LibreSSL support #1679
LibreSSL support #1679
Conversation
LibreSSL no longer uses compression in ssl.h, so the case that was formerly activated by defining OPENSSL_NO_COMP is now the default, and COMP_METHOD isn't defined (it's defined in comp.h, but that's no longer included by ssl.h). In order to make all the type definitions here line up with what's actually in LibreSSL's ssl.h, define COMP_METHOD as void. This definition is still compatible with the later type declaration in ssl.py: typedef ... COMP_METHOD;
Some features added to newer OpenSSL versions are absent in LibreSSL, so don't mark these as present if LIBRESSL_VERSION_NUMBER is defined.
LibreSSL aims to be source-compatible with OpenSSL, so there is no good reason to fail this test simply because the name has changed.
This changelog entry represents the previous three commits, which all fix compile- or test-time problems running against LibreSSL.
|
This PR looks on-ball to me. Will wait to see how the tests look. |
|
I'll retest jenkins once it finishes failing. We can't run with |
|
Test FAILed. |
|
retest this please |
|
Test PASSed. |
|
@reaperhulk besides the final reviewer, are there blockers infrastructure wise to merging this? |
|
Once we're ready to merge we need to add the libre config to the cryptography-master job (it's only on pr-experimental at the moment). That's the only infra item left. Do we want anyone else to weigh in or is this ready? |
| */ | ||
| #ifdef LIBRESSL_VERSION_NUMBER | ||
| #define COMP_METHOD void | ||
| #endif |
There was a problem hiding this comment.
Should compression related be in CONDITIONAL?
There was a problem hiding this comment.
Yeah, although that starts to bleed into #1675 😀
I'll move this so we can merge today.
There was a problem hiding this comment.
Sorry, move what?
On Thu, Feb 19, 2015 at 2:53 PM, Paul Kehrer [email protected]
wrote:
In src/cryptography/hazmat/bindings/openssl/ssl.py
#1679 (comment):@@ -7,6 +7,14 @@
INCLUDES = """
#include <openssl/ssl.h>+/* LibreSSL has removed support for compression, and with it the
- * COMP_METHOD use in ssl.h. This is a hack to make the function types
- * in this code match those in ssl.h.
- */
+#ifdef LIBRESSL_VERSION_NUMBER
+#define COMP_METHOD void
+#endifYeah, although that starts to bleed into #1675
#1675 [image: 😀]I'll move this so we can merge today.
—
Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/pull/1679/files#r25019164.
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084
There was a problem hiding this comment.
Move the COMP_METHOD definition.
|
Besides that one question, this LGTM. |
|
Test FAILed. |
|
Test PASSed. |
|
Thanks! I've tested the latest master with tox on my OpenBSD -current system, and all tests pass with both python2.7 and python3.4 (just thought I'd confirm in case there were differences with LibreSSL portable). Out of curiosity, I ran the pyOpenSSL test suite against this version, and got 3 failures and 6 errors (out of 397 tests). I've uploaded the failed test output in case anyone is interested in the details. I may look at fixing those if I have time, but pyOpenSSL is working well enough for my real-world use case (the |
This PR replaces #1673, #1674, and #1636.
Huge thanks to @AnchorCat for the hard work. I've adapted the cryptodev and @Sp1l's EGD work to be a conditional binding. (Be aware that with EGD downstream consumers like pyOpenSSL may not work. I haven't tested.).
I've also added a libre builder to jenkins so if we choose to merge this we'll need to rebase all other PRs (and, conversely, if we choose to not merge this I need to remove that builder).