bcrypt_sha256 - #24073
bcrypt_sha256#24073Sjord wants to merge 4 commits into
Conversation
|
|
||
| #define PHP_PASSWORD_DEFAULT PHP_PASSWORD_BCRYPT | ||
| #define PHP_PASSWORD_BCRYPT_COST 12 | ||
| #define PHP_PASSWORD_BCRYPT_SHA256_COST 12 |
There was a problem hiding this comment.
It may be simpler to just reuse PHP_PASSWORD_BCRYPT_COST here.
There was a problem hiding this comment.
It makes sense to me to have a separate define with an adjusted name, but it should just "pass through" to the other one.
|
|
||
| /* If the key is longer than the block size (64), hash it first. Our key is | ||
| * always the 22-byte salt, so this branch is never taken in practice. */ | ||
| if (key_len > 64) { |
| } | ||
|
|
||
| // Unlike bcrypt, a NUL byte is allowed: the HMAC pre-hash removes the quirk. | ||
| var_dump(password_verify("foo\x00bar", password_hash("foo\x00bar", PASSWORD_BCRYPT_SHA256))); |
There was a problem hiding this comment.
Please use the minimal acceptable cost here to keep the test fast.
| // The string ident works too | ||
| $h2 = password_hash("foo", "bcrypt-sha256"); | ||
| var_dump(password_verify("foo", $h2)); |
There was a problem hiding this comment.
I don't think testing this is useful. This is not how the API is supposed to be used.
| <?php | ||
| //-=-=-=- | ||
|
|
||
| $h10 = password_hash("foo", PASSWORD_BCRYPT_SHA256, ["cost" => 10]); |
There was a problem hiding this comment.
Use a smaller cost for performance.
There was a problem hiding this comment.
This test should likely be marked as slow.
| (c >= '0' && c <= '9'); | ||
| } | ||
|
|
||
| static void php_password_hmac_sha256(const unsigned char *key, size_t key_len, |
There was a problem hiding this comment.
We likely should add an internal HMAC API to ext/hash instead of reimplementing it everywhere and to avoid going through the userland API.
| if (len < 82 || len > 83) { | ||
| return false; | ||
| } | ||
| if (memcmp(h, PHP_PASSWORD_BCRYPT_SHA256_PREFIX, PHP_PASSWORD_BCRYPT_SHA256_PREFIX_LEN) != 0) { |
There was a problem hiding this comment.
zend_string_starts_with_literal(hash, PHP_PASSWORD_BCRYPT_SHA256_PREFIX)
| size_t i; | ||
| int c; |
There was a problem hiding this comment.
Reduce the scope as much as possible.
| if (!php_password_bcrypt_sha256_parse(hash, &cost, &salt, &digest)) { | ||
| return true; | ||
| } | ||
| if (options && (znew_cost = zend_hash_str_find(options, "cost", sizeof("cost") - 1)) != NULL) { |
There was a problem hiding this comment.
| if (options && (znew_cost = zend_hash_str_find(options, "cost", sizeof("cost") - 1)) != NULL) { | |
| if (options && (znew_cost = zend_hash_str_find(options, "cost", strlen("cost"))) != NULL) { |
For new code.
|
|
||
| /* Relabel the $2y$ result into the bcrypt-sha256 format. The digest is the | ||
| * last 31 characters of the 60-byte bcrypt output. */ | ||
| out_len = snprintf(out, sizeof(out), "$bcrypt-sha256$v=2,t=2b,r=%" ZEND_LONG_FMT_SPEC "$%s$%s", |
There was a problem hiding this comment.
You can use zend_strpprintf() to directly print into a zend_string.
| return false; | ||
| } | ||
|
|
||
| /* Constant-time comparison of the 31-byte digests. The salt portion of the |
Don't accept leading zeroes on rounds int. Changed bad test vectors to be more like v2 good hashes.
2 participants
No description provided.