On Wed, 18 Jan 2023 09:51:35 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:

>> 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.

When introducing the call to isInMetaInf in isSigningRelated, I accidentally 
broke the matching of MANIFEST.MF and SIG-* files.

When fixing this regression, I now match against the full path instead of the 
existing prefix stripping substring. (A nice side effect of this is that 
isBlockOrSF is now always called with the full path)

Since the regression was not caught by any existing test, I'm also adding a 
sanity check that a basic signed JAR has the expected sections in MANIFEST.MF. 
(The regression introduced a section for META-INF/MANIFEST.MF which seemed to 
not be caught by tests)

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

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

Reply via email to