-
Notifications
You must be signed in to change notification settings - Fork 421
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
Begin making examples Python 3 compatible #245
Conversation
Conflicts: examples/mk_simple_certs.py
setattr(subj, key, value) | ||
|
||
req.set_pubkey(pkey) | ||
req.sign(pkey, digest) | ||
return req | ||
|
||
def createCertificate(req, (issuerCert, issuerKey), serial, (notBefore, notAfter), digest="md5"): | ||
def createCertificate(req, issuerCertKey, serial, validityPeriod, digest="sha256"): |
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.
According to PEP 3113 tuple unpacking is deprecated.
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’s actually even removed in Python 3 (which kind of sucks when doing Twisted)
print 'Got certificate: %s' % cert.get_subject() | ||
certsubject = crypto.X509Name(cert.get_subject()) | ||
commonname = certsubject.commonName | ||
print(('Got certificate: ' + commonname)) |
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.
Why are these changes needed, particularly the crypto.X509Name(...)
bit?
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.
I should have made a comment to clarify :).
This is what you get on master:
Got certificate: <X509Name object '/CN=Certificate Authority'>
Got certificate: <X509Name object '/CN=Simple Server'>
This is what you get with that:
Got certificate: Certificate Authority
Got certificate: Simple Server
I think that is the direction the orig author was going in when he said: "# This obviously has to be updated". I can remove the comment if you agree. I suppose there are less spoofable things that we could use to identify the certs in addition to the CN, like the fingerprint.
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.
yeah please get rid of the comment. there’s nothing obvious about it.
So, stupid question: is there currently an easy way to actually test the examples? |
the 'easy' way I used to test them from the examples directory was: then type some text in, and server.py should echo it back to you, as well as tell you what certificate it received. It actually just gives the CN as opposed to something that couldn't be spoofed like a fingerprint. Then I ran the same against python3. |
I see, thanks! Please add |
1 similar comment
Thanks! |
Begin making examples Python 3 compatible
This PR makes a few of the examples work under Python 3 in addition to Python 2. I ran some tests under 2.7.8 and 3.4.2.
I also found a couple of md5s being used during signing in certgen.py, I'm proposing that they be bumped up to SHA-256.