[TLS 1.3] Codepoints for ECDH w/ Brainpool (RFC 8734)

[reneme]

IETF deprecated the code points for the brainpool curves that were used in TLS 1.2 and earlier. Those were initially defined in RFC 7027 (26-28). Therefore, for TLS 1.3 new code points were proposed in RFC 8734 (31-33). 😓

This basically introduced the new code points as aliases for the old ones (similarly to what we've done with some yet-to-be-defined code points for some hybrid algorithms). Technically, for TLS 1.3 handshakes we should ensure that the new ones are used and vice versa. However, I think, for interoperability's sake, we should just let the user configure whatever combination they want.

Also, I added TLS::Group_Params::usable_in_version(Protocol_Version) that returns false when the key exchange group is not compatible with the version parameter. Currently, that filters out the described brainpool constellations and also hybrid key exchanges in TLS 1.2. However, the filter is applied only when creating Client Hello messages. I.e. if we send a Client Hello that insists on TLS 1.3, only the new brainpool code points will be advertised and vice versa. A Client Hello that allows both protocol version may also advertise both brainpool code point families.

I'm open for better suggestions, of course.

[coveralls]

coverage: 92.043% (-0.02%) from 92.06%
when pulling 837453c on Rohde-Schwarz:tls13/brainpool_for_kex
into 20279c6 on randombit:master.

[securitykernel]

This is really unfortunate and unnecessarily complicates things. Just one comment on the curve string to group params matching.

[securitykernel]

IMHO the policy files should not be touched by this change. Users may find it confusing that brainpool curves are present twice here each, I think it would become a common source for error. "brainpool512r1" should be matched by the code to the corresponding code point automatically, just as Group_Params::to_algorithm_spec() does the other way around.

[reneme]

I fully agree on the potential end user confusion. Though, I'm somewhat on the fence whether users may still need the additional flexibility in configuration that it brings.

Just to make sure we're on the same page: You're suggesting, that brainpool384r1 (for instance) should translate into advertising both code points (and potentially offering key share values for both in a TLS 1.3 ClientHello), right?

On the one hand, this would somewhat complicate the logic of the code point selection and introduce a potential source of bugs (as we will likely need to handle this Brainpool-specific special case in more than one place). On the other hand, it would prevent users from explicitly choosing one code point over the other.

This is explicitly not meant to be an objection, though. However, if we introduce the additional logic, I want to suggest a compromise: Let's add the following configuration options:

• brainpool*r1 (the existing name) - add both code points
• brainpool*r1_legacy - add the old code points only
• brainpool*r1_tls13 - add the new code points only

That approach would also make the "special case" more explicit in the code base. The existing name could map to some special Group_Params_Code::BRAINPOOL_META_VALUE which we can handle explicitly in a special case (and map to ::BRAINPOOL_TLS12 and ::BRAINPOOL_TLS13).

Whatever we do: its a mess. 😢

[randombit]

What a totally unnecessary mess this is. Alas.

One concern I have here is that we can't quite treat these two points as equivalent. In particular, IIUC, technically speaking if a client sends a 1.2+1.3 client hello that contains just a 1.2 brainpool curve ID, we must not use brainpool with 1.3.

I'm not sure what the best fix is.

One interesting "out" is that afaik RFC 8734 does not prohibit using the 1.3 brainpool IDs in TLS 1.2 also. But that will cause interop problems with stacks which only support the 1.2 IDs.

[securitykernel]

You're suggesting, that brainpool384r1 (for instance) should translate into advertising both code points (and potentially offering key share values for both in a TLS 1.3 ClientHello), right?

What I think would be the best for users is to avoid having them to deal with the fact that there are two points at all. For the user, the only selection he should make is between the three brainpool*r1 -- in policy files as well as programatically. Botan should then figure out which code point to advertise. This puts the burden on the library's shoulders, I see.

[securitykernel]

I just looked at how OpenSSL solved this mess, and it seems they first implemented the approach I suggest, and then later refactored to what essentially this PR implements.

[reneme]

FWIW: We tested this with OpenSSL 3.2.0 and it works with both TLS 1.2 and 1.3.

Botan TLS policy:

allow_tls13 = true
allow_tls12 = true
allow_dtls10 = false
allow_dtls12 = false
key_exchange_groups = brainpool256r1tls13 brainpool256r1

OpenSSL CLI commands:

# TLS 1.3
openssl s_client -connect localhost:50447 -debug -curves brainpoolP256r1tls13:brainpoolP256r1:secp256r1 -tls1_3

# TLS 1.2
openssl s_client -connect localhost:50447 -debug -curves brainpoolP256r1tls13:brainpoolP256r1:secp256r1 -tls1_2

[randombit]

What happens with these

openssl s_client -connect localhost:50447 -debug -curves brainpoolP256r1:secp256r1 -tls1_3

openssl s_client -connect localhost:50447 -debug -curves brainpoolP256r1tls13:secp256r1 -tls1_2

[reneme]

Those cases don't result in a successful handshake. OpenSSL doesn't seem to offer the groups cross-version. Perhaps we should also try with OpenSSL as the server. Whether they accept TLS 1.3 with the legacy ID and vice versa.

-curves brainpoolP256r1:brainpoolP256r1tls13:secp256r1

When no TLS version restriction is given, tls13 is offered but non-TLS1.3 is marked as supported.

Key Shares: brainpool256r1tls13
Supported Groups: brainpool256r1 brainpool256r1tls13 secp256r1

-curves brainpoolP256r1:secp256r1 -tls1_3

Cross-protocol groups are not offered.

Key Shares: secp256r1
Supported Groups: secp256r1

-curves brainpoolP256r1tls13:secp256r1 -tls1_2

Supported Groups: secp256r1

[randombit]

Those cases don't result in a successful handshake.

By not successful do you mean the handshake ends up using P256? Or an alert occurs?

[reneme]

By not successful do you mean the handshake ends up using P256? Or an alert occurs?

In my particular example, it failed with "no common group". But I configured the botan-based server to exclusively support brainpool. Otherwise, it would just have used P256.

[reneme]

Perhaps we should also try with OpenSSL as the server.

Turns out that an OpenSSL server won't negotiate a connection with TLS 1.3 and the legacy code points either.

I tested with a Botan client that is configured to insist on TLS 1.3 but that offers the legacy brainpool code points only. The OpenSSL server is configured to allow both TLS 1.2 and 1.3 and supports both the new and old brainpool code points. This server rejects the handshake with "no suitable key share".

FWIW: OpenSSL prior to 3.2.0 -- that doesn't support the new code points yet -- also rejects negotiating brainpool (using the old code points) in TLS 1.3.

[reneme]

For the user, the only selection he should make is between the three brainpool*r1 -- in policy files as well as programatically.

Yeah, I agree, that would be great from a user's perspective. And the experiments with OpenSSL above also support such an approach. I'll see tomorrow, if there's a reasonable way to implement it like this. I'm hoping to centralize the necessary mappings somehow. If we have to scatter them across the TLS implementation(s), it's too big of a mess with lots of room for future fuck-up, all for too little user benefit, in my opinion.

[reneme]

The more I look at that, the worse it gets, frankly. Handling all the edge cases properly, requires special stuff in several locations of the code base. Those special cases are also non-trivial to test. With our current test infrastructure we would probably end up building end-to-end tests against OpenSSL 3.2+.

When trying to build this as an abstract policy, one needs to mentally mix in hybrid key exchange algorithms, which we currently support in TLS 1.3 but not 1.2. I.e. that must not be offered when also offering TLS 1.2. In contrast to brainpool, where just the offered code point changes contextually.

What @securitykernel suggested is neat for sure: i.e. hide this complexity from the user by transparently juggling the right code points depending on the available protocol version. Though, that's a completely new concept (i.e. contextually mapping one NamedGroup to one, the other or both code points, and vice versa) in our policy architecture on top of the whole mess this already is.

As an illustration, here are some combinations to keep in mind when implementing this one way or the other:

• Client
  • Offering just TLS 1.2, just 1.3 and both (allowing for downgrade)
  • With new, old and both code points
  • Check that we abort if the peer mixes 1.3 and the old points, and vice versa
  • (probably more)
• Server
  • Allow for just TLS 1.2, just 1.3 and both (allowing for downgrade)
  • Ensure that selected protocol version also results in correct code point revision
    • That might include a Hello Retry Request if a client offers a key share with the old points but allows for usage of TLS 1.3
  • Check that we abort if the peer mixes 1.3 and the old points, and vice versa
  • (probably much more)

All of that extra plumbing for the slim use case that Brainpool is at the end of the day.

Long story short, I'd like to suggest to KISS! Let's introduce the new code points and don't do any additional handling or validation. That will result in Botan allowing to mix and match protocol version and code point revisions. And this might result in interop issues with other implementations. But it gives the library user full flexibility to work around interop issues for their specific use case.

@securitykernel @randombit What do you think?