-
-
Notifications
You must be signed in to change notification settings - Fork 350
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
DTLS support #2047
DTLS support #2047
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2047 +/- ##
==========================================
- Coverage 99.56% 99.02% -0.55%
==========================================
Files 115 119 +4
Lines 14856 16165 +1309
Branches 2837 3101 +264
==========================================
+ Hits 14792 16007 +1215
- Misses 41 111 +70
- Partials 23 47 +24
|
This is still blocked waiting on cryptography+pyopenssl to release, but I believe everything on our end is done, so if anyone is excited about reviewing, go ahead! Docs are viewable here: https://trio--2047.org.readthedocs.build/en/2047/reference-io.html#datagram-tls-support Aside from the DTLS support itself, this also adds a MVP of a mock network layer for testing (#170). It's not exposed publicly yet, because so far it only supports UDP, the mechanism for manipulating packets in flight is awkward, and I just generally didn't want to make finalizing that API a blocker for this PR. But the DTLS tests use it extensively, and using it felt fabulous, so I'm excited. This is a bit of a weird situation as far as reviewing goes, because this is a huge complex piece of work, and I got paid for it but y'all didn't. So like... any review is much appreciated, but please don't feel obliged to spend days on it or anything! It is not the end of the world if some of this code doesn't get intense scrutiny – it has an extremely thorough test suite and we can always fix stuff later. Thanks again to [redacted] for sponsoring this work. |
Split out the address resolution code so that fake net can reuse it more easily
- use correct initial seqno on server in case of packet loss - calculate timeouts correctly - moar tests
This works around a problem on windows where recvfrom on an unbound socket errors out.
Noticed while testing on Ubuntu 18.04's weird old OpenSSL that this test was getting stuck in an infinite loop, even though it passed with more recent OpenSSL. I'm not sure why it was different, exactly, but on closer examination, writing the test this way makes more sense anyway, and now the tests pass on Ubuntu 18.04 too.
Update: pyopenssl has pushed their release, so this no longer needs hacky workarounds to test against the dev version. And I think I tracked down the last few test failures that were happening on pypy? Including finding a fun pypy bug: https://foss.heptapod.net/pypy/pypy/-/issues/3656 Let's see what CI says. |
Seems like test coverage has gone down by a small amount causing the CI check failures. @njsmith do you think this PR is ready for review/merge? |
The coverage does go down but it's not blocking in my opinion. |
Here's an initial draft of #2010. It's not done -- in particular, needs a ton more tests and docs, as well as some strategy to avoid adding a hard dependency on pyopenssl -- but the core logic is all there in some form.
It depends on upstream changes pyca/cryptography#6138 and pyca/pyopenssl#1026, so the tests are all going to fail for now.