Alexey Serbin has posted comments on this change. Change subject: KUDU-1955 refuse to use world-readable keytabs ......................................................................
Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/7249/6/src/kudu/security/init.cc File src/kudu/security/init.cc: PS6, Line 57: security_ Is this crucial to have 'security' as the part of the flag? What if we call it just allow_world_readable_credentials ? PS6, Line 58: this server This file is linked into non-server components as well (e.g., the kudu CLI tool) and in future we may support client-side TLS certificates as well. So, consider dropping 'this server' and updating the description to "Allow to use ..." or whatever you think is better wording here. PS6, Line 58: Enable this server to use Is it crucial to PS6, Line 80: LOG(ERROR) << "could not verify keytab file does not have world-readable permissions: " : << FLAGS_keytab_file; I think it's worth including the information from the Status 's' for troubleshooting if this errors happens. http://gerrit.cloudera.org:8080/#/c/7249/6/src/kudu/util/env.h File src/kudu/util/env.h: PS6, Line 334: FileIsWorldReadable Given the naming scheme seen in this file, this should be names IsFileWorldReadable (see IsOnExtFilesystem, IsDirectory). Also, for Status we have methods named as IsIOError, IsTimedOut, etc. -- 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: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sam Okrent <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[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
