RFR: 8253952: Work around wrong usage of ZipOutputStream.putNextEntry() in user code
### Summary
Work around wrong usage of `ZipOutputStream.putNextEntry()` in user code which
can lead to the `ZipException "invalid
entry compressed size"`.
### Motivation
In general it is not safe to directly write a ZipEntry obtained from
`ZipInputStream.getNextEntry()`,
`ZipFile.entries()`, `ZipFile.getEntry()` or `ZipFile.stream()` with
`ZipOutputStream.putNextEntry()` to a
`ZipOutputStream` and then read the entries data from the `ZipInputStream` and
write it to the `ZipOutputStream` as
follows:
ZipEntry entry;
ZipInputStream zis = new ZipInputStream(...);
ZipOutputStream zos = new ZipOutputStream(...);
while((entry = zis.getNextEntry()) != null) {
zos.putNextEntry(entry);
zis.transferTo(zos);
}
The problem with this code is that the zip file format does not record the
compression level used for deflation in its
entries. In general, it doesn't even mandate a predefined compression ratio per
compression level. Therefore the
compressed size recorded in a `ZipEntry` read from a zip file might differ from
the new compressed size produced by the
receiving `ZipOutputStream`. Such a difference will result in a `ZipException`
with the following message:
java.util.zip.ZipException: invalid entry compressed size (expected 12 but got
7 bytes)
The correct way of copying all entries from one zip file into another requires
the creation of a new `ZipEntry` or at
least resetting of the compressed size field. E.g.:
while((entry = zis.getNextEntry()) != null) {
ZipEntry newEntry = new ZipEntry(entry.getName());
zos.putNextEntry(newEntry);
zis.transferTo(zos);
}
or:
while((entry = zis.getNextEntry()) != null) {
entry.setCompressedSize(-1);
zos.putNextEntry(entry);
zis.transferTo(zos);
}
Unfortunately, there's a lot of user code out there which gets this wrong and
uses the bad coding pattern described
before. Searching for `"java.util.zip.ZipException: invalid entry compressed
size (expected 12 but got 7 bytes)"` gives
~2500 hits (~100 on StackOverflow). It's also no hard to find plenty of
instances of this anti-pattern on GitHub when
doing a code search for `ZipEntry` and `putNextEntry()`. E.g. [Gradle 4.x
wrapper task][1] is affected as well as the
latest version of the [mockableAndroidJar task][2]. I've recently fixed two
occurrences of this pattern in OpenJDK (see
[JDK-8240333][3] and [JDK-8240235][4]) but there still exist more of them (e.g.
[`test/jdk/java/util/zip/ZipFile/CopyJar.java`][5] which is there since 1999 :).
### Description
So while this has clearly been a problem before, it apparently wasn't painful
enough to trigger any action from the
side of the JDK. However, recently quite some zlib forks with [superior
deflate/inflate performance have evolved][6].
Using them with OpenJDK is quite straight-forward: one just has to configure
the alternative implementations by setting
`LD_LIBRARY_PATH` or `LD_PRELOAD` correspondingly. We've seen big saving by
using these new zlib implementations for
selected services in production and the only reason why we haven't enabled them
by default until now is the problem
I've just described. The reason why these new libraries uncover the described
anti-pattern much more often is because
their compression ratio is slightly different from that of the default zlib
library. This can easily trigger a
`ZipException` even if an application is not using a different compression
levels but just a zip file created with
another zlib version.
I'd therefore like to propose the following workaround for the wrong
`ZipOutputStream.putNextEntry()` usage in user
code:
- ignore the compressed size if it was implicitly determined from the zip file
and not explicitly set by calling
`ZipEntry.setCompressedSize()`.
- Change the API-documentation of `ZipOutputStream.putNextEntry()` and
`JarOutputStream.putNextEntry()` to explain the
problem and why `putNextEntry()` will ignore the compressed size of a
`ZipEntry` if that was set implicitely when
reading that entry from a `ZipFile` or `ZipInputStream`.
### Technical Details
A zip file consists of a stream of File Entries followed by a Central Directory
(see [here for a more detailed
specification][7]). Each File Entry is composed of a Local File Header (LFH)
followed by the compressed Data and an
optional Data Descriptor. The LFH contains the File Name and among other
attributes the Compressed and Uncompressed
size and CRC of the Data. In the case where the latter three attributes are not
available at the time when the LFH is
created, this fact will be recorded in a flag of the LFH and will trigger the
creation of a Data Descriptor with the
corresponding information right after the Data section. Finally, the Central
Directory contains one Central Directory
File Header (CDFH) for each entry of the zip archive. The CDFH is an extended
version of the LFH and the ultimate
reference for the contents of the zip archive. The redundancy between LFH and
CDF
RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate
8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate - Commit messages: - 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate Changes: https://git.openjdk.java.net/jdk/pull/528/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=528&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8254081 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/528.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/528/head:pull/528 PR: https://git.openjdk.java.net/jdk/pull/528
Re: RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate
On Tue, 6 Oct 2020 16:00:12 GMT, Rajan Halade wrote:
> 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to
> an expired certificate
test/jdk/java/security/cert/PolicyNode/GetPolicyQualifiers.java line 57:
> 55: params.setRevocationEnabled(false);
> 56: // Certificates expired on Oct 6th, 2020
> 57: params.setDate(new Date("July 01, 2020"));
The Date() constructor is deprecated, would you like to use the replacement
method DateFormat.parse()?
-
PR: https://git.openjdk.java.net/jdk/pull/528
Re: RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate [v2]
> 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to > an expired certificate Rajan Halade has updated the pull request incrementally with one additional commit since the last revision: 8254081: Use DateFormat instead of Date - Changes: - all: https://git.openjdk.java.net/jdk/pull/528/files - new: https://git.openjdk.java.net/jdk/pull/528/files/f8ff3bbc..0de3b2a4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=528&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=528&range=00-01 Stats: 5 lines in 1 file changed: 3 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/528.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/528/head:pull/528 PR: https://git.openjdk.java.net/jdk/pull/528
Re: RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate [v2]
On Tue, 6 Oct 2020 16:21:34 GMT, Xue-Lei Andrew Fan wrote:
>> Rajan Halade has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8254081: Use DateFormat instead of Date
>
> test/jdk/java/security/cert/PolicyNode/GetPolicyQualifiers.java line 57:
>
>> 55: params.setRevocationEnabled(false);
>> 56: // Certificates expired on Oct 6th, 2020
>> 57: params.setDate(new Date("July 01, 2020"));
>
> The Date() constructor is deprecated, would you like to use the replacement
> method DateFormat.parse()?
I have fixed it, thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/528
Re: RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate [v2]
On Tue, 6 Oct 2020 16:29:55 GMT, Rajan Halade wrote: >> 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to >> an expired certificate > > Rajan Halade has updated the pull request incrementally with one additional > commit since the last revision: > > 8254081: Use DateFormat instead of Date Marked as reviewed by mullan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/528
Re: RFR: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate [v2]
On Tue, 6 Oct 2020 16:33:22 GMT, Rajan Halade wrote: >> 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to >> an expired certificate > > Rajan Halade has updated the pull request incrementally with one additional > commit since the last revision: > > 8254081: Use DateFormat instead of Date Marked as reviewed by xuelei (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/528
Integrated: 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate
On Tue, 6 Oct 2020 16:00:12 GMT, Rajan Halade wrote: > 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to > an expired certificate This pull request has now been integrated. Changeset: 54b340b4 Author:Rajan Halade URL: https://git.openjdk.java.net/jdk/commit/54b340b4 Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod 8254081: java/security/cert/PolicyNode/GetPolicyQualifiers.java fails due to an expired certificate Perform backdated validation of test certificate. Reviewed-by: mullan, xuelei - PR: https://git.openjdk.java.net/jdk/pull/528
Re: RFR: 8156071: List.of: reduce array copying during creation
On Tue, 6 Oct 2020 05:07:37 GMT, Tagir F. Valeev wrote:
>> Sorry to be late to the party. I thought that all reviews labeled with
>> core-libs should be mirrored to core-libs-dev
>> mailing list but I haven't seen it there :(
>> Please note that the integrated implementation exposes listFromTrustedArray
>> to everybody. No dirty unsafe reflection is
>> necessary, only single unchecked cast:
>> static List untrustedArrayToList(T[] array) {
>> @SuppressWarnings("unchecked")
>> Function, List> finisher =
>> (Function, List>)
>> Collectors.toUnmodifiableList().finisher();
>> ArrayList list = new ArrayList<>() {
>> @Override
>> public Object[] toArray() {
>> return array;
>> }
>> };
>> return finisher.apply(list);
>> }
>>
>> This might be qualified as a security issue.
>
> This could be fixed by adding a classword check to the finisher, like this:
>
>list -> {
> if (list.getClass() !=
> ArrayList.class) {
> throw new
> IllegalArgumentException();
> }
> return (List)
> SharedSecrets.getJavaUtilCollectionAccess()
>
> .listFromTrustedArray(list.toArray());
>},
Thanks for pointing this out. I've filed bug
[JDK-8254090](https://bugs.openjdk.java.net/browse/JDK-8254090). I think
we're ok as long as this gets fixed before JDK 16 ships.
I think the notification messages for this did end up on core-libs-dev, but
perhaps there were some email delays over
the weekend.
-
PR: https://git.openjdk.java.net/jdk/pull/449
Re: RFR: 8153005: Upgrade the default PKCS12 encryption/MAC algorithms
On Fri, 2 Oct 2020 19:07:20 GMT, Weijun Wang wrote:
>> test/jdk/sun/security/mscapi/VeryLongAlias.java line 51:
>>
>>> 49: public static void main(String[] args) throws Throwable {
>>> 50:
>>> 51: // Using the old algorithms to make sure the file is recognized
>>
>> Do we also want to have a test that uses the new algorithms?
>
> I only know Windows Server 2019 can accept the new algorithms.
Ok, but maybe we can split this test in two and use the jtreg @requires tag to
run the newer algorithms on Windows
Server 2019? It would be a useful test if this is the only test where we test
PKCS12 interop with Windows.
-
PR: https://git.openjdk.java.net/jdk/pull/473
Re: RFR: 8153005: Upgrade the default PKCS12 encryption/MAC algorithms
On Tue, 6 Oct 2020 18:34:34 GMT, Sean Mullan wrote: >> I only know Windows Server 2019 can accept the new algorithms. > > Ok, but maybe we can split this test in two and use the jtreg @requires tag > to run the newer algorithms on Windows > Server 2019? It would be a useful test if this is the only test where we test > PKCS12 interop with Windows. OK. Or I can see if there is an existing method in test/lib that can detects the version. - PR: https://git.openjdk.java.net/jdk/pull/473
