Skip to content

Conversation

@njsmith
Copy link
Contributor

@njsmith njsmith commented Jun 26, 2021

Also OP_NO_RENEGOTIATION I guess, which is useful because renegotiation is the worst.

@njsmith
Copy link
Contributor Author

njsmith commented Jun 27, 2021

...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.

@alex
Copy link
Member

alex commented Jun 27, 2021

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.

@njsmith
Copy link
Contributor Author

njsmith commented Jun 28, 2021

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...

@njsmith
Copy link
Contributor Author

njsmith commented Jun 28, 2021

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;
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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...

@alex
Copy link
Member

alex commented Jun 30, 2021

Looks like there's some flake8 issues.

@njsmith
Copy link
Contributor Author

njsmith commented Jun 30, 2021

Huh. Codecov output is a bit mangled, but I think it might be saying that in _comparisons.py, we're never detecting DTLS_get_data_mtu as being falsey? That doesn't make sense to me -- we're definitely setting it to NULL on libressl and openssl 1.1.0, and I confirmed locally that CFFI treats null function pointers as falsey (bool(ffi.cast("void (*)(void)", 0)) == False).

AFAICT it works fine in CFFI's ABI mode, but I can't figure out how to
do it in the API mode.
@njsmith
Copy link
Contributor Author

njsmith commented Jun 30, 2021

image

@alex alex merged commit 1b281ba into pyca:main Jun 30, 2021
@njsmith njsmith deleted the dtls-wrappers branch June 30, 2021 18:37
vishwin pushed a commit to vishwin/py-cryptography that referenced this pull request Dec 23, 2022
…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.
Josh-Rusch pushed a commit to Josh-Rusch/cryptography that referenced this pull request Aug 11, 2024
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants