cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

Add x509 Certificate Validation

Open danetrain opened this issue 8 years ago • 49 comments

Currently, cryptography contains no functionality to validate a certificate chain against a trusted root certificate. This is a fairly standard operation; it is described in detail by RFC 5280. I would like to implement this by adding a x509Validator interface to the cryptography.x509 module.

danetrain avatar Sep 28 '15 14:09 danetrain

references #1957 and #1660 (although that issue overlaps but is not the same as this).

If the scope of this issue is limited to validation of a cert by an already provided chain (rather than trying to build a chain from an arbitrary set of certificates) we can solve that and then move on to chain building.

OpenSSL obviously implements chain building, but most versions still in use have limitations that can be problematic (see: https://lukasa.co.uk/2015/04/Certifi_State_Of_Union/ and https://github.com/certifi/python-certifi/issues/26).

I thought we had an issue discussing what an API for this might look like, but I can't find it. Do you have a proposal @danetrain?

reaperhulk avatar Sep 28 '15 15:09 reaperhulk

I'm thinking the API should expose pretty limited functionality. Basically I think the user will just initialize the validating object from a list of trusted certs and call a validate method with the cert chain of an untrusted cert. Like signature verification, it'll either return true or throw exceptions.

I'm thinking it'll go something like this from the user end:

from cryptography.x509 import x509Validator

trusted_certs = [ ] #array of cryptography.x509 objects which are self-signed trusted roots# cert_chain = [ ] #array of cryptography.x509 objects which is the cert chain for an untrusted cert# validator = x509Validator(trusted_certs)

try: validator.validate(cert_chain) except: #throws exceptions for invalid signature chain, if root of cert chain is not trusted# #or if root of cert chain is not signed by a trusted root#

danetrain avatar Sep 28 '15 15:09 danetrain

pyopenssl has the code for basic cert verification. Would be trivial to port to cryptography.

#1660 has some basic code to do it and allow callbacks too.

It is not perfect though, as it does not set all the possible flags for verification of timestamps (do you check at the time of issue, current time, time of signature?), check CRLs, OCSP etc.

schlenk avatar Sep 29 '15 09:09 schlenk

I'm too, gonna need this. I proposed a way in my first pull requests for CRLs (which also need validation methods). OpenSSL does offer the X509_*_verify() method which allows to verify the object against a public key. This requires that the chain would have to be validated manually but the code I'm trying to port already does that anyway.

I'm willing to do the work for this if someone decides this is a viable solution to the problem.

etrauschke avatar Sep 29 '15 15:09 etrauschke

Here's a code sample of what I'm working on so far. The only user-facing methods are verify_certificate_chain and verify_certificate_signature. Note that the verify_certificate_signature method is only a stub for the moment.

I'd like to request that we expose the signature bytes through the cryptography.x509.Certificate API. On a general level, it fits with the overall purpose of the x509 Certificate object, since the function of this object is generally just to parse the underlying certificate bytes. In the context of my feature, it'd simplify the implementation of the verify_certificate_signature method since I could rely solely on functionality and data exposed by the cryptography API, and wouldn't have to directly invoke any bound OpenSSL methods. I could retrieve the hash method, retrieve the signature bytes, and generate the cert's fingerprint through the x509 Certificate interface. Then, I could retrieve the backend object and the public key through the signing cert and use cryptography's asymmetric modules to verify the signature.

class CertificateValidator(object):

    def __init__(self, root_certs):
        if not all(
            isinstance(cert, Certificate) for cert in root_certs
        ):
            raise TypeError(
                "root_certs must be a list of "
                "cryptography.x509.Certificate objects"
            )
        if not all(
            self._is_root_cert(cert) for cert in root_certs
        ):
            raise ValueError(
                "root_certs must be a list of "
                "root certificates"
            )
        if not all(
            self._is_valid_at_current_time(cert) for cert in root_certs
        ):
            raise ValueError(
                "At least one root certificate is not "
                "valid at this time"
            )
        self._root_certs = root_certs

    def _is_root_cert(self, cert):
        return cert.subject == cert.issuer

    def _is_valid_at_current_time(self, cert):
        now = datetime.datetime.utcnow()
        return (now > cert.not_valid_before) and (now < cert.not_valid_after)

    def _get_issuing_root_cert(self, cert):
        for root_cert in self._root_certs:
            root_name = root_cert.subject
            if (root_name == cert.issuer or root_name == cert.subject):
                return root_cert
        return None

    '''When implemented, this will return True or raise an error'''
    def verify_certificate_signature(self, issuing_cert, subject_cert):
        return True

    '''Returns True or throws error'''
    def validate_certificate_chain(self, cert_chain):
        if not all(
            isinstance(cert, Certificate) for cert in cert_chain
        ):
            raise TypeError(
                "cert_chain must be a list of "
                "cryptography.x509.Certificate objects"
            )
        if not all(
            self._is_valid_at_current_time(cert) for cert in cert_chain
        ):
            raise ValueError(
                "cert_chain must consist of cryptography.x509.Certificate "
                "objects which are valid at the current time"
            )

        '''Start by checking that the chain issues from a trusted root'''
        issuing_cert = cert_chain[0]
        issuing_root = self._get_issuing_root_cert(issuing_cert)

        if not issuing_root:
            raise ValueError(
                "No matching root certificate found for certificate chain"
            )
        self.validate_certificate_signature(issuing_root, issuing_cert)

        '''Verify certificate signatures down the chain'''
        for i in range(len(cert_chain) - 1):
            self.validate_certificate_signature(cert_chain[i],
                                                cert_chain[i + 1])

        return True

danetrain avatar Sep 29 '15 18:09 danetrain

Yes, that was basically what I had in mind too, however, I'd make this part of the X509 object itself. So to verify a signature against a CA or just its public key you would just do that: cert.verify(pub_key)

For verifying the whole chain, I think it's a good idea to have this as part of a certstore. So you just throw all the certs in there and then check if you have a valid chain.

Why would the helper functions like _is_root_cert() be "protected"?

etrauschke avatar Sep 29 '15 18:09 etrauschke

The code is probably too simple to handle all the tricky details in RFC 5280. Especially the _is_root_cert() and _get_issuing_root_cert() code is naive and will probably fail miserably for the cases @reaperhulk mentioned in his two links about certifi (cross certification).

If you have want to have any hope of getting verification right, assume you have the proper certificate chain completely and in the correct order upfront. Do not try to do path building in a first step.

I'm not sure if it is better to have a rather limited API for cert validation on the python layer or if a wrapper for the openssl verify code is better. It should be possible to write it with the cryptography API alone.

schlenk avatar Sep 29 '15 19:09 schlenk

@etrauschke : The decision to build a Validator class instead of adding additional functionality to the x509 object is that it presents a more convenient way for the user to validate things against a root(s) of trust. If we just have a cert.verify(pub_key), then we pass the work of testing the candidate cert against all the trusted public keys on to the user. The Validator approach provides a pretty literal representation of roots of trust to the user. They can just add in root certs for CAs they trust, and verify candidate certs against them with a single line of python.

@schlenk : I'm confused by your comments. This code is meant primarily to illustrate a potential API change, not represent a complete implementation. Of course it will 'fail miserably', one of the most important methods is a stub that returns True. I'm also not attempting path building; the code assumes that the given certificate chain is more or less complete (either begins with a trusted root or begins with a cert signed by a trusted root) and assumes that it is ordered correctly (i.e. that each cert is signed by the previous cert in the chain).

danetrain avatar Sep 29 '15 19:09 danetrain

@danetrain Sorry for confusing.

Basically if you pass a 'certificate chain', why do you need to search for the root CA with _get_issuing_root_cert() ? cert_chain[0] should be the root CA to use. If it is not, your certificate chain is incomplete and you need (a very limited form of) path building to chain it up to your root certificates. There should be no ambiguity in the API more or less complete is a bit too vague for my taste. Example: If you have two CA certificates with different Key Identifiers but identical subjects your API randomly picks one of those and misvalidates.

A bit similar issues for the timestamp checks. You verify the Root CAs are valid right now. That is great if you want to verify a SSL/TLS certificate for a web browser. But it is wrong if you want to verify a digital signature with a policy like 'chain-model' for validation where the timestamp of the signature is the one to check. And if there are timestamp checks, the chain certificates should also be checked if they are in the valid range.

schlenk avatar Sep 29 '15 19:09 schlenk

@schlenk: "cert_chain[0] should be the root CA to use" is inconsistent with RFC 5280. Section 6.1 indicates that the first certificate should be issued by a trust anchor. These trust anchors are represented in my code by _root_certs attribute of the Validator class. That being said, I will make the code stricter so that it will not accept cert chains which include the actual trust anchor. I'll also clean up the language of the class so it maps more directly to the specification in RFC 5280.

I see your point about the temporal checks needing to be based on the signing times rather than the current time. I'll make this change as well.

danetrain avatar Sep 30 '15 14:09 danetrain

Added signature #2387, but we're going to need something that outputs tbsCertificate as well if you want to verify a signature.

reaperhulk avatar Oct 02 '15 01:10 reaperhulk

Thanks! Yeah I realized that this would also be an issue yesterday. Unless the fingerprint() method only hashes the tbsCertificate bytes then we have to do one of the following in order to get the proper data to verify the signature:

  • Add another API method to return the cert bytes without the signature
  • Manually remove the signature from the end of the certificate bytes in the Validator code

I'd prefer to go the former route since it seems cleaner and less error-prone, but if there are arguments for the latter I'd be open to hearing them.

danetrain avatar Oct 02 '15 12:10 danetrain

Any update yet on which path we want to go down with this?

etrauschke avatar Oct 20 '15 16:10 etrauschke

Hi all, I'm going to be stepping in to assist @danetrain with the implementation for the CertificateValidator. I'm hoping to get a pull request up by next week.

@etrauschke As far as I know for Option 1, @reaperhulk hasn't updated #2387 with an API call that provides access to the tbs_certificate bytes. I'm currently going forward with Option 2 (manually obtaining the tbs_certificate bytes by "subtracting out" the signature bytes from the certificate) but I'm definitely open to using the API if it gets updated.

PeterHamilton avatar Oct 21 '15 15:10 PeterHamilton

I think that was because the usage of an interface like this was fairly cumbersome. Imo we should use the functions the OpenSSL library already provides for this. You can still add all the other magic like assembling the whole cert chain, retrieving CRLs, etc..

But as was mentioned before (can't find it anymore, though) you don't want to make network connections to retrieve CRLs or test revocation via OCSP by default. I'm not sure if it forcing the user to provide a whole cert chain actually buys you that much. It should be up to the user if you just want to verify a sub-chain, the whole chain but without revocations or the whole chain with revocations.

etrauschke avatar Oct 21 '15 15:10 etrauschke

If we end up going down this path we'll either need a tbs_certificate property so you can use the hazmat asym verification primitives or else we'll need to define new backend methods (e.g. verify_x509_signature(self, cert, ca)) that can abstract it away. I'm ambivalent right now as to which way we should go. The advantage of the backend method would be that we can leverage backend specific behaviors (like X509_verify in the case of OpenSSL).

reaperhulk avatar Oct 21 '15 15:10 reaperhulk

@etrauschke I agree that making additional network connections when doing lookups should be avoided, at least for this first cut. Right now, the implementation we're working on requires the full chain from the user. I think once we have that working and in place, we can build off of it to support the other options you listed.

@reaperhulk If I had to choose, I'd pick adding the tbs_certificate property, to make working with the different parts of the certificate bytes consistent.

PeterHamilton avatar Oct 22 '15 11:10 PeterHamilton

Hi all. First off, thanks for 'cryptography', I've tried all the different python crypto libraries over many years and this is the best design and API I've seen yet. I have written about 80% of an extensible certificate path validation package, but I need access to the tbs_certficate property you mentioned above as well as the signatureValue from the outer body of the certificate so I can verify the signature on the certificate. I just started looking at the openssl backend to see if I could expose this information myself until you decide on how you will make signature verificative possible officially. Any pointers on how to get to the toBeSigned DER encoded bytes and the DER encoded signatureValue would be greatly appreciated. Also I would like to share my Validation module with you if you're interested.

rowland-smith avatar Nov 02 '15 01:11 rowland-smith

@rowland-smith Check out https://github.com/pyca/cryptography/pull/2387. It adds a signature property to the Certificate API that allows you to get the signature bytes. I'm hoping that'll get merged soon. We're still debating on the right approach for getting the tbs_certificate bytes (pinging @reaperhulk :).

I'm also in the process of adding certificate chain validation to cryptography, see https://github.com/pyca/cryptography/pull/2460. Any comments you might have on what I have so far are definitely appreciated.

PeterHamilton avatar Nov 02 '15 13:11 PeterHamilton

Hi Peter, I'll check that out today. FYI my code is now on GitHub: https://github.com/river2sea/X509Validation

Thanks, Rowland

On Nov 2, 2015, at 8:16 AM, Peter Hamilton [email protected] wrote:

@rowland-smith Check out #2387. It adds a signature property to the Certificate API that allows you to get the signature bytes. I'm hoping that'll get merged soon. We're still debating on the right approach for getting the tbs_certificate bytes (pinging @reaperhulk :).

I'm also in the process of adding certificate chain validation to cryptography, see #2460. Any comments you might on what I have so far are definitely appreciated.

— Reply to this email directly or view it on GitHub.

rowland-smith avatar Nov 02 '15 14:11 rowland-smith

Peter, I was able to integrate the changes (made by reaperhulk?) that I found in issue #2387 with a clone of the latest cryptography master. I can get the signature from the Certificate now. Any word on getting the tbs_bytes?

rowland-smith avatar Nov 02 '15 20:11 rowland-smith

@rowland-smith No word yet on the tbs_certificate bytes. It definitely needs to be added to cryptography, either as an update to the Certificate API or to the backend API. @etrauschke and @reaperhulk were still debating it last I heard. I'll try pinging them again on it if they don't pop in to discuss soon.

PeterHamilton avatar Nov 02 '15 21:11 PeterHamilton

OK, Thanks, I'll keep checking back periodically. I'm trying a workaround using pyasn1 for the tbs_certificate bytes, we'll see how that goes.

rowland-smith avatar Nov 02 '15 22:11 rowland-smith

@rowland-smith FYI, @reaperhulk updated https://github.com/pyca/cryptography/pull/2387 to include a tbs_certificate_bytes property in the Certificate API. Hopefully this will merge soon.

PeterHamilton avatar Nov 03 '15 17:11 PeterHamilton

Great, thanks for the heads up!

Sent from my iPhone

On Nov 3, 2015, at 12:02 PM, Peter Hamilton [email protected] wrote:

@rowland-smith FYI, @reaperhulk updated #2387 to include a tbs_certificate_bytes property in the Certificate API. Hopefully this will merge soon.

— Reply to this email directly or view it on GitHub.

rowland-smith avatar Nov 03 '15 17:11 rowland-smith

@PeterHamilton If you're interested in talking about certificate path validation, my email address is on my GitHub profile page. As far as I can tell you can't send someone a direct email from GitHub(?). Apologies to all for using the issues forum as an email client ;)

rowland-smith avatar Nov 04 '15 01:11 rowland-smith

tbs_certificate_bytes is now on master

alex avatar Nov 04 '15 12:11 alex

@alex W00t! Thanks, I'm hoping to push an update to https://github.com/pyca/cryptography/pull/2460 today, so this definitely helps me out.

PeterHamilton avatar Nov 04 '15 14:11 PeterHamilton

Reviving this old thread, I am wondering if there are any plans to have certificate validation as a part cryptography package? I saw some pull requests regarding this feature, but it seems the discussion on that was not conclusive. Are there any updates on this matter?

fantamiracle avatar May 02 '17 19:05 fantamiracle

FWIW, I started hacking on an external x.509 validator atop cryptography: https://github.com/alex/x509-validator

No one has ever reviewed it, don't use it.

alex avatar Aug 22 '17 15:08 alex