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

[Severin Gehwolf]

Hi Martin,

On Wed, 2021-02-03 at 16:04 -0300, Martin Balao wrote:
> Hi Severin,
> 
> Thanks for proposing this backport.

Thanks for the review!

> On 2/3/21 10:41 AM, Severin Gehwolf wrote:
>  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.
> 
> In the case of tSADigestAlg and digestalg, my understanding is that you
> are assuming that they cannot be null because in 8u they are initialized
> upon object construction. I.e.: String digestalg = "SHA-256". However,
> I've seen "if (digestalg != null ..." and "if (tSADigestAlg != null ..."
> statements

Can you be more specific? If I grep for 'digestalg' in the jdk source
tree only usages in jarsigner/Main.java come up and related
Resources.java files. Similar for tSADigestAlg.

>  in 8u which makes me think that this is not necessarily true,
> as if the instance variable can eventually turn null after the object is
> created. Otherwise, those checks would be redundant. I've seen a couple
> of places where the value is updated. Have you ruled out this possibility?

I believe I have. The only place where 'digestalg' is assigned a value
is on line 437 in jarsigner/Main.java:

            } else if (collator.compare(flags, "-digestalg") ==0) {
                if (++n == args.length) usageNoArg();
                digestalg = args[n];

where the value comes from the arguments string passed in via jarsigner
CLI args which cannot be null. The argument for tSADigestAlg is
similar. Line 400 of jarsigner/Main.java reads:

            } else if (collator.compare(flags, "-tsadigestalg") ==0) {
                if (++n == args.length) usageNoArg();
                tSADigestAlg = args[n];

I might have missed some spots, but I think this needs to be viewed in
the context of being used as a CLI tool: jarsigner.

Thanks,
Severin