Skip to content

Conversation

@masaori335
Copy link
Contributor

An AuTest filed with boringssl. It looks like boringssl doesn't like the change for the openssl 3.0 (#9753).

file /tmp/_sandbox/tls_client_versions/ts/log/diags.log : Diags log file diags.log should not contain errors - Failed
             Reason: Contents of /tmp/_sandbox/tls_client_versions/ts/log/diags.log contains expression: "ERROR:"
                Details:
                  [Jul  6 02:15:55.834] traffic_server ERROR: SSL::140521672264768:error:1000009e:SSL routines:OPENSSL_internal:INVALID_COMMAND:/opt/boringssl/ssl/ssl_cipher.cc:1108 : 21
                  [Jul  6 02:15:55.834] traffic_server ERROR: invalid cipher suite in records.config : 22
                  [Jul  6 02:15:55.834] traffic_server ERROR: SSL::140521672264768:error:1000009e:SSL routines:OPENSSL_internal:INVALID_COMMAND:/opt/boringssl/ssl/ssl_cipher.cc:1108 : 23
                  [Jul  6 02:15:55.834] traffic_server ERROR: invalid cipher suite in records.config : 24
                  [Jul  6 02:15:55.865] [ET_NET 8] ERROR: SSL::140521545656064:error:100000b9:SSL routines:OPENSSL_internal:NULL_SSL_CTX:/opt/boringssl/ssl/ssl_lib.cc:633: peer address is 127.0.0.1 : 45
                  [Jul  6 02:15:55.865] [ET_NET 8] ERROR: failed to create SSL server session : 46
                  [Jul  6 02:15:55.978] [ET_NET 9] ERROR: SSL::140521544603392:error:100000b9:SSL routines:OPENSSL_internal:NULL_SSL_CTX:/opt/boringssl/ssl/ssl_lib.cc:633: peer address is 127.0.0.1 : 47
                  [Jul  6 02:15:55.978] [ET_NET 9] ERROR: failed to create SSL server session : 48

@masaori335 masaori335 added this to the 10.0.0 milestone Jul 11, 2023
@masaori335 masaori335 self-assigned this Jul 11, 2023
@randall randall changed the title Do not set @SECLEVEL with borginssl Do not set @SECLEVEL with boringssl Jul 11, 2023
@maskit
Copy link
Member

maskit commented Jul 11, 2023

Too much duplication. I'd suggest:

cipher_suite = "The basic ones"
if openssl_version == 3
  cipher_suite += ":@SECLEVEL=0"
endif

and

'proxy.config.ssl.server.cipher_suite': '{0}'.format(cipher_suite),

@bneradt
Copy link
Contributor

bneradt commented Jul 11, 2023

SECLEVEL=0 was OK for a stopgap, but I assume there's a better way to address this. Should the cipher suite value be updated for the more stringent openssl3? This would presumably make boringssl happy too.

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

I assume tests/gold_tests/tls/tls_client_versions_minmax.test.py will need to be updated with a similar change?

@bneradt
Copy link
Contributor

bneradt commented Jul 11, 2023

SECLEVEL=0 was OK for a stopgap, but I assume there's a better way to address this. Should the cipher suite value be updated for the more stringent openssl3? This would presumably make boringssl happy too.

Ah, nevermind. I suppose the issue with these tests is that they force the older TLS versions which OpenSSL 3.0 now requires an explicit SECLEVEL=0 configuration to use.

@bneradt
Copy link
Contributor

bneradt commented Jul 11, 2023

Too much duplication. I'd suggest:

cipher_suite = "The basic ones"
if openssl_version == 3
  cipher_suite += ":@SECLEVEL=0"
endif

and

'proxy.config.ssl.server.cipher_suite': '{0}'.format(cipher_suite),

Looks good. But no need for the format string. This should work just fine:

'proxy.config.ssl.server.cipher_suite': cipher_suite,

Thank you @masaori335 for updating for boringssl.

@masaori335 masaori335 force-pushed the autest-tls_client_versions-boringssl branch from 28470c8 to 055d4ca Compare July 11, 2023 23:21
@masaori335
Copy link
Contributor Author

Updated to reduce dups. Please take another look.

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

Thanks for the update for OpenSSL 3.x

@masaori335 masaori335 merged commit b462f08 into apache:master Jul 17, 2023
zwoop pushed a commit that referenced this pull request Jul 17, 2023
@zwoop
Copy link
Contributor

zwoop commented Jul 17, 2023

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.2 Jul 17, 2023
masaori335 added a commit to masaori335/trafficserver that referenced this pull request Sep 26, 2023
(cherry picked from commit b462f08)
(cherry picked from commit 64c648f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants