On Thu, May 28, 2020 at 10:22 PM Niels Möller nisse@lysator.liu.se wrote:
If I get it right, the parameters log2rounds and salt can either be provided as explicit arguments, or derived from the scheme string. In some way, it would be clenaer with one function taking all parameters as explicit arguments (without any scheme string to parse at all), and a different one taking it all from the scheme string. Do we need anything in the middle, like parsing log2rounds from the scheme string, but the salt as a separate binary string?
Cleaner, yes. Easier to use, no. If we split it up, it becomes cleaner, but people using the interface will have more work to do and can mess things up (more than now). So, in this case I would prefer ease of use.
It's unusual in the Nettle API to use NUL-terminated strings, we usually
pass length and data. E.g., the pbkdf2 functions take the input password as size_t key_length, const uint8_t *key, even though it's usually an ascii string. It's not clear what's best here. Will the algorithm break down if we let it process a key input with NUL character in the middle?
I don't think it will break down, but I'd need to verify the code. I understand that this would keep it consistent with the rest of the Nettle conventions. I'll see what I can do.
The length input (first argument) seems redundant, since its only use is to check that it's >= BLOWFISH_BCRYPT_HASH_SIZE. When the sie really is fixed, it's better to document that and drop that argument.
Yes, this can be done. But here I specifically chose to do it this way so that anyway needing to program to the interface has to look up less reference docs (more consistent with the other Nettle conventions). But if you insist, I can change this, of course.
If you would like to make some easy progress, you could split out the patch to create blowfish-internal.h and declare _nettle_blowfish_encround in that file. That has no api implications and would reduce the size of the main patch.
I'll see what I can do.