Adar Dembo has posted comments on this change.

Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................


Patch Set 5:

(5 comments)

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

Line 269:   const vector<string> argv = { binary, "--unlock_experimental_flags",
Nit: can you put unlock_experimental_flags on its own line, like you did for 
the others?


http://gerrit.cloudera.org:8080/#/c/7249/5/src/kudu/security/init.cc
File src/kudu/security/init.cc:

PS5, Line 58:       "Enable this server to use keytab files and TLS private 
keys with "
            :       "world-readable permissions.");
Nit: these lines should be aligned with "allow..."


http://gerrit.cloudera.org:8080/#/c/7249/5/src/kudu/util/env.h
File src/kudu/util/env.h:

PS5, Line 333: , o
Nit: more readable if you convert this comma to a period.


http://gerrit.cloudera.org:8080/#/c/7249/5/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 1564:   Status FileIsWorldReadable(const string& path, bool* result) 
override {
Could you add ThreadRestrictions::AssertIOAllowed() as well as a TRACE_EVENT1() 
call with the path?


Line 1570:     (*result) = (s.st_mode & S_IROTH) != 0;
Nit: don't need parens around *result.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

Reply via email to