Skip to content

Commit 2c4f6f8

Browse files
dghgitdghgit
committed
Merge branch '2013-openpgp-pgpsignaturesubpacketgenerator' into 'main'
PGPSignatureSubpacketGenerator: setXYZ() now removes duplicates. See merge request root/bc-java!77
2 parents 95e67cd + 6da0f6e commit 2c4f6f8

File tree

2 files changed

+55
-34
lines changed

2 files changed

+55
-34
lines changed

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

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,7 @@ public PGPSignatureSubpacketGenerator(PGPSignatureSubpacketVector sigSubV)
6969
*/
7070
public void setRevocable(boolean isCritical, boolean isRevocable)
7171
{
72-
if (contains(SignatureSubpacketTags.REVOCABLE))
73-
{
74-
throw new IllegalStateException("Revocable exists in the Signature Subpacket Generator");
75-
}
72+
removePacketsOfType(SignatureSubpacketTags.REVOCABLE);
7673
packets.add(new Revocable(isCritical, isRevocable));
7774
}
7875

@@ -97,21 +94,19 @@ public void setExportable(boolean isExportable)
9794
*/
9895
public void setExportable(boolean isCritical, boolean isExportable)
9996
{
100-
if (contains(SignatureSubpacketTags.EXPORTABLE))
101-
{
102-
throw new IllegalStateException("Exportable Certification exists in the Signature Subpacket Generator");
103-
}
97+
removePacketsOfType(SignatureSubpacketTags.EXPORTABLE);
10498
packets.add(new Exportable(isCritical, isExportable));
10599
}
106100

107101
/**
108102
* Specify the set of features of the key.
109103
*
110104
* @param isCritical true if should be treated as critical, false otherwise.
111-
* @param feature features
105+
* @param feature features bitmap
112106
*/
113107
public void setFeature(boolean isCritical, byte feature)
114108
{
109+
removePacketsOfType(SignatureSubpacketTags.FEATURES);
115110
packets.add(new Features(isCritical, feature));
116111
}
117112

@@ -126,6 +121,7 @@ public void setFeature(boolean isCritical, byte feature)
126121
*/
127122
public void setTrust(boolean isCritical, int depth, int trustAmount)
128123
{
124+
removePacketsOfType(SignatureSubpacketTags.TRUST_SIG);
129125
packets.add(new TrustSignature(isCritical, depth, trustAmount));
130126
}
131127

@@ -150,6 +146,7 @@ public void setKeyExpirationTime(long seconds)
150146
*/
151147
public void setKeyExpirationTime(boolean isCritical, long seconds)
152148
{
149+
removePacketsOfType(SignatureSubpacketTags.KEY_EXPIRE_TIME);
153150
packets.add(new KeyExpirationTime(isCritical, seconds));
154151
}
155152

@@ -175,6 +172,7 @@ public void setSignatureExpirationTime(long seconds)
175172
*/
176173
public void setSignatureExpirationTime(boolean isCritical, long seconds)
177174
{
175+
removePacketsOfType(SignatureSubpacketTags.EXPIRE_TIME);
178176
packets.add(new SignatureExpirationTime(isCritical, seconds));
179177
}
180178

@@ -200,6 +198,7 @@ public void setSignatureCreationTime(Date date)
200198
*/
201199
public void setSignatureCreationTime(boolean isCritical, Date date)
202200
{
201+
removePacketsOfType(SignatureSubpacketTags.CREATION_TIME);
203202
packets.add(new SignatureCreationTime(isCritical, date));
204203
}
205204

@@ -212,6 +211,7 @@ public void setSignatureCreationTime(boolean isCritical, Date date)
212211
*/
213212
public void setPreferredHashAlgorithms(boolean isCritical, int[] algorithms)
214213
{
214+
removePacketsOfType(SignatureSubpacketTags.PREFERRED_HASH_ALGS);
215215
packets.add(new PreferredAlgorithms(SignatureSubpacketTags.PREFERRED_HASH_ALGS, isCritical,
216216
algorithms));
217217
}
@@ -225,6 +225,7 @@ public void setPreferredHashAlgorithms(boolean isCritical, int[] algorithms)
225225
*/
226226
public void setPreferredSymmetricAlgorithms(boolean isCritical, int[] algorithms)
227227
{
228+
removePacketsOfType(SignatureSubpacketTags.PREFERRED_SYM_ALGS);
228229
packets.add(new PreferredAlgorithms(SignatureSubpacketTags.PREFERRED_SYM_ALGS, isCritical,
229230
algorithms));
230231
}
@@ -238,6 +239,7 @@ public void setPreferredSymmetricAlgorithms(boolean isCritical, int[] algorithms
238239
*/
239240
public void setPreferredCompressionAlgorithms(boolean isCritical, int[] algorithms)
240241
{
242+
removePacketsOfType(SignatureSubpacketTags.PREFERRED_COMP_ALGS);
241243
packets.add(new PreferredAlgorithms(SignatureSubpacketTags.PREFERRED_COMP_ALGS, isCritical,
242244
algorithms));
243245
}
@@ -254,6 +256,7 @@ public void setPreferredCompressionAlgorithms(boolean isCritical, int[] algorith
254256
@Deprecated
255257
public void setPreferredAEADAlgorithms(boolean isCritical, int[] algorithms)
256258
{
259+
removePacketsOfType(SignatureSubpacketTags.PREFERRED_AEAD_ALGORITHMS);
257260
packets.add(new PreferredAlgorithms(SignatureSubpacketTags.PREFERRED_AEAD_ALGORITHMS, isCritical,
258261
algorithms));
259262
}
@@ -268,6 +271,7 @@ public void setPreferredAEADAlgorithms(boolean isCritical, int[] algorithms)
268271
*/
269272
public void setPreferredAEADCiphersuites(boolean isCritical, PreferredAEADCiphersuites.Combination[] algorithms)
270273
{
274+
removePacketsOfType(SignatureSubpacketTags.PREFERRED_AEAD_ALGORITHMS);
271275
packets.add(new PreferredAEADCiphersuites(isCritical, algorithms));
272276
}
273277

@@ -280,6 +284,7 @@ public void setPreferredAEADCiphersuites(boolean isCritical, PreferredAEADCipher
280284
*/
281285
public void setPreferredAEADCiphersuites(PreferredAEADCiphersuites.Builder builder)
282286
{
287+
removePacketsOfType(SignatureSubpacketTags.PREFERRED_AEAD_ALGORITHMS);
283288
packets.add(builder.build());
284289
}
285290

@@ -300,6 +305,7 @@ public void setPreferredAEADCiphersuites(PreferredAEADCiphersuites.Builder build
300305
@Deprecated
301306
public void setPreferredLibrePgpEncryptionModes(boolean isCritical, int[] algorithms)
302307
{
308+
removePacketsOfType(SignatureSubpacketTags.LIBREPGP_PREFERRED_ENCRYPTION_MODES);
303309
packets.add(new LibrePGPPreferredEncryptionModes(isCritical, algorithms));
304310
}
305311

@@ -309,8 +315,22 @@ public void setPreferredLibrePgpEncryptionModes(boolean isCritical, int[] algori
309315
*
310316
* @param isCritical true if the subpacket should be treated as critical
311317
* @param uri key server URI
318+
* @deprecated use {@link #addPreferredKeyServer(boolean, String)} instead.
312319
*/
320+
@Deprecated
313321
public void setPreferredKeyServer(boolean isCritical, String uri)
322+
{
323+
addPreferredKeyServer(isCritical, uri);
324+
}
325+
326+
/**
327+
* Specify a preferred key server for the signed user-id / key.
328+
* Note, that the key server might also be a http/ftp etc. URI pointing to the key itself.
329+
*
330+
* @param isCritical true if the subpacket should be treated as critical
331+
* @param uri key server URI
332+
*/
333+
public void addPreferredKeyServer(boolean isCritical, String uri)
314334
{
315335
packets.add(new PreferredKeyServer(isCritical, uri));
316336
}
@@ -341,6 +361,7 @@ public void setKeyFlags(int flags)
341361
*/
342362
public void setKeyFlags(boolean isCritical, int flags)
343363
{
364+
removePacketsOfType(SignatureSubpacketTags.KEY_FLAGS);
344365
packets.add(new KeyFlags(isCritical, flags));
345366
}
346367

@@ -443,6 +464,7 @@ public void addEmbeddedSignature(boolean isCritical, PGPSignature pgpSignature)
443464

444465
public void setPrimaryUserID(boolean isCritical, boolean isPrimaryUserID)
445466
{
467+
removePacketsOfType(SignatureSubpacketTags.PRIMARY_USER_ID);
446468
packets.add(new PrimaryUserID(isCritical, isPrimaryUserID));
447469
}
448470

@@ -485,6 +507,7 @@ public void addNotationData(boolean isCritical, boolean isHumanReadable, String
485507
*/
486508
public void setRevocationReason(boolean isCritical, byte reason, String description)
487509
{
510+
removePacketsOfType(SignatureSubpacketTags.REVOCATION_REASON);
488511
packets.add(new RevocationReason(isCritical, reason, description));
489512
}
490513

@@ -523,6 +546,7 @@ public void addRevocationKey(boolean isCritical, int keyAlgorithm, byte[] finger
523546
*/
524547
public void setIssuerKeyID(boolean isCritical, long keyID)
525548
{
549+
removePacketsOfType(SignatureSubpacketTags.ISSUER_KEY_ID);
526550
packets.add(new IssuerKeyID(isCritical, keyID));
527551
}
528552

@@ -536,6 +560,7 @@ public void setIssuerKeyID(boolean isCritical, long keyID)
536560
*/
537561
public void setSignatureTarget(boolean isCritical, int publicKeyAlgorithm, int hashAlgorithm, byte[] hashData)
538562
{
563+
removePacketsOfType(SignatureSubpacketTags.SIGNATURE_TARGET);
539564
packets.add(new SignatureTarget(isCritical, publicKeyAlgorithm, hashAlgorithm, hashData));
540565
}
541566

@@ -558,6 +583,7 @@ public void setIssuerFingerprint(boolean isCritical, PGPSecretKey secretKey)
558583
*/
559584
public void setIssuerFingerprint(boolean isCritical, PGPPublicKey publicKey)
560585
{
586+
removePacketsOfType(SignatureSubpacketTags.ISSUER_FINGERPRINT);
561587
packets.add(new IssuerFingerprint(isCritical, publicKey.getVersion(), publicKey.getFingerprint()));
562588
}
563589

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

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@
3030
import org.bouncycastle.bcpg.SignatureSubpacketTags;
3131
import org.bouncycastle.bcpg.SymmetricKeyAlgorithmTags;
3232
import org.bouncycastle.bcpg.attr.ImageAttribute;
33+
import org.bouncycastle.bcpg.sig.Exportable;
3334
import org.bouncycastle.bcpg.sig.Features;
3435
import org.bouncycastle.bcpg.sig.IntendedRecipientFingerprint;
3536
import org.bouncycastle.bcpg.sig.KeyFlags;
3637
import org.bouncycastle.bcpg.sig.NotationData;
3738
import org.bouncycastle.bcpg.sig.PolicyURI;
3839
import org.bouncycastle.bcpg.sig.PreferredAEADCiphersuites;
3940
import org.bouncycastle.bcpg.sig.RegularExpression;
41+
import org.bouncycastle.bcpg.sig.Revocable;
4042
import org.bouncycastle.bcpg.sig.RevocationKey;
4143
import org.bouncycastle.bcpg.sig.RevocationKeyTags;
4244
import org.bouncycastle.bcpg.sig.RevocationReason;
@@ -2169,7 +2171,7 @@ public void testPGPSignatureSubpacketVector()
21692171
isTrue("Trust should be null", trustSignature != null);
21702172
isTrue("Trust level depth should be " + depth, trustSignature.getDepth() == depth);
21712173
isTrue("Trust amount should be " + trustAmount, trustSignature.getTrustAmount() == trustAmount);
2172-
isTrue("Exporable should be false", !hashedPcks.isExportable());
2174+
isTrue("Exportable should be false", !hashedPcks.isExportable());
21732175
isTrue(hashedPcks.getIssuerFingerprint().getKeyVersion() == publicKey.getVersion());
21742176
isTrue("isPrimaryUserID should be true", hashedPcks.isPrimaryUserID());
21752177

@@ -2195,7 +2197,7 @@ public void testPGPSignatureSubpacketVector()
21952197
isTrue("Trust should be null", trustSignature != null);
21962198
isTrue("Trust level depth should be " + depth, trustSignature.getDepth() == depth);
21972199
isTrue("Trust amount should be " + trustAmount, trustSignature.getTrustAmount() == trustAmount);
2198-
isTrue("Exporable should be false", !hashedPcks.isExportable());
2200+
isTrue("Exportable should be false", !hashedPcks.isExportable());
21992201
isTrue(hashedPcks.getIssuerFingerprint().getKeyVersion() == publicKey.getVersion());
22002202
isTrue("isPrimaryUserID should be true", hashedPcks.isPrimaryUserID());
22012203

@@ -2207,27 +2209,20 @@ public void testPGPSignatureSubpacketVector()
22072209

22082210
hashedGen = new PGPSignatureSubpacketGenerator();
22092211
hashedGen.setExportable(false, true);
2210-
try
2211-
{
2212-
hashedGen.setExportable(false, false);
2213-
fail("Duplicated settings for Exportable");
2214-
}
2215-
catch (IllegalStateException e)
2216-
{
2217-
isTrue("Exportable Certification exists in the Signature Subpacket Generator",
2218-
messageIs(e.getMessage(), "Exportable Certification exists in the Signature Subpacket Generator"));
2219-
}
2212+
hashedGen.setExportable(false, false);
2213+
isEquals("Calling setExportable multiple times MUST NOT introduce duplicates",
2214+
1, hashedGen.getSubpackets(SignatureSubpacketTags.EXPORTABLE).length);
2215+
Exportable exportable = (Exportable) hashedGen.getSubpackets(SignatureSubpacketTags.EXPORTABLE)[0];
2216+
isTrue("Last invocation of setExportable MUST take precedence.",
2217+
!exportable.isExportable());
2218+
22202219
hashedGen.setRevocable(false, true);
2221-
try
2222-
{
2223-
hashedGen.setRevocable(false, false);
2224-
fail("Duplicated settings for Revocable");
2225-
}
2226-
catch (IllegalStateException e)
2227-
{
2228-
isTrue("Revocable exists in the Signature Subpacket Generator",
2229-
messageIs(e.getMessage(), "Revocable exists in the Signature Subpacket Generator"));
2230-
}
2220+
hashedGen.setRevocable(false, false);
2221+
isEquals("Calling setRevocable multiple times MUST NOT introduce duplicates.",
2222+
1, hashedGen.getSubpackets(SignatureSubpacketTags.REVOCABLE).length);
2223+
Revocable revocable = (Revocable) hashedGen.getSubpackets(SignatureSubpacketTags.REVOCABLE)[0];
2224+
isTrue("Last invocation of setRevocable MUST take precedence.",
2225+
!revocable.isRevocable());
22312226

22322227
try
22332228
{
@@ -2273,13 +2268,13 @@ public void testPGPSignatureSubpacketVector()
22732268
hashedPcks = sig.getHashedSubPackets();
22742269
isTrue("URL should be " + url, hashedPcks.getPolicyURI().getURI().equals(url));
22752270
isTrue(areEqual(hashedPcks.getPolicyURI().getRawURI(), Strings.toUTF8ByteArray(url)));
2276-
isTrue("Exporable should be true", hashedPcks.isExportable());
2277-
isTrue("Test Singner User ID", hashedPcks.getSignerUserID().equals(""));
2271+
isTrue("Exportable should be false", !hashedPcks.isExportable());
2272+
isTrue("Test Signer User ID", hashedPcks.getSignerUserID().equals(""));
22782273
isTrue("Test for empty description", hashedPcks.getRevocationReason().getRevocationDescription().equals(""));
22792274
Features features = hashedPcks.getFeatures();
22802275
isTrue(features.supportsSEIPDv2());
22812276
isTrue(features.getFeatures() == Features.FEATURE_SEIPD_V2);
2282-
isTrue(hashedPcks.getRevocable().isRevocable());
2277+
isTrue("Revocable should be false", !hashedPcks.getRevocable().isRevocable());
22832278
}
22842279

22852280
public void testECNistCurves()

0 commit comments

Comments
 (0)