kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/3] kudu git commit: sasl: disable error messages assertions on el6
Date Fri, 11 Nov 2016 05:05:02 GMT
sasl: disable error messages assertions on el6

MIT krb5 on RHEL 6 has a bug which means that many of the error messages
from GSSAPI don't get exposed in a reasonable way. So, assertions in
sasl_rpc-test have been failing.

I couldn't find any way to workaround the actual bug and get better
error messages, so this patch just disables those assertions on krb5
1.10 or earlier.

I used a hack to determine the version of Kerberos since spending the
effort to learn enough cmake to do so didn't seem worth the effort for a
test-local purpose.

Change-Id: I1e780e2060dceb322bbbbb772d06b68271ddf675
Reviewed-on: http://gerrit.cloudera.org:8080/5046
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <danburkert@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/e1433ff6
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/e1433ff6
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/e1433ff6

Branch: refs/heads/master
Commit: e1433ff6c29a3a8fab4843cf922e376f7faeb9e8
Parents: ed01e91
Author: Todd Lipcon <todd@apache.org>
Authored: Thu Nov 10 14:37:04 2016 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Fri Nov 11 02:06:21 2016 +0000

----------------------------------------------------------------------
 src/kudu/rpc/sasl_rpc-test.cc | 29 ++++++++++++++++++++++++++++-
 src/kudu/rpc/sasl_server.cc   |  6 ++++++
 2 files changed, 34 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/e1433ff6/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 a0283a0..8e882b0 100644
--- a/src/kudu/rpc/sasl_rpc-test.cc
+++ b/src/kudu/rpc/sasl_rpc-test.cc
@@ -43,6 +43,17 @@
 using std::string;
 using std::thread;
 
+// HACK: MIT Kerberos doesn't have any way of determining its version number,
+// but the error messages in krb5-1.10 and earlier are broken due to
+// a bug: http://krbdev.mit.edu/rt/Ticket/Display.html?id=6973
+//
+// Since we don't have any way to explicitly figure out the version, we just
+// look for this random macro which was added in 1.11 (the same version in which
+// the above bug was fixed).
+#ifndef KRB5_RESPONDER_QUESTION_PASSWORD
+#define KRB5_VERSION_LE_1_10
+#endif
+
 namespace kudu {
 namespace rpc {
 
@@ -273,7 +284,9 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
       std::bind(RunGSSAPINegotiationClient, std::placeholders::_1,
                 [](const Status& s, SaslClient& client) {
                   CHECK(s.IsNotAuthorized());
+#ifndef KRB5_VERSION_LE_1_10
                   CHECK_GT(s.ToString().find("No Kerberos credentials available"), 0);
+#endif
                 }));
 
 
@@ -303,7 +316,9 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
       std::bind(RunGSSAPINegotiationClient, std::placeholders::_1,
                 [kErrorMsg](const Status& s, SaslClient& client) {
                   CHECK(s.IsNotAuthorized());
+#ifndef KRB5_VERSION_LE_1_10
                   CHECK_EQ(s.message().ToString(), kErrorMsg);
+#endif
                 }));
 
   // Create and kinit as a client user.
@@ -335,14 +350,18 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
       std::bind(RunGSSAPINegotiationServer, std::placeholders::_1,
                 [](const Status& s, SaslServer& server) {
                   CHECK(s.IsNotAuthorized());
+#ifndef KRB5_VERSION_LE_1_10
                   ASSERT_STR_CONTAINS(s.ToString(),
                                       "No key table entry found matching kudu/127.0.0.1");
+#endif
                 }),
       std::bind(RunGSSAPINegotiationClient, std::placeholders::_1,
                 [](const Status& s, SaslClient& client) {
                   CHECK(s.IsNotAuthorized());
+#ifndef KRB5_VERSION_LE_1_10
                   ASSERT_STR_CONTAINS(s.ToString(),
                                       "No key table entry found matching kudu/127.0.0.1");
+#endif
                 }));
 
 }
@@ -352,8 +371,10 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
 TEST_F(TestSaslRpc, TestPreflight) {
   // Try pre-flight with no keytab.
   Status s = SaslServer::PreflightCheckGSSAPI(kSaslAppName);
+  ASSERT_FALSE(s.ok());
+#ifndef KRB5_VERSION_LE_1_10
   ASSERT_STR_MATCHES(s.ToString(), "Key table file.*not found");
-
+#endif
   // Try with a valid krb5 environment and keytab.
   MiniKdc kdc;
   ASSERT_OK(kdc.Start());
@@ -367,14 +388,20 @@ TEST_F(TestSaslRpc, TestPreflight) {
   // Try with an inaccessible keytab.
   CHECK_ERR(chmod(kt_path.c_str(), 0000));
   s = SaslServer::PreflightCheckGSSAPI(kSaslAppName);
+  ASSERT_FALSE(s.ok());
+#ifndef KRB5_VERSION_LE_1_10
   ASSERT_STR_MATCHES(s.ToString(), "error accessing keytab: Permission denied");
+#endif
   CHECK_ERR(unlink(kt_path.c_str()));
 
   // Try with a keytab that has the wrong credentials.
   ASSERT_OK(kdc.CreateServiceKeytab("wrong-service/127.0.0.1", &kt_path));
   CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/));
   s = SaslServer::PreflightCheckGSSAPI(kSaslAppName);
+  ASSERT_FALSE(s.ok());
+#ifndef KRB5_VERSION_LE_1_10
   ASSERT_STR_MATCHES(s.ToString(), "No key table entry found matching kudu/.*");
+#endif
 }
 
 ////////////////////////////////////////////////////////////////////////////////

http://git-wip-us.apache.org/repos/asf/kudu/blob/e1433ff6/src/kudu/rpc/sasl_server.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_server.cc b/src/kudu/rpc/sasl_server.cc
index 11d0137..7170e35 100644
--- a/src/kudu/rpc/sasl_server.cc
+++ b/src/kudu/rpc/sasl_server.cc
@@ -467,6 +467,12 @@ int SaslServer::PlainAuthCb(sasl_conn_t * /*conn*/, const char * /*user*/,
const
 }
 
 Status SaslServer::PreflightCheckGSSAPI(const string& app_name) {
+  // TODO(todd): the error messages that come from this function on el6
+  // are relatively useless due to the following krb5 bug:
+  // http://krbdev.mit.edu/rt/Ticket/Display.html?id=6973
+  // This may not be useful anymore given the keytab login that happens
+  // in security/init.cc.
+
   // Initialize a SaslServer with a null socket, and enable
   // only GSSAPI.
   //


Mime
View raw message