On 1/6/16 5:25 AM, KUBOTA Yuji wrote:
Hi Sherman,

Thank you for your feedback!

At first, I'd like to confirm your thought about ZipCryption to
prevent misunderstand.
By following reasons, we should remove (or modified) ZipCryption to
not have any "encryption" related field/method in DeflaterOutputStream
/ InflaterInputStream.

  1. For security, to hide "passwd/encryption" support by not just
adding "password" specific method.
  2. To avoid that JDK API developers implement "passwd/encryption"
through ZipCryption.

Is it right? In my previous email, I thought we should have the public
interface to provide scalability for supporting any open-source
encryption implementation such as WinZip AES.
If the reason is right, your suggestion sounds reasonable to us as below.

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 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.s...@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.y...@gmail.com>:
Hi Sherman,

2015-12-20 16:35 GMT+09:00 Xueming Shen<xueming.s...@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-boun...@openjdk.java.net>  on behalf
of
Yasumasa Suenaga<yasue...@gmail.com>
Sent: Wednesday, December 16, 2015 8:47 AM
To: Sergey Bylokhov; Xueming Shen
Cc: core-libs-dev@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.s...@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.y...@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



Reply via email to