From 341c3d16eeaf9ff2a35227827d22bc53d40857d0 Mon Sep 17 00:00:00 2001 From: Naveed Khan Date: Fri, 2 Oct 2026 01:53:37 +0530 Subject: [PATCH] use containsKey for a supplied cmsAlgorithmProtect attribute --- docs/releasenotes.md | 2 + ...tAuthenticatedAttributeTableGenerator.java | 8 +-- .../DefaultSignedAttributeTableGenerator.java | 6 +- .../cms/test/NewAuthenticatedDataTest.java | 55 +++++++++++++++++++ .../cms/test/NewSignedDataTest.java | 46 ++++++++++++++++ 5 files changed, 110 insertions(+), 7 deletions(-) diff --git a/docs/releasenotes.md b/docs/releasenotes.md index 3983bd59b6..617bae922a 100644 --- a/docs/releasenotes.md +++ b/docs/releasenotes.md @@ -64,6 +64,8 @@ Date: 2026, TBD - The raw JCA provider bounded the PBKDF2 iteration count taken from an encoding (org.bouncycastle.pbe.max_iteration_count, default 10,000,000) but not the counts of the legacy PBES1 (PKCS#5 scheme 1) and PKCS#12 PBE families beside it. Their AlgorithmParameters (PKCS12PBE and its OID aliases, PBKDF1) accepted any count, narrowing one beyond the int range with intValue() so that 2^32 arrived as 0, and every Cipher, Mac and SecretKeyFactory derivation ran with whatever count it was given - including a count decoded by another provider's AlgorithmParameters, as when javax.crypto.EncryptedPrivateKeyInfo.getKeySpec() decrypts a PKCS#12 PBE-protected key with BC. As these schemes carry the count in unauthenticated parameters and derive before anything can be checked, a supplied blob could hold a derivation for tens of minutes. The parameter parse now rejects a negative, beyond-int or over-limit count, and the derivations reject a negative or over-limit count, under the same property as PBKDF2. The PKCS#12 key store derives through the same code, so a org.bouncycastle.pkcs12.max_it_count raised above 10,000,000 now needs org.bouncycastle.pbe.max_iteration_count raised with it. - The light-weight CryptoProWrapEngine (RFC 4357 sec. 6.3) diversified the key encryption key in the caller's own array, so after init the KeyParameter it was given held the diversified key, and initialising again with the same parameters - to unwrap what had just been wrapped, say - diversified it a second time and used a different key. It also failed with a NullPointerException when given no S-box, although init has a branch for that case. It now diversifies a copy, and given no S-box uses the GOST 28147 engine's default S-box for the diversification, the one the wrap itself then uses. The provider's GOST 28147 key wrap ciphers were unaffected, as they always supply an S-box and build a new KeyParameter on every init. +- The CMS signed and authenticated attribute generators let the caller override a generated attribute with an entry in the AttributeTable passed to the constructor - contentType, signingTime and messageDigest were each guarded with Hashtable.containsKey() - but the RFC 6211 CMS algorithm protection attribute beside them was guarded with Hashtable.contains(), which tests values rather than keys, so with the table holding Attribute values against ASN1ObjectIdentifier keys the condition was never true and the generated attribute always replaced a supplied one. DefaultSignedAttributeTableGenerator and DefaultAuthenticatedAttributeTableGenerator now test the key as the attributes beside it already did, so the AttributeTable passed to the constructor can pin the algorithm identifiers the attribute binds rather than a wrapper having to rewrite the table the generator returns. No generator in the library supplies the attribute itself, so generated output is unchanged. + ### 2.1.3 Additional Features and Functionality - The CRMF certificate request message controls now include the RFC 4211 sec. 6.6 protocolEncrKey control, which names the key a CA is to encrypt its response with: org.bouncycastle.cert.crmf.ProtocolEncrKeyControl carries the SubjectPublicKeyInfo the control is defined to take, and CertificateRequestMessage.getControl() recognises id-regCtrl-protocolEncrKey alongside the regToken, authenticator and pkiArchiveOptions controls it already returned (github PR #2443). diff --git a/pkix/src/main/java/org/bouncycastle/cms/DefaultAuthenticatedAttributeTableGenerator.java b/pkix/src/main/java/org/bouncycastle/cms/DefaultAuthenticatedAttributeTableGenerator.java index 6cfb7e3e85..a231d9b923 100644 --- a/pkix/src/main/java/org/bouncycastle/cms/DefaultAuthenticatedAttributeTableGenerator.java +++ b/pkix/src/main/java/org/bouncycastle/cms/DefaultAuthenticatedAttributeTableGenerator.java @@ -49,9 +49,9 @@ public DefaultAuthenticatedAttributeTableGenerator( /** * Create a standard attribute table from the passed in parameters - this will - * normally include contentType and messageDigest. If the constructor - * using an AttributeTable was used, entries in it for contentType and - * messageDigest will override the generated ones. + * normally include contentType, messageDigest, and CMS algorithm protection. If the + * constructor using an AttributeTable was used, entries in it for contentType, + * messageDigest, and CMS algorithm protection will override the generated ones. * * @param parameters source parameters for table generation. * @@ -87,7 +87,7 @@ protected Hashtable createStandardAttributeTable( std.put(attr.getAttrType(), attr); } - if (!std.contains(CMSAttributes.cmsAlgorithmProtect)) + if (!std.containsKey(CMSAttributes.cmsAlgorithmProtect)) { Attribute attr = new Attribute(CMSAttributes.cmsAlgorithmProtect, new DERSet(new CMSAlgorithmProtection( (AlgorithmIdentifier)parameters.get(CMSAttributeTableGenerator.DIGEST_ALGORITHM_IDENTIFIER), diff --git a/pkix/src/main/java/org/bouncycastle/cms/DefaultSignedAttributeTableGenerator.java b/pkix/src/main/java/org/bouncycastle/cms/DefaultSignedAttributeTableGenerator.java index fb268b2965..a0f0deb4a0 100644 --- a/pkix/src/main/java/org/bouncycastle/cms/DefaultSignedAttributeTableGenerator.java +++ b/pkix/src/main/java/org/bouncycastle/cms/DefaultSignedAttributeTableGenerator.java @@ -52,8 +52,8 @@ public DefaultSignedAttributeTableGenerator( /** * Create a standard attribute table from the passed in parameters - this will * normally include contentType, signingTime, messageDigest, and CMS algorithm protection. - * If the constructor using an AttributeTable was used, entries in it for contentType, signingTime, and - * messageDigest will override the generated ones. + * If the constructor using an AttributeTable was used, entries in it for contentType, signingTime, + * messageDigest, and CMS algorithm protection will override the generated ones. * * @param parameters source parameters for table generation. * @@ -95,7 +95,7 @@ protected Hashtable createStandardAttributeTable( std.put(attr.getAttrType(), attr); } - if (!std.contains(CMSAttributes.cmsAlgorithmProtect)) + if (!std.containsKey(CMSAttributes.cmsAlgorithmProtect)) { Attribute attr = new Attribute(CMSAttributes.cmsAlgorithmProtect, new DERSet(new CMSAlgorithmProtection( (AlgorithmIdentifier)parameters.get(CMSAttributeTableGenerator.DIGEST_ALGORITHM_IDENTIFIER), diff --git a/pkix/src/test/java/org/bouncycastle/cms/test/NewAuthenticatedDataTest.java b/pkix/src/test/java/org/bouncycastle/cms/test/NewAuthenticatedDataTest.java index 1e55b4a6f6..05844632be 100644 --- a/pkix/src/test/java/org/bouncycastle/cms/test/NewAuthenticatedDataTest.java +++ b/pkix/src/test/java/org/bouncycastle/cms/test/NewAuthenticatedDataTest.java @@ -9,7 +9,9 @@ import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import javax.crypto.SecretKey; @@ -17,13 +19,19 @@ import junit.framework.Test; import junit.framework.TestCase; import junit.framework.TestSuite; +import org.bouncycastle.asn1.ASN1EncodableVector; import org.bouncycastle.asn1.ASN1Encoding; import org.bouncycastle.asn1.ASN1ObjectIdentifier; import org.bouncycastle.asn1.ASN1Primitive; import org.bouncycastle.asn1.DERNull; import org.bouncycastle.asn1.DEROctetString; +import org.bouncycastle.asn1.DERSet; +import org.bouncycastle.asn1.cms.Attribute; +import org.bouncycastle.asn1.cms.AttributeTable; import org.bouncycastle.asn1.cms.AuthenticatedData; import org.bouncycastle.asn1.cms.CCMParameters; +import org.bouncycastle.asn1.cms.CMSAlgorithmProtection; +import org.bouncycastle.asn1.cms.CMSAttributes; import org.bouncycastle.asn1.cms.CMSObjectIdentifiers; import org.bouncycastle.asn1.cms.ContentInfo; import org.bouncycastle.asn1.cms.GCMParameters; @@ -36,11 +44,13 @@ import org.bouncycastle.asn1.x509.AlgorithmIdentifier; import org.bouncycastle.cert.X509CertificateHolder; import org.bouncycastle.cms.CMSAlgorithm; +import org.bouncycastle.cms.CMSAttributeTableGenerator; import org.bouncycastle.cms.CMSRuntimeException; import org.bouncycastle.cms.CMSAuthenticatedData; import org.bouncycastle.cms.CMSAuthenticatedDataGenerator; import org.bouncycastle.cms.CMSException; import org.bouncycastle.cms.CMSProcessableByteArray; +import org.bouncycastle.cms.DefaultAuthenticatedAttributeTableGenerator; import org.bouncycastle.cms.OriginatorInfoGenerator; import org.bouncycastle.cms.PasswordRecipient; import org.bouncycastle.cms.PasswordRecipientInformation; @@ -138,6 +148,51 @@ public static Test suite() return new CMSTestSetup(new TestSuite(NewAuthenticatedDataTest.class)); } + public void testSuppliedAlgorithmProtectionAttribute() + throws Exception + { + AlgorithmIdentifier sha1 = new AlgorithmIdentifier(OIWObjectIdentifiers.idSHA1); + AlgorithmIdentifier sha256 = new AlgorithmIdentifier(NISTObjectIdentifiers.id_sha256); + AlgorithmIdentifier macAlgId = new AlgorithmIdentifier(PKCSObjectIdentifiers.id_hmacWithSHA256); + + Map parameters = new HashMap(); + + parameters.put(CMSAttributeTableGenerator.CONTENT_TYPE, CMSObjectIdentifiers.data); + parameters.put(CMSAttributeTableGenerator.DIGEST_ALGORITHM_IDENTIFIER, sha256); + parameters.put(CMSAttributeTableGenerator.MAC_ALGORITHM_IDENTIFIER, macAlgId); + parameters.put(CMSAttributeTableGenerator.DIGEST, new byte[32]); + + // with nothing supplied the attribute is generated from the parameters + AttributeTable generated = new DefaultAuthenticatedAttributeTableGenerator().getAttributes(parameters); + + assertEquals(new CMSAlgorithmProtection(sha256, CMSAlgorithmProtection.MAC, macAlgId), + algorithmProtectionOf(generated)); + + // an entry supplied through the constructor overrides it, as contentType and messageDigest + // already do + CMSAlgorithmProtection supplied = new CMSAlgorithmProtection( + sha1, CMSAlgorithmProtection.MAC, macAlgId); + + ASN1EncodableVector table = new ASN1EncodableVector(); + + table.add(new Attribute(CMSAttributes.cmsAlgorithmProtect, new DERSet(supplied))); + + AttributeTable overridden = new DefaultAuthenticatedAttributeTableGenerator( + new AttributeTable(table)).getAttributes(parameters); + + assertEquals(supplied, algorithmProtectionOf(overridden)); + } + + private static CMSAlgorithmProtection algorithmProtectionOf(AttributeTable table) + { + Attribute attr = table.get(CMSAttributes.cmsAlgorithmProtect); + + assertNotNull(attr); + assertEquals(1, attr.getAttrValues().size()); + + return CMSAlgorithmProtection.getInstance(attr.getAttrValues().getObjectAt(0)); + } + public void testKeyTransDESede() throws Exception { diff --git a/pkix/src/test/java/org/bouncycastle/cms/test/NewSignedDataTest.java b/pkix/src/test/java/org/bouncycastle/cms/test/NewSignedDataTest.java index d3c3ee2e66..1973583caf 100644 --- a/pkix/src/test/java/org/bouncycastle/cms/test/NewSignedDataTest.java +++ b/pkix/src/test/java/org/bouncycastle/cms/test/NewSignedDataTest.java @@ -52,6 +52,7 @@ import org.bouncycastle.asn1.bsi.BSIObjectIdentifiers; import org.bouncycastle.asn1.cms.Attribute; import org.bouncycastle.asn1.cms.AttributeTable; +import org.bouncycastle.asn1.cms.CMSAlgorithmProtection; import org.bouncycastle.asn1.cms.CMSAttributes; import org.bouncycastle.asn1.cms.CMSObjectIdentifiers; import org.bouncycastle.asn1.cms.ContentInfo; @@ -1940,6 +1941,51 @@ public AttributeTable getAttributes(Map parameters) verifyRSASignatures(s, md.digest("Hello world!".getBytes())); } + public void testSuppliedAlgorithmProtectionAttribute() + throws Exception + { + AlgorithmIdentifier sha1 = new AlgorithmIdentifier(OIWObjectIdentifiers.idSHA1); + AlgorithmIdentifier sha256 = new AlgorithmIdentifier(NISTObjectIdentifiers.id_sha256); + AlgorithmIdentifier sigAlgId = new AlgorithmIdentifier(PKCSObjectIdentifiers.sha256WithRSAEncryption); + + Map parameters = new HashMap(); + + parameters.put(CMSAttributeTableGenerator.CONTENT_TYPE, CMSObjectIdentifiers.data); + parameters.put(CMSAttributeTableGenerator.DIGEST_ALGORITHM_IDENTIFIER, sha256); + parameters.put(CMSAttributeTableGenerator.SIGNATURE_ALGORITHM_IDENTIFIER, sigAlgId); + parameters.put(CMSAttributeTableGenerator.DIGEST, new byte[32]); + + // with nothing supplied the attribute is generated from the parameters + AttributeTable generated = new DefaultSignedAttributeTableGenerator().getAttributes(parameters); + + assertEquals(new CMSAlgorithmProtection(sha256, CMSAlgorithmProtection.SIGNATURE, sigAlgId), + algorithmProtectionOf(generated)); + + // an entry supplied through the constructor overrides it, as contentType, signingTime and + // messageDigest already do + CMSAlgorithmProtection supplied = new CMSAlgorithmProtection( + sha1, CMSAlgorithmProtection.SIGNATURE, sigAlgId); + + ASN1EncodableVector table = new ASN1EncodableVector(); + + table.add(new Attribute(CMSAttributes.cmsAlgorithmProtect, new DERSet(supplied))); + + AttributeTable overridden = new DefaultSignedAttributeTableGenerator( + new AttributeTable(table)).getAttributes(parameters); + + assertEquals(supplied, algorithmProtectionOf(overridden)); + } + + private static CMSAlgorithmProtection algorithmProtectionOf(AttributeTable table) + { + Attribute attr = table.get(CMSAttributes.cmsAlgorithmProtect); + + assertNotNull(attr); + assertEquals(1, attr.getAttrValues().size()); + + return CMSAlgorithmProtection.getInstance(attr.getAttrValues().getObjectAt(0)); + } + public void testSignerInformationExtension() throws Exception {