code review request: 7023056: NPE from sun.security.util.ManifestEntryVerifier.verify during Maven build

[Sean Mullan]

Hi Max,

The fix looks good. I suggest you also remove this comment in ManifestEntryVerifier:

198         // MANIFEST.MF should not be skipped. It has signers.

and add a similar comment to JarSigner.doneWithMeta.

Also, in the test, I think you should add a try/finally clause and close the 2 
InputStreams.

--Sean

On 3/24/11 9:12 AM, Weijun Wang wrote:
> Hi Sean
>
> This is a regression made by my former treat-MANIFEST.MF-as-signed code change.
> Webrev here:
>
> http://cr.openjdk.java.net/~weijun/7023056/webrev.00/
>
> For the reason, see the evaluation below.
>
> === *Description* ================================================
> Running a Maven build of Glassfish sources fails using JDK 7.
>
> java.lang.NullPointerException
> at java.util.Hashtable.remove(Hashtable.java:474)
> at sun.security.util.ManifestEntryVerifier.verify(ManifestEntryVerifier.java:226)
>
> === *Evaluation* =================================================
> This is a regression made by
> 7004035: signed jar with only META-INF/* inside is not verifiable.
>
> The jar verification has always been done in two steps:
>
> 1. verify the signature of SF file against its BLOCK file. This generates a map
> of entry name vs its possible signers, saved in sigFileSigners. This is
> performed only once for each SF file.
>
> 2. verify the digest of each entry. Each time an entry gets verified, its
> possible signers are moved to verified signers, saved in another map --
> verifiedSigners. For each entry, this step should be performed exactly once, by
> looking at if it has a digest line and if it's still inside sigFileSigners.
>
> Since 7004035, the MANIFEST.MF file is treated as signed. But it's a very
> special signed entry:
>
> it has no digest line in itself or any SF file
>
> Therefore, in step 2 above for this file, we ignore the skip flag and always try
> the move (well, it has to be moved once to be treated as signed). This triggers
> an inconsistency: a name argument for ManifestEntryVerifier.verify() method is
> set to null to trigger the skip flag, but we ignore it, and then there comes a
> stage when this argument is used, and NPE.
>
> Solution: MANIFEST.MF is special so we treat it specially, by moving its
> possible signers to verifiedSigners as soon as the META-INF entries are
> processed, that is, in the JarFile.doneWithMeta() method. Back to
> ManifestEntryVerifier.verify(), we'll keep revert to the old behavior to always
> honoring the skip flag. In fact, this flag will be always true for the
> MANIFEST.MF entry.
>
> Thanks
> Max
>