You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2017/03/30 08:26:31 UTC
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Hao Hao has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6514
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
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 unauthenticated
connections from publicly routable IPs. Even though it is highly recommended to
disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
7 files changed, 137 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/1
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. If this flag is set explicitly, all unauthenticated
or unencrypted connections are prohibited except the ones from the
specified address blocks. Otherwise, private network (127.0.0.0/8,
etc.) and local subnets of all local network interfaces will be used.
Set it to '0.0.0.0/0' allows unauthenticated/unencrypted connections
from all remote IP addresses. However, if network access is not
otherwise restricted by a firewall, malicious users may be able to
gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Reviewed-on: http://gerrit.cloudera.org:8080/6514
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/socket.h
7 files changed, 337 insertions(+), 7 deletions(-)
Approvals:
Dan Burkert: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 38
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 36:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6514/36/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 961: if (google::GetCommandLineFlagInfoOrDie("trusted_subnets").is_default) {
> Does not quite follow if we move this if condition to call_once, how do we
We only flags marked with the 'runtime' tag to be set after startup. Typically string flags can't be marked runtime due to thread safety issues. Is there a reason we would want it to be runtime updatable? We probably shouldn't be using call_once at all if that's the case.
--
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: 36
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#19).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
It also provides two flags: one adavance flag 'trusted_subnets' to
whitelist trusted subnets. Connections from trustred subnets will
be allowed even if unauthenticaed; another unsafe flag
'allow_unauthenticated_public_connections' is provided to enable
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/init.cc
M src/kudu/security/init.h
M src/kudu/server/server_base.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
12 files changed, 246 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/19
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#16).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/init.cc
M src/kudu/security/init.h
M src/kudu/server/server_base.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
12 files changed, 243 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/16
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#35).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. If this flag is set explicitly, all unauthenticated
or unencrypted connections are prohibited except the ones from the
specified address blocks. Otherwise, private network (127.0.0.0/8,
etc.) and local subnets of all local network interfaces will be used.
Set it to '0.0.0.0/0' allows unauthenticated/unencrypted connections
from all remote IP addresses. However, if network access is not
otherwise restricted by a firewall, malicious users may be able to
gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/socket.h
7 files changed, 337 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/35
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 35
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#7).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 142 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/7
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#21).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavance flag 'trusted_subnets', a trusted subnet whitelist.
All unauthenticated or unencrypted connections are prohibited
except these from the specified subnets and local subnets of
all local network interfaces. Set the flag to '0.0.0.0/0' can
completely disable this restriction. However network access is
not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 288 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/21
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 36:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6514/36/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 961: if (google::GetCommandLineFlagInfoOrDie("trusted_subnets").is_default) {
Can this if condition be moved inside the call_once, and only evaluated it once per process? As much as possible should be moved inside the call_once as possible.
--
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: 36
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#33).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. If this flag is set explicitly, all unauthenticated
or unencrypted connections are prohibited except the ones from the
specified address blocks. Otherwise, private network (127.0.0.0/8,
etc.) and local subnets of all local network interfaces will be used.
Set it to '0.0.0.0/0' allows unauthenticated/unencrypted connections
from all remote IP addresses. However, if network access is not
otherwise restricted by a firewall, malicious users may be able to
gain unauthorized acces
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 334 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/33
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 33
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 1:
(1 comment)
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,
> Checking against the range of (datacenter/cloud infra mix) cluster IPs of t
Any idea how many of those 15% are _actually_ publicly accessible by mistake? I would guess that more than a few of them are actually accidentally on the public internet and exposed to attackers reading data, etc.
Maybe a compromise would be to allow requests from within the subnet? It would still be "broken out of the box" for some people who have multi-subnet-but-firewalled setups, but that might be preferable to the alternative.
--
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 <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 3:
(3 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,
> Figured we'd just use the configured netmask on the local interface. Unfort
So looks like we all agree to enable it by default but give users a choice to config trusted subnet?
Todd, what do you mean by 'configured netmask on the local interface'? Why is local interface?
http://gerrit.cloudera.org:8080/#/c/6514/1/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 149: negotiated_authn_ == AuthenticationType::INVALID)) {
> The authentication type should never be INVALID at this point (that's just
Yeah, initially I thought the same. But it turns out here negotiated_mech_ is not set yet, so we does not know if it is SaslMechanism::PLAIN or not. It appears to me negotiated_mech_ is only set at step 4?
Line 685: if (!FLAGS_allow_unauthenticated_public_connections &&
> I don't think you need to check here, it should be handled correctly by the
I am checking here because negotiated_mech_ is only set properly at line 679. Am I missing something?
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#17).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/init.cc
M src/kudu/security/init.h
M src/kudu/server/server_base.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
12 files changed, 243 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/17
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 34:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/6514/34/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
PS34, Line 66: allows
nit: "to allow"
Line 95: vector<Network>* g_local_networks = new vector<kudu::Network>();
I think it's better to initialize this to nullptr, and then set it to the vector only after parsing/initializing the value. That way if we have a bug where we accidentally use it before it's been properly set, we'd get a crash instead of silent wrong results.
Also, please wrap this in an anonymous namespace so that the symbol doesn't get exported to other translation units
PS34, Line 209: NegotiatePB request;
: RETURN_NOT_OK(RecvNegotiatePB(&request, &recv_buf));
can you add a comment why we need to consume this? not quite following, since 'request' is unused
PS34, Line 212: or unauthenticated
this error can be more specific to unencrypted, right?
PS34, Line 729: unencrypted or unauthenticated
this one can be specific to unauthenticated
Line 914: std::call_once(once, [] {
the initialization of g_local_networks could be moved inside the 'if' statement below on line 925. this would help us provide a workaround in the case that, for some reason, GetLocalNetworks crashes, we can configure explicit networks and avoid calling it at all.
PS34, Line 915: GetLocalNetworks
does this need WARN_NOT_OK or something?
PS34, Line 919: KUDU_C
can just be CHECK_OK (the 'KUDU_...' variants are only used in the context of client headers)
PS34, Line 926: =
can use |= here instead of the || on line 928
http://gerrit.cloudera.org:8080/#/c/6514/34/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
PS34, Line 95: is
nit: are
PS34, Line 113: vector Network
nit: "vector of"
--
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: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#25).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/25
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 25
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 22:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/6514/22//COMMIT_MSG
Commit Message:
PS22, Line 13: adavance
typo: advanced
nit: change to a full sentence please
PS22, Line 17: However network
"However, if"
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 82: kudu::g_trusted_subnets->push_back(network);
I don't think the validator is supposed to have side effects -- it's not clear how many times it will get called, etc
PS22, Line 85: can only be interpreted as
"must be specified in"
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/rpc/server_negotiation.h
File src/kudu/rpc/server_negotiation.h:
Line 42: extern vector<Network>* g_trusted_subnets;
hm, where is this used externally? we try to avoid globals with such wide scope
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util-test.cc
File src/kudu/util/net/net_util-test.cc:
PS22, Line 95: TestPrivateAddresses
rename to TestWithinNetwork (this isn't specific to public/private)
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
PS22, Line 186: string Network::ToString() const {
: return Substitute("$0:$1", addr_, mask_);
: }
this ToString isn't really human-readable. I think it would be better to either implement one that returns something like CIDR or host/netmask, or just remove it
Line 252: Status GetLocalNetwork(std::vector<Network>* net) {
nit: rename param to 'nets' since it is a list, not a single one
PS22, Line 256: auto cleanup = MakeScopedCleanup([&]() {
: freeifaddrs(ifap);
: });
this should probably be moved below the error check, otherwise we may freeifaddrs() an uninitialized 'ifap'.
Alternatively, initialize 'ifap' to nullptr, and add an 'if (ifap) freeifaddrs(ifap);' here
Line 265:
should probably 'net->clear()' here, since the docs say it gets them, not appends them.
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
Line 98: Network(uint32_t addr, uint32_t mask);
add doc here regarding endianness, etc.
PS22, Line 107: ParseString
rename to ParseCIDR so it's clear this is a CIDR style string and not a netmask-style
PS22, Line 111: mask_
I think conventionally people refer to this as 'netmask' instead of 'mask'. Can you change here and the above parameters/getters/etc?
PS22, Line 131: GetLocalNetwork
since this gives back a list, change to GetLocalNetworks
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6514/1/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 149: negotiated_authn_ == AuthenticationType::INVALID)) {
> Yeah, initially I thought the same. But it turns out here negotiated_mech_
Hmm, good point. We really only need to be checking when SASL PLAIN is being used, so perhaps the check is only necessary below, and not here? You are right that we don't know whether we're using SASL GSSAPI or SASL PLAIN until below.
Line 685: if (!FLAGS_allow_unauthenticated_public_connections &&
> I am checking here because negotiated_mech_ is only set properly at line 67
argh, good point. I forgot that negotiated_mech_ isn't set till later. The mech should still not be INVALID here, though.
--
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 <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/6514/1/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 145: if (!FLAGS_allow_unauthenticated_public_connections &&
> Instead of linking to the blog, it's probably best to reference the associa
Done
Line 149: Status s = RejectUntrustedPublicConnection(addr);
> Hmm, good point. We really only need to be checking when SASL PLAIN is bei
Done
Line 685: // verified.
> argh, good point. I forgot that negotiated_mech_ isn't set till later. Th
Done
http://gerrit.cloudera.org:8080/#/c/6514/1/src/kudu/util/net/sockaddr.cc
File src/kudu/util/net/sockaddr.cc:
PS1, Line 117: 0xff;
> could this just be 0xff? Having an odd # of hex digits is somewhat unordin
Done
Line 121: return true;
> The google style guide allows omitting the braces here, but I think our usu
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#23).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However network access is
not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 317 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/23
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#36).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. If this flag is set explicitly, all unauthenticated
or unencrypted connections are prohibited except the ones from the
specified address blocks. Otherwise, private network (127.0.0.0/8,
etc.) and local subnets of all local network interfaces will be used.
Set it to '0.0.0.0/0' allows unauthenticated/unencrypted connections
from all remote IP addresses. However, if network access is not
otherwise restricted by a firewall, malicious users may be able to
gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/socket.h
7 files changed, 345 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/36
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 36
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#34).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. If this flag is set explicitly, all unauthenticated
or unencrypted connections are prohibited except the ones from the
specified address blocks. Otherwise, private network (127.0.0.0/8,
etc.) and local subnets of all local network interfaces will be used.
Set it to '0.0.0.0/0' allows unauthenticated/unencrypted connections
from all remote IP addresses. However, if network access is not
otherwise restricted by a firewall, malicious users may be able to
gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/socket.h
7 files changed, 330 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/34
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#20).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavance flag 'trusted_subnets', a trusted subnet whitelist.
All unauthenticated or unencrypted connections are prohibited
except these from the specified subnets and local subnets of
all local network interfaces. Set the flag to '0.0.0.0/0' can
completely disable this restriction. However network access is
not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 287 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/20
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 20
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#15).
Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
WIP KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 182 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/15
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 12:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6514/11/src/kudu/rpc/negotiation-test.cc
File src/kudu/rpc/negotiation-test.cc:
PS11, Line 155: tring("192.
> This looks like an IP network address, not a single IP. Also, why not to j
Done
PS11, Line 222: unique_ptr<Socket>(new Socket());
:
> nit: why not to do that at the point of declaration of server_socket?
Done
http://gerrit.cloudera.org:8080/#/c/6514/11/src/kudu/rpc/server_negotiation.h
File src/kudu/rpc/server_negotiation.h:
PS11, Line 208: const So
> Is this signature intended for the move semantics?
Right, will change it. Thanks for pointing out.
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#28).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 319 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/28
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#29).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 319 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/29
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 29
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#5).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
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 unauthenticated
connections from publicly routable IPs. Even though it is highly recommended to
disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 138 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/5
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 37: Code-Review+2
--
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: 37
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 18:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/6514/17/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 873: for (const auto const& t : strings::Split(FLAGS_trusted_subnets, ",", strings::SkipEmpty())) {
> warning: the variable '__end' is copy-constructed from a const reference bu
Done
http://gerrit.cloudera.org:8080/#/c/6514/17/src/kudu/security/init.cc
File src/kudu/security/init.cc:
Line 69:
> warning: the variable '__end' is copy-constructed from a const reference bu
Done
http://gerrit.cloudera.org:8080/#/c/6514/17/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
Line 230: Sockaddr addr(*reinterpret_cast<struct sockaddr_in*>(ifa->ifa_addr));
> warning: C-style casts are discouraged; use reinterpret_cast [google-readab
Done
Line 231: Sockaddr netmask(*reinterpret_cast<struct sockaddr_in*>(ifa->ifa_netmask));
> warning: C-style casts are discouraged; use reinterpret_cast [google-readab
Done
http://gerrit.cloudera.org:8080/#/c/6514/17/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
Line 114: Status GetFQDN(std::string* hostname);
> warning: function 'kudu::GetFQDN' has a definition with different parameter
Done
http://gerrit.cloudera.org:8080/#/c/6514/13/src/kudu/util/net/sockaddr.cc
File src/kudu/util/net/sockaddr.cc:
Line 120:
> warning: redundant boolean literal in conditional return statement [readabi
Done
Line 121: // Strip any whitespace from the cidr_addr.
> warning: don't use else after return [readability-else-after-return]
Done
http://gerrit.cloudera.org:8080/#/c/6514/14/src/kudu/util/net/sockaddr.h
File src/kudu/util/net/sockaddr.h:
Line 71: bool WithInSubnet(const std::string& net_cidr_addr) const;
> warning: function 'kudu::Sockaddr::WithInSubnet' has a definition with diff
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: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 11:
Hi Dan, yeah, I have not updated the part for how to specify the trusted subnet yet, as the discussion on mailing list is still ongoing.
--
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: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#13).
Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
WIP KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 135 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/13
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 36:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6514/36/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 961: if (google::GetCommandLineFlagInfoOrDie("trusted_subnets").is_default) {
> Can this if condition be moved inside the call_once, and only evaluated it
Does not quite follow if we move this if condition to call_once, how do we know if the flag has been set explicitly later?
--
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: 36
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#31).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 334 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/31
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 31
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 19:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/rpc/negotiation-test.cc
File src/kudu/rpc/negotiation-test.cc:
PS19, Line 155: 192.169
> since this is supposed to be testing a public IP, I think it's confusing th
Done
PS19, Line 220: unique_ptr<Socket> server_socket = desc.use_test_socket ?
: unique_ptr<Socket>(new NegotiationTestSocket()) :
: unique_ptr<Socket>(new Socket());
> would be shorter to just write server_socket(desc.use_test_socket ? new Neg
Done
Line 923: }
> how about a case with an authenticated connection from a public routable IP
Done
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 61: DEFINE_bool(allow_unauthenticated_public_connections, false,
> Is this option actually still relevant if we have trusted_subnets? Isn't se
Done
PS19, Line 62: "Allow connections from unauthenticated public routable IPs. If enabled, "
: "any people on the internet can connect and browse the data, which is "
: "very dangerous. It is highly recommended to keep it disabled.
> I'd suggest rephrasing this as:
Done
PS19, Line 157: encryption_ == RpcEncryption::DISABLED) {
> not quite sure whether this is right. I would have guessed that this would
Done
PS19, Line 879: unauthenticated
> this doesn't quite match the logic, if we're also checking encryption. ie t
Done
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/security/init.cc
File src/kudu/security/init.cc:
PS19, Line 500: vector<string> addrs;
: RETURN_NOT_OK(GetNetworkAddresses(&addrs));
: string val = StrCat(FLAGS_trusted_subnets, JoinStrings(addrs, ","));
: SetCommandLineOptionWithMode("trusted_subnets",
: val.c_str(),
: google::SET_FLAGS_DEFAULT);
> it strikes me as a little strange to be writing back into the flags rather
Done
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/security/init.h
File src/kudu/security/init.h:
Line 34: Status InitTrustedSubnets();
> I think it would be better to encapsulate all of the logic somewhere in net
Done
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/net_util-test.cc
File src/kudu/util/net/net_util-test.cc:
Line 99: EXPECT_TRUE(addr.WithInSubnet("10.0.0.0/8"));
> in these tests it seems like you're always testing the case where the subne
It looks like "1.2.3.4/8" is legal address https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing. Will add one test.
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
PS19, Line 229: ==A
> nit: spaces around ==s
Done
Line 236: freeifaddrs(ifap);
> can you use MakeScopedCleanup here so that the freeifaddrs is placed as clo
Done
Line 333: // Strip any whitespace from the cidr_addr.
> should the stripping behavior be part of the ParseString below?
Done
Line 340: return (s.ok() && success);
> should verify that bits <= 32
Done
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
PS19, Line 110: // Returns the local network addresses.
: Status GetNetworkAddresses(std::vector<std::string>* addresses);
> The docs should be clearer here to indicate that it's returning networks, n
Done
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/sockaddr.cc
File src/kudu/util/net/sockaddr.cc:
PS19, Line 118: std::pair<string, string> p = strings::Split(net_cidr_addr,
: strings::delimiter::Limit("/", 1));
:
: // Strip any whitespace from the cidr_addr.
: StripWhiteSpace(&p.first);
: Sockaddr net_addr;
: Status s = net_addr.ParseString(p.first, 0);
:
: uint32_t bits;
: bool success = SimpleAtoi(p.second, &bits);
> this seems to be duplicated code from elsewhere. Can you share this code?
Done
PS19, Line 130: NetworkByteOrder::FromHost32(~(0xffffffff >> bits))
> hm, I think I would have expected to see "(1ULL << bits) - 1"
Not sure if "(1ULL << bits) - 1" will work here, as net_mask should be big endian?
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/sockaddr.h
File src/kudu/util/net/sockaddr.h:
PS19, Line 71: WithIn
> nit: "Within" rather than 'WithIn'
Done
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/socket.h
File src/kudu/util/net/socket.h:
Line 100: virtual Status GetPeerAddress(Sockaddr *cur_addr) const;
> Can you add a comment here that it's virtual so it can be overridden by tes
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: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 11:
I think I would structure this patch a little bit differently. I think we need to be able to specify the whitelist of address blocks via the command line flag, instead of being hardcoded.
--
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: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 19:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/rpc/negotiation-test.cc
File src/kudu/rpc/negotiation-test.cc:
PS19, Line 155: 192.169
since this is supposed to be testing a public IP, I think it's confusing that it looks so close to one of the private blocks. How about using 8.8.8.8 (a google DNS server that a lot of people might have memorized)
PS19, Line 220: unique_ptr<Socket> server_socket = desc.use_test_socket ?
: unique_ptr<Socket>(new NegotiationTestSocket()) :
: unique_ptr<Socket>(new Socket());
would be shorter to just write server_socket(desc.use_test_socket ? new NegotiationTestSocket() : new Socket()) right?
Line 923: }
how about a case with an authenticated connection from a public routable IP?
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 61: DEFINE_bool(allow_unauthenticated_public_connections, false,
Is this option actually still relevant if we have trusted_subnets? Isn't setting this to 'true' equivalent to adding 0.0.0.0/0 as a trusted subnet?
PS19, Line 62: "Allow connections from unauthenticated public routable IPs. If enabled, "
: "any people on the internet can connect and browse the data, which is "
: "very dangerous. It is highly recommended to keep it disabled.
I'd suggest rephrasing this as:
Allow unauthenticated connections from public routable IP addresses. If this is enabled and network access is not otherwise restricted by a firewall, malicious users may be able to gain unauthorized access. It is highly recommended to keep this option disabled.
PS19, Line 157: encryption_ == RpcEncryption::DISABLED) {
not quite sure whether this is right. I would have guessed that this would be based on whether encryption is negotiated, not based on the server side config.
PS19, Line 879: unauthenticated
this doesn't quite match the logic, if we're also checking encryption. ie the client could be authenticated, but have disabled encryption, and still reach this error, right?
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/security/init.cc
File src/kudu/security/init.cc:
PS19, Line 500: vector<string> addrs;
: RETURN_NOT_OK(GetNetworkAddresses(&addrs));
: string val = StrCat(FLAGS_trusted_subnets, JoinStrings(addrs, ","));
: SetCommandLineOptionWithMode("trusted_subnets",
: val.c_str(),
: google::SET_FLAGS_DEFAULT);
it strikes me as a little strange to be writing back into the flags rather than having some global variable which contains the parsed info. Per the suggestion elsewhere, if we had a structured way of storing the subnets, then we could have a global with pre-parsed data, which is populated from this initialization code
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/security/init.h
File src/kudu/security/init.h:
Line 34: Status InitTrustedSubnets();
I think it would be better to encapsulate all of the logic somewhere in netutil, and use something like std::once_call to do the initialization. That way we don't need to explicitly init it, and all the related code could be in one place
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/net_util-test.cc
File src/kudu/util/net/net_util-test.cc:
Line 99: EXPECT_TRUE(addr.WithInSubnet("10.0.0.0/8"));
in these tests it seems like you're always testing the case where the subnet address ends in enough 0 bits that any non-masked bits are 0. I'm not sure what the definition of a valid CIDR address is -- is it legal to refer to a network like "1.2.3.4/8"? Or is it only legal to parse one like "1.0.0.0/8"? We should respect whatever the actual spec says.
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
PS19, Line 229: ==A
nit: spaces around ==s
Line 236: freeifaddrs(ifap);
can you use MakeScopedCleanup here so that the freeifaddrs is placed as close as possible to the getifaddrs() call? that way if we added some early-return in the loop, we wouldn't leak.
Line 333: // Strip any whitespace from the cidr_addr.
should the stripping behavior be part of the ParseString below?
Line 340: return (s.ok() && success);
should verify that bits <= 32
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
PS19, Line 110: // Returns the local network addresses.
: Status GetNetworkAddresses(std::vector<std::string>* addresses);
The docs should be clearer here to indicate that it's returning networks, not individual addresses. I think if you added some struct for a Network (combination of an address and a mask) and had this return a vector of those structs, it would be easier to use and have less string-related overhead.
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/sockaddr.cc
File src/kudu/util/net/sockaddr.cc:
PS19, Line 118: std::pair<string, string> p = strings::Split(net_cidr_addr,
: strings::delimiter::Limit("/", 1));
:
: // Strip any whitespace from the cidr_addr.
: StripWhiteSpace(&p.first);
: Sockaddr net_addr;
: Status s = net_addr.ParseString(p.first, 0);
:
: uint32_t bits;
: bool success = SimpleAtoi(p.second, &bits);
this seems to be duplicated code from elsewhere. Can you share this code?
eg perhaps some struct like Subnet which has an address and mask, with corresponding function to parse (which can return a status) and a ToString?
PS19, Line 130: NetworkByteOrder::FromHost32(~(0xffffffff >> bits))
hm, I think I would have expected to see "(1ULL << bits) - 1"
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/sockaddr.h
File src/kudu/util/net/sockaddr.h:
PS19, Line 71: WithIn
nit: "Within" rather than 'WithIn'
http://gerrit.cloudera.org:8080/#/c/6514/19/src/kudu/util/net/socket.h
File src/kudu/util/net/socket.h:
Line 100: virtual Status GetPeerAddress(Sockaddr *cur_addr) const;
Can you add a comment here that it's virtual so it can be overridden by tests?
--
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: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#26).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/26
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 26
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#27).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/27
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 36:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6514/36/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 961: if (google::GetCommandLineFlagInfoOrDie("trusted_subnets").is_default) {
> We only flags marked with the 'runtime' tag to be set after startup. Typic
I see. I do not see a reason we have to make it runtime updatable. That was my misunderstanding. Thanks!
--
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: 36
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#12).
Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
WIP KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 138 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/12
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 22:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 82: kudu::g_trusted_subnets->push_back(network);
> I don't think the validator is supposed to have side effects -- it's not cl
Hmm, what is the best way you think to init g_trusted_subnets?
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Harsh J (Code Review)" <ge...@cloudera.org>.
Harsh J has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 1:
(1 comment)
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,
> So looks like we all agree to enable it by default but give users a choice
I re-ran the checks with testing public connectivity to some of their used ports, and all of them failed in a ~250 clusters run, so they appear to be fire-walled correctly or are internally used.
I'm ok with this being enabled by default as a safe-than-sorry thing, but would it be possible to have better exposure for new installers? Or if that's a Management UI thing we could probably discuss it elsewhere.
--
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 <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#4).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
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 unauthenticated
connections from publicly routable IPs. Even though it is highly recommended to
disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
A src/kudu-1875.patch
A src/kudu-1875patch
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
10 files changed, 932 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/4
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#24).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However network access is
not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 315 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/24
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 24
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#18).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
It also provides two flags: one adavance flag 'trusted_subnets' to
whitelist trusted subnets. Connections from trustred subnets will
be allowed even if unauthenticaed; another unsafe flag
'allow_unauthenticated_public_connections' is provided to enable
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/security/init.cc
M src/kudu/security/init.h
M src/kudu/server/server_base.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
12 files changed, 246 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/18
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 36:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6514/35/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 96:
> Instead of only caching local_networks, could you cache the full set of tru
Done
http://gerrit.cloudera.org:8080/#/c/6514/35/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
Line 22: #include <sys/socket.h>
> insert this in alphabetical order
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: 36
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#32).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 342 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/32
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 32
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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 <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 5:
(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 unauthenticated
: connections from publicly routable IPs. Even though it is highly recommended to
: disable it.
nit: could you keep the lines under 72 chars length?
You can get more information on the commit guidelines at https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines
That doc is referenced from http://kudu.apache.org/docs/contributing.html#_submitting_patches
http://gerrit.cloudera.org:8080/#/c/6514/5/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
PS5, Line 149: Status s = RejectUntrustedPublicConnection(addr);
: if (!s.ok()) return s;
nit: why not just
RETURN_NOT_OK(RejectUntrustedPublicConnection(addr));
PS5, Line 679: Status s = RejectUntrustedPublicConnection(addr);
: if (!s.ok()) return s;
RETURN_NOT_OK(RejectUntrustedPublicConnection(addr));
PS5, Line 858: = Status::OK();
nit: Status::OK() is set by the default constructor, so no assignment is needed here.
PS5, Line 861: is unacceptable
are prohibited
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 last /24 networks in the range (i.e. 169.254.1.0 -- 169.254.254.0) ?
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
uint32_t first_byte = NetworkByteOrder::FromHost32(addr_.sin_addr.s_addr) >> 24;
uint32_t second_byte = (NetworkByteOrder::FromHost32(addr_.sin_addr.s_addr) >> 16) & 0xff;
http://gerrit.cloudera.org:8080/#/c/6514/5/src/kudu/util/net/sockaddr.h
File src/kudu/util/net/sockaddr.h:
PS5, Line 71: // 10.0.0.0 - 10.255.255.255,
: // 172.16.0.0 - 172.31.255.255,
: // 192.168.0.0 - 192.168.255.255.
What about link-local IPv4 addresses (RFC 6890 and RFC 3927)?
--
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: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 32:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/6514/32/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 60: DEFINE_string(trusted_subnets,
We discussed offline about this, just going to write up what we decided:
If the user specifies the flag explicitly, then we won't add in unroutable IPs or local subnets. Instead, we'll just use the address blocks specified.
PS32, Line 65: can completely disable this restriction
I think this sentence will read better as "Set it to '0.0.0.0/0' to allow unauthenticated connections from all remote IP addresses."
Line 76: for (auto const& t : strings::Split(value, ",", strings::SkipEmpty())) {
> warning: the variable '__end' is copy-constructed from a const reference bu
It's typical to make the iteration variable a 'const auto&', which means the reference is to a non-mutable object.
PS32, Line 212: public
nit: publicly
Also, add a reference to the flag that controls it, like "... prohibited. See --trusted_subnets flag for more information."
Line 728: "from public routable IPs are prohibited",
Same here
Line 916: Status s = Network::ParseCIDRStrings(FLAGS_trusted_subnets, &trusted_subnets);
You can probably just wrap this in a KUDU_CHECK_OK call. It shouldn't ever fail since it's already been validated, right?
http://gerrit.cloudera.org:8080/#/c/6514/32/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
Line 96: // (same as network byte order)
add period to end of sentence.
http://gerrit.cloudera.org:8080/#/c/6514/32/src/kudu/util/net/sockaddr.h
File src/kudu/util/net/sockaddr.h:
Line 73: bool WithinNetwork(const Network& netaddr) const;
What do you think about defining this operator on Network instead of Sockaddr? It's debatable, but I think it makes sense to keep it there.
Line 76: bool WithinNetworks(const std::vector<Network>& netaddrs) const;
I'm not sure it's worth defining this, since it's easily done with std::any_of on a variety of input collections.
--
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: 32
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 34:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/6514/32/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 60: DEFINE_string(trusted_subnets,
> We discussed offline about this, just going to write up what we decided:
Done
PS32, Line 65: bnets of all local network interfaces w
> I think this sentence will read better as "Set it to '0.0.0.0/0' to allow u
Done
Line 76:
> It's typical to make the iteration variable a 'const auto&', which means th
Done
PS32, Line 212: rypted
> nit: publicly
Done
Line 728: if (!IsTrustedConnection(addr)) {
> Same here
Done
Line 916: });
> You can probably just wrap this in a KUDU_CHECK_OK call. It shouldn't ever
Done
http://gerrit.cloudera.org:8080/#/c/6514/32/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
Line 96: // (same as network byte order).
> add period to end of sentence.
Done
http://gerrit.cloudera.org:8080/#/c/6514/32/src/kudu/util/net/sockaddr.h
File src/kudu/util/net/sockaddr.h:
Line 73: // the default auto-generated copy constructor is fine here
> What do you think about defining this operator on Network instead of Sockad
Done
Line 76: };
> I'm not sure it's worth defining this, since it's easily done with std::any
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: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 36: Code-Review+1
--
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: 36
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#30).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. All unauthenticated or unencrypted connections
are prohibited except these from the specified subnets and local
subnets of all local network interfaces. Set the flag to '0.0.0.0/0'
can completely disable this restriction. However, if network access
is not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 320 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/30
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 30
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 35:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/6514/34/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
PS34, Line 66: to all
> nit: "to allow"
Done
Line 95: namespace {
> I think it's better to initialize this to nullptr, and then set it to the v
Done
PS34, Line 209:
: if (!IsTrustedConnection(addr)) {
> can you add a comment why we need to consume this? not quite following, sin
Done
PS34, Line 212: used,
> this error can be more specific to unencrypted, right?
Done
PS34, Line 729: routable IPs if authentication
> this one can be specific to unauthenticated
Done
Line 914: }
> the initialization of g_local_networks could be moved inside the 'if' state
Make sense. Updated it.
PS34, Line 915: We always allow
> does this need WARN_NOT_OK or something?
Done
PS34, Line 919: ol Ser
> can just be CHECK_OK (the 'KUDU_...' variants are only used in the context
Done
PS34, Line 926: el
> can use |= here instead of the || on line 928
Done
http://gerrit.cloudera.org:8080/#/c/6514/34/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
PS34, Line 95: are
> nit: are
Done
PS34, Line 113: vector of Netw
> nit: "vector of"
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: 35
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Harsh J (Code Review)" <ge...@cloudera.org>.
Harsh J has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 1:
(1 comment)
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,
Checking against the range of (datacenter/cloud infra mix) cluster IPs of the various customers I work with, the use of IPs outside the range labelled as private is rather significant (~15% in a small sample of ~500 clusters).
I therefore don't think this blockage should be enabled by default, as Kudu would simply not work out of the box for such environments?
I'd be more comfortable knowing this was an opt-in, and also had a configurable IP range so users can self-exclude their network admins' choices of numbering when applying this useful restriction.
--
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 <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#8).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 143 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/8
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 22:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 82: kudu::g_trusted_subnets->push_back(network);
> Hmm, what is the best way you think to init g_trusted_subnets?
I think something like:
static std::once_flag once;
std::call_once(once, [*]() {
InitTrustedSubnets();
});
although keeping the validation there (without side effects) is also a good idea.
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#22).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavance flag 'trusted_subnets', a trusted subnet whitelist.
All unauthenticated or unencrypted connections are prohibited
except these from the specified subnets and local subnets of
all local network interfaces. Set the flag to '0.0.0.0/0' can
completely disable this restriction. However network access is
not otherwise restricted by a firewall, malicious users may be
able to gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
9 files changed, 288 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/22
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#37).
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
This rejects unauthenticated connections from publicly routable IPs,
even if authentication and encryption are not configured.
An adavanced flag 'trusted_subnets' is provided to whitelist
trusted subnets. If this flag is set explicitly, all unauthenticated
or unencrypted connections are prohibited except the ones from the
specified address blocks. Otherwise, private network (127.0.0.0/8,
etc.) and local subnets of all local network interfaces will be used.
Set it to '0.0.0.0/0' allows unauthenticated/unencrypted connections
from all remote IP addresses. However, if network access is not
otherwise restricted by a firewall, malicious users may be able to
gain unauthorized access.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
M src/kudu/util/net/socket.h
7 files changed, 337 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/37
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 37
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6514
to look at the new patch set (#14).
Change subject: WIP KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
WIP KUDU-1875: Refuse unauthenticated connections from publicly routable
IP addrs
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
unauthenticated connections from publicly routable IPs. Even though it
is highly recommended to disable it.
Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
---
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/server_negotiation.cc
M src/kudu/rpc/server_negotiation.h
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/sockaddr.cc
M src/kudu/util/net/sockaddr.h
M src/kudu/util/net/socket.h
8 files changed, 131 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6514/14
--
To view, visit http://gerrit.cloudera.org:8080/6514
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6c3fbb5491785874c5701d6c9d866949cfac905e
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 22:
TestNegotiation is passing locally. Checking why it is failed here.
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 11:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6514/11/src/kudu/rpc/negotiation-test.cc
File src/kudu/rpc/negotiation-test.cc:
PS11, Line 155: 192.169.0.0
This looks like an IP network address, not a single IP. Also, why not to just do
return cur_addr->ParseString(...);
Is it all intentional? If yes, please add a comment explaining those points.
PS11, Line 222: server_socket = desc.use_test_socket ? unique_ptr<Socket>(new NegotiationTestSocket()) :
: unique_ptr<Socket>(new Socket());
nit: why not to do that at the point of declaration of server_socket?
http://gerrit.cloudera.org:8080/#/c/6514/11/src/kudu/rpc/server_negotiation.h
File src/kudu/rpc/server_negotiation.h:
PS11, Line 208: Sockaddr
Is this signature intended for the move semantics?
I'm not sure Sockaddr is a type which fits for the move semantics: when moving, it would be necessary to copy over the aggregated sockaddr_in structure field-by-field.
Maybe it's better to have const reference instead?
--
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: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 35:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6514/35/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 96: vector<Network>* g_local_networks = nullptr;
Instead of only caching local_networks, could you cache the full set of trusted networks? That way the trusted_subnets flag wouldn't need to be reparsed for every connection.
http://gerrit.cloudera.org:8080/#/c/6514/35/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
Line 22: #include <ifaddrs.h>
insert this in alphabetical order
--
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: 35
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 1:
(1 comment)
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,
> Thanks for running the numbers, Harsh. I'm hoping we can figure out a way
Figured we'd just use the configured netmask on the local interface. Unfortunately it requires a fair bit more coding to enumerate local interfaces and determine local networks.
Harsh, any way you can check whether those 15% of clusters are running insecurely vs on firewalled public IPs?
--
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 <ha...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-1875: Refuse unauthenticated connections from publicly routable IP addrs
......................................................................
Patch Set 22:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/6514/22//COMMIT_MSG
Commit Message:
PS22, Line 13: adavance
> typo: advanced
Done
PS22, Line 17: However network
> "However, if"
Done
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
Line 82: kudu::g_trusted_subnets->push_back(network);
> I think something like:
Done
PS22, Line 85: can only be interpreted as
> "must be specified in"
Done
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/rpc/server_negotiation.h
File src/kudu/rpc/server_negotiation.h:
Line 42: extern vector<Network>* g_trusted_subnets;
> hm, where is this used externally? we try to avoid globals with such wide s
Done
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util-test.cc
File src/kudu/util/net/net_util-test.cc:
PS22, Line 95: TestPrivateAddresses
> rename to TestWithinNetwork (this isn't specific to public/private)
Done
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
PS22, Line 186: string Network::ToString() const {
: return Substitute("$0:$1", addr_, mask_);
: }
> this ToString isn't really human-readable. I think it would be better to ei
Done
Line 252: Status GetLocalNetwork(std::vector<Network>* net) {
> nit: rename param to 'nets' since it is a list, not a single one
Done
PS22, Line 256: auto cleanup = MakeScopedCleanup([&]() {
: freeifaddrs(ifap);
: });
> this should probably be moved below the error check, otherwise we may freei
Done
Line 265:
> should probably 'net->clear()' here, since the docs say it gets them, not a
Done
http://gerrit.cloudera.org:8080/#/c/6514/22/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
Line 98: Network(uint32_t addr, uint32_t mask);
> add doc here regarding endianness, etc.
Done
PS22, Line 107: ParseString
> rename to ParseCIDR so it's clear this is a CIDR style string and not a net
Done
PS22, Line 111: mask_
> I think conventionally people refer to this as 'netmask' instead of 'mask'.
Done
PS22, Line 131: GetLocalNetwork
> since this gives back a list, change to GetLocalNetworks
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Harsh J <ha...@harshj.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes