PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries

[Xueming Shen]

Yasumasa

It's a tricky call. To be honest, as I said at the very beginning, I'm not sure whether
or not it's a good idea and worth the efffort to push this into the j.u.zip package to
support the "traditional PKWare encryption", which is known to be "seriously flawed,
and in particular is vulnerable to known-plaintext attacks" (from wiki), while I fully
understood it is not a concern in your "problem-free" use scenario. Just wonder
if there is anyone else on the list that has/had the need for such encryption feature
in the past. It would be preferred to have more input (agree, disagree) to make the
final decision.

Here are some comments regarding the proposed implementation,

(1) The changes in native code are probably no longer needed. The native path is
      left there for vm directly access.
(2) I'm concerned about the footprint increase for the ZipEntry class (the proposed
      change is adding 3 more fields into the entry class). Given this is something rarely
      needed/used, and we might have hundreds and thousands of zip/jar entries during
      startup/runtime, it might be desired to avoid adding these fields into ZipEntry class.
      A possible alternative to have a dedicated entry class extended from the ZipEntry
      to hold those extra fields and methods, EncryptionZipEntry for example. So the
      proposed encryption support code will not have any impact to the existing use of
      ZipInput/OutputStream/File.
(3) I'm also not comfortable to add the "encryption engine" logic into the in/deflater
      stream classes, while it might be convenient to do so from implementation point
      of view. Given the encryption is something between the raw bites in the zip file
      and the in/deflating layer, it might need an extra layer there, though I'm not sure
      if it's easy to do so.

The webrev below is what I think might be better for the ZipFile and ZipOutputStream
to have an extra layer in between to handle the encryption. Not try to test if it really
works, just throw in the idea for discussion.

http://cr.openjdk.java.net/~sherman/zencrypt/webrev/

No, I did not try the ZIS, the "pushback" stream there might be a little tricky:-)

-Sherman

On 03/28/2016 02:34 AM, Yasumasa Suenaga wrote:
> PING: Could you review it?
> We want to move forward this enhancement.
>
> Thanks,
>
> Yasumasa
> 2016/03/19 22:01 "Yasumasa Suenaga"<yasuenag at gmail.com>:
>
>> Hi Alan,
>>
>>> I think the main issue here is to decide whether the API
>>> should be extended for encryption or not.
>> We've discussed on the premise that we add the API for supporting ZIP
>> encryption.
>> In this context, Sherman tried to implement AES encryption extending
>> current API. [1]
>>
>> IMHO, ZIP encryption should be implemented as extention of current ZIP API
>> because encryption/decryption will process for each ZIP entries.
>>
>> According to Developers' Guide, guideline for adding new API is TBD. [2]
>> What should I do next?
>>
>>
>> Thanks,
>>
>> Yasumsa
>>
>>
>> [1]
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/037903.html
>> [2] http://openjdk.java.net/guide/changePlanning.html#api
>>
>>
>> On 2016/03/19 0:25, Alan Bateman wrote:
>>> On 18/03/2016 15:02, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> We (including me and Yuji Kubota (ykubota: OpenJDK jdk9 Author))
>> discussed
>>>> about this issue from Nov. 2015. [1]
>>>> We heard several comments and we applied them to our patch.
>>>>
>>>> I have not heard new comment for our latest patch.
>>>> So I send review request for it. Could you review it?
>>>>
>>>>     webrev:
>>>>       http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/
>>>>
>>>>     Usage of new API:
>>>>
>> http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/Test.java
>>> Yasumasa - I think the main issue here is to decide whether the API
>>> should be extended for encryption or not. If exposing it in the API make
>>> sense then I assume that alternative names to java.util.zip.ZipCryption
>>> needs to be explored.
>>>
>>> -Alan.
>>>