-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Expose a few more OpenSSL functions that are useful for DTLS support #6138
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
Conversation
|
...This thing where each CI run has to be re-approved again is pretty annoying. I guess feel free to add me to the org temporarily? I promise that the only way I will abuse my power would be to kick myself out again ASAP. |
|
An easier move, more compatible with how we manage permissions generally, would be if you could send some other small PR (docs fix, typo, etc.) and then we could merge that, freeing this up to be run CI jobs without our intervention. |
it's probably not very effective
|
Well, I guess I'll keep an eye out for typos then? :-) In the mean time, I think it should pass now, so probably it will only take another 3 attempts... |
|
Well would you look at that. I guess this is ready for review then. |
| #endif | ||
| #if CRYPTOGRAPHY_OPENSSL_LESS_THAN_111 | ||
| size_t (*DTLS_get_data_mtu)(SSL *) = NULL; |
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.
Take a look in _conditional.py to see what we do when a symbol isn't actually available.
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.
It looks like currently, _conditional.py doesn't actually remove any conditionally-available functions (e.g. DTLS_get_link_min_mtu, visible just above my new code in the diff, doesn't appear there). So I made my best guess at how it should work for DTLS_get_data_mtu -- take a look.
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.
Ha, I would argue that is a bug (we don't want callables that are null func ptrs!). Looks like that's a thing we need to separately fix for the DTLS bindings though...
|
Looks like there's some flake8 issues. |
|
Huh. Codecov output is a bit mangled, but I think it might be saying that in |
AFAICT it works fine in CFFI's ABI mode, but I can't figure out how to do it in the API mode.
…yca#6138) * Expose a few more OpenSSL functions that are useful for DTLS support * Move BIO_ADDR gunk to proper place * const correct * Throw more #ifdefs at the wall and see if they stick * njsmith used "think about what he's doing" it's probably not very effective * LibreSSL is not my favorite library * Attempt to hide my new undefined symbols * deflake * Give up on trying to check function pointers for NULLness AFAICT it works fine in CFFI's ABI mode, but I can't figure out how to do it in the API mode.
…yca#6138) * Expose a few more OpenSSL functions that are useful for DTLS support * Move BIO_ADDR gunk to proper place * const correct * Throw more #ifdefs at the wall and see if they stick * njsmith used "think about what he's doing" it's probably not very effective * LibreSSL is not my favorite library * Attempt to hide my new undefined symbols * deflake * Give up on trying to check function pointers for NULLness AFAICT it works fine in CFFI's ABI mode, but I can't figure out how to do it in the API mode.

Also
OP_NO_RENEGOTIATIONI guess, which is useful because renegotiation is the worst.