On Wed, 12 Jan 2022 21:57:22 GMT, Sean Mullan <mul...@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`.
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.
src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java line
272:
> 270: }
> 271:
> 272: // Gets the permitted algs for the signers of this entry.
This can probably be another `computeIfAbsent`.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7056
