CHANGE EVERYTHING #4
Conversation
The old convenience API was a bit too inconvenient. The new one is more convenient. Also it now supports PyOpenSSL.
Codecov Report
@@ Coverage Diff @@
## master #4 +/- ##
======================================
Coverage 100% 100%
======================================
Files 3 3
Lines 117 221 +104
Branches 2 9 +7
======================================
+ Hits 117 221 +104
Continue to review full report at Codecov.
|
|
@njsmith My vote goes for the imperative form, but I'd prefer "this" be the pronoun and not "me", e.g., |
|
@markrwilliams: that's the whole problem though -- it's supposed to mean "context, trust this CA"! This is why I'm kinda leaning towards the |
|
@njsmith |
|
|
- New names for the context configuration methods: configure_trust and configure_cert. - Switch *_pem to a new convenience class Blob - Redo tempfile handling to be Windows friendly - Replace the concatenated cert_chain_pem with a list cert_chain_pems.
njsmith
changed the title
|
@markrwilliams: Well this branch got a bit out of hand, but see what you think? |
|
@markrwilliams Guess you're busy. No worries! But I want to get this off my stack (and hopefully minimize the number of people who start using the 0.1.0 API), so if I don't hear anything I'll probably merge on Monday or so. |
| # ----- Using your shiny new certs ----- | ||
|
|
||
| # You can configure SSL context objects to trust this CA: | ||
| ca.configure_trust(ssl_context) |
There was a problem hiding this comment.
👍
This is clear if less pithy than the alternatives we discussed.
| # You can configure SSL context objects to trust this CA: | ||
| ca.configure_trust(ssl_context) | ||
| # Or configure them to present the server certificate | ||
| server_cert.configure_cert(ssl_context) |
tests/test_trustme.py
Outdated
|
|
||
| def doit(ca, hostname, server_cert): | ||
| # socketpair and ssl don't work together on py2, because... reasons | ||
| #raw_client_sock, raw_server_sock = socket.socketpair() |
There was a problem hiding this comment.
Why comment code out when you can delete it?
| with ThreadPoolExecutor(2) as tpe: | ||
| f1 = tpe.submit(fake_ssl_client, ca, raw_client_sock, hostname) | ||
| f2 = tpe.submit(fake_ssl_server, server_cert, raw_server_sock) | ||
| f1.result() |
There was a problem hiding this comment.
Should there be a timeout here just in case?
There was a problem hiding this comment.
I think we're OK -- the only way this could lock up is something truly weird happens (like -- Python semantics violating weird), and there's not much that can be done to defend against that. Also I'm not sure how to do a timeout here (there's no way to cancel a thread). Worst case Travis times out after 10 minutes or whatever.
| @@ -21,6 +23,9 @@ | |||
| # not 2 seconds. | |||
| _KEY_SIZE = 1024 | |||
|
|
|||
| def _smells_like_pyopenssl(ctx): | |||
There was a problem hiding this comment.
I'm not thrilled by this but I don't see a simpler way :(
|
@markrwilliams: okay, pushed changes to address some nits, will merge after travis passes (probably after I wake up). Thanks! |
Now instead of having methods to return a
ssl.SSLContextobject, wehave methods that take an
ssl.SSLContextobject and reconfigure itin place. This is more flexible and probably fits better into existing
codebases.
Plus, it makes it easy to support PyOpenSSL without lots of awkward
namespacing!
I'm still not 100% happy with the phrasing of
because the subject/object seem backwards.
Maybe they should be top-level functions? Or have better names?
ca.trust_me(ctx),cert.use_me(ctx),ca.trusted_by(ctx),cert.used_by(ctx)?