RFR: 8305972: Update XML Security for Java to 3.0.2 [v2]

Sean Mullan mullan at openjdk.org
Mon May 8 20:48:26 UTC 2023


On Mon, 8 May 2023 17:22:53 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Update XML Security for Java to 3.0.2. Some change to tests:
>> 
>> 1. No more Xalan. One test case is singled out to demonstrate how to use a special configuration.
>> 2. EdDSA does not support `KeyValue`. Use X.509 certificate instead.
>
> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   cleanup commented out code and unnecessary bug id

src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/SignatureMethod.java line 63:

> 61: public interface SignatureMethod extends XMLStructure, AlgorithmMethod {
> 62: 
> 63:     // All methods can be found in RFC 9231.

Only the ones with "xmldsig-more" in the URI, actually.

src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/SignatureMethod.java line 260:

> 258:     /**
> 259:      * The <a href="http://www.w3.org/2021/04/xmldsig-more#eddsa-ed25519">
> 260:      * ED25519</a> signature method algorithm URI.

I think it would be beneficial to add a reference to the standards. Most of these URIs are defined in RFC 9131, but a few are from the W3C Recommendation. I suggest adding the following sentence to the class description:

"The signature method algorithm URIs defined in this class are specified in the [W3C Recommendation for XML-Signature Syntax and Processing](http://www.w3.org/TR/xmldsig-core/) and [RFC 9131](https://www.rfc-editor.org/info/rfc9131)."

test/jdk/javax/xml/crypto/dsig/GenerationTests.java line 105:

> 103:             rsaSha1mgf1, rsaSha224mgf1, rsaSha256mgf1, rsaSha384mgf1, rsaSha512mgf1,
> 104:             rsaShaPSS,
> 105:             ed25519, ed448;

Nit: combine these lines.

test/jdk/javax/xml/crypto/dsig/XalanTest.java line 1:

> 1: /*

Can you add a comment to this test with more details, something like: "This test used to be part of ValidationTests but was moved into its own test because it tests a signature that contains the `here()` function which depends on Xalan internals. The Xalan dependency has been removed from the DSig implementation but can be optionally configured ..."

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13840#discussion_r1187870857
PR Review Comment: https://git.openjdk.org/jdk/pull/13840#discussion_r1187866459
PR Review Comment: https://git.openjdk.org/jdk/pull/13840#discussion_r1187845348
PR Review Comment: https://git.openjdk.org/jdk/pull/13840#discussion_r1187862788


More information about the security-dev mailing list