rustls icon indicating copy to clipboard operation
rustls copied to clipboard

AWS LC fails against golang TLS server while ring works fine

Open robsdedude opened this issue 10 months ago • 6 comments

Checklist

  • [x] I've searched the issue tracker for similar bugs.

Describe the bug I've stumbled over a very specific set-up where when using the AWS LC provider, a connection cannot be established while the ring implementation works fine. I'm not 100% sure this is an issue with this crate, but wan't able to trace it down any further.

The set-up: using a server written in golang enforcing TLS 1.2 (I couldn't reproduce the error with a server written in rust with rustls).

To Reproduce The setup is quite large, even though I tried to reduce it as much as I could.

https://github.com/robsdedude/rusttls-ring-vs-aws-issue/tree/4f42dcf51beadce18dc2c44df610a5184bafa0f9

There's plenty more description about the output setup and the observations in the README of the linked repo.

The bottom line is that when using AWS LC, the client fails with

Custom { kind: InvalidData, error: InvalidCertificate(BadSignature) }

and the server complains

remote error: tls: error decrypting message

Whereas with ring everything works fine. Going through the trace logs, the only difference I could find is that LC offers ECDSA_NISTP521_SHA512 as SignatureAlgorithm while ring doesn't. So I cannot rule out that go's TLS implementation might be at fault here. Sadly I don't know enough TLS details to debug this any further.

robsdedude avatar Apr 22 '24 09:04 robsdedude

Thanks for filing this issue. It sounds interesting and having a repro is a big help (even if its on the larger side). I won't have a chance to investigate for a little while but wanted to leave a comment so you knew it's on our radar.

cpu avatar Apr 25 '24 15:04 cpu

Had a short look at this and it looks like there are no relevant TLS-level differences between the providers. Next step is to minimise the reproducer to just the signature verification, then we can send it upstream.

ctz avatar Apr 25 '24 16:04 ctz

Thank you, both. Your work is much appreciated :relaxed:

robsdedude avatar Apr 25 '24 19:04 robsdedude

So just adding some information here from what I've dug into so far:

Signature algorithms offered by the ring-based client: ring_client_hello

Signature algorithms offered by the aws-lc-rs-based client: lc_client_hello

The increase in algorithms here makes sense due to aws-lc-rs offering a larger set of supported algoirthms in its signature module.

The corresponding Go server key exchange message for a ring client: ring_server_hello

The corresponding Go server key exchange message for aws-lc-rs client: lc_server_hello

What has me a bit puzzled though is why the Go server in both these instances is offering a signature algorithm to the client that can't be used with the certificate? My understanding is that the signature offered in the key exchange message is supposed to be a signed message of the key exchange parameters using ECDSA using p256r1 w/ SHA256 given that the private key is a p256r1 key and that scheme matches the certificate. So why is the Go server offering p384 and p521 in both instances if that doesn't match the servers private key? Interesting that it succeeds with the ring. Or is Wireshark just lying here and really this is just two bytes that specify the digest algorithm and digital signature scheme.

Attaching the debugger on the failure case confirms this is the failure case: https://github.com/rustls/rustls/blob/main/rustls/src/client/tls12.rs#L897. The actual error being returned is UnsupportedSignatureAlgorithmForPublicKey from here.

From rfc4492 section 2.2:

In ECDHE_ECDSA, the server's certificate MUST contain an ECDSA- capable public key and be signed with ECDSA.

The server sends its ephemeral ECDH public key and a specification of the corresponding curve in the ServerKeyExchange message. These parameters MUST be signed with ECDSA using the private key corresponding to the public key in the server's Certificate.

The client generates an ECDH key pair on the same curve as the server's ephemeral ECDH key and sends its public key in the ClientKeyExchange message.

Both client and server perform an ECDH operation (Section 5.10) and use the resultant shared secret as the premaster secret.

Of course it is possible I have a gap in my knowledge here, but figured I'd drop some information of what I had seen so far.

Update: I believe this is likely the intended behavior for TLS 1.2, in that the digest algorithm isn't bound to a specific key size. See this comment from the Go TLS implementation: https://github.com/golang/go/blob/master/src/crypto/tls/auth.go#L182-L192

skmcgrail avatar Apr 26 '24 01:04 skmcgrail

Ah, thanks for the detailed look!

I agree on your conclusion, and I think it means that offering ecdsa_secp521r1_sha512 for TLS1.2 & TLS1.3 implies support for SHA512 signatures for all supported curves on TLS1.2. That is very annoying!

That previously wasn't an issue, because the set of hash functions we were willing to do verifications for {SHA256, SHA384} were supported for each curve.

ctz avatar Apr 26 '24 06:04 ctz

Ah I see since aws-lc-rs doesn't expose SHA512 for the other curves that is creating an issue. Let me revisit that and get back to you, I'm not seeing an immediate reason why we wouldn't allow SHA512 with the non-p521 curves.

Update

I think the direction you have chosen for the fix is likely the best path forward given that you only recently extended support for SHA512.

There is a few online discussions that are of interest around this topic of why you may not want to use larger digest sizes with the smaller keys in addition to reusing the same key across different digest algorithms: https://groups.google.com/g/mozilla.dev.security.policy/c/mCKvUmYUMb0/m/VVW_eSNrCAAJ https://groups.google.com/a/chromium.org/g/security-dev/c/SlfABuvvQas/m/qOil2X4UBQAJ

In Go's fixed preference order for signature algorithms in clients ECDSA SHA512 is actually ranked lower in priority then SHA256 and SHA384 similar to your change here. I imagine these concerns are largely why TLS 1.3 binds the digest used to a specific key size.

skmcgrail avatar Apr 26 '24 16:04 skmcgrail