On Thu, 13 Jan 2022 19:54:44 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> 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.
Or maybe the key/signature algorithm is weak and ignored. I was only thinking
it's not worth calculating the hash anymore. Of course there will be a behavior
change. If there's a hash mismatch, it used to be a security exception, but if
ignored, it's just a plain unsigned entry.
>> 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?
Maybe, I don't know how problematic if lambda is used this early.
Anyway, I still prefer moving the update of `permittedAlgs` here. Updating it
inside the method seems too faraway.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7056
