Custom extensions and backwards compatibility

[wbl]

Currently custom extensions are only settable if openssl hasn't added support itself, and then later versions will disable support. This raises some binary compatibility issues when new extensions are added, but the whole situation is thorny. Either we doubly parse the extension raising questions of who wins, or applications see behavioral changes that might be significant and have to adapt, or we disable support when custom extensions that cover are added vs. drop them, but that might be a problem in other places.

(Sidenote: we ran into this when trying to port the BoringSSL QUIC patches to OpenSSL 3.3. Either implementation works, but both together fail tests because of this issue. I'm not sure why the QUIC layer feels the need to pretend to be a source of custom extensions vs. integrating support)

Given that we deploy dynamically linked packages with many applications relying on them, this could get messy.

[nhorman]

I'm not sure I completely follow here:

1. To the question of who wins when custom extensions are registered twice, ossl_tls_add_custom_ext_intern checks for internal support prior to allowing the registration of an extension, so in that case OpenSSL wins, and, as you note, we are left with the unforseen potential change in behavior from end users. However, it would seem to me that custom extensions are only added when ratified by a standards document so (in an ideal world) the behavior should match, or at least be in compliance with whatever standard its added against. I understand that may not always occur

2. As for why quic implements a custom extension (I assume you're referring to the SSL_EXT_TLS1_3_ENCRYPTED_EXTENSION), I would guess that we weren't in a position to support that extension in TCP TLS or DTLS, but I'm unsure.

Ostensibly, if you were backporting borinssl implementations of QUIC to openssl 3.3, that should be done with openssls quic support disabled (but I'm just guessing there)

@hlandau @mattcaswell can you please comment here?

[wbl]

With 1 there's a problem where the extension communicates additional data that the application provides or vice versa. Whatever mechanism was used in the custom handler is not available to openssl, so the behavior will not match. I think we need to consider having custom handlers take priority if we want to really have compatibility.

With 2: why not use the boringssl api and build the openssl quic on top if this layering is an issue? Applications have two different nonconflicting paths to set up an SSL object: for the Boring API they call a specific function, for the OpenSSL one they inject a specific SSL method. I wouldn't be surprised if there are some rough edges, but it's conceptually doable.

Forcing a choice here means that administrators and vendors of shared libraries have to pick one or the other, with consequences for shipping applications today that use QUIC that we need to support, and for further applications that have to decide what QUIC stack to support. I suspect that particularly for servers the existing, deployed today server side stacks would win out over future unproven work.

[mattcaswell]

As for why quic implements a custom extension (I assume you're referring to the SSL_EXT_TLS1_3_ENCRYPTED_EXTENSION), I would guess that we weren't in a position to support that extension in TCP TLS or DTLS, but I'm unsure

I assume @wbl is referring to this code:

openssl/ssl/quic/quic_tls.c

Lines 758 to 768 in 067fbc0

	if (!ossl_tls_add_custom_ext_intern(NULL, &sc->cert->custext,
	qtls->args.is_server ? ENDPOINT_SERVER
	: ENDPOINT_CLIENT,
	TLSEXT_TYPE_quic_transport_parameters,
	SSL_EXT_TLS1_3_ONLY
	| SSL_EXT_CLIENT_HELLO
	| SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
	add_transport_params_cb,
	free_transport_params_cb, qtls,
	parse_transport_params_cb, qtls))
	return RAISE_INTERNAL_ERROR(qtls);

At the time of writing that code I was seeking to keep the changes required to support QUIC in the core TLS code to an absolute minimum. I had refactored the record layer to enable the setting of a custom record layer. That enabled the QUIC code to set its own record layer without introducing knowledge of QUIC into the core TLS code. By the same thinking, this extension was something only useful for QUIC so I was seeking to keep it within the QUIC code, and so chose to use a custom extension to do that. It is certainly a valid alternative design to implement the extension in the normal way - but that would require the core TLS extensions code to be modified in some way to know only to use the extension in QUIC (probably with some extra "context" flag, e.g. SSL_EXT_QUIC_ONLY)

[hlandau]

@wbl To be clear — the custom extension (QUIC Transport Parameters) is only added by the QUIC code. So if you aren't using the OpenSSL QUIC stack, it shouldn't be causing any problems if you are trying to add QUIC Transport Parameters as a custom extension itself (even if OpenSSL is compiled with QUIC support enabled).

I'm not sure if that answers your question — let me know.

[davidben]

This kind of issue, combined with observing the original API did not work when upgrading TLS 1.2 to TLS 1.3, was why we removed the custom extensions API in BoringSSL. (BoringSSL does not use custom extensions to implement its QUIC API. We just taught the library how to handle the various QUIC extensions.)

While it's handy to allow callers to add extensions without the TLS library supporting them, it conflicts with wanting to be able to evolve the TLS library backwards-compatibly. We prefer APIs with semantics that can survive the TLS library moving to new versions or adopting new features.