[PING] PoC for JDK-4347142: Need method to set Password protection to Zip entries
KUBOTA Yuji
kubota.yuji at gmail.com
Thu Jan 7 15:01:16 UTC 2016
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