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: > > (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! I'll address the nits in a separate patchset. -- 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:58 +0000 Gerrit-HasComments: No
