Skip to content

Conversation

@gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Sep 25, 2024

Forward-port of #9541, plus adaptations needed to handle the incompatible changes between 3.6 and 4.0:

Closes #9072.

PR checklist

  • changelog not required because: test only
  • development PR here
  • framework PR not required
  • 3.6 PR Backport 3.6: test sample programs in ssl-opt.sh #9541
  • 2.28 PR not required because: let's not bother given that there's likely going to be only one more 2.28.x release
  • tests provided

This is necessary for the SSL sample programs: they hard-code port 4433.

Signed-off-by: Gilles Peskine <[email protected]>
Test ssl_client1 with both TLS 1.2 and TLS 1.3.
Test against both OpenSSL and GnuTLS.

Clean up compile-time requirements in ssl_client1.c: any certificate-based
key exchange is ok, so don't insist on built-in RSA.

Signed-off-by: Gilles Peskine <[email protected]>
Test against both OpenSSL and GnuTLS.

Don't use a proxy. It's not particularly useful here, and would complicate
figuring out port numbers.

Clean up compile-time requirements in dtls_client.c: any certificate-based
key exchange is ok, so don't insist on built-in RSA.

Signed-off-by: Gilles Peskine <[email protected]>
Test ssl_server with both TLS 1.2 and TLS 1.3.
Test against both OpenSSL and GnuTLS.

Clean up compile-time requirements in ssl_server.c: any certificate-based
key exchange is ok, so don't insist on built-in RSA.

Signed-off-by: Gilles Peskine <[email protected]>
Test ssl_pthread_server with both TLS 1.2 and TLS 1.3.
Test against both OpenSSL and GnuTLS.

In the server, flush more often. Otherwise, when stdout is redirected to a
file, the server gets killed before it writes important information, such as
the logs that we expect in the test cases.

Clean up compile-time requirements in ssl_pthread_server.c: any certificate-based
key exchange is ok, so don't insist on built-in RSA.

Signed-off-by: Gilles Peskine <[email protected]>
Test ssl_fork_server with both TLS 1.2 and TLS 1.3.
Test against both OpenSSL and GnuTLS.

In the server, flush more often. Otherwise, when stdout is redirected to a
file, the server gets killed before it writes important information, such as
the logs that we expect in the test cases.

In the server, only write output for 10 seconds, not 100. That's enough time
to start concurrent clients if desired. 100 seconds causes ssl-opt to take a
very long time when the client actually listens to the whole input (which
`gnutls-cli` does, but not `openssl s_client`).

Clean up compile-time requirements in ssl_fork_server.c: any certificate-based
key exchange is ok, so don't insist on built-in RSA.

Signed-off-by: Gilles Peskine <[email protected]>
Test against both OpenSSL and GnuTLS.

Don't use a proxy. It's not particularly useful here, and would complicate
figuring out port numbers.

Clean up compile-time requirements dtls_server.c: any certificate-based key
exchange is ok, so don't insist on built-in RSA.

Signed-off-by: Gilles Peskine <[email protected]>
GnuTLS 3.4.x doesn't allow repeated `-p PORT` arguments.

OpenSSL 1.0.2 has different logs. For TLS 1.2 test cases, use a line that
is present in logs from OpenSSL 1.0.2g, 3.3.0 and presumably all versions
in between.

Signed-off-by: Gilles Peskine <[email protected]>
This is necessary when testing against OpenSSL 1.0.2g.

In the server, flush more often. Otherwise, when stdout is redirected to a
file, the server gets killed before it writes important information, such as
the logs that we expect in the test cases.

Signed-off-by: Gilles Peskine <[email protected]>
Default to connecting to "localhost", like ssl_client1.

Signed-off-by: Gilles Peskine <[email protected]>
dtls_client connects to "localhost", which is usually IPv6 on modern
systems. On our CI, $OPENSSL is OpenSSL 1.0.2g which doesn't support IPv6.
Pitching dtls_client against $OPENSSL works on the CI at the moment, but
only because the CI runs in Docker with default network settings which has
IPv6 disabled. This would stop working if we changed the CI's Docker setup,
and the test case is likely to fail on a developer machine. So switch the
test case to using $OPENSSL_NEXT (which is a version of OpenSSL that has
IPv6 support).

Signed-off-by: Gilles Peskine <[email protected]>
When building with `configs/config-suite-b.h`, the SSL I/O buffer size is
1024 bytes. Experimentally, this isn't quite enough for the test certificate
that we use: the server aborts the handshake with
`MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL` raised from
`mbedtls_ssl_write_certificate()`. State an ad hoc minimum output buffer
size to skip testing `ssl_server` in `config-suite-b`.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added needs-backports Backports are missing or are pending review and approval. component-tls needs-ci Needs to pass CI tests component-tls13 size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Sep 25, 2024
In Mbed TLS 4.0, all cryptography goes through PSA, so calling
psa_crypto_init() is now mandatory before starting a TLS connection (as was
the case in Mbed TLS 3.x with MBEDTLS_USE_PSA_CRYPTO enabled).

Switch the TLS sample programs to calling psa_crypto_init() unconditionally.
Otherwise TLS 1.3 connections fail, and (D)TLS 1.2 connections soon will.

This commit omits the test programs ssl_client2 and ssl_server2, which don't
require a change right now. They will be covered when we make
MBEDTLS_USE_PSA_CRYPTO always on.

Signed-off-by: Gilles Peskine <[email protected]>
TLS 1.3 session tickets require additional handling in the client.
Mbed-TLS#8749

Disable session tickets for ssl_client1 when using TLS 1.3
until Mbed-TLS#6640 is resolved
and (if relevant) implemented in ssl_client1.

Signed-off-by: Gilles Peskine <[email protected]>
Stop testing configurations without PSA (MBEDTLS_PSA_CRYPTO_C or at least
MBEDTLS_PSA_CRYPTO_CLIENT). No future release from this branch will support
such configurations, and we can no longer build the SSL sample programs
without psa_crypto_init.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, and removed needs-ci Needs to pass CI tests labels Sep 26, 2024
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - faithful forward-port plus 3 new commits adapting to differences between branches.

Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, clean forward-port, I've reviewed the 3 new commits for development

@davidhorstmann-arm davidhorstmann-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, needs-backports Backports are missing or are pending review and approval. labels Sep 26, 2024
@davidhorstmann-arm davidhorstmann-arm added this pull request to the merge queue Sep 26, 2024
Merged via the queue into Mbed-TLS:development with commit 1a09caa Sep 26, 2024
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls-docs that referenced this pull request Oct 2, 2024
Needed after Mbed-TLS/mbedtls#9638, no big deal
before.

Signed-off-by: Gilles Peskine <[email protected]>
@davidhorstmann-arm davidhorstmann-arm mentioned this pull request Oct 29, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Design and code approved - may be waiting for CI or backports component-tls component-tls13 priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)

Development

Successfully merging this pull request may close these issues.

ssl_client1 fails on TLS 1.3

3 participants