You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2026/09/22 09:18:08 UTC

[Impala-ASF-CR] IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24912


Change subject: IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump
......................................................................

IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Thrift 0.24.0 bump (IMPALA-15354) changed SSL_CTX initialization
to use SSL_CTX_set_min_proto_version() instead of SSL_CTX_set_options()
to set (dis)allowed protocol versions. An unintended
consequence was allowing lower versions than the system's default minimum,
which was not possible with flags like SSL_OP_NO_TLSv1.

This caused test failures in some environments (RHEL 8.6) in cases
where the minimum version was not set (ThriftServerBuilder::ssl_version()),
passing default tls1 which overrode 1.2 from the system default. In tests
that disable tls1.2, this could lead to a version list with a gap like
tls1, tls1.1, tls1.3. AFAIK normally tls1.3 would be negotiated in this
case, but on RHEL 8.6 the SSL handshake failed.

In production the tls version is always set, and the only effect was that
Impala may lower the version to 1.2 when the system policy mandates 1.3.

Two separate fixes are added:
- ThriftServerBuilder's default is set to tls1.2 (only affects tests)
- SSL_CTX_set_min_proto_version is only called if the version is
  increased, not lowered

Assisted-by: Claude Opus 4.8 (Claude Code)
Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
---
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
2 files changed, 10 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/24912/1
-- 
To view, visit http://gerrit.cloudera.org:8080/24912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
Gerrit-Change-Number: 24912
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>

[Impala-ASF-CR] IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24912 )

Change subject: IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/24130/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
Gerrit-Change-Number: 24912
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Sep 2026 09:45:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24912 )

Change subject: IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump
......................................................................

IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Thrift 0.24.0 bump (IMPALA-15354) changed SSL_CTX initialization
to use SSL_CTX_set_min_proto_version() instead of SSL_CTX_set_options()
to set (dis)allowed protocol versions. An unintended
consequence was allowing lower versions than the system's default minimum,
which was not possible with flags like SSL_OP_NO_TLSv1.

This caused test failures in some environments (RHEL 8.6) in cases
where the minimum version was not set (ThriftServerBuilder::ssl_version()),
passing default tls1 which overrode 1.2 from the system default. In tests
that disable tls1.2, this could lead to a version list with a gap like
tls1, tls1.1, tls1.3. AFAIK normally tls1.3 would be negotiated in this
case, but on RHEL 8.6 the SSL handshake failed.

In production the tls version is always set, and the only effect was that
Impala may lower the version to 1.2 when the system policy mandates 1.3.

Two separate fixes are added:
- ThriftServerBuilder's default is set to tls1.2 (only affects tests)
- SSL_CTX_set_min_proto_version is only called if the version is
  increased, not lowered

Assisted-by: Claude Opus 4.8 (Claude Code)
Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
Reviewed-on: http://gerrit.cloudera.org:8080/24912
Reviewed-by: Peter Rozsa <pr...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
2 files changed, 10 insertions(+), 5 deletions(-)

Approvals:
  Peter Rozsa: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
Gerrit-Change-Number: 24912
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24912 )

Change subject: IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump
......................................................................


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
Gerrit-Change-Number: 24912
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Sep 2026 15:22:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/24912 )

Change subject: IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
Gerrit-Change-Number: 24912
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Sep 2026 10:50:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24912 )

Change subject: IMPALA-15393: Fix TLS minimum version handling after Thrift 0.24 bump
......................................................................


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/14116/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7542445bcf6623b15ba8c66aaba585983e11afc7
Gerrit-Change-Number: 24912
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Sep 2026 10:53:41 +0000
Gerrit-HasComments: No