On Sun, 19 Oct 2025 01:52:02 GMT, Koushik Muthukrishnan Thirupattur
<[email protected]> wrote:
>> Looking at RFC 9879 on PBES2 and PBMAC1 in PKCS12, algorithm identifiers for
>> HmacSHA*** (like SHA***) should always contain NULL as params. We can update
>> the list at AlgorithmId.encode(DOS) to enforce this rule.
>
> Koushik Muthukrishnan Thirupattur has updated the pull request incrementally
> with two additional commits since the last revision:
>
> - 8367008: Algorithm identifiers for HmacSHA* should always have NULL as
> params
> - 8367008: Algorithm identifiers for HmacSHA* should always have NULL as
> params
test/jdk/sun/security/x509/AlgorithmId/AlgorithmIdEqualsHashCode.java line 136:
> 134: + e);
> 135: throw e; // Rethrow to mark test failure
> 136: }
The whole section can be simply
// Construct an AlgorithmId with explicit DER NULL parameters
DerValue explicitNullParams = new DerValue(DerValue.tag_Null, new byte[0]);
AlgorithmId aiNullParams = new AlgorithmId(AlgorithmId.SHA256_oid,
explicitNullParams);
// The constructor should canonicalize this to "no parameters"
Asserts.assertTrue(aiNullParams.getEncodedParams() == null);
AlgorithmId aiNormal = AlgorithmId.get("SHA-256");
Asserts.assertEquals(aiNullParams, aiNormal);
Asserts.assertEquals(aiNullParams.hashCode(), aiNormal.hashCode());
test/jdk/sun/security/x509/AlgorithmId/AlgorithmIdEqualsHashCode.java line 150:
> 148: throw new Exception("FAILED invalid ASN.1 NULL test:
> unexpected exception" +
> 149: " type", e);
> 150: }
I wonder if it's necessary to throw a wrapped exception here. As long as the
exception has stack traces that lead you to the point of failure, that's
enough. In fact, this whole section can be written as
DerValue invalidNull = new DerValue(DerValue.tag_Null, new byte[]{0x00});
Asserts.assertThrows(IOException.class,
() -> new AlgorithmId(AlgorithmId.SHA256_oid, invalidNull));
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27700#discussion_r2445391121
PR Review Comment: https://git.openjdk.org/jdk/pull/27700#discussion_r2445385142
