Skip to content

Commit e2e9bb2

Browse files
dghgitdghgit
committed
Merge branch '1913-openpgp-v6-null-pointer' into 'main'
1913 openpgp v6 null pointer See merge request root/bc-java!43
2 parents 8896ae0 + 9a5c220 commit e2e9bb2

File tree

5 files changed

+55
-54
lines changed

5 files changed

+55
-54
lines changed

‎pg/src/main/java/org/bouncycastle/openpgp/PGPEncryptedDataGenerator.java‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public class PGPEncryptedDataGenerator
8888
private SecureRandom rand;
8989
// If true, force generation of a session key, even if we only have a single password-based encryption method
9090
// and could therefore use the S2K output as session key directly.
91-
private boolean forceSessionKey = false;
91+
private boolean forceSessionKey = true;
9292

9393
/**
9494
* Base constructor.
@@ -121,7 +121,9 @@ public PGPEncryptedDataGenerator(PGPDataEncryptorBuilder encryptorBuilder, boole
121121
* Some versions of PGP always expect a session key, this will force use
122122
* of a session key even if a single PBE encryptor is provided.
123123
*
124-
* @param forceSessionKey true if a session key should always be used, default is false.
124+
* @param forceSessionKey true if a session key should always be used, default is true.
125+
* @see <a href="https://www.rfc-editor.org/rfc/rfc9580.html#section-5.3.1-4">
126+
* RFC9580 - Description of the optional encrypted session key field</a>
125127
*/
126128
public void setForceSessionKey(boolean forceSessionKey)
127129
{
@@ -217,40 +219,38 @@ private OutputStream open(
217219
pOut = new BCPGOutputStream(out, !useOldFormat);
218220

219221
byte[] sessionKey; // session key, either protected by - or directly derived from session key encryption mechanism.
220-
byte[] sessionInfo; // sessionKey with prepended alg-id, appended checksum
221-
222+
byte[] sessionInfo = null; // sessionKey with prepended alg-id, appended checksum, null indicates direct use of S2K output as sessionKey/messageKey
222223
byte[] messageKey; // key used to encrypt the message. In OpenPGP v6 this is derived from sessionKey + salt.
223224

224225
boolean directS2K = !forceSessionKey && methods.size() == 1 &&
225-
methods.get(0) instanceof PBEKeyEncryptionMethodGenerator;
226-
if (directS2K)
227-
{
228-
sessionKey = ((PBEKeyEncryptionMethodGenerator)methods.get(0)).getKey(defAlgorithm);
229-
sessionInfo = null; // null indicates direct use of S2K output as sessionKey/messageKey
230-
}
231-
else
232-
{
233-
sessionKey = PGPUtil.makeRandomKey(defAlgorithm, rand);
234-
// prepend algorithm, append checksum
235-
sessionInfo = createSessionInfo(defAlgorithm, sessionKey);
236-
}
237-
messageKey = sessionKey;
238-
239-
// In OpenPGP v6, we need an additional step to derive a message key and IV from the session info.
240-
// Since we cannot inject the IV into the data encryptor, we append it to the message key.
241-
boolean isV5StyleAEAD = dataEncryptorBuilder.isV5StyleAEAD();
226+
methods.get(0) instanceof PBEKeyEncryptionMethodGenerator; // not public key
227+
boolean isV5StyleAEAD = dataEncryptorBuilder.isV5StyleAEAD(); //v5
242228
if (dataEncryptorBuilder.getAeadAlgorithm() != -1 && !isV5StyleAEAD)
243229
{
230+
sessionKey = PGPUtil.makeRandomKey(defAlgorithm, rand);
231+
// In OpenPGP v6, we need an additional step to derive a message key and IV from the session info.
232+
// Since we cannot inject the IV into the data encryptor, we append it to the message key.
244233
byte[] info = SymmetricEncIntegrityPacket.createAAData(
245234
SymmetricEncIntegrityPacket.VERSION_2,
246235
defAlgorithm,
247236
dataEncryptorBuilder.getAeadAlgorithm(),
248237
dataEncryptorBuilder.getChunkSize());
249-
250238
// messageKey = key and IV, will be separated in the data encryptor
251239
messageKey = AEADUtil.deriveMessageKeyAndIv(
252240
dataEncryptorBuilder.getAeadAlgorithm(), defAlgorithm, sessionKey, salt, info);
253241
}
242+
else if (directS2K)
243+
{
244+
sessionKey = ((PBEKeyEncryptionMethodGenerator)methods.get(0)).getKey(defAlgorithm);
245+
messageKey = sessionKey;
246+
}
247+
else
248+
{
249+
sessionKey = PGPUtil.makeRandomKey(defAlgorithm, rand);
250+
// prepend algorithm, append checksum
251+
sessionInfo = createSessionInfo(defAlgorithm, sessionKey);
252+
messageKey = sessionKey;
253+
}
254254

255255
PGPDataEncryptor dataEncryptor = dataEncryptorBuilder.build(messageKey);
256256
digestCalc = dataEncryptor.getIntegrityCalculator();
@@ -269,7 +269,9 @@ private OutputStream open(
269269
}
270270
else // data is encrypted by v2 SEIPD (AEAD), so write v6 SKESK packet
271271
{
272-
writeOpenPGPv6ESKPacket(method, aeadDataEncryptor.getAEADAlgorithm(), sessionInfo);
272+
//https://www.rfc-editor.org/rfc/rfc9580.html#section-3.7.2.1 Table 2
273+
//AEAD(HKDF(S2K(passphrase), info), secrets, packetprefix)
274+
writeOpenPGPv6ESKPacket(method, aeadDataEncryptor.getAEADAlgorithm(), sessionKey);
273275
}
274276
}
275277
// OpenPGP v4
@@ -320,7 +322,7 @@ private OutputStream open(
320322
{
321323
if (digestCalc != null)
322324
{
323-
encOut = SymmetricEncIntegrityPacket.createVersion1Packet();
325+
encOut = SymmetricEncIntegrityPacket.createVersion1Packet();
324326
if (useOldFormat)
325327
{
326328
throw new PGPException("symmetric-enc-integrity packets not supported in old PGP format");

‎pg/src/main/java/org/bouncycastle/openpgp/operator/PBEKeyEncryptionMethodGenerator.java‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public byte[] getKey(int encAlgorithm)
162162

163163
@Override
164164
public ContainedPacket generateV5(int kekAlgorithm, int aeadAlgorithm, byte[] sessionInfo)
165-
throws PGPException
165+
throws PGPException
166166
{
167167
return generate(kekAlgorithm, sessionInfo);
168168
// TODO: Implement v5 SKESK creation properly.
@@ -179,44 +179,45 @@ public ContainedPacket generateV6(int kekAlgorithm, int aeadAlgorithm, byte[] se
179179
// If we use this method, roundtripping v5 AEAD is broken.
180180
// TODO: Investigate
181181
private ContainedPacket generateV5ESK(int kekAlgorithm, int aeadAlgorithm, byte[] sessionInfo)
182-
throws PGPException
182+
throws PGPException
183183
{
184184
byte[] ikm = getKey(kekAlgorithm);
185-
byte[] info = new byte[] {
186-
(byte) 0xC3,
187-
(byte) SymmetricKeyEncSessionPacket.VERSION_5,
188-
(byte) kekAlgorithm,
189-
(byte) aeadAlgorithm
185+
byte[] info = new byte[]{
186+
(byte)0xC3,
187+
(byte)SymmetricKeyEncSessionPacket.VERSION_5,
188+
(byte)kekAlgorithm,
189+
(byte)aeadAlgorithm
190190
};
191191

192192
byte[] iv = new byte[AEADUtils.getIVLength(aeadAlgorithm)];
193193
random.nextBytes(iv);
194194

195195
int tagLen = AEADUtils.getAuthTagLength(aeadAlgorithm);
196-
byte[] eskAndTag = getEskAndTag(kekAlgorithm, aeadAlgorithm, sessionInfo, ikm, iv, info);
196+
byte[] sessionKey = getSessionKey(sessionInfo);
197+
byte[] eskAndTag = getEskAndTag(kekAlgorithm, aeadAlgorithm, sessionKey, ikm, iv, info);
197198
byte[] esk = Arrays.copyOfRange(eskAndTag, 0, eskAndTag.length - tagLen);
198199
byte[] tag = Arrays.copyOfRange(eskAndTag, esk.length, eskAndTag.length);
199200

200201
return SymmetricKeyEncSessionPacket.createV5Packet(kekAlgorithm, aeadAlgorithm, iv, s2k, esk, tag);
201202
}
202203

203-
private ContainedPacket generateV6ESK(int kekAlgorithm, int aeadAlgorithm, byte[] sessionInfo)
204-
throws PGPException
204+
private ContainedPacket generateV6ESK(int kekAlgorithm, int aeadAlgorithm, byte[] sessionKey)
205+
throws PGPException
205206
{
206207
byte[] ikm = getKey(kekAlgorithm);
207-
byte[] info = new byte[] {
208-
(byte) 0xC3,
209-
(byte) SymmetricKeyEncSessionPacket.VERSION_6,
210-
(byte) kekAlgorithm,
211-
(byte) aeadAlgorithm
208+
byte[] info = new byte[]{
209+
(byte)0xC3,
210+
(byte)SymmetricKeyEncSessionPacket.VERSION_6,
211+
(byte)kekAlgorithm,
212+
(byte)aeadAlgorithm
212213
};
213214
byte[] kek = generateV6KEK(kekAlgorithm, ikm, info);
214215

215216
byte[] iv = new byte[AEADUtils.getIVLength(aeadAlgorithm)];
216217
random.nextBytes(iv);
217218

218219
int tagLen = AEADUtils.getAuthTagLength(aeadAlgorithm);
219-
byte[] eskAndTag = getEskAndTag(kekAlgorithm, aeadAlgorithm, sessionInfo, kek, iv, info);
220+
byte[] eskAndTag = getEskAndTag(kekAlgorithm, aeadAlgorithm, sessionKey, kek, iv, info);
220221
byte[] esk = Arrays.copyOfRange(eskAndTag, 0, eskAndTag.length - tagLen);
221222
byte[] tag = Arrays.copyOfRange(eskAndTag, esk.length, eskAndTag.length);
222223

@@ -227,7 +228,7 @@ private ContainedPacket generateV6ESK(int kekAlgorithm, int aeadAlgorithm, byte[
227228
* Generate a V4 SKESK packet.
228229
*
229230
* @param encAlgorithm the {@link SymmetricKeyAlgorithmTags encryption algorithm} being used
230-
* @param sessionInfo session data generated by the encrypted data generator.
231+
* @param sessionInfo session data generated by the encrypted data generator.
231232
* @return v4 SKESK packet
232233
* @throws PGPException
233234
*/
@@ -250,10 +251,17 @@ public ContainedPacket generate(int encAlgorithm, byte[] sessionInfo)
250251
return SymmetricKeyEncSessionPacket.createV4Packet(encAlgorithm, s2k, encryptSessionInfo(encAlgorithm, key, nSessionInfo));
251252
}
252253

254+
protected byte[] getSessionKey(byte[] sessionInfo)
255+
{
256+
byte[] sessionKey = new byte[sessionInfo.length - 3];
257+
System.arraycopy(sessionInfo, 1, sessionKey, 0, sessionKey.length);
258+
return sessionKey;
259+
}
260+
253261
abstract protected byte[] encryptSessionInfo(int encAlgorithm, byte[] key, byte[] sessionInfo)
254262
throws PGPException;
255263

256-
abstract protected byte[] getEskAndTag(int kekAlgorithm, int aeadAlgorithm, byte[] sessionInfo, byte[] key, byte[] iv, byte[] info)
264+
abstract protected byte[] getEskAndTag(int kekAlgorithm, int aeadAlgorithm, byte[] sessionKey, byte[] key, byte[] iv, byte[] info)
257265
throws PGPException;
258266

259267
abstract protected byte[] generateV6KEK(int kekAlgorithm, byte[] ikm, byte[] info)

‎pg/src/main/java/org/bouncycastle/openpgp/operator/bc/BcPBEKeyEncryptionMethodGenerator.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,9 @@ protected byte[] generateV6KEK(int kekAlgorithm, byte[] ikm, byte[] info)
113113
return BcAEADUtil.generateHKDFBytes(ikm, null, info, SymmetricKeyUtils.getKeyLengthInOctets(kekAlgorithm));
114114
}
115115

116-
protected byte[] getEskAndTag(int kekAlgorithm, int aeadAlgorithm, byte[] sessionInfo, byte[] key, byte[] iv, byte[] info)
116+
protected byte[] getEskAndTag(int kekAlgorithm, int aeadAlgorithm, byte[] sessionKey, byte[] key, byte[] iv, byte[] info)
117117
throws PGPException
118118
{
119-
byte[] sessionKey = new byte[sessionInfo.length - 3];
120-
System.arraycopy(sessionInfo, 1, sessionKey, 0, sessionKey.length);
121-
122119
AEADCipher aeadCipher = BcAEADUtil.createAEADCipher(kekAlgorithm, aeadAlgorithm);
123120
aeadCipher.init(true, new AEADParameters(new KeyParameter(key), 128, iv, info));
124121
int outLen = aeadCipher.getOutputSize(sessionKey.length);

‎pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcePBEKeyEncryptionMethodGenerator.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,9 @@ protected byte[] generateV6KEK(int kekAlgorithm, byte[] ikm, byte[] info)
156156
return JceAEADUtil.generateHKDFBytes(ikm, null, info, SymmetricKeyUtils.getKeyLengthInOctets(kekAlgorithm));
157157
}
158158

159-
protected byte[] getEskAndTag(int kekAlgorithm, int aeadAlgorithm, byte[] sessionInfo, byte[] key, byte[] iv, byte[] info)
159+
protected byte[] getEskAndTag(int kekAlgorithm, int aeadAlgorithm, byte[] sessionKey, byte[] key, byte[] iv, byte[] info)
160160
throws PGPException
161161
{
162-
byte[] sessionKey = new byte[sessionInfo.length - 3];
163-
System.arraycopy(sessionInfo, 1, sessionKey, 0, sessionKey.length);
164-
165162
AEADCipher aeadCipher = BcAEADUtil.createAEADCipher(kekAlgorithm, aeadAlgorithm);
166163
aeadCipher.init(true, new AEADParameters(new KeyParameter(key), 128, iv, info));
167164
int outLen = aeadCipher.getOutputSize(sessionKey.length);

‎pg/src/test/java/org/bouncycastle/openpgp/test/PGPAeadTest.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,9 @@
4444
import org.bouncycastle.openpgp.operator.jcajce.JcePBEDataDecryptorFactoryBuilder;
4545
import org.bouncycastle.openpgp.operator.jcajce.JcePBEKeyEncryptionMethodGenerator;
4646
import org.bouncycastle.openpgp.operator.jcajce.JcePGPDataEncryptorBuilder;
47-
import org.bouncycastle.test.DumpUtil;
4847
import org.bouncycastle.util.Arrays;
4948
import org.bouncycastle.util.Exceptions;
50-
import org.bouncycastle.util.Pack;
5149
import org.bouncycastle.util.Strings;
52-
import org.bouncycastle.util.encoders.Hex;
5350
import org.bouncycastle.util.io.Streams;
5451
import org.bouncycastle.util.test.SimpleTest;
5552

@@ -429,7 +426,7 @@ private void testJceDecryption(String armoredMessage, char[] password, byte[] ex
429426
public static void printHex(byte[] bytes)
430427
{
431428
// -DM System.out.println
432-
System.out.println(DumpUtil.hexdump(bytes));
429+
//System.out.println(DumpUtil.hexdump(bytes));
433430
}
434431

435432
private static String algNames(int aeadAlg, int symAlg)

0 commit comments

Comments
 (0)