[8u] RFR Backport 8177334: Update xmldsig implementation to Apache Santuario 2.1.1

[Andrew Hughes]

On 25/05/2020 21:53, Elliott Baron wrote:
> Hi,
> 
> On 2020-05-08 2:58 p.m., Elliott Baron wrote:
>> Hi,
>>
>> On 2020-04-16 7:54 p.m., Elliott Baron wrote:
>>> Hi,
>>>
>>> I'd like to request a review to backport 8177334 to 8u.
>>>
>>> Original fix:
>>> https://bugs.openjdk.java.net/browse/JDK-8177334
>>> http://hg.openjdk.java.net/jdk/jdk/rev/3810c9a2efa1
>>>
>>> The JDK 11 fix did not apply cleanly. Below, I have detailed the
>>> modifications I made in order to backport this fix to 8u. There are
>>> some major changes that I believe may require some discussion, and
>>> many minor changes outlined after those. The changes within each
>>> section are listed roughly in order of the patch.
>>>
>>> Thank you to Martin Balao for his assistance in tracking down the
>>> dependencies for this fix.
>>>
>>> I should point out that there are some known bugfixes that fix
>>> problems introduced by this update. These should probably go into the
>>> same 8u update as this fix. They are:
>>> - 8217878: ENVELOPING XML signature no longer works
>>> - 8218629: XML Digital Signature throws NAMESPACE_ERR exception on
>>> OpenJDK 11, works 8/9/10 (I believe this is the same fix as above)
>>> - 8236645: JDK 8u231 introduces a regression with incompatible
>>> handling of XML messages
>>>
>>> 8u webrev:
>>> https://cr.openjdk.java.net/~ebaron/jdk8u/JDK-8177334/webrev.00/
>>
>> Has anyone had a chance to take a look at this yet?
>>
>>>
>>> [snip]
> 
> Here is an updated webrev that fixes conflicts introduced by
> 8231415: Better signatures in XML.
> 
> 8u webrev:
> https://cr.openjdk.java.net/~ebaron/jdk8u/JDK-8177334/webrev.01/
> 
> Thanks,
> Elliott
> 

Thanks for completing what is a huge backport.

Can you comment more on what changes you believe require discussion?
>From comparing with the 11u version, most of it seems fine:

* Movement of checkRegisterPermission in JavaUtils.java is omitted as
it's already been moved to its current location in 8u
* Omission of new 11u algorithms in DigestMethod.java and
SignatureMethod.java seems appropriate
* Leaving out the type checking in e.g. DOMKeyInfo.java seems
appropriate as these API methods did not previously throw ClassCastException

My main concern is that, although we don't add the new algorithms to
DigestMethod & SignatureMethod, or their tests, we do seem to be adding
support in e.g. JCEMapper for new algorithms like SHA3. I think we
should be consistent here and if we're not going to put new algorithms
in DigestMethod & SignatureMethod, and not include tests of them, they
shouldn't be being implemented.

I also wonder why getSignature was pulled into DOMSignatureMethod.java
from JDK-8042967. Why was this needed?

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222