-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update for OpenSSL 1.1 #875
base: master
Are you sure you want to change the base?
Conversation
Replace all uses of the OpenSSL API with post-1.1 idioms. No effort has been made to retain portability with obsolete versions of OpenSSL. This enables rpki.net to work on systems that still have Python 2.7 but have updated OpenSSL beyond OpenSSL 1.1.
ext/POW.c
Outdated
@@ -10198,7 +10217,9 @@ init_POW(void) | |||
* if you tinker with the build script and start seeing nasty | |||
* memory-related issues, this might be the cause. | |||
*/ | |||
#ifdef notyet | |||
CRYPTO_set_mem_functions(PyMem_Malloc, PyMem_Realloc, PyMem_Free); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From https://man.openbsd.org/CRYPTO_set_mem_functions#DESCRIPTION
DESCRIPTION
Do not use any of the interfaces documented here. They are provided
purely for compatibility with legacy application code.
CRYPTO_get_mem_functions() assigns pointers to the C library functions
malloc(3), realloc(3), and free(3) to those of its arguments that are not
NULL.
CRYPTO_set_mem_functions(), CRYPTO_mem_ctrl(), CRYPTO_mem_leaks(),
CRYPTO_mem_leaks_fp(), and CRYPTO_mem_leaks_cb() have no effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I forgot about this. I also found that documentation you cite.
Note that, unlike LibreSSL, OpenSSL 1.1 still has a working CRYPTO_set_mem_functions()
.
For completeness, here is the Python perspective on this, from https://docs.python.org/3/c-api/memory.html:
In most situations, however, it is recommended to allocate memory from the Python heap specifically because the latter is under control of the Python memory manager. For example, this is required when the interpreter is extended with new object types written in C. Another reason for using the Python heap is the desire to inform the Python memory manager about the memory needs of the extension module. Even when the requested memory is used exclusively for internal, highly-specific purposes, delegating all memory requests to the Python memory manager causes the interpreter to have a more accurate image of its memory footprint as a whole. Consequently, under certain circumstances, the Python memory manager may or may not trigger appropriate actions, like garbage collection, memory compaction or other preventive procedures. Note that by using the C library allocator as shown in the previous example, the allocated memory for the I/O buffer escapes completely the Python memory manager.
This is clearly "encouraging" language, as per the comment.
I am still torn about the best way forward. On the one hand, I feel better about OpenSSL managing its memory without potential interference from Python. On the other hand, I can easily imagine Python getting very confused about memory that isn't under its control. The code does work fine as-is, without using the Python memory allocator. Though I've not done any specific testing to try to catch edge cases.
For what it's worth, everyone currently running rpki.net is using the Python memory allocator... Given that this is very much code in maintenance mode (if not outright suspended animation), I'm leaning towards providing wrappers to OpenSSL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[...] from https://docs.python.org/3/c-api/memory.html [...]
The Python 2.7 manual uses the same language: https://docs.python.org/2.7/c-api/memory.html
Remove the #ifdef notyet fence around CRYPTO_set_mem_functions(). While it it generally preferable to let OpenSSL manage its memory, existing users of rpki.net have been using the Python memory allocator all along.
Correct the check for whether OpenSSL has been compiled with RFC3779 support. v3_addr_validate_path is spelled X509v3_addr_validate_path in OpenSSL 1.1 and later.
Picked up two commits from @sraustein. These add wrappers around the Python memory management functions and update the included OpenSSL distribution for users whose system OpenSSL is not built with RFC 3779 support. What this branch really needs now is more thorough testing in a real-world environment. |
This replaces all uses of the OpenSSL API with post-1.1 idioms. It builds on the preparations put in place by @sraustein several years ago. No effort has been made to retain portability with obsolete versions of OpenSSL. It is clear that those versions of OpenSSL should no longer be used anywhere.
With these changes, rpki.net will work (and hopefully work well!) on systems that still have Python 2.7 (also obsolete) but that have moved beyond OpenSSL version 1.1.
I would very much appreciate help testing these changes. My testing has been largely focussed on unit-testing the Python OpenSSL Wrappers and the relying-party code. The CA components have seen almost no testing from me. This pull request should probably not be merged before at least one other person compiles this branch!