tlse icon indicating copy to clipboard operation
tlse copied to clipboard

Implement TLS_ECDSA_SIGN_SHA256_OID (prevents access to Cloudflare Domains)

Open turbo opened this issue 3 years ago • 16 comments

A significant amount of internet traffic is shielded by Cloudflare. Including my website, which is why I was surprised to see certificate validation fail for them. Here's why and what would need to be done to fix it.

When we modify the non-blocking low-level example (tlsclienthello) to use proper certificate validation, we'll run into an issue with tls_certificate_chain_is_valid. For domains like google.com and tio.run, this works fine but fails for domains like std.fyi which use Cloudflare's SSL.

A quick openssl s_client -connect <host>:443 shows us why. google.com's certificate (Google CA) and tio.run's cert (LetsEncrypt CA) use RSA as the signature type and SHA256 as the digest. The digest is verified by tls_certificate_verify_signature which is called by tls_certificate_chain_is_valid.

With a cert using ECDSA as the signature algorithm, tls_certificate_verify_signature will fail. It does so because the algorithm field is completely empty. The reason for that is a bug in the tls_certificate_set_algorithm function. That function doesn't error if no matching OID was found for the given length, instead it just silently returns, leaving the algorithm and single fields zero'd out.

If we compile with debugging and catch the moment where the sig alg is supposed to be set:

if (_is_field(fields, algorithm_id)){
  DEBUG_PRINT(" !!!SIGN_ALGO!!! ");
  tls_certificate_set_algorithm(&cert->algorithm, &buffer[pos], length);
}

we can see that Cloudflare's cert returns this OID (I also added a debug print to set_algo to catch aforementioned silent error):

HANDSHAKE MESSAGE
 => CERTIFICATE
1          SEQUENCE
1.1          SEQUENCE
1.1.1          CONTEXT-SPECIFIC
1.1.1.1          INTEGER(1): 02 
1.1.2.1        INTEGER(16): 0F 2D 3B A8 03 88 D7 77 0A 84 D6 75 1E 28 5A 79 
1.1.3.1        SEQUENCE
1.1.3.1           !!!SIGN_ALGO!!!  (ERROR: L8 ALGO DOESN'T MATCH ANY OID) OBJECT IDENTIFIER(8): 2A 86 48 CE 3D 04 03 02 

This confirms my suspicion. In fact, the received OID 2A 86 48 CE 3D 04 03 02 matches TLS_ECDSA_SIGN_SHA256_OID, which is part of a commented line in tlse.c, but none of the ECDSA algos are handled by signature verification functions.

To summarize, two problems identified here:

  • tls_certificate_set_algorithm should fail early when no matching OID was found instead of silently carrying on
  • ECSDA support needs to be added. Cloudflare customers can't change the default cert settings and I predict a steady increase in CAs switching to ECDSA sigs over the next few years.

The only workaround, for now, is to either no-op the entire tls_certificate_chain_is_valid call or no-op tls_certificate_verify_signature for the specific case of empty algorithm fields.

turbo avatar Jul 16 '20 18:07 turbo

Note this only applied to TLS1.2 with ECDSA. TLS 1.3 works fine.

turbo avatar Jul 16 '20 19:07 turbo

Were you able to implement a fix for this?

ronaaron avatar Aug 09 '20 17:08 ronaaron

No, I don't know enough about ECDSA (yet) to do this properly. For now, I'm using a fallback in my own fork, see the flowchart at the end of https://github.com/turbo/nuTLS

turbo avatar Aug 10 '20 13:08 turbo

Thanks. Hopefully Eduard is ok and will make an appearance soon.

ronaaron avatar Aug 10 '20 15:08 ronaaron

Hello! I’m fine, thank you Ron, still sailing. Hope to get back sometime at the end of this month. I will update this soon. Now I’m enjoying the nice weather and the breeze.

eduardsui avatar Aug 10 '20 16:08 eduardsui

Oh, I'm happy to hear that! Be well and stay safe.

ronaaron avatar Aug 10 '20 16:08 ronaaron

That's good to hear!

Side note: if you want to get rid of the google license if x25519 is used, the NaCl code (public domain) works as a drop-in replacement, see: https://github.com/turbo/nuTLS/blob/master/nutls.c#L18563-L18815

turbo avatar Aug 10 '20 17:08 turbo

It looks like one of my issues is actually this. Perhaps both of them are.

ronaaron avatar Oct 26 '20 06:10 ronaaron

@turbo, can you provide a test domain for this issue?

Thanks!

eduardsui avatar Mar 20 '21 19:03 eduardsui

Feel free to use the domains from the issue. std.fyi is my domain.

turbo avatar Mar 20 '21 19:03 turbo

I'm trying to understand what is happening. I'm getting an alert, just after the hello message (0x28 - I think this is handshake failure). I'm not sure why... TLSe already reports supporting ecdsa_secp256r1_sha256(0x0403)... but I get this alert. Does anyone has some idea why?

eduardsui avatar Mar 22 '21 14:03 eduardsui

I think I'm seeing the same thing from a user trying to access https://api.tiingo.com/

ronaaron avatar Mar 29 '21 19:03 ronaaron

Enabling DEBUG in tlssimple.c with the 'api.tiigo.com' host, I get:

Initializing dependencies
Message type: 15, length: 2
ALERT MESSAGE
02 50 Consumed -12 bytes
ERROR IN CONSUME: -12
SSL write error -6

ronaaron avatar Mar 30 '21 04:03 ronaaron

It seems that SHA384 ciphers don't work as expected. If I remove the SHA348 ciphers, everything works fine.

eduardsui avatar Apr 07 '21 11:04 eduardsui

Ok, fixed the SHA384 issue, @ronaaron, now it should be ok. It seems that there were multiple issues with the client hello. I use this library mainly as a server so thank you for testing the client.

eduardsui avatar Apr 07 '21 13:04 eduardsui

Excellent work, Eduard. I can confirm that the problem I was having with that one site is over (had to specify TLS ver 1.3 to connect, but it now works).

ronaaron avatar Apr 07 '21 15:04 ronaaron