Skip to content
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

ECDSA Keys and Certificates #291

Closed
3goats opened this issue Jul 10, 2015 · 18 comments
Closed

ECDSA Keys and Certificates #291

3goats opened this issue Jul 10, 2015 · 18 comments

Comments

@3goats
Copy link

3goats commented Jul 10, 2015

Hi,

Could you please provide an example of using PyOpenssl to create an ECDSA key pair and associated self signed x.509 certificate.

Best Regards,

@jsha
Copy link

jsha commented Feb 4, 2017

@carlskii: By my reading of the code it is not currently possible.

@reaperhulk: What are the blockers to adding ECDSA support? I'd like to add ECDSA support to Certbot, but I think it's blocked on support in pyOpenSSL. cc @bmw @ohemorange for confirmation.

@plinss
Copy link

plinss commented Feb 4, 2017

It's not possible currently using the PyOpenSSL API directly, but it is possible to generate an ECDSA private key and load it into a PyOpenSSL PKey using lower-level crypto APIs:

from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives.asymmetric import rsa, ec
from cryptography.hazmat.primitives.serialization import Encoding, PrivateFormat, NoEncryption

import OpenSSL

def generate_ecdsa_key(self, key_curve):
    key_curve = key_curve.lower()
    if ('secp256r1' == key_curve):
        key = ec.generate_private_key(ec.SECP256R1(), default_backend())
    elif ('secp384r1' == key_curve):
        key = ec.generate_private_key(ec.SECP384R1(), default_backend())
    elif ('secp521r1' == key_curve):
        key = ec.generate_private_key(ec.SECP521R1(), default_backend())
    else:
        print('Unsupported key curve: ' + key_curve + '\n')
        return None
    key_pem = key.private_bytes(encoding=Encoding.PEM, format=PrivateFormat.TraditionalOpenSSL, encryption_algorithm=NoEncryption())
    return OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, key_pem)

From there you can generate a CSR, see https://github.com/plinss/acmebot/ for more code examples. (There are more curves you can use, I only used those three in my bot.)

@hynek
Copy link
Contributor

hynek commented Feb 4, 2017

pyOpenSSL has direct support for loading cryptography PKeys as of 16.1. Certificates will follow hopefully soon.

Therefore I’m closing this issue. Our mid-term plan is to get rid of code duplication between cryptography and pyOpensSSL. Specifically x509 is the first to go.

@hynek hynek closed this as completed Feb 4, 2017
@plinss
Copy link

plinss commented Feb 5, 2017

@hynek FWIW, loading a cryptography key directly into a PKey currently fails for ECDSA keys (as of 16.2.0), results in "TypeError: Unsupported key type", the PEM workaround I posted above does work.

@hynek
Copy link
Contributor

hynek commented Feb 5, 2017

Ah right...is there a reason why we don't allow EC keys @reaperhulk? Or was that just for parity?

@reaperhulk
Copy link
Member

That was because pyOpenSSL doesn't officially support EC keys so I figured I should just have it error on that. We could lift that restriction but I wouldn't guarantee that an EC PKey object would behave properly in all cases.

@hynek
Copy link
Contributor

hynek commented Feb 5, 2017

Do you have a hunch what might go wrong or is it just a general bad feeling?

@reaperhulk
Copy link
Member

A general bad feeling. Looking at the public methods generate_key would fail, but that's not an issue here. It might be okay.

In the specific case of certbot it's probably worth revisiting what all they use pyOpenSSL for since it's possible cryptography covers the complete set now.

@hynek
Copy link
Contributor

hynek commented Feb 5, 2017

I know better than to argue against your bad feeling. :)

I’m thinking about removing the exception but leave it experimental for now?

@plinss
Copy link

plinss commented Feb 5, 2017

As a data point, I'm using the following PyOpenSSL (16.2) methods in acmebot with ECDSA private keys, public keys, and certificates, and all seems fine (in that I am able to issue functioning ECDSA certificates from Let's Encrypt):

OpenSSL.crypto.load_privatekey()
OpenSSL.crypto.dump_privatekey()
OpenSSL.crypto.load_certificate()
OpenSSL.crypto.dump_certificate()
PKey.to_cryptography_key()
X509Req.set_pubkey()
X509Req.sign()
X509.get_pubkey()

@reaperhulk
Copy link
Member

If that's the only part of pyOpenSSL you're using in acmebot then you could actually directly depend on cryptography (you have it as a transitive dependency via pyOpenSSL right now). CSRBuilder, Certificate, and CertificateSigningRequest contain what you need.

@plinss
Copy link

plinss commented Feb 6, 2017

I'm using Let's Encrypt's acme client which uses PyOpenSSL X509Reqs and returns X509 certificates. If I ever switch to my own acme client code then yeah, I can most likely go directly to cryptography.

@reaperhulk
Copy link
Member

reaperhulk commented Feb 6, 2017

Just as a warning: the acme client doesn't require pyOpenSSL 16.2 (just >=0.13) so PKey.to_cryptography_key isn't guaranteed to be present for all users.

@plinss
Copy link

plinss commented Feb 6, 2017

Thanks for the heads-up, I'll update my requirements file.

@jsha
Copy link

jsha commented Feb 6, 2017

@reaperhulk:

In the specific case of certbot it's probably worth revisiting what all they use pyOpenSSL for since it's possible cryptography covers the complete set now.

Talked about this with @bmw today. It seems likely (haven't verified 100%) that we could use cryptography in Certbot every place we use pyOpenSSL, but there may be some use cases for which OpenSSL's APIs are shorter and cleaner (like verifying that certs match private keys, that issuer chains validate, etc). Besides chopping a dependency, are there other reasons you're recommend switching entirely to cryptography?

@reaperhulk
Copy link
Member

Chopping a dependency would be the primary reason -- the secondary is that as hynek noted we're slowly moving pyOpenSSL to maintenance mode as cryptography takes over its capabilities. That's a long term project though (the primary use case for pyOpenSSL is making TLS connections and cryptography has no interface for that at the present time), so it's entirely reasonable to keep using pyOpenSSL if you'd like. Just be aware that things like EC keys may need some contributions to get working on the pyOpenSSL side (and even if we do add them you'll be forced to upgrade your pyOpenSSL dep to get that feature).

@jsha
Copy link

jsha commented Feb 7, 2017

Good to know, thanks. @bmw pointed out that we already take the PyOpenSSL dependency because we depend on requests, which depends on PyOpenSSL. So even if we chop the direct dependency, we'd still require users to download PyOpenSSL. So the main reason for us to move at the moment is better EC support, which we can do without fulling removing our PyOpenSSL dep.

@bmw
Copy link

bmw commented Feb 7, 2017

Right. To clarify a bit, we currently unconditionally depend on requests security extras to provide reasonable SSL support for ancient versions of Python. The security extras include PyOpenSSL in newer versions of requests so until we change our unconditional requirement, dropping PyOpenSSL doesn't have much benefit for us. We can make our dependency unconditional here, but it's actually a bit of work on our end due to how things such as certbot-auto work.

As more of PyOpenSSL's functionality is implemented in cryptography and old OSes we support get EOL'd though, we'll work towards dropping PyOpenSSL.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

6 participants