Allow simple configuration of supported TLS1.3 groups

[baentsch]

Describe the feature you'd like supported

TLS1.3 permits announcing algorithm groups used during key exchange via the supported_groups extension.

The feature requested is to permit setting these groups for msquic applications to different values via some simple (command line or environment variable) manner to allow easy adaptation to different group (lists).

msquic currently fixes this list to 4 groups as per this code.

Proposed solution

An example how it may be implemented is the openssl -groups parameter.

Additional context

While the chosen msquic preference list is somewhat understandable from a classic cryptography point of view, some users may want to request stronger classic algorithms, e.g., by leaving away weaker algorithms like "P-256". In the context of quantum safe cryptography where many different algorithms could be chosen by way of specifying different group names, the issue is even more acute.

[nibanks]

This would require updating the QUIC_CREDENTIAL_CONFIG to allow the app to pass the allowable groups down. It'd be nice to support a model similar to how we do QUIC_ALLOWED_CIPHER_SUITE_FLAGS but I think the number of groups might be too big.

[baentsch]

It'd be nice to support a model similar to how we do QUIC_ALLOWED_CIPHER_SUITE_FLAGS but I think the number of groups might be too big.

Agreed, this would be "boundless". But why limit/hard-code the (set of) allowable groups in this way? Why not simply adding a (colon-delimited) string of default groups to QUIC_CREDENTIAL_CONFIG? That way everyone can define the list as short (my preference :) or long as s/he wants. To rephrase: Why not move CXPLAT_TLS_DEFAULT_SSL_CURVES into the QUIC_CREDENTIAL_CONFIG struct (as --then modifiable-- string, not as a set of pre-ordained bits à la "QUIC_ALLOWED_CIPHER_SUITE_FLAGS")?

For completeness, here's the diff to the current code base showing how I implemented this feature for now:

diff --git a/src/platform/tls_openssl.c b/src/platform/tls_openssl.c
index 42d6ee70..6f2ccc82 100644
--- a/src/platform/tls_openssl.c
+++ b/src/platform/tls_openssl.c
@@ -1173,7 +1173,7 @@ CxPlatTlsSecConfigCreate(
     Ret =
         SSL_CTX_set1_groups_list(
             SecurityConfig->SSLCtx,
-            CXPLAT_TLS_DEFAULT_SSL_CURVES);
+            getenv("SSL_DEFAULT_GROUPS")?getenv("SSL_DEFAULT_GROUPS"):CXPLAT_TLS_DEFAULT_SSL_CURVES);
     if (Ret != 1) {
         QuicTraceEvent(
             LibraryErrorStatus,

This env-var based approach is completely in line with --for example-- the SSL_CERT_FILE environment variable to define the behaviour (root of trust in this case) for openssl.

Adding this into the code base of openssl would be another approach, but as msquic is actively setting the groups list, overriding this in the underlying library doesn't seem proper -- assuming the calling library/app (msquic in this case) has a good reason for setting specific groups. Without such reason, it might even be a more simple approach to close out this issue by completely removing the call to SSL_CTX_set1_groups_list to give back control over this configuration setting to the underlying crypto library (openssl in this case).

So, may I ask why msquic opted to fix the current key exchange groups?

[nibanks]

why msquic opted to fix the current key exchange groups?

We've had no request (until now) to expose a knob to control the value.

Why not simply adding a (colon-delimited) string of default groups to QUIC_CREDENTIAL_CONFIG?

Yes, I believe that's the direction we'll have to go, but the complexity is Schannel. The MsQuic API supports multiple TLS providers, and anything we expose at the MsQuic API needs to map to them all. So we'd have to write custom code to map that string list into whatever Schannel takes as input.

[baentsch]

We've had no request (until now) to expose a knob to control the value.

That I understand. My question is more about what was the rationale for msquic to "overrule" the defaults of openssl, i.e., to call SSL_CTX_set1_groups_list at all?

into whatever Schannel takes as input.

Hmm, I just tried to find out how Schannel handles/configures key exchange group (announcements), but had no luck... Initially I even got the impression that TLS1.3 isn't really supported -- but then again it couldn't be used for QUIC at all :) So, I can't really help on this one.

[nibanks]

call SSL_CTX_set1_groups_list at all?

I was under the impression I was supposed to call it. 😄 Probably just a copy/paste from some other code.

Initially I even got the impression that TLS1.3 isn't really supported

It's supported in Windows Server 2022 and Windows 11 (it may eventually be backported to Windows 10, but I'm not sure).

I just tried to find out how Schannel handles/configures key exchange group (announcements), but had no luck

It looks like we'd set key exchange via the TlsParametersCngAlgUsageKeyExchange in the CRYPTO_SETTINGS, similar to how we do cipher suits.

[baentsch]

I was under the impression I was supposed to call it. smile Probably just a copy/paste from some other code.

I don't think so. We could close out the whole discussion (and save some work) by you removing this call and me adding the capability into (oqs-)openssl.

This approach would also resolve the "hidden" issue namely that msquic allows/enforces unnecessarily "weak" (in the eye of the beholder :) crypto (key exchange groups) -- at least when using openssl. It might be "interesting" to check what Schannel does by default in this regard, though....

[nibanks]

Feel free to send a PR to remove that code. If the CI all passes still, that'd probably be fine to merge. I still think it's probably worth while to expose a knob to the app eventually though.

[baentsch]

Feel free to send a PR to remove that code.

Sorry -- I'd have done that already (would've saved a lot of typing :) if I'd been OK with the contribution conditions to this project. Hope this doesn't cause you to look upon our interaction with disappointment or disdain now.

I still think it's probably worth while to expose a knob to the app eventually though.

It definitely is. Until then I keep my env var "solution".

[nibanks]

if I'd been OK with the contribution conditions to this project.

I'd be curious to know what issue(s) you have with the agreement. I can try to forward any feedback you have along to the owners of that. Perhaps it's something that can be modified in the future.

Hope this doesn't cause you to look upon our interaction with disappointment or disdain now.

It does not. 😄 We still appreciate working with you!

[baentsch]

Perhaps it's something that can be modified in the future.

Highly improbable (I've been working for more than 2 decades for a US IT company :). More seriously: I'm missing the "AS-IS" and "no support obligation" language I know from (and like in) the OpenSSL ICLA. But more generally, this conceptually doesn't sound right: In order to do free and voluntary work for a company generating multi-billion-dollar profits one needs to provide guarantees and assurances to that company?? Shouldn't that be the other way around? For example, when working (equally free and voluntary) for the Red Cross I a) don't have to sign such stinging contracts, b) don't get tied to a US court and --believe it or not-- c) get insurance cover from the Red Cross when working for them.

[baentsch]

Rethinking options for the above, might it be an idea for @christianpaquin to take a look at the above (basically remove

msquic/src/platform/tls_openssl.c

Lines 1173 to 1185 in 692be23

	Ret =
	SSL_CTX_set1_groups_list(
	SecurityConfig->SSLCtx,
	CXPLAT_TLS_DEFAULT_SSL_CURVES);
	if (Ret != 1) {
	QuicTraceEvent(
	LibraryErrorStatus,
	"[ lib] ERROR, %u, %s.",
	ERR_get_error(),
	"SSL_CTX_set1_groups_list failed");
	Status = QUIC_STATUS_TLS_ERROR;
	goto Exit;
	}

and check CI via a PR)? This would ease activation of liboqs algorithms in msquic substantially and the MSFT contract wouldn't need to be signed by someone already employed, right?