Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10529 )
Change subject: IMPALA-6990: TestClientSsl.test_tls_v12 failing due to Python SSL error ...................................................................... Patch Set 3: Code-Review+1 > (5 comments) > > My main question is to confirm that we understand why the Python > upgrade broke this. Before it was also broken, but we were silently > ignoring that? > > The rest is python nits, which you can ignore if you'd like. I did some more digging to answer the questions and this is what I've come up with (I've posted this in the JIRA as well, but pasting here for better context): I missed a detail which was that this test never ran on RHEL6 due to all our RHEL6 machines having OpenSSL 1.0.0 which doesn't support TLSv1.2, causing them to be skipped. On RHEL7, this used to work before the Thrift upgrade because the old Thrift cpp library (0.9.0) was somehow accepting TLSv1 connections even though we explicitly set TLSv1.2 on the server. I'm unable to figure out why that was happening, and it looks like a bug, but I'll keep looking. It could be a bug in the Python 'ssl' library, or the Thrift 0.9.0 python library, or the Thrift 0.9.0 CPP library, or even OpenSSL. In Thrift 0.9.3, we explicitly select TLSv1.2 if that's what the user specified which fixes the above mentioned bug. Our test caught this bug, since the client side doesn't support TLSv1.2 unless we are equipped with Python 2.7.9 or up. As for a weaker test, we already run test_ssl() which is a weaker test as it doesn't enforce any ciphers or TLS versions which allows the client and server to negotiate a protocol that they're both aware of. So to summarize, things weren't working as expected before and we were passing the test when it should have been failing, but the upgrade fixed the original problem and caused the test to start failing. Carry +1. Thanks for the review! -- To view, visit http://gerrit.cloudera.org:8080/10529 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92c66ecaeb94b0c83ee6f1396c082709c21b3187 Gerrit-Change-Number: 10529 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Comment-Date: Wed, 30 May 2018 22:32:42 +0000 Gerrit-HasComments: No
