-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add H3C/Huawei/HPE hash format #5784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thank you for the contribution @SamuraiOcto! Can you please add test vectors for the maximum supported password lengths (with and without SIMD)? I wonder if this hash is implementable as a dynamic format. If so, maybe we'd need to switch this format to a "thin" wrapper around dynamic, as we already have in a few formats. |
Is it possible to include the null bytes with a dynamic format, like |
We could, but why "need"? This one doesn't look too bad. Perhaps we could avoid using scalar buffers for a small boost if we really wanted to. |
Oh, I used the wrong word. We certainly can accept this as a separate format as well. I just thought we could prefer to reduce code size / duplication at some point. Formats like these have rather simple logic, but much code shared with other formats. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some comments, but really those can be addressed separately.
len = strspn(ciphertext, BASE64_ALPHABET); | ||
if (len != CIPHERTEXT_LENGTH) | ||
return 0; | ||
return 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we have similar logic in sha512_common_valid_nsldap
, and in both places we don't fully check that the base64 encoding is valid (=
only used as padding). We might want to enhance both with a test base64_convert
run and checking its return value. A way to do that may be to have get_binary
return NULL if base64_convert
fails to provide the expected length data, call get_binary
from valid
and check its return value. Since we would also be fixing the pre-existing code, this (or at least that part) should be a separate commit (may also be separate PR).
Oh, I don't know. Maybe not. Good point. |
Did the two test vectors here come from actual devices? |
True. Another possibility could be to amend the Anyway, format code like this current PR is the nicest start for implementing an OpenCL version. Easy to follow and easy to move valid/binary/salt to shared code. And a big problem with dynamic is it doesn't have an active maintainer. |
I tried doing this in dynamic compiler format just for the hell of it (re-encoding the Base64 into hex manually), but it failed with no clue as to why (possibly the NUL constant - or maybe \xHH isn't even supported but taken as-is, there's very little documentation). I then looked into doing it with the config style dynamic but the various docs contradict each other so I gave up. In that case \xHH is said to be supported but it doesn't mention \x00 so that is an unknown already. |
I didn't look into an improved Base64 check but I think we can merge this now. |
I did and because of that I noticed that
Only the first one, the others are calculated by Python. |
@SamuraiOcto are you planning to add an OpenCL version as well, or should I put that on my to-do list? |
I'm not very familiar with OpenCL so I wasn't planning on it. So if that's something that you'd want to work on then by all means go ahead. |
AIX 7.1
Also, the |
I have fixes for these pending. And it turns out we have the |
Fixes pushed. Also for stricter valid(), as it previously would accept any trailing string, and |
This adds support for hashes with
$h$6$
prefix that are used in various H3C/Huawei/HPE network devices. The SHA512-based format is described in this hashcat issue and this PR.