|Marked as misclassified||Yes|
Based on: Craig McDaniel to jython-users Aug 25 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.
|2016-08-28 23:58:16||darjus||set||messageid: <firstname.lastname@example.org>|
|2016-08-28 23:58:16||darjus||set||recipients: + darjus|
|2016-08-28 23:58:16||darjus||link||issue2516 messages|