[PING] PoC for JDK-4347142: Need method to set Password protection to Zip entries
KUBOTA Yuji
kubota.yuji at gmail.com
Mon Feb 1 09:23:21 UTC 2016
Hi Sherman and all,
Could you please let know your thought and the past case about AES?
Thanks,
Yuji
2016-01-08 0:01 GMT+09:00 KUBOTA Yuji <kubota.yuji at gmail.com>:
> Hi Sherman,
>
> Thank you for sharing!
>
> 2016-01-07 4:04 GMT+09:00 Xueming Shen <xueming.shen at oracle.com>:
>> The reason that I'm not convinced that we really need a public interface of
>> ZipCryption here
>> is I don't know how useful/helpful/likely it would be going forward that
>> someone might really
>> use this interface to implement other encryption(s), especially the pkware
>> proprietary one,
>> I doubt it might be not that straightforward.
>
> In this proposal, we aim to support "traditional" because most people need it
> in secure environment. BTW, could you please share the reason why you did
> not support WinZip AES? Do you have a plan to support in the future?
>
> If you can share the reason, we want to decide the way of implementation with
> consideration for your information. I think we can implement by two
> way as below.
>
> 1. Implementing by reference to
> http://cr.openjdk.java.net/~sherman/zipmisc/ZipFile.java
> This is good simply API. If we need to implement other encryption(s),
> try to refactor it.
>
> 2. Implementing with a package private interface of ZipCryption for next step.
> This has two problems as your advice.
>
> We agree with that the "encryption" and "compression" should be
> separated logically.
> However, current implementation compress the encrypted data, and buffering it.
> It is too tightly-coupled, so we need refactoring to separate the
> managing buffer
> processing and the stream processing of InflaterInputStream /
> DeflaterOutputStream.
>
> About "push back the bytes belong to next entry", we think
> InflaterInputStream.originBuf
> of our PoC do not required the needed info. Do this implements have problem?
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.00/src/java.base/share/classes/java/util/zip/InflaterInputStream.java.cdiff.html
>
> Thanks,
> Yuji
>
>> In fact I did have a draft implementation that supports WinZip AES about 5-6
>> years ago :-)
>> (which also supports compression methods bzip and lzma, btw) Here is the
>> top class, It appears
>> a general interface might not be that helpful and it might be ideal to
>> simply implement it inside
>> the JDK, as what is proposed here, when it's really desired.
>>
>> http://cr.openjdk.java.net/~sherman/zipmisc/ZipFile.java
>>
>> It is a ZipFile based implementation, so it does not have the headache that
>> ZipInputStream has,
>> such as to push back the bytes belong to next entry, since the loc might not
>> have the needed
>> info regarding the size/csize in stream mode.
>>
>> From abstract point of view. The "encryption" and "compression" are
>> different layers, it would
>> be ideal to have them in separate classes logically, instead of mixing the
>> encryption into
>> compression. Sure, it might be convenient and probably may have better
>> performance to mix
>> them in certain use scenario, but the "encryption" should never appear in
>> the public interface
>> of those compression classes. Package private interface should be fine, if
>> have to.
>>
>> -Sherman
>>
>>
>>>
>>> 2016-01-06 7:10 GMT+09:00 Xueming Shen <xueming.shen at oracle.com>:
>>>>
>>>> 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);
>>>
>>> Thanks advice, I agree. We should re-design the API to extend the
>>> ZipEntry class.
>>>
>>>> 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.
>>>
>>> Agree. For this proposal, we aim to support only traditional
>>> encryption. So I think we should also check bit 6.
>>>
>>>> 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.
>>>
>>> Yes, we have to add note the flow of codes to the JavaDoc.
>>>
>>>> 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.
>>>
>>> Agree, but I think we might be faced technical difficulty about a
>>> processing between zlib and the internal buffer of InflaterInputStream
>>> / DeflaterOutputStream. Please give us time to implement.
>>>
>>>> -Sherman
>>>
>>> Thanks,
>>> Yuji
>>>
>>>
>>>> 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
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>
More information about the core-libs-dev
mailing list