Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9060 )
Change subject: IMPALA-6418: Find a reliable way to detect supported TLS versions ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/9060/3/be/src/rpc/thrift-server.cc File be/src/rpc/thrift-server.cc: http://gerrit.cloudera.org:8080/#/c/9060/3/be/src/rpc/thrift-server.cc@94 PS3, Line 94: bool is_openssl_1_0_0_or_lower = (max_supported_tls_version < TLS1_1_VERSION); : if (is_openssl_1_0_0_or_lower) return (protocol == TLSv1_0_plus || protocol == TLSv1_0) if (max_supported_tls_version < TLS_1_VERSION) { return protocol == TLSv1_0_plus || protocol == TLSv1_0; } http://gerrit.cloudera.org:8080/#/c/9060/3/be/src/rpc/thrift-server.cc@97 PS3, Line 97: // All other versions supported by OpenSSL 1.0.1 and later. The old code made the assumption that Open SSL 1.0.1 or later will always support TLS 1.1 and TLS 1.2. The new code is inferring the version by checking the max TLS version supported but this may break if there exists a version of the library which supports TLS 1.1 but not TLS 1.2. I am not entirely sure if such a version exists but we don't have proof that it doesn't either. I wonder if the code will be more robust if we do the following instead: switch (max_supported_tls_version) { case TLS1_VERSION: return protocol == TLSv1_0_plus || protocol == TLSv1_0; case TLS1_1_VERSION: return protocol != TLSv1_2_plus && protocol != TLS_v1_2_0; default: return true; } -- To view, visit http://gerrit.cloudera.org:8080/9060 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd40219b7be5889b3c24457acdb79a28bdcd9bfb Gerrit-Change-Number: 9060 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Mon, 22 Jan 2018 19:35:52 +0000 Gerrit-HasComments: Yes
