You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/03/01 01:35:37 UTC

[kudu-CR] KUDU-1896 (part 2): enable redaction on the web UI

Hello Dan Burkert, Hao Hao, Adar Dembo,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/6193

to review the following change.

Change subject: KUDU-1896 (part 2): enable redaction on the web UI
......................................................................

KUDU-1896 (part 2): enable redaction on the web UI

When we first added log redaction, we decided not to enable redaction of
the web UI. The reasoning at the time was that the primary motivation
for log redaction was so that people could easily share logs outside of
their network without fear of leaking sensitive data, but that defending
data against "inside the network" readers was useless given lack of
authentication.

Now, we have authentication and basic authorization support, so it makes
more sense to consider preventing internal users from seeing data.
Someone may have network access to the web UI but not supposed to have
access to read data stored in Kudu.

Given that, this patch enables redaction on the web pages.

Note that this has a very important side effect as well: redaction is
enabled for the /tracing and /rpcz endpoints. This is critical since
both allow a remote user to snoop on RPC requests and responses, and
those may include data such as authentication credentials as well as
user data. With redaction now enabled, I verified that the traced data
is properly redacted as well.

I also verified that range partitions are still displayed un-redacted as
desired, while RowSet boundary keys (which are user data) are now
redacted.

Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
---
M src/kudu/server/webserver.cc
1 file changed, 0 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/6193/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6193
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>

[kudu-CR] KUDU-1896 (part 2): enable redaction on the web UI

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: KUDU-1896 (part 2): enable redaction on the web UI
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6193/1/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

PS1, Line 307: 
             : 
If I'm understanding your commit message correctly, as the (complex) redaction patchset evolved, this became no longer true, right? That is, we shouldn't have been concerned about needlessly redacting "useful" metadata like partition keys, table names, column names, etc?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1896 (part 2): enable redaction on the web UI

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1896 (part 2): enable redaction on the web UI
......................................................................


KUDU-1896 (part 2): enable redaction on the web UI

When we first added log redaction, we decided not to enable redaction of
the web UI. The reasoning at the time was that the primary motivation
for log redaction was so that people could easily share logs outside of
their network without fear of leaking sensitive data, but that defending
data against "inside the network" readers was useless given lack of
authentication.

Now, we have authentication and basic authorization support, so it makes
more sense to consider preventing internal users from seeing data.
Someone may have network access to the web UI but not supposed to have
access to read data stored in Kudu.

Given that, this patch enables redaction on the web pages.

Note that this has a very important side effect as well: redaction is
enabled for the /tracing and /rpcz endpoints. This is critical since
both allow a remote user to snoop on RPC requests and responses, and
those may include data such as authentication credentials as well as
user data. With redaction now enabled, I verified that the traced data
is properly redacted as well.

I also verified that range partitions are still displayed un-redacted as
desired, while RowSet boundary keys (which are user data) are now
redacted.

Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
Reviewed-on: http://gerrit.cloudera.org:8080/6193
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/server/webserver.cc
1 file changed, 0 insertions(+), 5 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-1896 (part 2): enable redaction on the web UI

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1896 (part 2): enable redaction on the web UI
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6193/1/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

PS1, Line 307: 
             : 
> If I'm understanding your commit message correctly, as the (complex) redact
sorta... we'll now be redacting default value and scan predicates I believe, but I think it's OK collateral damage, and perhaps desirable.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1896 (part 2): enable redaction on the web UI

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: KUDU-1896 (part 2): enable redaction on the web UI
......................................................................


Patch Set 1: Code-Review+2 -Verified

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1896 (part 2): enable redaction on the web UI

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: KUDU-1896 (part 2): enable redaction on the web UI
......................................................................


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d31f87fd10d177adc2d98dcb049f3bcf6ecdbe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No