pyopenssl icon indicating copy to clipboard operation
pyopenssl copied to clipboard

Allow elliptic curve keys in from_cryptography_key().

Open theno opened this issue 7 years ago • 4 comments

With this change an (Certificate Transparency) SCT can be verified against the public key of a CT-Log. CT-Logs usually are signed with an elliptic curve digest. The argument cert of the function verify(cert, signature, data, digest) is just a wrapper for its pkey attribute. This attribute now can contain an ec-pubkey. Here is an example of this usage: https://github.com/theno/ctutlz/blob/60cd6b9aeee7a7c6f0f90b0262a306a292808985/ctutlz/sct/verification.py#L29

Elliptic curve support in PKey objects still needs to be implemented.

theno avatar Jun 10 '17 02:06 theno

Thanks for this! However, I think you're approaching this backwards. Without support for EC keys in PKey, I don't think we should allow them to be loaded: I'd much rather see EC PKey support added first.

I'd even more rather see SCR verification land in cryptography, as this seems right up @Alex's street.

Lukasa avatar Jun 10 '17 07:06 Lukasa

Codecov Report

Merging #636 into master will decrease coverage by 0.08%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
- Coverage   96.78%   96.69%   -0.09%     
==========================================
  Files          18       18              
  Lines        5625     5625              
  Branches      390      390              
==========================================
- Hits         5444     5439       -5     
- Misses        121      125       +4     
- Partials       60       61       +1
Impacted Files Coverage Δ
tests/test_crypto.py 98.38% <100%> (-0.2%) :arrow_down:
src/OpenSSL/crypto.py 96.21% <100%> (-0.19%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c9280c5...6e7c49d. Read the comment docs.

codecov[bot] avatar Jun 10 '17 10:06 codecov[bot]

For convenience I marked the failing test to be skipped when running pytest to make the unit tests passing.

This even makes the arguments of Lukasa more obvious.

theno avatar Jun 10 '17 10:06 theno

I was just about to submit a pull request very similar to this. I have a need for this functionality in my AsyncSSH package. I'm in the middle of adding support for X.509 certificates in SSH and I have things working for RSA certificates. However, I ran into this issue when attempting to use EC certificates.

As it turns out, EC certificates work just fine when you feed them to OpenSSL.crypto.load_certificate(). However, to support generating certificates, I need to be able to convert a cryptography EC key to a PKey, and it looks like these explicit checks are the only thing stopping that from working.

While it's true that PyOpenSSL doesn't support creating new EC PKeys directly yet, I don't need that in my application and supporting conversion from already constructed PyCA keys would be a good first step.

I might be able to work around this by generating a certificate using the native PyCA X.509 support and then using OpenSSL.crypto.load_certificate() to get a PyOpenSSL certificate object out of that. However, I'd prefer not to have to do that.

Long term, I'd love to convert everything over to PyCA and not depend on PyOpenSSL at all, but I can't do that until PyCA adds support for X.509 certificate chain validation.

ronf avatar Jul 11 '17 04:07 ronf