[openssl/openssl] 99fb78: Fix SSL_select_next_proto

Matt Caswell noreply at github.com
Thu Jun 27 09:37:54 UTC 2024


  Branch: refs/heads/openssl-3.2
  Home:   https://github.com/openssl/openssl
  Commit: 99fb785a5f85315b95288921a321a935ea29a51e
      https://github.com/openssl/openssl/commit/99fb785a5f85315b95288921a321a935ea29a51e
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M ssl/ssl_lib.c

  Log Message:
  -----------
  Fix SSL_select_next_proto

Ensure that the provided client list is non-NULL and starts with a valid
entry. When called from the ALPN callback the client list should already
have been validated by OpenSSL so this should not cause a problem. When
called from the NPN callback the client list is locally configured and
will not have already been validated. Therefore SSL_select_next_proto
should not assume that it is correctly formatted.

We implement stricter checking of the client protocol list. We also do the
same for the server list while we are about it.

CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 015255851371757d54c2560643eb3b3a88123cf1
      https://github.com/openssl/openssl/commit/015255851371757d54c2560643eb3b3a88123cf1
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M ssl/statem/extensions_clnt.c

  Log Message:
  -----------
  More correctly handle a selected_len of 0 when processing NPN

In the case where the NPN callback returns with SSL_TLEXT_ERR_OK, but
the selected_len is 0 we should fail. Previously this would fail with an
internal_error alert because calling OPENSSL_malloc(selected_len) will
return NULL when selected_len is 0. We make this error detection more
explicit and return a handshake failure alert.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 6cc511826f09e513b4ec066d9b95acaf4f86d991
      https://github.com/openssl/openssl/commit/6cc511826f09e513b4ec066d9b95acaf4f86d991
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M ssl/quic/quic_tserver.c

  Log Message:
  -----------
  Use correctly formatted ALPN data in tserver

The QUIC test server was using incorrectly formatted ALPN data. With the
previous implementation of SSL_select_next_proto this went unnoticed. With
the new stricter implemenation it was failing.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 8e81c57adbbf703dfb63955f65599765fdacc741
      https://github.com/openssl/openssl/commit/8e81c57adbbf703dfb63955f65599765fdacc741
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M doc/man3/SSL_CTX_set_alpn_select_cb.pod

  Log Message:
  -----------
  Clarify the SSL_select_next_proto() documentation

We clarify the input preconditions and the expected behaviour in the event
of no overlap.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: add5c52a25c549cec4a730cdf96e2252f0a1862d
      https://github.com/openssl/openssl/commit/add5c52a25c549cec4a730cdf96e2252f0a1862d
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M test/sslapitest.c

  Log Message:
  -----------
  Add a test for SSL_select_next_proto

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 7ea1f6a85b299b976cb3f756b2a7f0153f31b2b6
      https://github.com/openssl/openssl/commit/7ea1f6a85b299b976cb3f756b2a7f0153f31b2b6
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M test/helpers/handshake.c
    M test/ssl-tests/08-npn.cnf
    M test/ssl-tests/08-npn.cnf.in
    M test/ssl-tests/09-alpn.cnf
    M test/ssl-tests/09-alpn.cnf.in

  Log Message:
  -----------
  Allow an empty NPN/ALPN protocol list in the tests

Allow ourselves to configure an empty NPN/ALPN protocol list and test what
happens if we do.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 53f5677f358c4a4f69830d944ea40e71950673b8
      https://github.com/openssl/openssl/commit/53f5677f358c4a4f69830d944ea40e71950673b8
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M ssl/statem/extensions_srvr.c

  Log Message:
  -----------
  Correct return values for tls_construct_stoc_next_proto_neg

Return EXT_RETURN_NOT_SENT in the event that we don't send the extension,
rather than EXT_RETURN_SENT. This actually makes no difference at all to
the current control flow since this return value is ignored in this case
anyway. But lets make it correct anyway.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 195e15421df113d7283aab2ccff8b8fb06df5465
      https://github.com/openssl/openssl/commit/195e15421df113d7283aab2ccff8b8fb06df5465
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M ssl/statem/extensions_clnt.c

  Log Message:
  -----------
  Add ALPN validation in the client

The ALPN protocol selected by the server must be one that we originally
advertised. We should verify that it is.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 7c95191434415d1c9b7fe9b130df13cce630b6b5
      https://github.com/openssl/openssl/commit/7c95191434415d1c9b7fe9b130df13cce630b6b5
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    M test/sslapitest.c

  Log Message:
  -----------
  Add explicit testing of ALN and NPN in sslapitest

We already had some tests elsewhere - but this extends that testing with
additional tests.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


  Commit: 301b870546d1c7b2d8f0d66e04a2596142f0399f
      https://github.com/openssl/openssl/commit/301b870546d1c7b2d8f0d66e04a2596142f0399f
  Author: Matt Caswell <matt at openssl.org>
  Date:   2024-06-27 (Thu, 27 Jun 2024)

  Changed paths:
    A test/recipes/70-test_npn.t
    M util/perl/TLSProxy/Message.pm
    A util/perl/TLSProxy/NextProto.pm
    M util/perl/TLSProxy/Proxy.pm

  Log Message:
  -----------
  Add a test for an empty NextProto message

It is valid according to the spec for a NextProto message to have no
protocols listed in it. The OpenSSL implementation however does not allow
us to create such a message. In order to check that we work as expected
when communicating with a client that does generate such messages we have
to use a TLSProxy test.

Follow on from CVE-2024-5535

Reviewed-by: Neil Horman <nhorman at openssl.org>
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24717)


Compare: https://github.com/openssl/openssl/compare/1967b7316dfa...301b870546d1

To unsubscribe from these emails, change your notification settings at https://github.com/openssl/openssl/settings/notifications


More information about the openssl-commits mailing list