Re: Possible bug in cert validation? _sslcerts.py

Darjus Loktevic <[email protected]> Mon, 29 Aug 2016 00:08:52 +0000
Newsgroups gmane.comp.lang.jython.user
Message-ID <CAFiRLmmSsdcQfZRCEssJnJQnKnNhAtRk+qOfvkgWKBtFfQiDEQ@mail.gmail.com>
http://bugs.jython.org/issue2516

On Mon, Aug 29, 2016 at 9:53 AM Darjus Loktevic <[email protected]> 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 <
> [email protected]> 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
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/jython-users
>>
>

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

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