Re: [Jython-users] Possible bug in cert validation? _sslcerts.py

[Darjus L. <da...@gm...>]

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 <cra...@da...>
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
> Jyt...@li...
> https://lists.sourceforge.net/lists/listinfo/jython-users
>