Verify certificate key usage

[ocheron]

This is to address one issue reported in #155:

 FAIL certificate has invalid key usage for HTTPS connection [reject bad-key-usage.badtls.io:11005]
      output: 200 OK

[kazu-yamamoto]

@vincenthz Would you review this PR?
I don't have enough knowledge to review this PR.

[kazu-yamamoto]

Ping @vincenthz

[vincenthz]

👍

[ocheron]

Thanks

[vdukhovni]

I am starting to find mail servers with self-signed certificates that now fail to negotiate DHE-RSA, even though the certificate keyUsage (and Extended Key Usage just because), include only:

            X509v3 Key Usage: 
                Key Encipherment, Data Encipherment
            X509v3 Extended Key Usage: 
                TLS Web Server Authentication

I should note that TLS 1.2 and earlier made no mention of keyUsage enforcement, and the server is doing TLS 1.2 (not TLS 1.3). Furthermore, since the motivation for this enforcement is to avoid Bleichenbacher attacks, it seems odd to me to restrict this certificate to just RSA key transport, which is what's vulnerable to the Bleichenbacher attacks.

I'll double check on the TLS WG list, but it seems to me at first blush that the check may be counter-producitve. FWIW, OpenSSL 1.1.1 s_client completes a handshake with the same server with no fuss.

[vdukhovni]

I think this will likely need revising. We'll see what folks say on the TLS WG list, but in any case basing the check on the cipher suite rather than the actual key exchange method looks wrong... Or did I miss another check in the TLS 1.3 code path, perhaps added later???

[vdukhovni]

I should further note that with the only mention of keyUsage enforcement in TLS RFCs being in TLS 1.3 (RFC8446, appendix E.8) it is rather ironic that we're enforcing the constraint based on the selected cipher, since in TLS 1.3 the ciphers don't actually select the key exchange method, that's based on the signature algorithms extension and certificate type. So the check is entirely ineffective with TLS 1.3!

So perhaps this PR not only causes some interoperability issues, but also fails to achieve the goals it set out to meet. :-(

[ocheron]

The PR was entirely focused on TLS 1.2, at this point I have no idea if it is compatible with 1.3.

For TLS 1.2 this is modeled out of RFC 5246 section 7.4.2. If it not aligned with other implementations and causes interoperability failures we can decrease the requirements or make some of them optional.

[ocheron]

Can you look if it is because of self-signed? Usually validation is less.

[vdukhovni]

@ocheron Can you look if it is because of self-signed? Usually validation is less.

It is self-signed, but it has DANE TLSA records, so with OpenSSL, Postfix, ... it passes validation, but with my DANE survey engine written in Haskell and newly recompiled against the latest TLS libraries, the TLS handshake fails, it seems that this behaviour is hard-coded and I cannot turn it off.

One person on the TLS WG list seems to think this is correct behaviour, but can't think of any relevant attack for RSA. Since almost all certificates are RSA, if nobody comes up with any compelling, I'd like to ask that we relax the check for RSA when the key usage appears to require RSA key exchange, and only enforce the check when the key usage forbids RSA key exchange (which is the problem case with RSA).

WIth ECDSA, I'd like to hear more about the possible attacks, but yes, ECDSA is somewhat brittle to key recovery with poor randomness or other issues, so I would not be too surprised...

[vdukhovni]

So, yes the certificate was created by a non-expert, who slightly messed up, but no obvious problem, and so far I am thinking that certificate should be tolerated.

And I think, that if we're serious about this check it should be extended to TLS 1.3, where it needs to check the key exchange message signature, not the cipher. So both relax it for some RSA cases perhaps, and at the same time extend it to TLS 1.3 if it should cover the other cases.

[vdukhovni]

I'm coming to the realization that the right place to enforce keyUsage is in the server application, NOT the client. Each server should avoid using its keys outside the designated keyUsage restrictions, as this can risk key compromise. If the risk is server key compromise, legitimate clients are not protected by enforcing the restriction when the server does not, because the attacker will recover the keys using his own client. The only thing that comes to mind is DROWN, but SSLv2 support in servers has been systematically disabled since then.

Therefore, I'd prefer to see this PR reverted, and the checks applied on the server side, with server only offering ciphersuites (TLS ≤ 1.2) and signature algorithms (TLS ≥ 1.2) that are compatible with the keyUsage of its own certificate(s).

[ocheron]

The server-side restrictions have already been here for years, and I agree exact mirroring on client side is probably too restrictive. Yes this is hardcoded, I didn't submit the part to override the behavior. For now I still think some form of client restriction is needed and I'll try to analyze what conditions to relax.

[vdukhovni]

My most up-to-date thoughts on the subject are in:

https://www.ietf.org/mail-archive/web/tls/current/msg27203.html

[vdukhovni]

Quick summary, enforce for ECDSA, enforce for TLS 1.3, but for RSA don't enforce with TLS ≤ 1.2 if we are allowing RSA key exchange ciphers, as I believe that in that case the client gains little from enforcing the keyUsage, and interoperability with past practice might be the more important factor...

[kazu-yamamoto]

Maybe we should provide an option to weaken the ristriction in the client side.

[vdukhovni]

I am not sure that a configuration option is needed. It seems the enforcement for ECDSA is already common across other toolkits, and also in TLS 1.3. So the questionable case is RSA for TLS ≤ 1.2. Perhaps my proposal is sufficient without more configuration knobs?

If there are more configuration knobs, instead of changing the API, perhaps some sort of general-purpose option parser is more appropriate, so that the library can recognize new options over time, without applications having to constantly adapt to a new configuration API.

Rather than all the "Default" instances, something similar to the Monoid changes for queries in DNS is more usable if you want a type-safe interface that's not tied into an ever changing option record layout. In DNS we managed to hide the internal representation and provide composable mutators. That's the type-safe version, but it still requires application code changes to take advantage of new options.

With TLS it really might make sense to lose a bit of type safety, and define a parser for either configuration files or a set of command-line switches (an application-provided prefix, allows each application to group the command-line arguments that the application will pass to the TLS library). Users can provide switches or config files that work with the latest TLS library, even if the application only "knows" about older versions.

For now, I'd resist adding more knobs to the somewhat clunky old settings. Only the "hooks" should require close integration between the library and application code, and even there we need to strive to maintain adequate generality, so that the interfaces don't easily grow stale...

[ocheron]

So it seems we have same behavior as GnuTLS. For example when I test connecting gnutls-cli to openssl s_server with a server RSA certificate having only KeyUsage_keyEncipherment bit set, the server picks ECDHE_RSA and the client rejects with:

|<1>| Peer's certificate does not allow digital signatures. Key usage violation detected.
*** Fatal error: Key usage violation in certificate has been detected.

On the other hand, openssl s_client checks the certificate key usage irrespective of key exchange or whether self-signed or not:

• Accepts with any of: KeyUsage_digitalSignature, KeyUsage_keyEncipherment or KeyUsage_keyAgreement
• Rejects with only values in: KeyUsage_nonRepudiation, KeyUsage_dataEncipherment, KeyUsage_keyCertSign, KeyUsage_cRLSign, KeyUsage_encipherOnly and KeyUsage_decipherOnly

I opened #301 to relax the verification conditions with the rules suggested.

An easy-to-use configuration mechanism can be built on top of Haskell-level configuration and hook-based pluggable behavior. This is also related to the policy mechanism described in #157.

For key usage I don't think configuration is required at this point as nobody explicitly asked yet. It would also more code to maintain, API to document and it does not remove the need for good default.

Note: After this PR I also noticed my source comment about ValidationCache was wrong. In theory it's possible to pack both port number and key exchange into ServiceID. Probably bad idea in practice to those who expect port number only.