You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Sam Okrent (Code Review)" <ge...@cloudera.org> on 2017/06/21 18:33:53 UTC
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Sam Okrent has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7249
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_security_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 67 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/1
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Adar Dembo, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7249
to look at the new patch set (#7).
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 129 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/7
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7249
to look at the new patch set (#2).
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_security_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 70 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/2
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 7: Code-Review+2
--
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: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Sam Okrent has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 5:
(4 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 fo
Done
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.
Done
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_EVEN
Done
Line 1570: (*result) = (s.st_mode & S_IROTH) != 0;
> Nit: don't need parens around *result.
Done
--
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 <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 6: Code-Review+2
--
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 <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7249
to look at the new patch set (#4).
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_security_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
A src/kudu/util/.test_macros.h.swo
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
6 files changed, 122 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/4
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Sam Okrent has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
Line 232: TEST_F(SecurityITest, TestWorldReadableKeytab) {
> Can you add a test for the cert case as well?
Done
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:
Line 273: RETURN_NOT_OK(Env::Default()->FileIsWorldReadable(FLAGS_rpc_private_key_file,
> Okay, do you think I should move the keytab file check to a validator as we
Done
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7249
to look at the new patch set (#5).
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_security_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 122 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/5
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 8: Code-Review+2
--
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: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 5: Code-Review+2
--
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 <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Sam Okrent has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/util/env.h
File src/kudu/util/env.h:
PS2, Line 333: O
> capitalization
Done
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:
Line 273: RETURN_NOT_OK(Env::Default()->FileIsWorldReadable(FLAGS_rpc_private_key_file,
> Okay, do you think I should move the keytab file check to a validator as we
Yah, if possible we prefer to put validation like this in an explicit gflags validator function. It runs earlier in server startup, which is usually good.
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Sam Okrent has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 6:
(3 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?
Done
PS6, Line 58: this server
> This file is linked into non-server components as well (e.g., the kudu CLI
Done
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
Done
--
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 <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Reviewed-on: http://gerrit.cloudera.org:8080/7249
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 128 insertions(+), 5 deletions(-)
Approvals:
Dan Burkert: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Sam Okrent has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:
Line 273: RETURN_NOT_OK(Env::Default()->FileIsWorldReadable(FLAGS_rpc_private_key_file,
> did you try adding this check to the ValidateExternalPkiFlags method? I th
Okay, do you think I should move the keytab file check to a validator as well?
Line 274: &world_readable_private_key));
> alignment
Done
Line 276: return Status::InvalidArgument(Substitute(
> Same comment about InvalidArgument as in the other check.
Done
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/security/init.cc
File src/kudu/security/init.cc:
Line 455: return Status::InvalidArgument(Substitute(
> Status::InvalidArgument can take an optional second string parameter which
Done
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
PS2, Line 1564: OVERRIDE
> use lowercase 'override'
Done
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
Line 232: TEST_F(SecurityITest, TestWorldReadableKeytab) {
Can you add a test for the cert case as well?
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:
Line 273: RETURN_NOT_OK(Env::Default()->FileIsWorldReadable(FLAGS_rpc_private_key_file,
did you try adding this check to the ValidateExternalPkiFlags method? I think it would be better there if possible.
Line 274: &world_readable_private_key));
alignment
Line 276: return Status::InvalidArgument(Substitute(
Same comment about InvalidArgument as in the other check.
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/security/init.cc
File src/kudu/security/init.cc:
Line 455: return Status::InvalidArgument(Substitute(
Status::InvalidArgument can take an optional second string parameter which is equivalent to joining with ': ', so you can drop the Substitute call and just pass the string and flag:
return Status::InvalidArgument(
"cannot use keytab file with world-readable permissions: $0",
FLAGS_keytab_file);
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/util/env.h
File src/kudu/util/env.h:
PS2, Line 333: O
capitalization
http://gerrit.cloudera.org:8080/#/c/7249/2/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
PS2, Line 1564: OVERRIDE
use lowercase 'override'
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
Patch Set 4:
Code looks good. The .test_macros.h.swo needs to be removed.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7249
to look at the new patch set (#3).
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_security_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
A src/kudu/util/.test_macros.h.swo
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
6 files changed, 122 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/3
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7249
to look at the new patch set (#6).
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_security_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 125 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/6
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1955 refuse to use world-readable keytabs
Posted by "Sam Okrent (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Adar Dembo, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7249
to look at the new patch set (#8).
Change subject: KUDU-1955 refuse to use world-readable keytabs
......................................................................
KUDU-1955 refuse to use world-readable keytabs
Allowing users to supply keytab files and TLS private keys
with world-readable permissions lessens a cluster's security.
During Kerberos/TLS initialization, servers now check the
permissions of these files and exit with bad statuses if they
have world-readable permissions. Additionally, if users wish
to override this safeguard, they may set the flag
'--allow_world_readable_credentials' to true. However, this
flag is tagged as unsafe.
Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/messenger.cc
M src/kudu/security/init.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
5 files changed, 128 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/7249/8
--
To view, visit http://gerrit.cloudera.org:8080/7249
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2ee84e71023304f0743ba0ad37ebb1eef24abc6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sam Okrent <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot