Add a CI workflow for no-tls1_2 no-tls1_3 by bernd-edlinger · Pull Request #18188 · openssl/openssl

bernd-edlinger · 2022-04-27T04:43:03Z

Currently this configuration seems to be failing several tests.

Checklist

documentation is added or updated
tests are added or updated

bernd-edlinger · 2022-04-27T06:27:17Z

I've fixed two test failures that I already knew how to fix, because they were also in 1.1.1,
but all others seem to be regressions from 1.1.1.
I'm not sure if I will find time to fix those, given how many test cases fail with
./config no-tls1_2 no-tls1_3, however no-tls1_2 and no-tls1_3 are okay when used alone.

bernd-edlinger · 2022-04-27T08:29:28Z

Hmm, that is strange, when configured with -d no-tls1_2 no-tls1_3
i simply can't connect s_server with s_client:

$ ../util/shlib_wrap.sh ./openssl s_server 
Using default temp DH parameters
ACCEPT
ERROR
406707F8C17F0000:error:0A000076:SSL routines:(unknown function):no suitable signature algorithm:ssl/t1_lib.c:3387:
shutting down SSL
CONNECTION CLOSED

vs.

$ ../util/shlib_wrap.sh ./openssl s_client 
Connecting to ::1
CONNECTED(00000003)
40579AC9C67F0000:error:0A000438:SSL routines:(unknown function):tlsv1 alert internal error:ssl/record/rec_layer_s3.c:1584:SSL alert number 80
---
no peer certificate available
---
No client certificate CA names sent
---
SSL handshake has read 7 bytes and written 122 bytes
Verification: OK
---
New, (NONE), Cipher is (NONE)
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.1
    Cipher    : 0000
    Session-ID: 
    Session-ID-ctx: 
    Master-Key: 
    PSK identity: None
    PSK identity hint: None
    SRP username: None
    Start Time: 1651047896
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
    Extended master secret: no
---

@mattcaswell is that expected?

t8m · 2022-04-27T08:38:15Z

You would need SECLEVEL=0 as TLS1.1/1.0 requires SHA1 signatures.

bernd-edlinger · 2022-04-27T08:42:50Z

Ah, of course...
But I confiigured no-tls1_2 and no-tls1_3 so it doesn't make sense to have
a security level != 0.
How about this

diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index 7be6d47..f65b4be 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -28,7 +28,11 @@ extern "C" {
 
 /* Default security level if not overridden at config time */
 # ifndef OPENSSL_TLS_SECURITY_LEVEL
-#  define OPENSSL_TLS_SECURITY_LEVEL 2
+#  if defined(OPENSSL_NO_TLS1_2) && defined(OPENSSL_NO_TLS1_3)
+#   define OPENSSL_TLS_SECURITY_LEVEL 0
+#  else
+#   define OPENSSL_TLS_SECURITY_LEVEL 2
+#  endif
 # endif
 
 /* TLS*_VERSION constants are defined in prov_ssl.h */

bernd-edlinger · 2022-04-27T09:08:32Z

That's A LOT better now:

Test Summary Report
-------------------
80-test_ssl_new.t                (Wstat: 256 Tests: 30 Failed: 1)
  Failed test:  12
  Non-zero exit status: 1
90-test_sslapi.t                 (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=248, Tests=3250, 166 wallclock secs ( 7.60 usr  0.86 sys + 272.33 cusr 40.61 csys = 321.40 CPU)
Result: FAIL

bernd-edlinger · 2022-05-13T10:32:50Z

Well, it's getting better:

Test Summary Report
-------------------
80-test_ssl_new.t                (Wstat: 3072 Tests: 30 Failed: 12)
  Failed tests:  1, 3, 5-6, 8-9, 12, 15, 17, 19, 23, 28
  Non-zero exit status: 12
80-test_cipherlist.t             (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
90-test_sslapi.t                 (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1

tests will amiss

t8m · 2022-05-16T13:50:47Z

I still wonder whether all this effort is worth it. Would it make more sense no-tls1_2 no-tls1_3 to imply no-tls? I.E. enable only DTLS? Honestly I cannot imagine a valid setup where only tls up to 1.1 is enabled.

bernd-edlinger · 2022-05-17T04:50:19Z

It works now, so no longer WIP. See also #18252 for the 1.1.1 variant of this PR.

t8m

Some nits

I had to re-base the PR because of a merge conflict in test/ssl-tests/12-ct.cnf

bernd-edlinger · 2022-06-04T08:04:51Z

So while the change in test/ssl-tests/12-ct.cnf is no longer necessary
The new test which was introduced with #17989 in
test/ssl-test/04-client_aith.cnf which is using rsa-pss
starts to fail if neither tls1_2 nor tls1_3 is available.
I've adjusted the test expectation accordingly.

This test needs security level 0.

In order to create a SSL connection with TLS<1.2 we need security level 0.

This test needs -cipher DEFAULT@SECLEVEL=0 to work in that configuration.

Lots of test cases use this proxy and fail, because the security level needs to be adjusted to use @SECLEVEL=0 in this configutation.

This needs en explicit @SECLEVEL=0 in the ciphers string when neither tls1_2 nor tls1_3 are configured.

When tls1_2 and tls1_3 are both disabled and the test does not define an explicit CipherString, use "DEFAULT\@SECLEVEL=0" instead of "DEFAULT".

When tls1_2 is not available this test needs @SECLEVEL=0.

There are a couple changes necessary here, mostly adjusting security level.

When only TLS<1.2 is available and the security level is 0, SSL_get1_supported_ciphers(ssl) retuns NULL. That changed with commit 7bf2e4d. Not sure of that is a bug or a feature, especially since that does not happen with no-tls1_2 no-ec no-dh. Test the current behaviour in case it changes again, so we will know.

The new test using rsa-pss fails if neither tls1_2 nor tls1_3 is available.

bernd-edlinger · 2022-07-29T11:36:43Z

removed the no-dtls1_2 fix from this PR since I found that #18848
just got merged and fixes the same issue.

openssl-machine · 2022-08-29T00:08:52Z

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

openssl-machine · 2022-09-29T00:08:47Z

This PR is in a state where it requires action by @openssl/committers but the last update was 61 days ago

openssl-machine · 2022-11-24T00:09:23Z

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

openssl-machine · 2022-12-25T00:08:58Z

This PR is in a state where it requires action by @openssl/committers but the last update was 61 days ago

openssl-machine · 2023-01-25T00:09:50Z

This PR is in a state where it requires action by @openssl/committers but the last update was 92 days ago

openssl-machine · 2023-02-25T00:10:02Z

This PR is in a state where it requires action by @openssl/committers but the last update was 123 days ago

openssl-machine · 2023-03-28T00:10:13Z

This PR is in a state where it requires action by @openssl/committers but the last update was 154 days ago

openssl-machine · 2023-04-28T00:09:40Z

This PR is in a state where it requires action by @openssl/committers but the last update was 185 days ago

openssl-machine · 2023-05-29T00:09:35Z

This PR is in a state where it requires action by @openssl/committers but the last update was 216 days ago

openssl-machine · 2023-06-29T00:09:40Z

This PR is in a state where it requires action by @openssl/committers but the last update was 247 days ago

tom-cosgrove-arm · 2023-06-29T06:50:23Z

So I'm happy to review this, but it seems there are conflicts, and it's been left so long that "2022" is no longer the current year for copyright lines.

@bernd-edlinger will you update this, or have you lost interest?

openssl-machine · 2023-07-31T00:09:35Z

This PR is waiting for the creator to make requested changes but it has not been updated for 30 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

openssl-machine · 2023-08-31T00:09:40Z

This PR is waiting for the creator to make requested changes but it has not been updated for 61 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

openssl-machine · 2023-09-29T00:09:48Z

This PR has been closed. It was waiting for the creator to make requested changes but it has not been updated for 90 days.

bernd-edlinger added branch: master Applies to master branch branch: 3.0 Applies to openssl-3.0 branch (EOL) labels Apr 27, 2022

bernd-edlinger added the help wanted We depend on community to resolve this issue. label Apr 27, 2022

t8m added the triaged: bug The issue/pr is/fixes a bug label Apr 27, 2022

t8m reviewed Apr 27, 2022

View reviewed changes

Comment thread include/openssl/tls1.h Outdated

bernd-edlinger force-pushed the add_ci_test_for_notls123 branch from a07f686 to ed504d2 Compare May 4, 2022 04:21

paulidale approved these changes May 4, 2022

View reviewed changes

bernd-edlinger mentioned this pull request May 5, 2022

Add a CI workflow for no-tls1_2 no-tls1_3 [1.1.1] #18252

Closed

2 tasks

paulidale previously approved these changes May 9, 2022

View reviewed changes

bernd-edlinger force-pushed the add_ci_test_for_notls123 branch from 494d729 to 8192e89 Compare May 9, 2022 06:38

bernd-edlinger changed the title ~~[WIP] Add a CI workflow for no-tls1_2 no-tls1_3~~ Add a CI workflow for no-tls1_2 no-tls1_3 May 17, 2022

t8m requested changes May 17, 2022

View reviewed changes

Comment thread test/cipherlist_test.c Outdated

Comment thread test/cipherlist_test.c Outdated

Comment thread test/cipherlist_test.c Outdated

Comment thread test/recipes/80-test_ssl_old.t Outdated

t8m previously approved these changes May 17, 2022

View reviewed changes

t8m added approval: review pending This pull request needs review by a committer and removed help wanted We depend on community to resolve this issue. labels May 17, 2022

faramir-dev force-pushed the master branch from 36c269c to 3c0e8bc Compare May 24, 2022 04:50

bernd-edlinger force-pushed the add_ci_test_for_notls123 branch from 50714b1 to ada8859 Compare June 4, 2022 06:35

bernd-edlinger force-pushed the add_ci_test_for_notls123 branch from 41d46b7 to f1f2330 Compare June 16, 2022 06:18

bernd-edlinger mentioned this pull request Jun 22, 2022

Build breakage in OpenSSL 3.0.4 when building with no-rfc3779 #18619

Closed

bernd-edlinger added 11 commits July 29, 2022 13:33

Fix recordlentest.c with no-tls1_2 no-tls1_3

73061e3

This test needs security level 0.

Fix sslbuffertest.c with no-tls1_2 no-tls1_3

1e76d5e

In order to create a SSL connection with TLS<1.2 we need security level 0.

Fix 82-test_tfo_cli.t with no-tls1_2 no-tls1_3

55c864c

This test needs -cipher DEFAULT@SECLEVEL=0 to work in that configuration.

Fix TLSProxy::Proxy with no-tls1_2 no-tls1_3

65e7e42

Lots of test cases use this proxy and fail, because the security level needs to be adjusted to use @SECLEVEL=0 in this configutation.

Fix 70-test_sslmessages.t with no-tls1_2 no-tls1_3

a95958a

This needs en explicit @SECLEVEL=0 in the ciphers string when neither tls1_2 nor tls1_3 are configured.

Fix some ssl_new tests with no-tls1_2 no-tls1_3

5f9b2cb

When tls1_2 and tls1_3 are both disabled and the test does not define an explicit CipherString, use "DEFAULT\@SECLEVEL=0" instead of "DEFAULT".

Fix ssl-tests/19-mac-then-encrypt.cnf with no-tls1_2

68b413e

When tls1_2 is not available this test needs @SECLEVEL=0.

Fix ssl-tests/23-srp.cnf with no-tls1_2

90be7b4

When tls1_2 is not available this test needs @SECLEVEL=0.

Fix sslapitest.c with no-tls1_2 no-tls1_3

a587527

There are a couple changes necessary here, mostly adjusting security level.

Fix ssl-tests/04-client_auth.cnf with no-tls1_2 no-tls1_3

35288d4

The new test using rsa-pss fails if neither tls1_2 nor tls1_3 is available.

bernd-edlinger force-pushed the add_ci_test_for_notls123 branch from 4cf6dc0 to 35288d4 Compare July 29, 2022 11:33

t8m added the branch: 3.1 Applies to openssl-3.1 (EOL) label Oct 24, 2022

paulidale added the waiting-for: contributor response This pull request is awaiting a response by the contributor label Jun 30, 2023

openssl-machine closed this Sep 29, 2023

Uh oh!

Uh oh!

Conversation

bernd-edlinger commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

bernd-edlinger commented Apr 27, 2022

Uh oh!

bernd-edlinger commented Apr 27, 2022

Uh oh!

t8m commented Apr 27, 2022

Uh oh!

bernd-edlinger commented Apr 27, 2022

Uh oh!

bernd-edlinger commented Apr 27, 2022

Uh oh!

Uh oh!

bernd-edlinger commented May 13, 2022

Uh oh!

t8m commented May 16, 2022

Uh oh!

bernd-edlinger commented May 17, 2022

Uh oh!

t8m left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bernd-edlinger commented Jun 4, 2022

Uh oh!

bernd-edlinger commented Jul 29, 2022

Uh oh!

openssl-machine commented Aug 29, 2022

Uh oh!

openssl-machine commented Sep 29, 2022

Uh oh!

openssl-machine commented Nov 24, 2022

Uh oh!

openssl-machine commented Dec 25, 2022

Uh oh!

openssl-machine commented Jan 25, 2023

Uh oh!

openssl-machine commented Feb 25, 2023

Uh oh!

openssl-machine commented Mar 28, 2023

Uh oh!

openssl-machine commented Apr 28, 2023

Uh oh!

openssl-machine commented May 29, 2023

Uh oh!

openssl-machine commented Jun 29, 2023

Uh oh!

tom-cosgrove-arm commented Jun 29, 2023

Uh oh!

openssl-machine commented Jul 31, 2023

Uh oh!

openssl-machine commented Aug 31, 2023

Uh oh!

openssl-machine commented Sep 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bernd-edlinger commented Apr 27, 2022 •

edited

Loading