Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23149 )

Change subject: [webserver] Add support for TLS 1.3
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/23149/4/src/kudu/server/webserver-test.cc
File src/kudu/server/webserver-test.cc:

http://gerrit.cloudera.org:8080/#/c/23149/4/src/kudu/server/webserver-test.cc@524
PS4, Line 524: TEST_F(Tls13WebserverTest, TestTlsMinVersion) {
> Since you added support of setting of TLSv1.3 ciphers into the embedded web
I did some manual testing (added this to the commit message now), but 
unfortunately, I couldn't find a way to test it with curl.


http://gerrit.cloudera.org:8080/#/c/23149/4/thirdparty/patches/squeasel-tls-min-version.patch
File thirdparty/patches/squeasel-tls-min-version.patch:

PS4:
> nit: maybe, consider renaming this patch to match what it has become in PS4
Done


http://gerrit.cloudera.org:8080/#/c/23149/4/thirdparty/patches/squeasel-tls-min-version.patch@58
PS4, Line 58: +  if (ctx->config[SSL_CIPHERS] != NULL || 
ctx->config[SSL_CIPHERSUITES] != NULL) {
            : +    // The sequence of SSL_CTX_set_ciphersuites() and 
SSL_CTX_set_cipher_list()
            : +    // calls below is essential to make sure the TLS engine ends 
up with usable,
            : +    // non-empty set of ciphers in case of early 1.1.1 releases 
of OpenSSL (like
            : +    // OpenSSL 1.1.1 shipped with Ubuntu 18).
            : +    //
            : +    // The SSL_CTX_set_ciphersuites() call cares only about 
TLSv1.3 ciphers, and
            : +    // those might be none. From the other side, the 
implementation of
            : +    // SSL_CTX_set_cipher_list() verifies that the overall 
result list of
            : +    // ciphers is valid and usable, reporting an error otherwise.
            : +    //
            : +    // If the sequence is reversed, no error would be reported 
from
            : +    // TlsContext::Init() in case of empty list of ciphers for 
some early-1.1.1
            : +    //
> IIRC, there were some funny details on the proper ordering of these calls f
Nice catch, thanks.



--
To view, visit http://gerrit.cloudera.org:8080/23149
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba21d62e22962c782ff8013d805b31ff058d9245
Gerrit-Change-Number: 23149
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Comment-Date: Mon, 21 Jul 2025 12:17:18 +0000
Gerrit-HasComments: Yes

Reply via email to