[PING] PoC for JDK-4347142: Need method to set Password protection to Zip entries

Wed Jan 6 13:45:07 UTC 2016

Hi Stephen,

Thank you for your note. After discussion about the design, we will
re-implement API. Please check it, and give feedback us when the time
comes :)

Thanks,
Yuji

2016-01-06 19:27 GMT+09:00 Stephen Colebourne <scolebourne at joda.org>:
> Just to note that I once needed this and even commented on Stack Overflow
> about it:
> http://stackoverflow.com/questions/166340/write-a-password-protected-zip-file-in-java
>
> I'd just note that setting the password on each entry is a bit painful, as
> you'd normally expect to just set the password once. But having the feature
> in the JDK is better than not. Having pluggable encryption will I suspect
> be useful though, because the encryption used will no doubt change over
> time.
>
> Stephen
>
>
> On 5 January 2016 at 22:10, Xueming Shen <xueming.shen at oracle.com> wrote:
>
>> Yuji,
>>
>> I'm not convinced that the ZipCryption is a public interface we'd like to
>> expose,
>> at least for now, given the proprietary nature of the "strong encryption"
>> defined
>> by PKWARE.
>>
>> As I said in my previous email, it might be desired to hide the
>> "passwd/encryption"
>> support for the "traditional zip encryption" as an implementation detail.
>>
>> Looking at the existing methods that deal with "entry" in ZipFile,
>> ZipInputStream
>> and ZipOutputStream,
>>
>> ZipFile.getInputStream(ZipEntry e);
>> ZipOutputStream.putNextEntry(ZipEntry e);
>> ZipInputStream.getNextEntry();
>>
>> it appears that instead of adding "password" specific method to these
>> classes
>> directly, it might be more appropriate to extend the ZipEntry class for
>> such
>> "password" functionality. For example, with a pair of new methods
>>
>> boolean ZipEntry.isTraditionalEncryption().
>> void ZipEntry.setTraditionalEncryption(String password);
>>
>> The encryption support should/can be added naturally/smoothly with
>> ZipFile.getInputStream(e), ZipInputstream and
>> ZipOutputStream.putNextEntry(e),
>> with no extra new method in these two classes. The implementation checks
>> the flag (bit0, no bit 6) first and then verifies the password, as an
>> implementation
>> details.
>>
>> For ZipFile and ZipInputStream, we can add note to the api doc to force the
>> invoker to check if the returned ZipEntry indicates it's an encrypted
>> entry. If yes,
>> it must to set the appropriate password to the returned ZipEntry via
>> ZipEntry.setTraditionalEncryption(password); before reading any byte from
>> the
>> input stream.
>>
>> Again, we should not have any "encryption" related public field/method in
>> DeflaterOutputStream/InflaterInputStream. Ideally these two classes really
>> should
>> not be aware of "encryption" at all.
>>
>> -Sherman
>>
>>
>>
>> On 01/04/2016 06:26 AM, KUBOTA Yuji wrote:
>>
>>> Hi Sherman and all,
>>>
>>> Happy new year to everyone!
>>>
>>> Please let know your feedback about this proposal. :-)
>>>
>>> Thanks,
>>> Yuji
>>>
>>> 2015-12-21 22:38 GMT+09:00 KUBOTA Yuji<kubota.yuji at gmail.com>:
>>>
>>>> Hi Sherman,
>>>>
>>>> 2015-12-20 16:35 GMT+09:00 Xueming Shen<xueming.shen at oracle.com>:
>>>>
>>>>> It is no longer necessary to touch the native code (zip_util.c/h) after
>>>>> the
>>>>> native ZipFile implementation has been moved up to the java level. Those
>>>>> native code are for vm access only now, which I dont think care about
>>>>> the
>>>>> password support at all.
>>>>>
>>>> Thanks for your information. We do not take care the native.
>>>>
>>>> I discussed with Yasumasa, and our thought is as below.
>>>>
>>>> (1) what's the benefit of exposing the public interface ZipCryption? the
>>>>> real
>>>>> question is whether or not this interface is good enough for other
>>>>> encryption
>>>>> implementation to plugin their implementation to support the
>>>>> ZipFile/Input/
>>>>> OutputStream to their encryption spec.
>>>>>
>>>> We aimed that the public interface ZipCryption supports the
>>>> extensibillity for other encrypt engine. The JDK core libs developers
>>>> have to implementation ZipyCryption only. If not provide, the JDK
>>>> developers must implement ZipStream/Entry by JDK API to design the
>>>> data structure of entry.
>>>> If you want to use binary key data such as PKI, you can implement new
>>>> encrypt/decrypt engine by ZipCryption interface.
>>>> So we think we should provide this interface to be clearly how to
>>>> implement a new engine, e.g., cipher algorithm, cipher strength and
>>>> converting the header, etc.
>>>>
>>>> (2) it seems like it might be possible to hide most of the implementation
>>>>> and only expose the "String password" (instead of the ZipCryption) as
>>>>> the
>>>>> public interface to support the "traditional" encryption. This depends
>>>>> on the
>>>>> result of (1) though.
>>>>>
>>>> Thanks for your clues. We think the string password at first. However,
>>>> we should also create a new binary interface given we support PKI in
>>>> the future.
>>>>
>>>> (3) I'm concerned of pushing ZipCryption into
>>>>> InflaterInputStream/DeflaterOutputStream.
>>>>> It might be worth considering to replace the ZipCryption implementation
>>>>> with
>>>>> a pair of FilterOutput/InputStream. It would be easy and reasonable to
>>>>> use
>>>>> the FilterOutputStream for the ZipOutputStream and the
>>>>> FilterInputStream for the
>>>>> ZipFile. The PushbackInputStream in ZipInputStream might be an issue ...
>>>>>
>>>> Thanks for your clues, too. Honestly speaking, we think the current
>>>> zip implementation may break the data when used PushbackInputStream
>>>> for the following reasons.
>>>>
>>>> * PushbackInputStream uses an unique internal buffer for re-read
>>>> operation.
>>>> * But, InflaterInputStream provide date to Inflater per reads and
>>>> buffer by JNI (zlib).
>>>> * So we think PushbackInputStream is poor compatibility with
>>>> InflaterInputStream.
>>>>
>>>> We generally use InputStream through ZipEntry#getInputStream(). We do
>>>> not touch FileInputStream for reading ZIP data. If we call unread()
>>>> when we use PushbackInputStream as reading ZIP archive, we guess that
>>>> it will break the reading data.
>>>> So, our approach do not affect the PushbackInputStream.
>>>> What do you think about this?
>>>>
>>>> (4) It seems the ZipOutputStream only supports the "stream based"
>>>>> password, while
>>>>> the ZipInputStream  supports the "entry based" password. Do we really
>>>>> need
>>>>> "entry based" support here?
>>>>>
>>>> As your suggestion, we should support "entry based". We will start to
>>>> implement "entry based" after this discussion is closed.
>>>>
>>>> Thanks,
>>>> Yuji
>>>>
>>>> On 12/17/15, 9:45 PM, Yasumasa Suenaga wrote:
>>>>>
>>>>>> Hi Jason,
>>>>>>
>>>>>> Thank you for your comment.
>>>>>> I've fixed it in new webrev:
>>>>>>    http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.03/
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2015/12/17 0:33, Jason Mehrens wrote:
>>>>>>
>>>>>>> The null check of 'entry' at line 351 of ZipFile.getInputStream  is in
>>>>>>> conflict with line 350 and 348.
>>>>>>>
>>>>>>> ________________________________________
>>>>>>> From: core-libs-dev<core-libs-dev-bounces at openjdk.java.net>  on
>>>>>>> behalf of
>>>>>>> Yasumasa Suenaga<yasuenag at gmail.com>
>>>>>>> Sent: Wednesday, December 16, 2015 8:47 AM
>>>>>>> To: Sergey Bylokhov; Xueming Shen
>>>>>>> Cc: core-libs-dev at openjdk.java.net
>>>>>>> Subject: Re: [PING] PoC for JDK-4347142: Need method to set Password
>>>>>>> protection to Zip entries
>>>>>>>
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> Thank you for your comment.
>>>>>>>
>>>>>>> I added that description in new webrev:
>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.02/
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2015/12/16 22:19, Sergey Bylokhov wrote:
>>>>>>>
>>>>>>>> Should the new methods describe how they will work in case of null
>>>>>>>> params?
>>>>>>>>
>>>>>>>> On 16/12/15 16:04, Yasumasa Suenaga wrote:
>>>>>>>>
>>>>>>>>> I adapted this enhancement after JDK-8145260:
>>>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.01/
>>>>>>>>>
>>>>>>>>> Could you review it?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2015/12/12 21:23, Yasumasa Suenaga wrote:
>>>>>>>>>
>>>>>>>>>> Hi Sherman,
>>>>>>>>>>
>>>>>>>>>> Our proposal is affected by JDK-8142508.
>>>>>>>>>> We have to change ZipFile.java and and ZipFile.c .
>>>>>>>>>> Thus we will create a new webrev for current (after 8142508)
>>>>>>>>>> jdk9/dev
>>>>>>>>>> repos.
>>>>>>>>>>
>>>>>>>>>> Do you have any comments about current webrev?
>>>>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.00/
>>>>>>>>>>
>>>>>>>>>> If you have comments, we will fix them in new webrev.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2015/12/03 16:51, KUBOTA Yuji wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Sherman,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for your quick response :)
>>>>>>>>>>>
>>>>>>>>>>> I aimed to implement the "traditional" at this proposal by the
>>>>>>>>>>> below
>>>>>>>>>>> reasons.
>>>>>>>>>>>
>>>>>>>>>>>     * We want to prepare API for encrypted zip files at first.
>>>>>>>>>>>       * Many people use the "traditional" in problem-free scope
>>>>>>>>>>> like a
>>>>>>>>>>> temporary file.
>>>>>>>>>>>     * We do not know which implementation of the "stronger" is
>>>>>>>>>>> best
>>>>>>>>>>> for
>>>>>>>>>>> openjdk.
>>>>>>>>>>>       * PKWare claims that they have patents about the "stronger"
>>>>>>>>>>> on
>>>>>>>>>>> Zip[1].
>>>>>>>>>>>       * OTOH, WinZip have the alternative implementation of the
>>>>>>>>>>> "stronger" [2][3].
>>>>>>>>>>>     * Instead, we prepared the extensibility by ZipCryption
>>>>>>>>>>> interface
>>>>>>>>>>> to
>>>>>>>>>>> implement other encrypt engine, such as the AES based.
>>>>>>>>>>>
>>>>>>>>>>> Thus, I think this PoC should support the "traditional" only.
>>>>>>>>>>> In the future, anyone who want to implement the "stronger" can
>>>>>>>>>>> easily
>>>>>>>>>>> add their code by virtue of this proposal.
>>>>>>>>>>>
>>>>>>>>>>> [1] https://pkware.cachefly.net/webdocs/APPNOTE/APPNOTE-6.3.3.TXT
>>>>>>>>>>>        (1.4 Permitted Use&  7.0 Strong Encryption Specification)
>>>>>>>>>>>
>>>>>>>>>>> [2]
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> https://en.wikipedia.org/wiki/Zip_(file_format)#Strong_encryption_controversy
>>>>>>>>>>>
>>>>>>>>>>> [3] http://www.winzip.com/aes_info.htm
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Yuji
>>>>>>>>>>>
>>>>>>>>>>> 2015-12-03 12:29 GMT+09:00 Xueming Shen<xueming.shen at oracle.com>:
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Yuji,
>>>>>>>>>>>>
>>>>>>>>>>>> I will take a look at your PoC.  Might need some time and even
>>>>>>>>>>>> bring
>>>>>>>>>>>> in the
>>>>>>>>>>>> security guy
>>>>>>>>>>>> to evaluate the proposal. It seems like you are only interested
>>>>>>>>>>>> in
>>>>>>>>>>>> the
>>>>>>>>>>>> "traditional PKWare
>>>>>>>>>>>> decryption", which is, based on the wiki, "known to be seriously
>>>>>>>>>>>> flawed, and
>>>>>>>>>>>> in particular
>>>>>>>>>>>> is vulnerable to known-plaintext attacks":-) Any request to
>>>>>>>>>>>> support
>>>>>>>>>>>> "stronger" encryption
>>>>>>>>>>>> mechanism, such as the AES based?
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Sherman
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 12/2/15 6:48 PM, KUBOTA Yuji wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>
>>>>>>>>>>>>> We need reviewer(s) for this PoC.
>>>>>>>>>>>>> Could you please review this proposal and PoC ?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Yuji
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2015-11-26 13:22 GMT+09:00 KUBOTA Yuji<kubota.yuji at gmail.com>:
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> * Sorry for my mistake. I re-post this mail because I sent
>>>>>>>>>>>>>> before
>>>>>>>>>>>>>> get
>>>>>>>>>>>>>> a response of subscription confirmation of core-libs-dev.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Our customers have to handle password-protected zip files.
>>>>>>>>>>>>>> However,
>>>>>>>>>>>>>> Java SE does not provide the APIs to handle it yet, so we must
>>>>>>>>>>>>>> use
>>>>>>>>>>>>>> third party library so far.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Recently, we found JDK-4347142: "Need method to set Password
>>>>>>>>>>>>>> protection to Zip entries", and we tried to implement it.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The current zlib in JDK is completely unaffected by this
>>>>>>>>>>>>>> proposal.
>>>>>>>>>>>>>> The
>>>>>>>>>>>>>> traditional zip encryption encrypts a data after it is has been
>>>>>>>>>>>>>> compressed by zlib.[1] So we do NOT need to change existing
>>>>>>>>>>>>>> zlib
>>>>>>>>>>>>>> implementation.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> We've created PoC and uploaded it as webrev:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.00/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>         Test code is as below. This code will let you know how
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> PoC
>>>>>>>>>>>>>> works.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.00/Test.java
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In NTT, a Japanese telecommunications company. We are providing
>>>>>>>>>>>>>> many
>>>>>>>>>>>>>> enterprise systems to customers. Some of them, we need to
>>>>>>>>>>>>>> implement to
>>>>>>>>>>>>>> handle password-protected zip file. I guess that this proposal
>>>>>>>>>>>>>> is
>>>>>>>>>>>>>> desired for many developers and users.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'm working together with Yasumasa Suenaga, jdk9 committer
>>>>>>>>>>>>>> (ysuenaga).
>>>>>>>>>>>>>> We want to implement it if this proposal accepted.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> [1]:
>>>>>>>>>>>>>> https://pkware.cachefly.net/webdocs/APPNOTE/APPNOTE-6.3.3.TXT
>>>>>>>>>>>>>> (6.0  Traditional PKWARE Encryption)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Yuji
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>
>>