Dan Burkert has posted comments on this change. Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs ......................................................................
Patch Set 1: (6 comments) 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, > Any idea how many of those 15% are _actually_ publicly accessible by mistak Thanks for running the numbers, Harsh. I'm hoping we can figure out a way to do this by default, since the vulnerability has had such serious consequences in other databases. Todd, that's an interesting idea. What would we use as the size of the subnet, /24? http://gerrit.cloudera.org:8080/#/c/6514/1/src/kudu/rpc/server_negotiation.cc File src/kudu/rpc/server_negotiation.cc: Line 145: // See https://krebsonsecurity.com/2017/01/ Instead of linking to the blog, it's probably best to reference the associated KUDU jira (which in turn includes that link). Line 149: negotiated_authn_ == AuthenticationType::INVALID)) { The authentication type should never be INVALID at this point (that's just a marker value which indicates the actual type hasn't been negotiated yet). I believe we should be checking that the auth type / mechanism it's negotiated is !(AuthenticationType::SASL && SaslMechanism::PLAIN). Line 685: if (!FLAGS_allow_unauthenticated_public_connections && I don't think you need to check here, it should be handled correctly by the checks above. http://gerrit.cloudera.org:8080/#/c/6514/1/src/kudu/util/net/sockaddr.cc File src/kudu/util/net/sockaddr.cc: PS1, Line 117: 0x0ff could this just be 0xff? Having an odd # of hex digits is somewhat unordinary. Line 121: return true; The google style guide allows omitting the braces here, but I think our usual style is to only allow omitting the braces if the condition and statement are on a single line. So I'd recommend adding explicit braces to these conditionals. -- 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: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Harsh J <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
