bpo-9216: Expose OpenSSL FIPS_mode() as _hashlib.get_fips_mode() #19703
Conversation
|
Original patch: stratakis@5fa9620 |
|
This should stay private, in |
RHEL has two more patches:
|
|
I modified my PR to return a boolean rather directly returning FIPS_mode() integer value. |
|
It may be interesting to log /proc/sys/crypto/fips_enabled in test.pythoninfo. |
test.pythoninfo logs OpenSSL FIPS_mode() and Linux /proc/sys/crypto/fips_enabled in a new "fips" section. Co-Authored-By: Petr Viktorin <encukou@gmail.com>
|
This PR contains no documentation nor NEWS entry on purpose: _hashlib.get_fips_mode() must be private. |
|
When you're done making the requested changes, leave the comment: |
| // then the function will return 0 with an error code of | ||
| // CRYPTO_R_FIPS_MODE_NOT_SUPPORTED (0x0f06d065)." | ||
| // But 0 is also a valid result value. | ||
| unsigned long errcode = ERR_peek_last_error(); |
There was a problem hiding this comment.
This will fail with other libraries that don't clean the OpenSSL last_error after themselves.
(Apparently that is normal in the OpenSSL world.)
There was a problem hiding this comment.
I added a call to ERR_clear_error() at the function entry point: does it solve the issue that you described?
There was a problem hiding this comment.
I don't know :(
Christian might.
There was a problem hiding this comment.
Yes, this will ensure that there is no error left on the error stack and the function only fails when FIPS_mode() puts an error on the error stack.
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @tiran: please review the changes made to this pull request. |
Copy/paste mistake!
|
I added ERR_clear_error() call: @tiran, @encukou, @stratakis: Would you mind to review my updated PR? It only adds a private _hashlib.get_fips_mode() method with no documentation. |
|
The patch looks OK: all the issues I had with the original implementation are addressed. But I don't feel qualified to review this – I could just point out what security experts pointed out to me before. |
| // then the function will return 0 with an error code of | ||
| // CRYPTO_R_FIPS_MODE_NOT_SUPPORTED (0x0f06d065)." | ||
| // But 0 is also a valid result value. | ||
| unsigned long errcode = ERR_peek_last_error(); |
There was a problem hiding this comment.
Yes, this will ensure that there is no error left on the error stack and the function only fails when FIPS_mode() puts an error on the error stack.
test.pythoninfo logs OpenSSL FIPS_mode() and Linux
/proc/sys/crypto/fips_enabled in a new "fips" section.
Co-Authored-By: Petr Viktorin encukou@gmail.com
https://bugs.python.org/issue9216
Automerge-Triggered-By: @tiran