You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2016/11/04 23:40:33 UTC
[kudu-CR] [ssl] disable SSL/TLS compression
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4962
Change subject: [ssl] disable SSL/TLS compression
......................................................................
[ssl] disable SSL/TLS compression
As for the best recommended practices for SSL/TLS deployment,
disable compression even if it's supported the both connection peers.
https://tools.ietf.org/html/rfc7525#section-3.3
Also, disabling SSL/TLS compression frees CPU resources.
Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
---
M src/kudu/util/net/ssl_factory.cc
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/4962/1
--
To view, visit http://gerrit.cloudera.org:8080/4962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [ssl] disable SSL/TLS compression
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [ssl] disable SSL/TLS compression
......................................................................
Patch Set 1:
(1 comment)
> (1 comment)
>
> Good catch. I wasn't aware of this. Also just FYI, it looks like
> it's disabled by default from OpenSSLv1.1.0.
I also have found that just recently while trying to understand how much penalty adding SSL/TLS costs. In particular, I was watching
https://www.youtube.com/watch?v=0EB7zh_7UE4
https://www.youtube.com/watch?v=0EB7zh_7UE4
http://gerrit.cloudera.org:8080/#/c/4962/1/src/kudu/util/net/ssl_factory.cc
File src/kudu/util/net/ssl_factory.cc:
PS1, Line 94: SSL_OP_NO_COMPRESSION
> Do you think adding a comment like this is necessary?
Yep, I think it's good idea, will add.
--
To view, visit http://gerrit.cloudera.org:8080/4962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [ssl] disable SSL/TLS compression
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4962
to look at the new patch set (#2).
Change subject: [ssl] disable SSL/TLS compression
......................................................................
[ssl] disable SSL/TLS compression
As for the best recommended practices for SSL/TLS deployment,
disable compression even if it's supported the both connection peers.
https://tools.ietf.org/html/rfc7525#section-3.3
Also, disabling SSL/TLS compression frees CPU resources.
Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
---
M src/kudu/util/net/ssl_factory.cc
1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/4962/2
--
To view, visit http://gerrit.cloudera.org:8080/4962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [ssl] disable SSL/TLS compression
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.
Change subject: [ssl] disable SSL/TLS compression
......................................................................
[ssl] disable SSL/TLS compression
As for the best recommended practices for SSL/TLS deployment,
disable compression even if it's supported the both connection peers.
https://tools.ietf.org/html/rfc7525#section-3.3
Also, disabling SSL/TLS compression frees CPU resources.
Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
Reviewed-on: http://gerrit.cloudera.org:8080/4962
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M src/kudu/util/net/ssl_factory.cc
1 file changed, 7 insertions(+), 1 deletion(-)
Approvals:
Dan Burkert: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [ssl] disable SSL/TLS compression
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.
Change subject: [ssl] disable SSL/TLS compression
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Good catch. I wasn't aware of this. Also just FYI, it looks like it's disabled by default from OpenSSLv1.1.0.
http://gerrit.cloudera.org:8080/#/c/4962/1/src/kudu/util/net/ssl_factory.cc
File src/kudu/util/net/ssl_factory.cc:
PS1, Line 94: SSL_OP_NO_COMPRESSION
Do you think adding a comment like this is necessary?
"Disable compression as it's subject to the CRIME attack"
--
To view, visit http://gerrit.cloudera.org:8080/4962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [ssl] disable SSL/TLS compression
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [ssl] disable SSL/TLS compression
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib470d1c00abb5a4bdf4650fc3ed19b6d588ea78f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No