public/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Attempt to use NULL listeners to avoid use after free

Browse source 
As per @sashan suggestion, try pre-creating user ssls with a NULL
listener

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26361)
This commit is contained in:
Neil Horman 2025-01-21 16:55:15 -05:00
parent ddfd561f3c
commit 56b6ab094e
3 changed files with 23 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

15
doc/man3/SSL_new_listener.pod
View file

@ -109,7 +109,9 @@ by a listener object, that listener object is returned. An SSL object which is
not a listener object and which is not descended from a listener object (e.g. a not a listener object and which is not descended from a listener object (e.g. a
connection obtained using SSL_accept_connection()) or indirectly from a listener connection obtained using SSL_accept_connection()) or indirectly from a listener
object (e.g. a QUIC stream SSL object obtained using SSL_accept_stream() called object (e.g. a QUIC stream SSL object obtained using SSL_accept_stream() called
on a connection obtained using SSL_accept_connection()) returns NULL. on a connection obtained using SSL_accept_connection()) returns NULL. Also note
that pending SSL connections on a QUIC listeners accept queue have some caveats,
see NOTES below.
The SSL_listen() function begins listening after a listener has been created. The SSL_listen() function begins listening after a listener has been created.
Appropriate BIOs must have been configured before calling SSL_listen(), along Appropriate BIOs must have been configured before calling SSL_listen(), along
@ -212,6 +214,17 @@ called on an unsupported SSL object type.
SSL_new_from_listener() returns a pointer to a new SSL object on success or NULL SSL_new_from_listener() returns a pointer to a new SSL object on success or NULL
on failure. On success, the caller assumes ownership of the reference. on failure. On success, the caller assumes ownership of the reference.
=head1 NOTES
SSL_get0_listener() behaves somewhat differently in SSL callbacks for QUIC
connections. As QUIC connections begin TLS handshake operations prior to them
being accepted via SSL_accept_connection(), an application may receive callbacks
for such pending connection prior to acceptance via SSL_accept_connection(). As
listner association takes place during the accept process, prior to being
returned from SSL_accept_connection(), calls to SSL_get0_listener() made from
such SSL callbacks will return NULL. This can be used as an indicator within
the callback that the referenced SSL object has not yet been accepted.
=head1 SEE ALSO =head1 SEE ALSO
L<OSSL_QUIC_server_method(3)>, L<SSL_free(3)>, L<SSL_set_bio(3)>, L<OSSL_QUIC_server_method(3)>, L<SSL_free(3)>, L<SSL_set_bio(3)>,

10
ssl/quic/quic_impl.c
View file

@ -660,7 +660,7 @@ static void qc_cleanup(QUIC_CONNECTION *qc, int have_lock)
ossl_quic_channel_free(qc->ch); ossl_quic_channel_free(qc->ch);
qc->ch = NULL; qc->ch = NULL;
if (qc->port != NULL && qc->listener == NULL) { /* TODO */ if (qc->port != NULL && qc->listener == NULL && qc->pending == 0) { /* TODO */
quic_unref_port_bios(qc->port); quic_unref_port_bios(qc->port);
ossl_quic_port_free(qc->port); ossl_quic_port_free(qc->port);
qc->port = NULL; qc->port = NULL;
@ -674,7 +674,7 @@ static void qc_cleanup(QUIC_CONNECTION *qc, int have_lock)
/* tsan doesn't like freeing locked mutexes */ /* tsan doesn't like freeing locked mutexes */
ossl_crypto_mutex_unlock(qc->mutex); ossl_crypto_mutex_unlock(qc->mutex);
if (qc->listener == NULL) if (qc->listener == NULL && qc->pending == 0)
ossl_crypto_mutex_free(&qc->mutex); ossl_crypto_mutex_free(&qc->mutex);
#endif #endif
} }
@ -4578,7 +4578,8 @@ SSL *ossl_quic_accept_connection(SSL *ssl, uint64_t flags)
conn_ssl = SSL_CONNECTION_GET_USER_SSL(SSL_CONNECTION_FROM_SSL(conn_ssl)); conn_ssl = SSL_CONNECTION_GET_USER_SSL(SSL_CONNECTION_FROM_SSL(conn_ssl));
qc = (QUIC_CONNECTION *)conn_ssl; qc = (QUIC_CONNECTION *)conn_ssl;
qc->accepted = 1; qc->accepted = 1;
qc->listener = ctx.ql;
qc->pending = 0;
if (!SSL_up_ref(&ctx.ql->obj.ssl)) { if (!SSL_up_ref(&ctx.ql->obj.ssl)) {
SSL_free(conn_ssl); SSL_free(conn_ssl);
SSL_free(ossl_quic_channel_get0_tls(new_ch)); SSL_free(ossl_quic_channel_get0_tls(new_ch));
@ -4608,7 +4609,8 @@ static QUIC_CONNECTION *create_qc_from_incoming_conn(QUIC_LISTENER *ql, QUIC_CHA
ossl_quic_channel_get_peer_addr(ch, &qc->init_peer_addr); /* best effort */ ossl_quic_channel_get_peer_addr(ch, &qc->init_peer_addr); /* best effort */
qc->accepted = 0; qc->accepted = 0;
qc->listener = ql; qc->listener = NULL;
qc->pending = 1;
qc->engine = ql->engine; qc->engine = ql->engine;
qc->port = ql->port; qc->port = ql->port;
qc->ch = ch; qc->ch = ch;

3
ssl/quic/quic_local.h
View file

@ -208,6 +208,9 @@ struct quic_conn_st {
/* Flag to indicate if this connection has been accepted */ /* Flag to indicate if this connection has been accepted */
unsigned int accepted : 1; unsigned int accepted : 1;
/* Flag to indicate waiting on accept queue */
unsigned int pending : 1;
/* Default stream type. Defaults to SSL_DEFAULT_STREAM_MODE_AUTO_BIDI. */ /* Default stream type. Defaults to SSL_DEFAULT_STREAM_MODE_AUTO_BIDI. */
uint32_t default_stream_mode; uint32_t default_stream_mode;