RFR: 8278851: Correct signer logic for jars signed with multiple digestalgs

[Sean Mullan]

On Thu, 13 Jan 2022 16:55:08 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> If a JAR is signed with multiple digest algorithms and one of the digest algorithms is disabled, `ManifestEntryVerifier.verify()` was incorrectly returning null indicating that the jar entry has no signers. 
>> 
>> This fixes the issue such that an entry is considered signed if at least one of the digest algorithms is not disabled and the digest match passes. This makes the fix consistent with how multiple digest algorithms are handled in the Signature File. This also fixes an issue in the `ManifestEntryVerifier.getParams()` method in which it was incorrectly checking the algorithm constraints against all signers of a JAR when it should check them only against the signers of the entry that is being verified. 
>> 
>> An additional cache has also been added to avoid checking if the digest algorithm is disabled more than once for entries signed by the same set of signers.
>
> src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java line 211:
> 
>> 209:         }
>> 210: 
>> 211:         CodeSigner[] entrySigners = sigFileSigners.get(name);
> 
> What if we return here if `entrySigners == null`? It seems the hash comparison will be skipped, but at the end there is no difference in `verifiedSigners`.

The algorithm constraints check will be skipped (because `permittedAlgs` will be `null`) but the hash check will not be skipped.

I don't think `null` would be returned in a normal case. The only case I can think of is if there was an entry in the Manifest, but no corresponding entry in the SF file.  I suppose we could still do a constraints check as if there were no signers. However, it doesn't seem that useful since technically the entry is not protected by the signature and the hash check is still done, unless I am missing something.

> src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java line 230:
> 
>> 228:                         params = new JarConstraintsParameters(entrySigners);
>> 229:                     }
>> 230:                     if (!checkConstraints(digestAlg, permittedAlgs, params)) {
> 
> Can we move the `permittedAlgs::put` call from inside the `checkConstraints` method to here? You can even call `computeIfAbsent` to make the intention clearer.

Yes, I can do that. However, I'm a bit wary of using lambdas in this code which may get exercised early in app startup. WDYT?

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

PR: https://git.openjdk.java.net/jdk/pull/7056