Repository: kudu Updated Branches: refs/heads/master ae07d0dc4 -> 2adb3aeb1
Misc. fixes for Kerberos compatibility on OS X This commit makes a few changes in order to have better compatibility with the system macOS Heimdal kerberos: 1. krb5kdc now uses UDP instead of TCP; the heimdal client library seems to have issues connecting via TCP. 2. The ticket cache is now the default FILE type instead of DIR. This has the downside of limiting our MiniKdc to one kinitted user, but heimdal apparently has issues with DIR, and FILE is expected to be how normal users will use Kerberos. 3. The SASL error string handling is special cased for the Heimdal errors on OS X. sasl_rpc-test is still failing on macOS, but these changes get us a bit closer to having it pass. Change-Id: I3b61af8cedf83745a5b7a6b806b68912f2655821 Reviewed-on: http://gerrit.cloudera.org:8080/4978 Reviewed-by: Adar Dembo <a...@cloudera.com> Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/123b9918 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/123b9918 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/123b9918 Branch: refs/heads/master Commit: 123b9918c7005f65010c5d431f4e3cac459e7a31 Parents: ae07d0d Author: Dan Burkert <danburk...@apache.org> Authored: Mon Nov 7 14:20:27 2016 -0800 Committer: Dan Burkert <danburk...@apache.org> Committed: Tue Nov 8 00:27:34 2016 +0000 ---------------------------------------------------------------------- src/kudu/rpc/sasl_common.cc | 16 ++++++++++------ src/kudu/rpc/sasl_rpc-test.cc | 15 +++++++++++---- src/kudu/security/test/mini_kdc-test.cc | 9 +++++++-- src/kudu/security/test/mini_kdc.cc | 10 +++------- src/kudu/util/test_util.cc | 2 +- src/kudu/util/test_util.h | 2 ++ 6 files changed, 34 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/123b9918/src/kudu/rpc/sasl_common.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_common.cc b/src/kudu/rpc/sasl_common.cc index 25e883c..7874f20 100644 --- a/src/kudu/rpc/sasl_common.cc +++ b/src/kudu/rpc/sasl_common.cc @@ -216,12 +216,16 @@ static string CleanSaslError(const string& err) { // with older libstdcxx. static regex_t re; static std::once_flag once; - std::call_once(once, []{ - CHECK_EQ(0, regcomp(&re, - "Unspecified GSS failure. +" - "Minor code may provide more information +" - "\\((.+)\\)", REG_EXTENDED)); - }); + +#if defined(__APPLE__) + static const char* kGssapiPattern = "GSSAPI Error: Miscellaneous failure \\(see text \\((.+)\\)"; +#else + static const char* kGssapiPattern = "Unspecified GSS failure. +" + "Minor code may provide more information +" + "\\((.+)\\)"; +#endif + + std::call_once(once, []{ CHECK_EQ(0, regcomp(&re, kGssapiPattern, REG_EXTENDED)); }); regmatch_t matches[2]; if (regexec(&re, err.c_str(), arraysize(matches), matches, 0) == 0) { return err.substr(matches[1].rm_so, matches[1].rm_eo - matches[1].rm_so); http://git-wip-us.apache.org/repos/asf/kudu/blob/123b9918/src/kudu/rpc/sasl_rpc-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_rpc-test.cc b/src/kudu/rpc/sasl_rpc-test.cc index 1a5de40..a0283a0 100644 --- a/src/kudu/rpc/sasl_rpc-test.cc +++ b/src/kudu/rpc/sasl_rpc-test.cc @@ -30,6 +30,7 @@ #include "kudu/gutil/gscoped_ptr.h" #include "kudu/gutil/map-util.h" +#include "kudu/gutil/strings/substitute.h" #include "kudu/rpc/constants.h" #include "kudu/rpc/sasl_client.h" #include "kudu/rpc/sasl_common.h" @@ -281,6 +282,14 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) { ASSERT_OK(kdc.CreateServiceKeytab("kudu/127.0.0.1", &kt_path)); CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/)); +#if defined(__APPLE__) + string kErrorMsg = strings::Substitute("get-pricipal open($0): " + "No such file or directory (negative cache)", + kInvalidPath); +#else + string kErrorMsg = "No Kerberos credentials available"; +#endif + // Try to negotiate with no krb5 credentials on the client. It should fail on both // sides. RunNegotiationTest( @@ -292,12 +301,11 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) { CHECK(s.IsNetworkError()); }), std::bind(RunGSSAPINegotiationClient, std::placeholders::_1, - [](const Status& s, SaslClient& client) { + [kErrorMsg](const Status& s, SaslClient& client) { CHECK(s.IsNotAuthorized()); - CHECK_EQ(s.message(), "No Kerberos credentials available"); + CHECK_EQ(s.message().ToString(), kErrorMsg); })); - // Create and kinit as a client user. ASSERT_OK(kdc.CreateUserPrincipal("testuser")); ASSERT_OK(kdc.Kinit("testuser")); @@ -317,7 +325,6 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) { CHECK_EQ(SaslMechanism::GSSAPI, client.negotiated_mechanism()); })); - // Change the server's keytab file so that it has inappropriate // credentials. // Authentication should now fail. http://git-wip-us.apache.org/repos/asf/kudu/blob/123b9918/src/kudu/security/test/mini_kdc-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/test/mini_kdc-test.cc b/src/kudu/security/test/mini_kdc-test.cc index 3c57340..535c5d7 100644 --- a/src/kudu/security/test/mini_kdc-test.cc +++ b/src/kudu/security/test/mini_kdc-test.cc @@ -38,12 +38,17 @@ TEST(MiniKdcTest, TestBasicOperation) { ASSERT_OK(kdc.Stop()); ASSERT_OK(kdc.Start()); + // Check that alice is kinit'd. + string klist; + ASSERT_OK(kdc.Klist(&klist)); + ASSERT_STR_CONTAINS(klist, "al...@krbtest.com"); + ASSERT_OK(kdc.CreateUserPrincipal("bob")); ASSERT_OK(kdc.Kinit("bob")); - string klist; + // Check that bob has replaced alice as the kinit'd principal. ASSERT_OK(kdc.Klist(&klist)); - ASSERT_STR_CONTAINS(klist, "al...@krbtest.com"); + ASSERT_STR_NOT_CONTAINS(klist, "al...@krbtest.com"); ASSERT_STR_CONTAINS(klist, "b...@krbtest.com"); ASSERT_STR_CONTAINS(klist, "krbtgt/krbtest....@krbtest.com"); http://git-wip-us.apache.org/repos/asf/kudu/blob/123b9918/src/kudu/security/test/mini_kdc.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/test/mini_kdc.cc b/src/kudu/security/test/mini_kdc.cc index 8e92080..126bcc2 100644 --- a/src/kudu/security/test/mini_kdc.cc +++ b/src/kudu/security/test/mini_kdc.cc @@ -73,7 +73,7 @@ map<string, string> MiniKdc::GetEnvVars() const { return { {"KRB5_CONFIG", JoinPathSegments(options_.data_root, "krb5.conf")}, {"KRB5_KDC_PROFILE", JoinPathSegments(options_.data_root, "kdc.conf")}, - {"KRB5CCNAME", "DIR:" + JoinPathSegments(options_.data_root, "krb5cc")} + {"KRB5CCNAME", JoinPathSegments(options_.data_root, "krb5cc")} }; } @@ -191,8 +191,7 @@ Status MiniKdc::Stop() { Status MiniKdc::CreateKdcConf() const { static const string kFileTemplate = R"( [kdcdefaults] -kdc_ports = "" -kdc_tcp_ports = $2 +kdc_ports = $2 [realms] $1 = { @@ -232,9 +231,6 @@ Status MiniKdc::CreateKrb5Conf() const { # This enables us to connect despite that mismatch. ignore_acceptor_hostname = true - # The KDC is configured to only use TCP, so the client should not prefer UDP. - udp_preference_limit = 0 - [realms] $1 = { kdc = 127.0.0.1:$0 @@ -259,7 +255,7 @@ Status MiniKdc::WaitForKdcPorts() { vector<string> cmd = { lsof, "-wbn", "-Ffn", "-p", std::to_string(kdc_process_->pid()), - "-a", "-i", "4TCP"}; + "-a", "-i", "4UDP"}; string lsof_out; for (int i = 1; ; i++) { http://git-wip-us.apache.org/repos/asf/kudu/blob/123b9918/src/kudu/util/test_util.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc index d381946..1c0b133 100644 --- a/src/kudu/util/test_util.cc +++ b/src/kudu/util/test_util.cc @@ -40,6 +40,7 @@ using strings::Substitute; namespace kudu { +const char* kInvalidPath = "/dev/invalid-path-for-kudu-tests"; static const char* const kSlowTestsEnvVariable = "KUDU_ALLOW_SLOW_TESTS"; static const uint64 kTestBeganAtMicros = Env::Default()->NowMicros(); @@ -104,7 +105,6 @@ void KuduTest::OverrideKrb5Environment() { // // NOTE: we don't simply *unset* the variables, because then we'd still pick up // the user's /etc/krb5.conf and other default locations. - const char* kInvalidPath = "/dev/invalid-path-for-kudu-tests"; setenv("KRB5_CONFIG", kInvalidPath, 1); setenv("KRB5_KTNAME", kInvalidPath, 1); setenv("KRB5CCNAME", kInvalidPath, 1); http://git-wip-us.apache.org/repos/asf/kudu/blob/123b9918/src/kudu/util/test_util.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/test_util.h b/src/kudu/util/test_util.h index 4d34c52..ba26742 100644 --- a/src/kudu/util/test_util.h +++ b/src/kudu/util/test_util.h @@ -30,6 +30,8 @@ namespace kudu { +extern const char* kInvalidPath; + class KuduTest : public ::testing::Test { public: KuduTest();
