kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [1/2] kudu git commit: Misc. fixes for Kerberos compatibility on OS X
Date Tue, 08 Nov 2016 00:28:02 GMT
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 <adar@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 <danburkert@apache.org>
Authored: Mon Nov 7 14:20:27 2016 -0800
Committer: Dan Burkert <danburkert@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, "alice@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, "alice@KRBTEST.COM");
+  ASSERT_STR_NOT_CONTAINS(klist, "alice@KRBTEST.COM");
   ASSERT_STR_CONTAINS(klist, "bob@KRBTEST.COM");
   ASSERT_STR_CONTAINS(klist, "krbtgt/KRBTEST.COM@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();


Mime
View raw message