Skip to content

ZIP formats: Honor 64-bit data length#5715

Merged
magnumripper merged 2 commits intoopenwall:bleeding-jumbofrom
magnumripper:zip-64-bit-len
Mar 29, 2025
Merged

ZIP formats: Honor 64-bit data length#5715
magnumripper merged 2 commits intoopenwall:bleeding-jumbofrom
magnumripper:zip-64-bit-len

Conversation

@magnumripper
Copy link
Member

This includes changing shared OpenCL HMAC-SHA1 to support this.

Closes #5712

Copy link
Member

@solardiz solardiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried we could be introducing performance impact for other uses of the HMAC - but probably not since it'd get inlined and the compiler would see that the actually passed value is a uint? Still, have you run any benchmarks or looked at kernel binary size changes or register number changes in builds of other kernels that use this HMAC?

Copy link
Member

@solardiz solardiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is not so easy at all. hmac_sha1 passes data_len into SHA1_Update, which currently also only accepts a uint. And there's the USE_DATA_BUF code version which also has uint blocks.

@magnumripper magnumripper marked this pull request as draft March 24, 2025 21:46
@magnumripper
Copy link
Member Author

Back to the drawing board

@magnumripper
Copy link
Member Author

magnumripper commented Mar 26, 2025

So I fixed the opencl_sha1_ctx.h as well. Tests on nvidia, compared to current bleeding:

ZIP itself: Binary size grew with 289 bytes. Kernel used one (1) more register. Performance varies a bit but probably didn't change.

DMG uses the HMAC-SHA1. Binary size grew from 1577956 to 1671130 (!?). A stack frame grew from 9360 to 9376 bytes but other than that I see no difference in ptxas output. Performance didn't change.

GPG uses only the affected SHA1-CTX. Binary size grew from 1340541 to 1434672 (!?). The kernel uses one (1) more register. Performance didn't change.

Those sizes growing with 100KB are puzzling.

Next is to create a test archive and see if this is worthwhile at all.

EDIT:
Tried encfs as well. Binary size grew from 1979962 to 2126108, even more than the others. One of its stack frames shrinked by 32 bytes, no other difference in ptxas output. Speed varies but doesn't seem affected.

Also, no warnings or problems with an all-formats test.

@magnumripper
Copy link
Member Author

Next is to create a test archive and see if this is worthwhile at all.

That was harder than expected, I thought I could do it with 7z but apparently it can only produce pkzip archives. I'll see if I can find some Windows luser willing to produce a sample file for me.

@ghost
Copy link

ghost commented Mar 26, 2025

Next is to create a test archive and see if this is worthwhile at all.

That was harder than expected, I thought I could do it with 7z but apparently it can only produce pkzip archives. I'll see if I can find some Windows luser willing to produce a sample file for me.

Looks like I managed to create such a file using Ubuntu (if I've understood all the requirements), but john itself can't load the file (it is too much to my poor hardware):

$ head -c 128 teste.hash 
myfile.zip/myfile:$zip2$*0*3*0*96e664ad0e2d714ea7246303831fd088*3f4e*14018fd26*4ddf34229f02c2760fe80f89727fd2beb0af378ea06eaa3c9

$ john teste.hash 
Morto # ==> (it means killed, I guess)

$ file myfile.zip 
myfile.zip: Zip archive data, at least v4.5 to extract, compression method=AES Encrypted

Even after allocating 17 G (VIRT field from top).

@magnumripper
Copy link
Member Author

Looks like I managed to create such a file using Ubuntu (if I've understood all the requirements), but john itself can't load the file (it is too much to my poor hardware):

Thanks! From the look of it I'd suspect the OOM killer got you. How did you create the file?

@ghost
Copy link

ghost commented Mar 26, 2025

Looks like I managed to create such a file using Ubuntu (if I've understood all the requirements), but john itself can't load the file (it is too much to my poor hardware):

Thanks! From the look of it I'd suspect the OOM killer got you. How did you create the file?

Straightforward. Using the Nautilus GUI.

head -c 5G </dev/urandom >myfile
# GUI, right click, ...
# 2john

@magnumripper
Copy link
Member Author

The OpenCL format, with this fix, works. Curiously enough the CPU format doesn't but that's a separate issue.

@magnumripper magnumripper marked this pull request as ready for review March 28, 2025 01:55
@solardiz
Copy link
Member

Those sizes growing with 100KB are puzzling.

To avoid this, how about we use the same approach as we use with HMAC_MSG_TYPE, etc. - SHA1_DATA_LENGTH_TYPE that would default to uint and be overridden to uint64_t only in formats where we need that?

@magnumripper magnumripper marked this pull request as draft March 28, 2025 22:46
@magnumripper magnumripper changed the title zip-opencl: Honor 64-bit data length ZIP formats: Honor 64-bit data length Mar 28, 2025
This includes adding 64-bit length support to shared HMAC-SHA1 and
in turn the CTX version of SHA1.

Closes openwall#5712
This was the only thing needed to get the ZIP CPU format to crack huge
archives - the format code was already correct.

Closes openwall#5728
@magnumripper
Copy link
Member Author

Those sizes growing with 100KB are puzzling.

To avoid this, how about we use the same approach as we use with HMAC_MSG_TYPE, etc. - SHA1_DATA_LENGTH_TYPE that would default to uint and be overridden to uint64_t only in formats where we need that?

Good idea, did that.

I also added support for 64-bit length in CPU side shared HMAC_SHA1 and HMAC_SHA-2. That shouldn't lead to any problem or performance regression but I did an all-formats -test=0 for good measure.

Both ZIP formats now crack a real > 4GB archive. This is ready to review.

@magnumripper magnumripper marked this pull request as ready for review March 28, 2025 23:13
@solardiz
Copy link
Member

I also added support for 64-bit length in CPU side shared HMAC_SHA1 and HMAC_SHA-2. That shouldn't lead to any problem or performance regression

There may be a performance regression in 32-bit builds, but I guess that's fine these days. Or we could use unsigned long or ARCH_WORD there, relying on data sizes larger than what fits in 32 bits not fitting in the address space on 32-bit anyway.

@magnumripper
Copy link
Member Author

magnumripper commented Mar 29, 2025

There may be a performance regression in 32-bit builds, but I guess that's fine these days. Or we could use unsigned long or ARCH_WORD there, relying on data sizes larger than what fits in 32 bits not fitting in the address space on 32-bit anyway.

I'd rather change it to just size_t. I must still be exhausted because I can't even find the scalar SHA1 in our tree. Are we somehow still using openssl? Sourcing sha.h does a #define SHA1_Update sph_sha1 and in turns sources sph_sha1.h which actually has it as size_t, in void sph_sha1(void *cc, const void *data, size_t len);. But that latter function is not in our tree. What's up with that?

@magnumripper
Copy link
Member Author

But that latter function is not in our tree. What's up with that?

Apparently I'm actually running libcrypto. That is openssl, right? Why do we have sph_sha1.h at all?

@magnumripper
Copy link
Member Author

There may be a performance regression in 32-bit builds, but I guess that's fine these days

BTW the underlying SHA1, whichever one we use, likely has it as 64-bit anyway - at least until we implement our own scalar SHA1 for CPU (I thought we had one). So I doubt there can be any measurable performance regression.

@magnumripper magnumripper merged commit ac6a02d into openwall:bleeding-jumbo Mar 29, 2025
39 checks passed
@magnumripper magnumripper deleted the zip-64-bit-len branch March 29, 2025 07:33
@solardiz
Copy link
Member

I'd rather change it to just size_t.

I agree, size_t would be appropriate.

Sourcing sha.h does a #define SHA1_Update sph_sha1 and in turns sources sph_sha1.h which actually has it as size_t, in void sph_sha1(void *cc, const void *data, size_t len);. But that latter function is not in our tree. What's up with that?

There's some preprocessor magic to define sph_sha1, from sha1.c:

#define RFUN   sha1_round
#define HASH   sha1
#define BE32   1
#include "md_helper.c"

magnumripper added a commit to magnumripper/john that referenced this pull request Mar 29, 2025
Recent commit had it as uint64_t.  This change means 32-bit hosts
won't support ZIP files of 4 GiB or larger. They will be rejected
with a warning with hints to use a 64-bit build or the zip-opencl
format for loading them.

See openwall#5715
magnumripper added a commit to magnumripper/john that referenced this pull request Mar 29, 2025
Recent commit had it as uint64_t.  This change means 32-bit hosts
won't support ZIP files of 4 GiB or larger. They will be rejected
with a warning, hinting to use a 64-bit build or the zip-opencl
format for loading them.

See openwall#5715
magnumripper added a commit to magnumripper/john that referenced this pull request Mar 30, 2025
magnumripper added a commit that referenced this pull request Mar 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zip-opencl comp_len truncation from 64 to 32 bits

2 participants