Greg Solovyev has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12474 )

Change subject: KUDU-1900: add loopback check and test
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12474/14/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:

http://gerrit.cloudera.org:8080/#/c/12474/14/src/kudu/integration-tests/security-itest.cc@330
PS14, Line 330:   KUDU_RETURN_NOT_OK_LOG(GetLocalNetworks(&local_networks), 
ERROR,
> You can use just RETURN_NOT_OK; the KUDU_ prefix is only really intended fo
Done


http://gerrit.cloudera.org:8080/#/c/12474/14/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:

http://gerrit.cloudera.org:8080/#/c/12474/14/src/kudu/util/net/net_util.h@96
PS14, Line 96: const
> No need for const.
Done


http://gerrit.cloudera.org:8080/#/c/12474/14/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:

http://gerrit.cloudera.org:8080/#/c/12474/14/src/kudu/util/net/net_util.cc@311
PS14, Line 311: std::
> Drop the std:: prefix.
Done


http://gerrit.cloudera.org:8080/#/c/12474/10/src/kudu/util/net/socket.cc
File src/kudu/util/net/socket.cc:

http://gerrit.cloudera.org:8080/#/c/12474/10/src/kudu/util/net/socket.cc@304
PS10, Line 304:     return true;
> Do we need to further condition this local.IsAnyLocalAddress()? I'm guessin
I think, we do not need to condition this on local.isAnyLocalAddress, because 
there is no possible scenario where the remote is a loopback and the connection 
is coming from outside. If a network interface is seeing a connection from 
127.x.x.x, this connection is guaranteed to be local. However, the condition 
"local.IsAnyLocalAddress() AND NOT remote.IsAnyLocalAddress()" does not 
guarantee that the connection is local.

Also, here is the conversation between Dan and Todd on KUDU-1900 about this:

D: So the check would become 'remote addr is in 127.0.0.0/8 OR remote addr == 
local addr'? I think that should be safe.

T: Yea, I think if the remote address is in that loopback subnet that you 
mentioned we can skip TLS.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3483a9729ddeeb7901e3738532a45b49e713208f
Gerrit-Change-Number: 12474
Gerrit-PatchSet: 10
Gerrit-Owner: Greg Solovyev <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Greg Solovyev <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 19 Feb 2019 19:02:47 +0000
Gerrit-HasComments: Yes

Reply via email to