Possible bug in cert validation? _sslcerts.py

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Possible bug in cert validation? _sslcerts.py

Craig McDaniel
I have potentially found one or more bugs in _sslcerts.py, but before I file a bug report, I would like to get some context and possibly understand the intent of the code in question, as I am not an SSL expert.

First, my use case: I have a customer that has a chain of SSL certs in a PEM file. It includes the leaf cert, then 2 intermediate certs, and finally their own root CA cert (which they've manually added as a trusted cert in their browsers throughout their org). When trying to establish an SSL server connection, the function _get_open_ssl_key_manager tries to validate that the private and public keys match, and is throwing an SSLError: "key values mismatch". This same chain of certs worked fine when we were on CPython and using pyOpenSSL.

I have traced the root cause. In _get_open_ssl_key_manager, it is looping over all the certs from the cert_file and checking that the modulus from the private key matches the modulus of the public key. However, this is only true for the first cert (the leaf). I am going to paste the code here, because it looks very suspect to me:

   keys_match = False
   for cert in certs:
       # TODO works for RSA only for now
       if not isinstance(cert.publicKey, RSAPublicKey) and isinstance(private_key, RSAPrivateCrtKey):
           keys_match = True
           continue

       if cert.publicKey.getModulus() == private_key.getModulus() \
               and cert.publicKey.getPublicExponent() == private_key.getPublicExponent():
           keys_match = True
       else:
           keys_match = False

   if key_file is not None and not keys_match:
       from _socket import SSLError, SSL_ERROR_SSL
       raise SSLError(SSL_ERROR_SSL, "key values mismatch")

The first condition in the loop is always false for my case. This isn't what bothers me. The keys_match flag is True on the first iteration of the loop, then False, False, False. Certainly this is not what is intended here. In fact, my co-worker reversed the order of the certs in the PEM file, and thus avoided the Error. The server started, but the browser simply would not recognize it as a valid cert chain (couldn't even accept it as a security exception).

It is theoretically possible that the 2nd or 3rd cert in the chain could pass the modulus check. The result would still be False after having temporarily been True. What does this mean? What is the intention here? Should it simply break out of the loop the first time it finds a match? Perhaps the problem is elsewhere...

So I looked a little deeper. Just before this loop:

    if cert_file is not None:
        _certs, _private_key = _extract_certs_for_paths([cert_file], password)
        private_key = _private_key if _private_key else private_key
        certs.extend(_certs)

Now, _extract_certs_for_paths does something interesting. It calls _extract_certs_from_keystore_file in a try/except block, and if it fails, it then tries _extract_cert_from_data, which reads it as a PEM file. Curious, I decided to convert the PEM file to a JKS keystore. This actually worked, and here I found an important difference between these 2 functions.

1. _extract_certs_from_keystore_file returns only the leaf cert. I verified that the JKS file has the full chain with keytool -list -v. Apparently keystore.getCertificate(alias) only returns the leaf. Thus the JKS file passes the check. However, it also loops over the aliases in the keystore, so if I had more than one key in the store, we would again have potential breakage.

2. _extract_cert_from_data returns all 4 certs in the chain. Thus the PEM file fails the check because the last (root) cert causes keys_match to be False.

Notice also (now I'm knit-picking) the plurality of the word "cert(s)" is even backwards considering how the 2 functions actually behave.

Before I file this as a bug, I would like to find out if anyone has some perspective to add. This certainly feels like one or more bugs to me, but as I said before - I am not an SSL expert.


------------------------------------------------------------------------------

_______________________________________________
Jython-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jython-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Possible bug in cert validation? _sslcerts.py

Darjus Loktevic
Hey Craig,

Thanks for the detailed description. This definitely sounds like a bug.
Since Jython isn't using OpenSSL, we have to emulate it's behavior and so far it's been quite challenging to right especially with the relatively few tests that we have.

The situation we're trying to emulate, is this: https://github.com/openssl/openssl/blob/master/crypto/x509/x509_cmp.c#L279

There they have a cert already associated with the private key, which makes it an easier lookup. In our case we had to do the check before loading into a key store (don't remember the exact reasoning).
The code is of course wrong in assuming there is only a private/public key match.

So, the quick "fix" is to see if there's at least one cert matching in the file.
I'll update the issue with the fix.

Cheers,
Darjus

On Thu, Aug 25, 2016 at 12:33 AM Craig McDaniel <[hidden email]> wrote:
I have potentially found one or more bugs in _sslcerts.py, but before I file a bug report, I would like to get some context and possibly understand the intent of the code in question, as I am not an SSL expert.

First, my use case: I have a customer that has a chain of SSL certs in a PEM file. It includes the leaf cert, then 2 intermediate certs, and finally their own root CA cert (which they've manually added as a trusted cert in their browsers throughout their org). When trying to establish an SSL server connection, the function _get_open_ssl_key_manager tries to validate that the private and public keys match, and is throwing an SSLError: "key values mismatch". This same chain of certs worked fine when we were on CPython and using pyOpenSSL.

I have traced the root cause. In _get_open_ssl_key_manager, it is looping over all the certs from the cert_file and checking that the modulus from the private key matches the modulus of the public key. However, this is only true for the first cert (the leaf). I am going to paste the code here, because it looks very suspect to me:

   keys_match = False
   for cert in certs:
       # TODO works for RSA only for now
       if not isinstance(cert.publicKey, RSAPublicKey) and isinstance(private_key, RSAPrivateCrtKey):
           keys_match = True
           continue

       if cert.publicKey.getModulus() == private_key.getModulus() \
               and cert.publicKey.getPublicExponent() == private_key.getPublicExponent():
           keys_match = True
       else:
           keys_match = False

   if key_file is not None and not keys_match:
       from _socket import SSLError, SSL_ERROR_SSL
       raise SSLError(SSL_ERROR_SSL, "key values mismatch")

The first condition in the loop is always false for my case. This isn't what bothers me. The keys_match flag is True on the first iteration of the loop, then False, False, False. Certainly this is not what is intended here. In fact, my co-worker reversed the order of the certs in the PEM file, and thus avoided the Error. The server started, but the browser simply would not recognize it as a valid cert chain (couldn't even accept it as a security exception).

It is theoretically possible that the 2nd or 3rd cert in the chain could pass the modulus check. The result would still be False after having temporarily been True. What does this mean? What is the intention here? Should it simply break out of the loop the first time it finds a match? Perhaps the problem is elsewhere...

So I looked a little deeper. Just before this loop:

    if cert_file is not None:
        _certs, _private_key = _extract_certs_for_paths([cert_file], password)
        private_key = _private_key if _private_key else private_key
        certs.extend(_certs)

Now, _extract_certs_for_paths does something interesting. It calls _extract_certs_from_keystore_file in a try/except block, and if it fails, it then tries _extract_cert_from_data, which reads it as a PEM file. Curious, I decided to convert the PEM file to a JKS keystore. This actually worked, and here I found an important difference between these 2 functions.

1. _extract_certs_from_keystore_file returns only the leaf cert. I verified that the JKS file has the full chain with keytool -list -v. Apparently keystore.getCertificate(alias) only returns the leaf. Thus the JKS file passes the check. However, it also loops over the aliases in the keystore, so if I had more than one key in the store, we would again have potential breakage.

2. _extract_cert_from_data returns all 4 certs in the chain. Thus the PEM file fails the check because the last (root) cert causes keys_match to be False.

Notice also (now I'm knit-picking) the plurality of the word "cert(s)" is even backwards considering how the 2 functions actually behave.

Before I file this as a bug, I would like to find out if anyone has some perspective to add. This certainly feels like one or more bugs to me, but as I said before - I am not an SSL expert.

------------------------------------------------------------------------------
_______________________________________________
Jython-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jython-users

------------------------------------------------------------------------------

_______________________________________________
Jython-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jython-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Possible bug in cert validation? _sslcerts.py

Darjus Loktevic

On Mon, Aug 29, 2016 at 9:53 AM Darjus Loktevic <[hidden email]> wrote:
Hey Craig,

Thanks for the detailed description. This definitely sounds like a bug.
Since Jython isn't using OpenSSL, we have to emulate it's behavior and so far it's been quite challenging to right especially with the relatively few tests that we have.

The situation we're trying to emulate, is this: https://github.com/openssl/openssl/blob/master/crypto/x509/x509_cmp.c#L279

There they have a cert already associated with the private key, which makes it an easier lookup. In our case we had to do the check before loading into a key store (don't remember the exact reasoning).
The code is of course wrong in assuming there is only a private/public key match.

So, the quick "fix" is to see if there's at least one cert matching in the file.
I'll update the issue with the fix.

Cheers,
Darjus

On Thu, Aug 25, 2016 at 12:33 AM Craig McDaniel <[hidden email]> wrote:
I have potentially found one or more bugs in _sslcerts.py, but before I file a bug report, I would like to get some context and possibly understand the intent of the code in question, as I am not an SSL expert.

First, my use case: I have a customer that has a chain of SSL certs in a PEM file. It includes the leaf cert, then 2 intermediate certs, and finally their own root CA cert (which they've manually added as a trusted cert in their browsers throughout their org). When trying to establish an SSL server connection, the function _get_open_ssl_key_manager tries to validate that the private and public keys match, and is throwing an SSLError: "key values mismatch". This same chain of certs worked fine when we were on CPython and using pyOpenSSL.

I have traced the root cause. In _get_open_ssl_key_manager, it is looping over all the certs from the cert_file and checking that the modulus from the private key matches the modulus of the public key. However, this is only true for the first cert (the leaf). I am going to paste the code here, because it looks very suspect to me:

   keys_match = False
   for cert in certs:
       # TODO works for RSA only for now
       if not isinstance(cert.publicKey, RSAPublicKey) and isinstance(private_key, RSAPrivateCrtKey):
           keys_match = True
           continue

       if cert.publicKey.getModulus() == private_key.getModulus() \
               and cert.publicKey.getPublicExponent() == private_key.getPublicExponent():
           keys_match = True
       else:
           keys_match = False

   if key_file is not None and not keys_match:
       from _socket import SSLError, SSL_ERROR_SSL
       raise SSLError(SSL_ERROR_SSL, "key values mismatch")

The first condition in the loop is always false for my case. This isn't what bothers me. The keys_match flag is True on the first iteration of the loop, then False, False, False. Certainly this is not what is intended here. In fact, my co-worker reversed the order of the certs in the PEM file, and thus avoided the Error. The server started, but the browser simply would not recognize it as a valid cert chain (couldn't even accept it as a security exception).

It is theoretically possible that the 2nd or 3rd cert in the chain could pass the modulus check. The result would still be False after having temporarily been True. What does this mean? What is the intention here? Should it simply break out of the loop the first time it finds a match? Perhaps the problem is elsewhere...

So I looked a little deeper. Just before this loop:

    if cert_file is not None:
        _certs, _private_key = _extract_certs_for_paths([cert_file], password)
        private_key = _private_key if _private_key else private_key
        certs.extend(_certs)

Now, _extract_certs_for_paths does something interesting. It calls _extract_certs_from_keystore_file in a try/except block, and if it fails, it then tries _extract_cert_from_data, which reads it as a PEM file. Curious, I decided to convert the PEM file to a JKS keystore. This actually worked, and here I found an important difference between these 2 functions.

1. _extract_certs_from_keystore_file returns only the leaf cert. I verified that the JKS file has the full chain with keytool -list -v. Apparently keystore.getCertificate(alias) only returns the leaf. Thus the JKS file passes the check. However, it also loops over the aliases in the keystore, so if I had more than one key in the store, we would again have potential breakage.

2. _extract_cert_from_data returns all 4 certs in the chain. Thus the PEM file fails the check because the last (root) cert causes keys_match to be False.

Notice also (now I'm knit-picking) the plurality of the word "cert(s)" is even backwards considering how the 2 functions actually behave.

Before I file this as a bug, I would like to find out if anyone has some perspective to add. This certainly feels like one or more bugs to me, but as I said before - I am not an SSL expert.

------------------------------------------------------------------------------
_______________________________________________
Jython-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jython-users

------------------------------------------------------------------------------

_______________________________________________
Jython-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jython-users
Loading...