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