RFR: 8300140: ZipFile.isSignatureRelated returns true for files in META-INF subdirectories [v7]

[Eirik Bjorsnos]

On Tue, 17 Jan 2023 16:46:28 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> I started there, but ran into some problems:
>> 
>> SignatureFileVerifier.isSigningRelated calls isBlockOrSF, but it removes the "META-INF/" prefix from the path first. So we can't assume that input to isBlockOrSF is the full path.
>> 
>> I could update SignatureFileVerifier.isSigningRelated to send the full path, but we still have another problem:
>> 
>> JarSigner.sign0 puts META-INF/ files in a vector, such that it can output them first. We want to update this method such that it outputs only files which live directly in META-INF/ first. So we still need to check for "directness" outside isBlockOrSF.
>> 
>> isBlockOrSF has 8 call sites, most of them in security sensitive and tricky code. In the end, I felt safer leaving isBlockOrSF alone and just fix the bug-relevant call sites instead. Also, being a new contributor to security-dev, I wanted to keep the PR relatively simple and easy to review.
>> 
>> WDYT?
>
> In almost every call to `isBlockOrSF`, there is already an `isInMetaInf` (or equivalent) call right before it so it's always safe. The only exception is in `jarsigner`'s `verifyJar` method:
> 
>                     hasSignature = hasSignature
>                             || SignatureFileVerifier.isBlockOrSF(name);
> 
> and it makes a subtle difference. The `unrelated-signature-files-modified.jar` your test created has SF/block files inside a sub-directory. Try verifying it with jarsigner. Here `hasSignature` is true but the SF/block is not considered signature-related, an error message will show `Signature is either not parsable or not verifiable`. If `isBlockOrSF` is modified to return false, it will be simply `jar is unsigned`.

Good catch!

I've added a guarding call to SignatureFileVerifier.isSigningRelated here, similar to the earlier call site of isBlockOrSf in this methid. I've also added a new check to the test to verify that there is no WARNING when calling jarsigner with the modified jar.

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

PR: https://git.openjdk.org/jdk/pull/11976