[commons-compress] traditional zip encryption patch to come

[Bear Giles <bg...@coyotesong.com>]

We discussed adding support for the traditional zip encryption algorithm a
while back. IIRC the concensus was loosely against it since it's so weak
and we don't want to mislead people.

Since then I can add two points.

First, I heard from someone who said that she has to use the traditional
encryption algorithm for legal reasons. To be more precise she don't have
to use the traditional zip encryption algorithm specifically - she has to
use some form of encryption and for various reasons (e.g., ancient embedded
systems that can't be updated) this is the best she can do. It's enough to
satisfy the legal requirement that the data not be sent in plaintext and
her exposure is limited since it's only used during the brief window when
the embedded systems make a dialup connection to the central server.

In any case it's someone with a good use case for why she needs to continue
using the traditional algorithm.

Second, I've been looking at the strong encryption algorithms (WinZip and
the un-distributable PKWare) and they'll need some way to insert encryption
and decryption into the ZIP streams. The traditional approach may
illustrate problems we can expect with WinZip strong encryption.

My approach is to create two Filter classes -
TraditionalZipEncryptionInputStream and
TraditionalZipEncryptionOutputStream - and inject them at the lowest level.

There are two problems that prevent this from being completely transparent.

First, the ciphertext is precisely 12 bytes larger than the plaintext due
to the presence of an encrypted salt. I know that ZipEntry has a
'compressed size' field that's used with reading and skipping the actual
stream but I don't know if the decompression logic uses an internal EOF
marker or if it uses the number of bytes. This might cause problems and I
don't want to make any assumptions.

Second, the algorithm requires the CRC of the original data. The last byte
of the encrypted salt should be the same as the low byte of the CRC. If not
it's assumed that the provided password is incorrect. (If it matches
there's still a remote chance that it's incorrect but overall we can save a
lot of wasted effort.) This isn't an issue in streaming mode - just use 0 -
but it requires a way to get that value to the constructor when the header
value is not 0.

Thoughts?

Bear

Re: [commons-compress] traditional zip encryption patch to come

[Stefan Bodewig <bo...@apache.org>]

On 2016-02-16, Bear Giles wrote:

> I can create test files. (I still need to create some PKWare test files as
> well...).

This would be good.

> I also need to figure out a way to extract the encrypted content to I
> can verify my implementation of the crypto logic itself. I think that
> will just require disabling any check for the 'encrypted' bit.

Yes, I also think this is correct. There probably isn't anything else
using that bit other than the traditional encryption.

Many thanks!

     Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org

Re: [commons-compress] traditional zip encryption patch to come

[Bear Giles <bg...@coyotesong.com>]

I can create test files. (I still need to create some PKWare test files as
well...). I also need to figure out a way to extract the encrypted content
to I can verify my implementation of the crypto logic itself. I think that
will just require disabling any check for the 'encrypted' bit.

​Source at
https://github.com/beargiles/commons-compress/blob/traditional-encryption/src/main/java/org/apache/commons/compress/archivers/zip/TraditionalZipEncryption.java
.

Test at
https://github.com/beargiles/commons-compress/blob/traditional-encryption/src/test/java/org/apache/commons/compress/archivers/zip/TraditionalZipEncryptionTest.java
​

​Bear​



On Tue, Feb 16, 2016 at 9:08 AM, Stefan Bodewig <bo...@apache.org> wrote:

> On 2016-02-15, Bear Giles wrote:
>
> > We discussed adding support for the traditional zip encryption algorithm
> a
> > while back. IIRC the concensus was loosely against it since it's so weak
> > and we don't want to mislead people.
>
> I'm pretty sure we should ad a big warning about the quality of
> encryption to expect, but there are good reasons to implement decryption
> - and not quite so good reasons for encryption, too :-)
>
> > Second, I've been looking at the strong encryption algorithms (WinZip and
> > the un-distributable PKWare) and they'll need some way to insert
> encryption
> > and decryption into the ZIP streams. The traditional approach may
> > illustrate problems we can expect with WinZip strong encryption.
>
> True. Even though WinZip strong encryption will need a different
> approach as it is a "method" of its own rather than used in addition to
> one of the other methods.
>
> > My approach is to create two Filter classes -
> > TraditionalZipEncryptionInputStream and
> > TraditionalZipEncryptionOutputStream - and inject them at the lowest
> level.
>
> > There are two problems that prevent this from being completely
> transparent.
>
> > First, the ciphertext is precisely 12 bytes larger than the plaintext due
> > to the presence of an encrypted salt. I know that ZipEntry has a
> > 'compressed size' field that's used with reading and skipping the actual
> > stream but I don't know if the decompression logic uses an internal EOF
> > marker or if it uses the number of bytes. This might cause problems and I
> > don't want to make any assumptions.
>
> Without encryption you get both flavors. DEFLATE uses an internal
> marker, STORED uses the number of bytes, for example. From looking
> through appnote I don't see a definitive answer, we should probably look
> at real world archives.
>
> > Second, the algorithm requires the CRC of the original data. The last
> byte
> > of the encrypted salt should be the same as the low byte of the CRC. If
> not
> > it's assumed that the provided password is incorrect. (If it matches
> > there's still a remote chance that it's incorrect but overall we can
> save a
> > lot of wasted effort.) This isn't an issue in streaming mode - just use
> 0 -
> > but it requires a way to get that value to the constructor when the
> header
> > value is not 0.
>
> We may even run into a situation - when writing to a RandomAccessFile -
> where we move back to write the CRC into the header after we've written
> the file's contents. This would likely require us to calculate the CRC
> before actually starting to encrypt the data.
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Re: [commons-compress] traditional zip encryption patch to come

[Stefan Bodewig <bo...@apache.org>]

On 2016-02-15, Bear Giles wrote:

> We discussed adding support for the traditional zip encryption algorithm a
> while back. IIRC the concensus was loosely against it since it's so weak
> and we don't want to mislead people.

I'm pretty sure we should ad a big warning about the quality of
encryption to expect, but there are good reasons to implement decryption
- and not quite so good reasons for encryption, too :-)

> Second, I've been looking at the strong encryption algorithms (WinZip and
> the un-distributable PKWare) and they'll need some way to insert encryption
> and decryption into the ZIP streams. The traditional approach may
> illustrate problems we can expect with WinZip strong encryption.

True. Even though WinZip strong encryption will need a different
approach as it is a "method" of its own rather than used in addition to
one of the other methods.

> My approach is to create two Filter classes -
> TraditionalZipEncryptionInputStream and
> TraditionalZipEncryptionOutputStream - and inject them at the lowest level.

> There are two problems that prevent this from being completely transparent.

> First, the ciphertext is precisely 12 bytes larger than the plaintext due
> to the presence of an encrypted salt. I know that ZipEntry has a
> 'compressed size' field that's used with reading and skipping the actual
> stream but I don't know if the decompression logic uses an internal EOF
> marker or if it uses the number of bytes. This might cause problems and I
> don't want to make any assumptions.

Without encryption you get both flavors. DEFLATE uses an internal
marker, STORED uses the number of bytes, for example. From looking
through appnote I don't see a definitive answer, we should probably look
at real world archives.

> Second, the algorithm requires the CRC of the original data. The last byte
> of the encrypted salt should be the same as the low byte of the CRC. If not
> it's assumed that the provided password is incorrect. (If it matches
> there's still a remote chance that it's incorrect but overall we can save a
> lot of wasted effort.) This isn't an issue in streaming mode - just use 0 -
> but it requires a way to get that value to the constructor when the header
> value is not 0.

We may even run into a situation - when writing to a RandomAccessFile -
where we move back to write the CRC into the header after we've written
the file's contents. This would likely require us to calculate the CRC
before actually starting to encrypt the data.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org