On Fri, 13 Jan 2023 22:31:28 GMT, Weijun Wang <wei...@openjdk.org> wrote:

>> Some call sites of SignatureFileVerifier.isBlockOrSF fails to check that 
>> files reside in META-INF directly, and not in a subdirectory of META-INF.
>> 
>> The mentioned call sites needs updates to check and ignore such files.
>> 
>> A new test VerifyUnrelatedSignatureFiles is added which verifies that [*.SF, 
>> *.RSA] files in META-INF/ subdirectories are indeed ignored.
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 1748:
> 
>> 1746:                 .isBlockOrSF(new String(name, off, len, UTF_8.INSTANCE)
>> 1747:                     .toUpperCase(Locale.ENGLISH)));
>> 1748: 
> 
> How about updating `SignatureFileVerifier.isBlockOrSF` so that it only 
> returns true for files inside `META-INF/`. This way it's consistent to this 
> method.

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?

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

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

Reply via email to