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

Eirik Bjorsnos duke at openjdk.org
Wed Jan 18 09:56:28 UTC 2023


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

>> This duplicated check annoyed me also, but the existing checks have different behavior:
>> 
>> - JarVerifier.beginEntry normalizes the path to uppercase, them checks that it starts with "META-INF/" or "/META-INF/"
>> - JarSigner.sign0 does not normalize to uppercase , then checks that the path starts with "META-INF/"
>> 
>> Introducing a common method would need change behaviour of one of these methods. This change of behaviour would not be relevant to the bug being fixed in this PR.
>> 
>> Since I'm cautious of changing behaviour, I decided to keep the two methods.
>
> While `JarSigner` has not normalize to uppercase, the check is the same. As for `/META-INF/`, it must be very broken now since `JarFile::maybeInstantiateVerifier` is using `JUZFA.getManifestName(this,true)` to read the manifest and since `ZipFile` cannot see the signature-related files starting with `/META-INF/` this method will always return null.

I agree this deserves a cleanup, and since we're already deep into it it can make sense to fix this in this PR.

I have introduced SignatureFileVerifier.isInMetaInf as a common method to replace duplicated logic in JarVerifier and  JarSigner. I also noticed that SignatureFileVerifier.isSigningRelated performs the same check, so I updated that method to also call the isInMetaInf.

This introduces two subtle behavioural changes:

- JarSigner is relaxed to ignore case when checking isInMetaInf, meaning "meta-inf/" files will now be moved to the beginning of the signed JAR
- JarVerifier is tightened to reject /META-INF/ as not in META-INF/. This is believed to have little practical impact, since ZipFile.Source.isMetaName already rejects such paths.

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

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



More information about the security-dev mailing list