[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-17 Thread Dan Burkert (Code Review) 
Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Reviewed-on: http://gerrit.cloudera.org:8080/7821
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-by: Alexey Serbin 
Reviewed-by: Sailesh Mukil 
---
M src/kudu/security/tls_context.cc
1 file changed, 36 insertions(+), 12 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, but someone else must approve
  Alexey Serbin: Looks good to me, approved
  Sailesh Mukil: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-17 Thread Sailesh Mukil (Code Review) 
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 3: Code-Review+2

I tested this on platforms with OpenSSL 1.0.0 and 1.0.1e, by building on one 
and running on the other, and vice versa. And it does what is expected. We will 
do the same in Impala.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 17 Jan 2018 23:12:42 +
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-17 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 17 Jan 2018 18:54:18 +
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-17 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 3:

> LD_LIBRARY_PATH against both versions before pushing just in case we missed 
> something

Confirmed


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 17 Jan 2018 18:42:31 +
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-16 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 3: Code-Review+1

lgtm, please test using LD_LIBRARY_PATH against both versions before pushing 
just in case we missed something


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 17 Jan 2018 06:18:38 +
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-16 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 3: Code-Review+1

LGTM (deferring to other reviewers who might have some more feedback).


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 17 Jan 2018 02:32:06 +
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-16 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 3:

I've updated according to Alexey's suggestion. The check now happens at 
startup, which is a definite improvement.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 17 Jan 2018 01:09:06 +
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-16 Thread Dan Burkert (Code Review) 
Hello Alexey Serbin, Henry Robinson, Adar Dembo, Sailesh Mukil, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/7821

to look at the new patch set (#3).

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
---
M src/kudu/security/tls_context.cc
1 file changed, 36 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/7821/3
--
To view, visit http://gerrit.cloudera.org:8080/7821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-16 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc@52
PS2, Line 52: // --rpc-tls-min-protocol=TLSv1.2 option, negotiations will fail 
at runtime with
: // a 'missing protocol' error:
: /
> I think it's possible to look at SSLv23_method()->version just after initia
In other words, SSLv23_method()->version reports the highest version of the 
TLS/SSL protocol supported by the OpenSSL  library.  And it's straightforward 
to compare the reported version with the required protocol version:

SSL2_VERSION0x0002   
SSL3_VERSION0x0300 
TLS1_VERSION0x0301
TLS1_1_VERSION  0x0302
TLS1_2_VERSION  0x0303



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 16 Jan 2018 19:19:05 +
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-11 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc@52
PS2, Line 52: // --rpc-tls-min-protocol=TLSv1.2 option, negotiations will fail 
at runtime with
: // a 'missing protocol' error:
: /
> is there any way we can make this fail earlier? ie at startup rather than a
I think it's possible to look at SSLv23_method()->version just after 
initialization of the OpenSSL library.  It looks like a hack, but it works for 
the way how the SSLv23_method() is implemented.

As a POC, I compiled the code below at CentOS 6.4 with OpenSSL 1.0.0-stable and 
then ran both against 1.0.0u and 1.0.1e version.  The output was (it's in 
hexadecimal):

1.0.0u: 301
1.0.1e: 303

301 corresponds to TLSv1
303 corresponds to TLSv1.2



#include 
#include 

#include 

using namespace std;

void init_openssl() {
SSL_load_error_strings();
OpenSSL_add_ssl_algorithms();
}

void cleanup_openssl() {
EVP_cleanup();
}

int main() {
init_openssl();
cout << std::hex << SSLv23_method()->version << endl;
cleanup_openssl();
return 0;
}





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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jan 2018 21:20:45 +
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-11 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc@155
PS2, Line 155:options |= SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1;
> No, negotiation will fail with the error on line 55.
Ah, right, thanks.  It seems I missed read that comment, starting from this 
place, my bad.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jan 2018 20:43:41 +
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-11 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc@155
PS2, Line 155:options |= SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1;
> I'm curious what happens if the built binary is run with the library that d
No, negotiation will fail with the error on line 55.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jan 2018 20:40:24 +
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2018-01-11 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc@155
PS2, Line 155:options |= SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1;
I'm curious what happens if the built binary is run with the library that does 
not support TLSv1.2.  Is it going just to silently run the with TLSv1.1 here 
regardless of the fact that the --rpc_tls_min_protocol=TLSv1.2 is set?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jan 2018 20:25:05 +
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-10-16 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7821 )

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc@71
PS2, Line 71: 0x1000U
> That's an interesting discovery, Henry. Let's try to avoid diverting the co
Squeasel did commit the equivalent of this commit, so maybe we should go with 
this commit to keep it the same as squeasel?

https://github.com/cloudera/squeasel/commit/9335b81317a6451d5a37c5dc7ec088eecbf68c82



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-Change-Number: 7821
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 16 Oct 2017 22:46:27 +
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-09-05 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

PS2, Line 71: 0x1000U
> Here is a fun thing I discovered:
That's an interesting discovery, Henry. Let's try to avoid diverting the code 
between Impala and Kudu here, so if you go with runtime detection we should do 
the same.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-31 Thread Henry Robinson (Code Review) 
Henry Robinson has posted comments on this change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

PS2, Line 71: 0x1000U
Here is a fun thing I discovered:

  
henry@hnr-optiplex:/data/henry/src/cloudera/impala-toolchain/openssl-1.0.0s/include
 (master) $ grep -r 0x1000L *
openssl/ssl.h:# define SSL_OP_PKCS1_CHECK_2
0x1000L  
  
henry@hnr-optiplex:/data/henry/src/cloudera/impala-toolchain/openssl-1.0.0s/include
 (master) $ cd ../../openssl-1.0.2l/include/
  
henry@hnr-optiplex:/data/henry/src/cloudera/impala-toolchain/openssl-1.0.2l/include
  $ grep -r 0x1000L *
openssl/ssl.h:# define SSL_OP_NO_TLSv1_1   
0x1000L

In OpenSSL 1.0.0, the constant that became SSL_OP_NO_TLSv1_1 in 1.0.1 was 
already in use for an esoteric option that messes with the cryptographic 
protocol for fault injection (deprecated in 1.0.1).

I can't recall enough about your requirements to puzzle through whether this is 
a real problem for you, but theoretically it does mean that anyone linked 
against 1.0.0 that tries to set --rpc_tls_min_protocol=tlsv1.1 will get some 
unexpected behaviour. IIRC, Kudu isn't expected to work against 1.0.0 anyhow, 
so this may be an academic issue for you. In Impala, we're probably going to 
have to use SSLeay() to detect the OpenSSL version at runtime.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-25 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

PS2, Line 52: // --rpc-tls-min-protocol=TLSv1.2 option, negotiations will fail 
at runtime with
: // a 'missing protocol' error:
: /
is there any way we can make this fail earlier? ie at startup rather than at 
negotiation-time? I'm surprised that it fails only during negotiation time 
since the call is during TlsContext::Init


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-25 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-24 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-24 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

Line 131: // Hard code the SSL_OP_NO_TLSv1 and SSL_OP_NO_TLSv1_1 flags from 
OpenSSL
> Makes sense, but rather than the macros here, can we do something like this
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-24 Thread Dan Burkert (Code Review) 
Dan Burkert has uploaded a new patch set (#2).

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
---
M src/kudu/security/tls_context.cc
1 file changed, 25 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/7821/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-24 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7821/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

Line 131: // Hard code the SSL_OP_NO_TLSv1 and SSL_OP_NO_TLSv1_1 flags from 
OpenSSL
Makes sense, but rather than the macros here, can we do something like this 
earlier, after including the openssl headers:

  #ifndef SSL_OP_NO_TLSv1
  #define SSL_OP_NO_TLSv1 0x0400U
  #endif

  #ifndef SSL_OP_NO_TLSv1_1
  #define SSL_OP_NO_TLSv1_1 0x1000U
  #endif

That should improve readability in the code down here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions

2017-08-24 Thread Dan Burkert (Code Review) 
Hello Henry Robinson, Todd Lipcon,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/7821

to review the following change.

Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions
..

rpc: allow setting --rpc_tls_min_protocol on older RHEL versions

Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
---
M src/kudu/security/tls_context.cc
1 file changed, 20 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/7821/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Todd Lipcon