[8u] RFR: 8172404: Tools should warn if weak algorithms are used before restricting them

[Andrew Hughes]

On 14:41 Wed 03 Feb     , Severin Gehwolf wrote:
> Hi!
> 
> Anyone willing to review this? Dependencies are in 8u-dev by now.
> 
> Thanks,
> Severin
> 
> On Mon, 2021-01-11 at 12:35 +0100, Severin Gehwolf wrote:
> > Hi,
> > 
> > Please review this 8u backport of JDK-8172404, an enhancement to warn
> > about insecure and soon-to-be-disabled algorithms in security tools.
> > This patch introduces a new security property,
> > jdk.security.legacyAlgorithms, and thus requires a CSR. Since Oracle
> > backported this patch, we are re-using that CSR.
> > 
> > The original patch doesn't apply cleanly so I had to adjust hunks
> > manually. In order for this backport to apply better, it depends on
> > JDK-8185934[1] and JDK-8233228[2]. Of particular note are the
> > differences in jarsigner/Main.java method signJar(). For this
> > backport,
> > I've moved initialization of a null sigalg a bit earlier in signJar()
> > by virtue of calling new private static method
> > getDefaultSignatureAlgorithm(privateKey). This allows one to reason
> > that sigalg, tSADigestAlg and digestalg will never be null when new
> > method checkWeakSign() is being called. The former, because of what
> > I've just explained earlier, the latter two because they are being
> > set
> > to default values on object construction in contrast to code in JDK
> > 11u.
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8172404
> > webrev: https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8172404/jdk8/02/webrev/
> > CSR (approved): https://bugs.openjdk.java.net/browse/JDK-8238640
> > 
> > Testing: keytool/jarsigner tests. Manual testing that warnings are
> > being printed for weak CA certs in cacerts
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> 
> 

Code looks good. Just a couple of typographical issues:

1. In jarsigner/Main.java, a newline has gone missing in the changes
there:

11u:

+        checkWeakSign(sigalg, SIG_PRIMITIVE_SET, false);
+
+        checkWeakSign(privateKey);

8u:

+        checkWeakSign(sigalg, SIG_PRIMITIVE_SET, false);
+        checkWeakSign(privateKey);

2. The test/sun/security/tools/keytool/WeakAlg.java changes
look quite different when comparing the 11u & 8u patches. Can
you confirm the patched files are the same (or close enough)?

No need for a new webrev just for the correction in #1 if #2 is not an
issue.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
OpenJDK Package Owner
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