Hao Hao has posted comments on this change.

Change subject: KUDU-1875: Refuse unauthenticated connections from publicly 
routable IP addrs
......................................................................


Patch Set 7:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/6514/5//COMMIT_MSG
Commit Message:

PS5, Line 9: 
           : This rejects unauthenticated connections from publicly routable 
IPs,
           : even if authentication and encryption are not configured. An unsafe
           : flag 'allow_unauthenticated_public_connections' is provided to 
enable
           : unauthentic
> nit: could you keep the lines under 72 chars length?
Done


http://gerrit.cloudera.org:8080/#/c/6514/1/src/kudu/rpc/negotiation.cc
File src/kudu/rpc/negotiation.cc:

Line 68: DEFINE_bool(allow_unauthenticated_public_connections, false,
> I re-ran the checks with testing public connectivity to some of their used 
Thanks Harsh! I am bringing this discussion to dev mailing list.


http://gerrit.cloudera.org:8080/#/c/6514/5/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

PS5, Line 149:     RETURN_NOT_OK(RejectUntrustedPublicConnection(addr));
             :   }
> nit: why not just
Done


PS5, Line 679:       negotiated_mech_ == SaslMechanism::PLAIN) {
             :     Sockaddr addr;
> RETURN_NOT_OK(RejectUntrustedPublicConnection(addr));
Done


PS5, Line 858: 
> nit: Status::OK() is set by the default constructor, so no assignment is ne
Done


PS5, Line 861: rivateAddress()
> are prohibited
Done


http://gerrit.cloudera.org:8080/#/c/6514/5/src/kudu/util/net/sockaddr.cc
File src/kudu/util/net/sockaddr.cc:

PS5, Line 114: Sockaddr::IsPrivateAddress
> What about link-local addresses like 169.254.0.0/16 except for first and la
Yeah, thanks for point it out. I think we should consider it as private 
address. This is under discussion in the dev mailing list.


PS5, Line 115:   uint32 first_byte, sec_byte;
             :   first_byte = 
NetworkByteOrder::FromHost32(addr_.sin_addr.s_addr) >> 24;
             :   sec_byte = 
(NetworkByteOrder::FromHost32(addr_.sin_addr.s_addr) >> 16) & 0xff;
> nit: why not to initialize the variables at the point of definition, like 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Harsh J <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to