Bug 1611784
| Summary: | libxcrypt: crypt no longer returns NULL on failure, but "*0" | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Jesse Haber-Kucharsky <jhaberku> | ||||
| Component: | libxcrypt | Assignee: | Björn Esser (besser82) <besser82> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
| Severity: | unspecified | Docs Contact: | |||||
| Priority: | unspecified | ||||||
| Version: | 28 | CC: | besser82, codonell, fweimer, jhaberku | ||||
| Target Milestone: | --- | ||||||
| Target Release: | --- | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | libxcrypt-4.1.1-4.fc28 | Doc Type: | If docs needed, set a value | ||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2018-08-11 19:12:01 UTC | Type: | Bug | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 1613537 | ||||||
| Attachments: |
|
||||||
(In reply to Jesse Haber-Kucharsky from comment #0) > Both the `$2y$` and `$2a$` schemes being supported, as they were in the past. Which past are you referring to? An earlier version of Fedora (27?), or a previous version of libxcrypt? (In reply to Florian Weimer from comment #1) > (In reply to Jesse Haber-Kucharsky from comment #0) > > Both the `$2y$` and `$2a$` schemes being supported, as they were in the past. > > Which past are you referring to? An earlier version of Fedora (27?), or a > previous version of libxcrypt? Earlier versions of Fedora, going back to (I believe) 26. My understanding is that libxcrypt replaced the glibc implementation at some point, though I'm not sure if that is relevant to this change in behavior or not. I simply cannot get your reproducer to run. Here's an attempt on Fedora 26: $ gcc -O2 -Wall -D_GNU_SOURCE t.c -lcrypt $ ./a.out $2y$: Failed with `Invalid argument` $2a$: Failed with `Invalid argument` $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` $6$: Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` $ rpm -qf /lib64/libcrypt.so.1 libcrypt-nss-2.25-13.fc26.x86_64 So you really need to provide additional information about the environment where this works. If I go back to Red Hat Enterprise Linux 6, I get this: $2y$: Got `$2RjvfkKIZaTI` $2a$: Got `$2RjvfkKIZaTI` $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` $6$: Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` But the first two aren't bcrypt hashes, they are DES hashes with the salt "q2". You can make such hashes work again by replacing '$' with 'q'. The old crypt implementation accepted characters outside the specified character set, mapping them to some other character value. (In reply to Florian Weimer from comment #3) > I simply cannot get your reproducer to run. Here's an attempt on Fedora 26: > > $ gcc -O2 -Wall -D_GNU_SOURCE t.c -lcrypt > $ ./a.out > $2y$: Failed with `Invalid argument` > $2a$: Failed with `Invalid argument` > $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` > $6$: Got > `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/ > D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` > $ rpm -qf /lib64/libcrypt.so.1 > libcrypt-nss-2.25-13.fc26.x86_64 > > So you really need to provide additional information about the environment > where this works. > > If I go back to Red Hat Enterprise Linux 6, I get this: > > $2y$: Got `$2RjvfkKIZaTI` > $2a$: Got `$2RjvfkKIZaTI` > $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` > $6$: Got > `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/ > D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` > > But the first two aren't bcrypt hashes, they are DES hashes with the salt > "q2". You can make such hashes work again by replacing '$' with 'q'. The > old crypt implementation accepted characters outside the specified character > set, mapping them to some other character value. Thank you for the additional information, Florian. It helped me to understand the false assumption I was making. In the reproducer attached, if the following line is inserted prior to the `if` block: printf("Got `%s`\n", e); Then these are the results on: - Fedora 26: Got `(null)` $2y$: Failed with `Invalid argument` Got `(null)` $2a$: Failed with `Invalid argument` Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` $6$: Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` - Fedora 27 Got `(null)` $2y$: Failed with `Invalid argument` Got `(null)` $2a$: Failed with `Invalid argument` Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` $6$: Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` - Fedora 28 Got `*0` $2y$: Failed with `Invalid argument` Got `*0` $2a$: Failed with `Invalid argument` Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` $6$: Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` Notice that on previous versions of Fedora, the returned pointer is `NULL` when the algorithm is not supported. This is consistent with the current man page for `crypt` on Fedora 28: On success, a pointer to the encrypted password is returned. On error, NULL is returned. However, in Fedora 28, on error the string `"*0"` is returned. Thus, I was mistaken in my original assertion (and premise) that bcrypt was previously supported but not currently. This is likely a documentation rather than implementation bug. What is the best course of action as far as this bug report is concerned? Can it be re-titled? Carlos, what do you think about the behavior change in libxcrypt? I think it was introduced by Solar Designer to avoid NULL pointer crashes, which were a problem at one point. One more note that may be helpful.
The info pages for `crypt` describe the behavior in 28, and contradict the man page:
Some implementations return `NULL` on failure, and others return an
_invalid_ hashed passphrase, which will begin with a `*` and will
not be the same as SALT.
(In reply to Florian Weimer from comment #5) > Carlos, what do you think about the behavior change in libxcrypt? I think > it was introduced by Solar Designer to avoid NULL pointer crashes, which > were a problem at one point. I think libxcrypt should be returning NULL. We want user code to fail early and loudly. This is not the kind of interface where we should return something which might be confused with a valid result. I was never a fan of the "*<something>" result. I would prefer crashes to incorrect program operation. (In reply to Carlos O'Donell from comment #7) > (In reply to Florian Weimer from comment #5) > > Carlos, what do you think about the behavior change in libxcrypt? I think > > it was introduced by Solar Designer to avoid NULL pointer crashes, which > > were a problem at one point. > > I think libxcrypt should be returning NULL. We want user code to fail early > and loudly. This is not the kind of interface where we should return > something which might be confused with a valid result. I was never a fan of > the "*<something>" result. I would prefer crashes to incorrect program > operation. I think the original motivation was people uploading .htpasswd files with corrupt salts/parameters (something that was only possible after extensions past DES were introduced, before that everything was accepted) and thereby crashing the web server in a login attempt. Should we make the change in Fedora and downstream and revert to the glibc behavior? (In reply to Florian Weimer from comment #8) > (In reply to Carlos O'Donell from comment #7) > > (In reply to Florian Weimer from comment #5) > > > Carlos, what do you think about the behavior change in libxcrypt? I think > > > it was introduced by Solar Designer to avoid NULL pointer crashes, which > > > were a problem at one point. > > > > I think libxcrypt should be returning NULL. We want user code to fail early > > and loudly. This is not the kind of interface where we should return > > something which might be confused with a valid result. I was never a fan of > > the "*<something>" result. I would prefer crashes to incorrect program > > operation. > > I think the original motivation was people uploading .htpasswd files with > corrupt salts/parameters (something that was only possible after extensions > past DES were introduced, before that everything was accepted) and thereby > crashing the web server in a login attempt. > > Should we make the change in Fedora and downstream and revert to the glibc > behavior? Yes, can we also file an upstream ticket also? Can we ask libxcrypt to return NULL instead of '*0"? Crashing the webserver is certainly a DoS, but you need to have access to the system the webserver is running on and you need have local .htpasswd support turned on, and your server has to not be fixed. Apache trunk has: support/passwd_common.c 228 cbuf = crypt(pw, salt); 229 if (cbuf == NULL) { 230 rv = APR_FROM_OS_ERROR(errno); 231 ctx->errstr = apr_psprintf(ctx->pool, "crypt() failed: %pm", &rv); 232 ret = ERR_PWMISMATCH; 233 break; 234 } That is at least 5 years old (rev 1395253) modules/lua/lua_passwd.c 123 cbuf = crypt(pw, salt); 124 if (cbuf == NULL) { 125 rv = APR_FROM_OS_ERROR(errno); 126 ctx->errstr = apr_psprintf(ctx->pool, "crypt() failed: %pm", &rv); 127 ret = ERR_PWMISMATCH; 128 break; 129 } Likewise in 2.4.x. I think you get yourself in more trouble returning "*0" with application code no ready for an invalid hash to be returned. Ah! I see there is an upstream ticket attached. Thanks! libxcrypt-4.1.1-3.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-b393b60802 libxcrypt-4.1.1-4.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-b393b60802 libxcrypt-4.1.1-4.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report. |
Created attachment 1472801 [details] Reproducer Description of problem: The implementation of the `crypt.h` header provided by Fedora previously supported the Blowfish encryption algorithm in the `crypt` function (indicated with a `$2a$` prefix). With Fedora 28, the function's behavior has changed: it returns "*0" and sets `errno` to 22 when this prefix is used. Version-Release number of selected component (if applicable): - Fedora 28 (Workstation edition) - libxcrypt (4.1.0, 1.fc28, x86_64) How reproducible: Please see the attached C source file which reproduces the observed behavior. Steps to Reproduce: 1. Compile the attached source file (`salt.c`): gcc -std=c11 -Wall -Wextra -pedantic salt.c -o salt -lcrypt 2. Execute the program: $ ./salt Actual results: $2y$: Failed with `Invalid argument` $2a$: Failed with `Invalid argument` $5$: Got `$5$aaaabbbbccccdddd$itDgeJQSL4W3BHZ2Bi0rQ1Igzi3C/qg9QauzcnYI8k5` $6$: Got `$6$aaaabbbbccccdddd$wQh87anrgaL9BYj7EVzezHiMF/D1XJNeF1mnN8kh4jAwvtHI9GrGwzE8Hr./CcilCWTZkYeI0QcDuw6LYW5or1` Expected results: Both the `$2y$` and `$2a$` schemes being supported, as they were in the past.