Expose some DTLS-related features by njsmith · Pull Request #1026 · pyca/pyopenssl (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This is enough to make it at least possible to set up a DTLS association and exchange encrypted data, though a high quality implementation still requires a ton of scaffolding code (see python-trio/trio#2010).
Depends on pyca/cryptography#6138
For now I'm just skipping the new DTLS test when running against older cryptography. I guess the actual plan would be to wait for cryptography to release, bump the minimum cryptography version to the new release, and then drop the skipif? But everything else is good to go AFAICT.
Would you mind shifting the CI changes to a separate PR? We can merge those immediately. Also, is it a hard requirement that we raise our version minimum or can we feature detect here in a sane fashion? (It's okay if the answer to that is no)
Also looks like we have a variety of missing lines of coverage, although that may just be codecov being stupid, I haven't looked.
Also, is it a hard requirement that we raise our version minimum or can we feature detect here in a sane fashion? (It's okay if the answer to that is no)
Depends on what kind of feature detection you want. Having DTLS_METHOD and friends automagically hide themselves from the SSL namespace seems like maybe more trouble than any of us are motivated to deal with. OTOH the tests still pass just fine with the older cryptography, as long as you skip that one DTLS test.
Also AFAICT all the missed coverage lines are to handle obscure things that can allegedly happen with libressl/old openssl/etc., so the issue is that pyopenssl doesn't test against these in CI. Do we care? I think the worst case is that people using exotic openssl-like libraries might get the wrong exception.
The unrelated CI fixes/py 2 dropping/etc. have now all been merged, and I rebased this on top of them. So now this PR is just the DTLS changes, and I guess r4r.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit more missing coverage than I'd prefer, but most of that is fixable if we add a 1.1.0 builder to our CI...which I am not volunteering for.
2 participants
