RFR: 8339280: jarsigner -verify performs cross-checking between CEN and LOC [v15]
Hai-May Chao
hchao at openjdk.org
Thu Mar 27 02:21:10 UTC 2025
On Wed, 26 Mar 2025 23:26:12 GMT, Weijun Wang <weijun at openjdk.org> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Update with comments from Sean and Weijun
>
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line 1215:
>
>> 1213: if (!cenEntries2.equals(locEntries)) {
>> 1214: crossChkWarnings.add(rb.getString(
>> 1215: "entries.mismatch.when.comparing.jarfile.and.jarinputstream"));
>
> Do we still need this warning? The meaning is not clear to me. Since we have already compared in both ways, does this only mean the orders are different?
This step checks content and order. As the order does matter, I have this step to explicitly warn about ordering issue.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties line 219:
>
>> 217: entry.1.present.in.jarfile.but.unreadable=Entry %s is present in JarFile but unreadable
>> 218: codesigners.different.for.entry.1.when.reading.jarfile.and.jarinputstream=Code signers are different for entry %s when reading from JarFile and JarInputStream
>> 219: entry.1.has.codesigners.in.jarfile.but.not.in.jarinputstream=Entry %s has codesigners in JarFile but not in JarInputStream
>
> Usually we don't say "has codesigners" or "has no codesigners", we say "is signed" and "is not signed". Same for the next one.
Change them to:
Entry %s is signed in JarFile but is not signed in JarInputStream
Entry %s is signed in JarInputStream but is not signed in JarFile
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties line 222:
>
>> 220: entry.1.has.codesigners.in.jarinputstream.but.not.in.jarfile=Entry %s has codesigners in JarInputStream but not in JarFile
>> 221: entries.mismatch.when.comparing.jarfile.and.jarinputstream=Entries mismatch when comparing JarFile and JarInputStream
>> 222: jar.contains.internal.inconsistencies.may.result.in.different.contents.when.reading.via.jarfile.and.jarinputstream=This JAR file contains internal inconsistencies that may result in different contents when reading via JarFile and JarInputStream
>
> Do you think it makes sense to add a ":" at the end of this header line?
Yes.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties line 224:
>
>> 222: jar.contains.internal.inconsistencies.may.result.in.different.contents.when.reading.via.jarfile.and.jarinputstream=This JAR file contains internal inconsistencies that may result in different contents when reading via JarFile and JarInputStream
>> 223: signature.verification.failed.on.entry.1.when.reading.via.jarinputstream=Signature verification failed on entry %s when reading via JarInputStream
>> 224: signature.verification.failed.on.entry.1.when.reading.via.jarfile.inputstream=Signature verification failed on entry %s when reading via JarFile InputStream
>
> I don't think you need to mention `InputStream` for the "JarFile" case.
The entry is read using an `InputStream` from `JarFile`, and is not directly via `JarFile`. So I added `InputStream`. Remove it to be more consistent with other warnings.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015407996
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015408034
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015408114
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015408205
More information about the security-dev
mailing list