You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2017/02/23 02:48:21 UTC
kudu git commit: rpc: separately capture full principal name and
short username
Repository: kudu
Updated Branches:
refs/heads/master d6dac682b -> a9137c0d2
rpc: separately capture full principal name and short username
This changes GSSAPI authentication to capture the full principal in one
field of RemoteUser while storing the "short" username in the 'username'
field. This is so that admins can configure ACLs (and later group
mapping) based on the expected short usernames.
The "short user" mapping respects the auth_to_local rules configured in
krb5.conf. If no appropriate rule can be found, then we fall back to
Hadoop's default of just using the first part of the principal.
This also changes CERTIFICATE authentication to expose the new X509
extensions in the same corresponding fields of RemoteUser.
Additionally, the authentication method is preserved in the RemoteUser
object, which will allow for some special-case authorization decisions
(e.g. we shouldn't issue tokens to existing users of tokens).
Change-Id: Iaf76abf8348dd677a47330b6d0ef110129d853c0
Reviewed-on: http://gerrit.cloudera.org:8080/6106
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a9137c0d
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a9137c0d
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a9137c0d
Branch: refs/heads/master
Commit: a9137c0d2027e7f634436be797732f4e13b9e998
Parents: d6dac68
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Feb 22 00:17:37 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Thu Feb 23 02:32:34 2017 +0000
----------------------------------------------------------------------
src/kudu/rpc/negotiation-test.cc | 26 +++++++++------
src/kudu/rpc/remote_user.cc | 9 ++++-
src/kudu/rpc/remote_user.h | 58 ++++++++++++++++++++++++++++++++-
src/kudu/rpc/server_negotiation.cc | 40 +++++++++++++++++++----
4 files changed, 114 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/a9137c0d/src/kudu/rpc/negotiation-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/negotiation-test.cc b/src/kudu/rpc/negotiation-test.cc
index ac8655b..d0214eb 100644
--- a/src/kudu/rpc/negotiation-test.cc
+++ b/src/kudu/rpc/negotiation-test.cc
@@ -51,6 +51,7 @@
#include "kudu/util/net/sockaddr.h"
#include "kudu/util/net/socket.h"
#include "kudu/util/subprocess.h"
+#include "kudu/util/user.h"
// 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
@@ -291,29 +292,34 @@ TEST_P(TestNegotiation, TestNegotiation) {
EXPECT_EQ(desc.rpc_encrypt_loopback, server_tls_socket);
// Check that the expected user subject is authenticated.
- // TODO(PKI): reenable this once subjects are handled correctly among authn types.
- /*
+ RemoteUser remote_user = server_negotiation.take_authenticated_user();
switch (server_negotiation.negotiated_authn()) {
case AuthenticationType::SASL:
switch (server_negotiation.negotiated_mechanism()) {
case SaslMechanism::PLAIN:
- EXPECT_EQ("client-plain", server_negotiation.authenticated_user());
+ EXPECT_EQ("client-plain", remote_user.username());
break;
case SaslMechanism::GSSAPI:
- EXPECT_EQ("client-gssapi", server_negotiation.authenticated_user());
+ EXPECT_EQ("client-gssapi", remote_user.username());
+ EXPECT_EQ("client-gssapi@KRBTEST.COM", remote_user.principal());
break;
case SaslMechanism::INVALID: LOG(FATAL) << "invalid mechanism negotiated";
}
break;
- case AuthenticationType::CERTIFICATE:
- EXPECT_EQ("client-certificate", server_negotiation.authenticated_user());
- break;
+ case AuthenticationType::CERTIFICATE: {
+ // We expect the cert to be using the local username, because it hasn't
+ // logged in from any Keytab.
+ string expected;
+ CHECK_OK(GetLoggedInUser(&expected));
+ EXPECT_EQ(expected, remote_user.username());
+ EXPECT_FALSE(remote_user.has_principal());
+ break;
+ }
case AuthenticationType::TOKEN:
- EXPECT_EQ("client-token", server_negotiation.authenticated_user());
- break;
+ EXPECT_EQ("client-token", remote_user.username());
+ break;
case AuthenticationType::INVALID: LOG(FATAL) << "invalid authentication negotiated";
}
- */
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/a9137c0d/src/kudu/rpc/remote_user.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/remote_user.cc b/src/kudu/rpc/remote_user.cc
index ad9b4d1..015aace 100644
--- a/src/kudu/rpc/remote_user.cc
+++ b/src/kudu/rpc/remote_user.cc
@@ -17,6 +17,7 @@
#include "kudu/rpc/remote_user.h"
+#include <boost/optional.hpp>
#include <string>
#include "kudu/gutil/strings/substitute.h"
@@ -27,7 +28,13 @@ namespace kudu {
namespace rpc {
string RemoteUser::ToString() const {
- return strings::Substitute("{username='$0'}", username_);
+ string ret;
+ strings::SubstituteAndAppend(&ret, "{username='$0'", username_);
+ if (has_principal()) {
+ strings::SubstituteAndAppend(&ret, ", principal='$0'", *principal_);
+ }
+ ret.append("}");
+ return ret;
}
} // namespace rpc
http://git-wip-us.apache.org/repos/asf/kudu/blob/a9137c0d/src/kudu/rpc/remote_user.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/remote_user.h b/src/kudu/rpc/remote_user.h
index e085343..529d2a2 100644
--- a/src/kudu/rpc/remote_user.h
+++ b/src/kudu/rpc/remote_user.h
@@ -18,6 +18,9 @@
#include <string>
+#include <boost/optional.hpp>
+#include <glog/logging.h>
+
namespace kudu {
namespace rpc {
@@ -27,10 +30,57 @@ namespace rpc {
// its initialization during RPC negotiation.
class RemoteUser {
public:
+ // The method by which the remote user authenticated.
+ enum Method {
+ // No authentication (authentication was not required by the server
+ // and the user provided a username but it was not validated in any way)
+ UNAUTHENTICATED,
+ // Kerberos-authenticated.
+ KERBEROS,
+ // Authenticated by a Kudu authentication token.
+ AUTHN_TOKEN,
+ // Authenticated by a client certificate.
+ CLIENT_CERT
+ };
+
+ Method authenticated_by() const {
+ return authenticated_by_;
+ }
+
const std::string& username() const { return username_; }
- void set_username(std::string username) {
+ bool has_principal() const {
+ return principal_ != boost::none;
+ }
+ const std::string& principal() const {
+ DCHECK(has_principal());
+ return *principal_;
+ }
+
+ void SetAuthenticatedByKerberos(std::string username,
+ std::string principal) {
+ authenticated_by_ = KERBEROS;
+ username_ = std::move(username);
+ principal_ = std::move(principal);
+ }
+
+ void SetUnauthenticated(std::string username) {
+ authenticated_by_ = UNAUTHENTICATED;
username_ = std::move(username);
+ principal_ = boost::none;
+ }
+
+ void SetAuthenticatedByClientCert(std::string username,
+ boost::optional<std::string> principal) {
+ authenticated_by_ = CLIENT_CERT;
+ username_ = std::move(username);
+ principal_ = std::move(principal);
+ }
+
+ void SetAuthenticatedByToken(std::string username) {
+ authenticated_by_ = AUTHN_TOKEN;
+ username_ = std::move(username);
+ principal_ = boost::none;
}
// Returns a string representation of the object.
@@ -41,6 +91,12 @@ class RemoteUser {
// principal, this has already been mapped to a local username.
// TODO(todd): actually do the above mapping.
std::string username_;
+
+ // The full principal of the remote user. This is only set in the
+ // case of a strong-authenticated user.
+ boost::optional<std::string> principal_;
+
+ Method authenticated_by_ = UNAUTHENTICATED;
};
} // namespace rpc
http://git-wip-us.apache.org/repos/asf/kudu/blob/a9137c0d/src/kudu/rpc/server_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc
index 5fac8dc..2549132 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -36,6 +36,7 @@
#include "kudu/rpc/constants.h"
#include "kudu/rpc/serialization.h"
#include "kudu/security/cert.h"
+#include "kudu/security/init.h"
#include "kudu/security/tls_context.h"
#include "kudu/security/tls_handshake.h"
#include "kudu/security/tls_socket.h"
@@ -502,13 +503,30 @@ Status ServerNegotiation::AuthenticateBySasl(faststring* recv_buf) {
}
RETURN_NOT_OK(s);
- const char* username = nullptr;
+ const char* c_username = nullptr;
int rc = sasl_getprop(sasl_conn_.get(), SASL_USERNAME,
- reinterpret_cast<const void**>(&username));
+ reinterpret_cast<const void**>(&c_username));
// We expect that SASL_USERNAME will always get set.
- CHECK(rc == SASL_OK && username != nullptr) << "No username on authenticated connection";
- authenticated_user_.set_username(username);
-
+ CHECK(rc == SASL_OK && c_username != nullptr) << "No username on authenticated connection";
+ if (negotiated_mech_ == SaslMechanism::GSSAPI) {
+ // The SASL library doesn't include the user's realm in the username if it's the
+ // same realm as the default realm of the server. So, we pass it back through the
+ // Kerberos library to add back the realm if necessary.
+ string principal = c_username;
+ RETURN_NOT_OK_PREPEND(security::CanonicalizeKrb5Principal(&principal),
+ "could not canonicalize krb5 principal");
+
+ // Map the principal to the corresponding local username. For example, admins
+ // can set up mappings so that joe@REMOTEREALM becomes something like 'remote-joe'
+ // locally for the purposes of group mapping, ACLs, etc.
+ string local_name;
+ RETURN_NOT_OK_PREPEND(security::MapPrincipalToLocalName(principal, &local_name),
+ strings::Substitute("could not map krb5 principal '$0' to username",
+ principal));
+ authenticated_user_.SetAuthenticatedByKerberos(std::move(local_name), std::move(principal));
+ } else {
+ authenticated_user_.SetUnauthenticated(c_username);
+ }
return Status::OK();
}
@@ -556,7 +574,7 @@ Status ServerNegotiation::AuthenticateByToken(faststring* recv_buf) {
// get a signed authn token without a subject.
return Status::RuntimeError("authentication token has no username");
}
- authenticated_user_.set_username(token.authn().username());
+ authenticated_user_.SetAuthenticatedByToken(token.authn().username());
// Respond with success message.
pb.Clear();
@@ -572,7 +590,15 @@ Status ServerNegotiation::AuthenticateByCertificate() {
// Grab the subject from the client's cert.
security::Cert cert;
RETURN_NOT_OK(tls_handshake_.GetRemoteCert(&cert));
- authenticated_user_.set_username(cert.SubjectName());
+
+ boost::optional<string> user_id = cert.UserId();
+ boost::optional<string> principal = cert.KuduKerberosPrincipal();
+
+ if (!user_id) {
+ return Status::NotAuthorized("did not find expected X509 userId extension in cert");
+ }
+
+ authenticated_user_.SetAuthenticatedByClientCert(*user_id, std::move(principal));
return Status::OK();
}