eduardsui / tlse

Single C file TLS 1.2/1.3 implementation, using tomcrypt as crypto library
Other
535 stars 87 forks source link

Implement TLS_ECDSA_SIGN_SHA256_OID (prevents access to Cloudflare Domains) #53

Closed turbo closed 1 year ago

turbo commented 4 years ago

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:

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 commented 4 years ago

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

ronaaron commented 4 years ago

Were you able to implement a fix for this?

turbo commented 4 years ago

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

ronaaron commented 4 years ago

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

eduardsui commented 4 years ago

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.

ronaaron commented 4 years ago

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

turbo commented 4 years ago

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

ronaaron commented 3 years ago

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

eduardsui commented 3 years ago

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

Thanks!

turbo commented 3 years ago

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

eduardsui commented 3 years ago

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?

ronaaron commented 3 years ago

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

ronaaron commented 3 years ago

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
eduardsui commented 3 years ago

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

eduardsui commented 3 years ago

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.

ronaaron commented 3 years ago

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).