Backport 3.6: ssl_client1 and ssl_server demo scripts #9505
Conversation
gilles-peskine-arm
added
component-tls
needs-ci
| grep '^ *client_version=' "$server_log" | ||
| grep '^ *KeyExchangeAlgorithm=' "$server_log" || # TLS 1.2 | ||
| grep -A1 '^ *extension_type=key_share' "$server_log" # TLS 1.3 | ||
| grep '^ *cipher_suite ' "$server_log" | ||
| grep 'ApplicationData' "$server_log" |
There was a problem hiding this comment.
I should give better feedback if one of these fails.
|
|
||
| EOF | ||
|
|
||
| if ! openssl version >/dev/null; then |
There was a problem hiding this comment.
So far, I've only tested with OpenSSL 3.0.2 on Ubuntu 22.04.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add demo scripts for the basic TLS client ssl_client1, running against an OpenSSL server. Have separate scripts for TLS 1.2 and TLS 1.3. This makes it easier to specify configuration dependencies and track failures. This commit does not yet handle DLTS with dtls_client. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
`openssl s_server -www` dumps a lot of information about the server. It's noise given that our demo is about the client. Now the client's debug logs no longer report that the server sent a CloseNotify alert (-0x7800 = MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY), but that it closed the TCP connection without sending an alert (-0x7280 = MBEDTLS_ERR_SSL_CONN_EOF). That's arguably less nice, but the overall output is nicer. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add demo scripts for the basic TLS server ssl_server, running against an OpenSSL client. Have separate scripts for TLS 1.2 and TLS 1.3. This makes it easier to specify configuration dependencies and track failures. This commit does not yet handle DLTS with dtls_server. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Only DTLS 1.2 since we don't support DTLS 1.3. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
|
@gilles-peskine-arm Is now a good time for me to start reviewing or should I wait for further work (like rebasing to get a green CI) first? |
|
I'm not very happy with the stability of the shell scripts. I fear that they might run into a race condition, or fail because they insist on port 4433. That being said, I started to rewrite this in Python, and I find it actually harder: it would take more time than I care to throw at this task. So I've changed my mind on the design: I want to try this as is, and if it turns out to be unstable on the CI, we'll either fix it in a hurry or temporarily disable it. Of course, if reviewers think this is too fragile, we'll look for other ways. I'm going to rebase on top of the 3.6.1 release, to give it a chance to pass the CI. |
gilles-peskine-arm
force-pushed
the
tls-demo-scripts-3.6
branch
from
f0eaaa0
to
8bd4c1b
Compare
|
Well, that's not good. The SSL demos are failing almost systematically on the CI ( I can't see an obvious cause in the logs. Sometimes the server can't bind port 4433, which suggests that the port is in use and would require the ability to change the port. But sometimes the connection seems to happen correctly and yet the demo is reported as failed. Reusing |
I had a superficial look yesterday and was wondering why not make this part of (I have no strong opinion between |
gilles-peskine-arm
added
needs-work
and removed
needs-reviewer
|
Superseded by #9541. Manuel's suggestion of |
Add demo scripts for
ssl_client1,ssl_server,ssl_pthread_serveranddtls_server. This way we have CI coverage for:The TLS 1.3 demo reveals #9072 and #8749.
Fixes #9072 in the sense that when this pull request passes the CI, it means the issue is fixed. (The actual fixes are in previous PR: #9501 and #9507.)
The sample programs hard-code port 4433, so the demos can't run concurrently. This may be a problem for CI reliability (although in principle it shouldn't be since each test run runs alone in a Docker instance).
Omitted:
mini_client— would require different interactions withopenssl s_server.dtls_client— would require different interactions withopenssl s_server.ssl_fork_server— would require fancier process handling.Priority: high because it is a non-regression test for #9072 and #8749. Not very-high because I don't propose to rush this into 3.6.1. We can do manual testing for 3.6.1, and have the automated testing before 3.6.2.
Status: I expect a CI failure until #8749 is fixed.
PR checklist