Conversation
salted-sha1 and SSHA512 are ($p.$s) XSHA and XSHA512 are ($s.$p)
solardiz
left a comment
There was a problem hiding this comment.
Not a full review, but I skimmed.
Where are these hashes used? Why specifically 1024 iterations? Do we need default iteration counts at all, or should we require the field?
This is optionally salted ($s.$p) and optionally iterated. Canonical ciphertext: $sisha1$iter$<salt><hash> where salt length can be 1-16 bytes and inferred from total length (note: no field delimiter). Raw ciphertext: The last of the ciphertext is always the SHA-1 hash and anything before that is the salt. 4 bytes salt defaults to 1 iteration (this is XSHA). Other salt lengths default to 1024 iterations. No salt still means 1024 iterations but you can force this format to do single raw sha1 using $sisha1$1$<hash> because, well, why not?
f146d09 to
e9ee5d0
Compare
|
I still would like to know what the motivation was for adding this, and why the 1024 default. We also need a NEWS entry. |
|
I think it started out in some CTF but the idea now was to have a flexible format (btw the dynamic format can't do iterations). The 1024 for other lengths was arbitrary. Maybe always default to 1 (not iterated) is more logical? Then it's like raw-sha1 (when no salt) and XSHA (salted) would be supported without any special exception (and it's obsolete anyway I guess). On another note I toyed with the idea we could have a |
Ideally, we'd have more maintainable code for dynamic format, so that we could reasonably add iterations to there, and also support many different ways iterations can be implemented (e.g., binary vs. hex as input to each iteration). But that's currently unrealistic - that code is too messy as it is.
I think there should be no default iteration count. Just refuse to work without it specified. I suggest that you revise the code added here to remove any default iteration counts.
I think we shouldn't have that. It's more confusing and error-prone than useful. |
|
I think the special case for length 48 makes with 1 iteration makes sense. It's syntax we used in the XSHA format anyway, so it's good the OpenCL format would be suggested as a possible alternative. On CPU and with auto-detection, will the XSHA or the iterated format be picked up by default for those hashes? Which one is faster? We want the faster one to be the default. |
|
Turns out the old XSHA format is uppercase (only). And actually it's way faster than this CPU format, as it's more optimized.
This new one (if made supporting uppercase), then Tiger and only then XSHA. A problem with running this OpenCL format for XSHA is that the internal mask will default to not being used (target 0). You need to use Perhaps we should simply not accept any bare ciphertexts. |
|
Oh, uppercase and Tiger...
Well, or we could require uppercase for those 48 hex strings. Can we set a mask-internal-target upon seeing the first one of those in XSHA on GPU is pretty much the only immediate use case for this PR's additions, I think. So it feels unreasonable not to support it well. |
That's not a bad idea. I will look into this. Also check what xsha format pot entries look like. |
Add iterated-sha1 format(s). Optionally salted ($s.$p) and optionally iterated. Actually they can even do raw-sha1 if formatted right!