On Sat, 14 Jan 2023 12:02:11 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:
>> 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?
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`.
-------------
PR: https://git.openjdk.org/jdk/pull/11976
