Repository: kudu Updated Branches: refs/heads/master 69f2619d0 -> d6dac682b
IPKI: change X509 attribute fields This switches the attributes of the IPKI certs as follows: CN = FQDN of host userId = short user name <custom OID> = long kerberos principal (if kerberized) Change-Id: Iec3b759c327ea7758a98e675b12ff5ef49294679 Reviewed-on: http://gerrit.cloudera.org:8080/6116 Reviewed-by: Dan Burkert <[email protected]> Tested-by: Todd Lipcon <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/ed9ce952 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/ed9ce952 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/ed9ce952 Branch: refs/heads/master Commit: ed9ce952d8fcdf2110ce590b1ceb299918ec2536 Parents: 69f2619 Author: Todd Lipcon <[email protected]> Authored: Wed Feb 22 10:54:32 2017 -0800 Committer: Todd Lipcon <[email protected]> Committed: Thu Feb 23 01:58:56 2017 +0000 ---------------------------------------------------------------------- src/kudu/rpc/messenger.cc | 12 +- src/kudu/rpc/messenger.h | 6 +- src/kudu/rpc/rpc-test-base.h | 2 +- src/kudu/security/ca/cert_management-test.cc | 9 +- src/kudu/security/ca/cert_management.cc | 10 ++ src/kudu/security/ca/cert_management.h | 12 +- src/kudu/security/cert-test.cc | 8 ++ src/kudu/security/cert.cc | 42 ++++++ src/kudu/security/cert.h | 12 ++ src/kudu/security/init.cc | 165 +++++++++++++++++----- src/kudu/security/init.h | 28 +++- src/kudu/security/security-test-util.cc | 4 +- src/kudu/security/test/mini_kdc-test.cc | 20 +++ src/kudu/security/test/mini_kdc.cc | 5 + src/kudu/security/tls_context.cc | 34 ++++- src/kudu/security/tls_context.h | 2 +- src/kudu/security/tls_handshake-test.cc | 4 +- src/kudu/server/server_base.cc | 2 +- 18 files changed, 315 insertions(+), 62 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/rpc/messenger.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc index 59ad79e..7a3a9e1 100644 --- a/src/kudu/rpc/messenger.cc +++ b/src/kudu/rpc/messenger.cc @@ -118,7 +118,9 @@ MessengerBuilder::MessengerBuilder(std::string name) num_reactors_(4), min_negotiation_threads_(0), max_negotiation_threads_(4), - coarse_timer_granularity_(MonoDelta::FromMilliseconds(100)) {} + coarse_timer_granularity_(MonoDelta::FromMilliseconds(100)), + enable_inbound_tls_(false) { +} MessengerBuilder& MessengerBuilder::set_connection_keepalive_time(const MonoDelta &keepalive) { connection_keepalive_time_ = keepalive; @@ -151,8 +153,8 @@ MessengerBuilder &MessengerBuilder::set_metric_entity( return *this; } -MessengerBuilder& MessengerBuilder::enable_inbound_tls(std::string server_uuid) { - enable_inbound_tls_server_uuid_ = server_uuid; +MessengerBuilder& MessengerBuilder::enable_inbound_tls() { + enable_inbound_tls_ = true; return *this; } @@ -194,7 +196,7 @@ Status MessengerBuilder::Build(shared_ptr<Messenger> *msgr) { } RETURN_NOT_OK(new_msgr->Init()); - if (new_msgr->encryption_ != RpcEncryption::DISABLED && enable_inbound_tls_server_uuid_) { + if (new_msgr->encryption_ != RpcEncryption::DISABLED && enable_inbound_tls_) { auto* tls_context = new_msgr->mutable_tls_context(); if (!FLAGS_rpc_certificate_file.empty() && @@ -212,7 +214,7 @@ Status MessengerBuilder::Build(shared_ptr<Messenger> *msgr) { return Status::InvalidArgument("--rpc_certificate_file, --rpc_private_key_file, and " "--rpc_ca_certificate_file flags must be set as a group"); } else { - RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey(*enable_inbound_tls_server_uuid_)); + RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey()); } } http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/rpc/messenger.h ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h index 89dd933..f3614a0 100644 --- a/src/kudu/rpc/messenger.h +++ b/src/kudu/rpc/messenger.h @@ -118,9 +118,7 @@ class MessengerBuilder { MessengerBuilder &set_metric_entity(const scoped_refptr<MetricEntity>& metric_entity); // Configure the messenger to enable TLS encryption on inbound connections. - // The 'server_uuid' will be used as the subject name for the server's - // certificate. - MessengerBuilder& enable_inbound_tls(std::string server_uuid); + MessengerBuilder& enable_inbound_tls(); Status Build(std::shared_ptr<Messenger> *msgr); @@ -132,7 +130,7 @@ class MessengerBuilder { int max_negotiation_threads_; MonoDelta coarse_timer_granularity_; scoped_refptr<MetricEntity> metric_entity_; - boost::optional<string> enable_inbound_tls_server_uuid_; + bool enable_inbound_tls_; }; // A Messenger is a container for the reactor threads which run event loops http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/rpc/rpc-test-base.h ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/rpc-test-base.h b/src/kudu/rpc/rpc-test-base.h index 47f209f..dcdd5c3 100644 --- a/src/kudu/rpc/rpc-test-base.h +++ b/src/kudu/rpc/rpc-test-base.h @@ -366,7 +366,7 @@ class RpcTestBase : public KuduTest { MessengerBuilder bld(name); if (enable_ssl) { - bld.enable_inbound_tls(name); + bld.enable_inbound_tls(); } bld.set_num_reactors(n_reactors); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/ca/cert_management-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/ca/cert_management-test.cc b/src/kudu/security/ca/cert_management-test.cc index b3ec45b..6e00df5 100644 --- a/src/kudu/security/ca/cert_management-test.cc +++ b/src/kudu/security/ca/cert_management-test.cc @@ -149,7 +149,10 @@ TEST_F(CertManagementTest, SignerInitWithExpiredCert) { // Generate X509 CSR and issues corresponding certificate. TEST_F(CertManagementTest, SignCert) { - const CertRequestGenerator::Config gen_config = PrepareConfig("test-cn"); + CertRequestGenerator::Config gen_config; + gen_config.cn = "test-cn"; + gen_config.user_id = "test-uid"; + gen_config.kerberos_principal = "kudu/[email protected]"; PrivateKey key; const auto& csr = PrepareTestCSR(gen_config, &key); Cert cert; @@ -158,7 +161,9 @@ TEST_F(CertManagementTest, SignCert) { EXPECT_EQ("C = US, ST = CA, O = MyCompany, CN = MyName, emailAddress = [email protected]", cert.IssuerName()); - EXPECT_EQ("CN = test-cn", cert.SubjectName()); + EXPECT_EQ("CN = test-cn, UID = test-uid", cert.SubjectName()); + EXPECT_EQ(gen_config.user_id, *cert.UserId()); + EXPECT_EQ(gen_config.kerberos_principal, *cert.KuduKerberosPrincipal()); } // Generate X509 CA CSR and sign the result certificate. http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/ca/cert_management.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/ca/cert_management.cc b/src/kudu/security/ca/cert_management.cc index 88bb3fb..4bdca21 100644 --- a/src/kudu/security/ca/cert_management.cc +++ b/src/kudu/security/ca/cert_management.cc @@ -37,6 +37,7 @@ #include "kudu/gutil/strings/substitute.h" #include "kudu/security/cert.h" +#include "kudu/security/init.h" #include "kudu/security/openssl_util.h" #include "kudu/util/scoped_cleanup.h" #include "kudu/util/status.h" @@ -89,6 +90,9 @@ Status CertRequestGeneratorBase::GenerateRequest(const PrivateKey& key, } while (false) CERT_SET_SUBJ_FIELD(config_.cn, "CN", "common name"); + if (config_.user_id) { + CERT_SET_SUBJ_FIELD(*config_.user_id, "UID", "userId"); + } #undef CERT_SET_SUBJ_FIELD // Set necessary extensions into the request. @@ -155,6 +159,12 @@ Status CertRequestGenerator::Init() { RETURN_NOT_OK(PushExtension(extensions_, NID_basic_constraints, "critical,CA:FALSE")); + if (config_.kerberos_principal) { + int nid = GetKuduKerberosPrincipalOidNid(); + RETURN_NOT_OK(PushExtension(extensions_, nid, + Substitute("ASN1:UTF8:$0", *config_.kerberos_principal))); + } + is_initialized_ = true; return Status::OK(); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/ca/cert_management.h ---------------------------------------------------------------------- diff --git a/src/kudu/security/ca/cert_management.h b/src/kudu/security/ca/cert_management.h index f0fd342..2f36efc 100644 --- a/src/kudu/security/ca/cert_management.h +++ b/src/kudu/security/ca/cert_management.h @@ -24,6 +24,8 @@ #include <string> #include <vector> +#include <boost/optional.hpp> + #include "kudu/gutil/macros.h" #include "kudu/gutil/strings/stringpiece.h" #include "kudu/security/crypto.h" @@ -52,10 +54,14 @@ namespace ca { // Base utility class for issuing X509 CSRs. class CertRequestGeneratorBase { public: - // Properties for the generated X509 CSR. Using server UUID for the common - // name field. + // Properties for the generated X509 CSR. struct Config { - std::string cn; // subject field: CN + // Common Name (CN) + std::string cn; + // userId (UID) + boost::optional<std::string> user_id; + // Our custom extension which stores the full Kerberos principal for IPKI certs. + boost::optional<std::string> kerberos_principal; }; explicit CertRequestGeneratorBase(Config config); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/cert-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/cert-test.cc b/src/kudu/security/cert-test.cc index d3883a1..2185a83 100644 --- a/src/kudu/security/cert-test.cc +++ b/src/kudu/security/cert-test.cc @@ -17,6 +17,9 @@ #include <utility> +#include <boost/optional.hpp> +#include <boost/optional/optional_io.hpp> + #include "kudu/gutil/strings/strip.h" #include "kudu/security/cert.h" #include "kudu/security/crypto.h" @@ -102,5 +105,10 @@ TEST_F(CertTest, CertMismatchesRsaPrivateKey) { } } +TEST_F(CertTest, TestGetKuduSpecificFieldsWhenMissing) { + EXPECT_EQ(boost::none, ca_cert_.UserId()); + EXPECT_EQ(boost::none, ca_cert_.KuduKerberosPrincipal()); +} + } // namespace security } // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/cert.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc index 3efb8f8..8bccc03 100644 --- a/src/kudu/security/cert.cc +++ b/src/kudu/security/cert.cc @@ -22,10 +22,14 @@ #include <openssl/evp.h> #include <openssl/pem.h> #include <openssl/x509.h> +#include <openssl/x509v3.h> + +#include <boost/optional.hpp> #include "kudu/security/crypto.h" #include "kudu/security/openssl_util.h" #include "kudu/security/openssl_util_bio.h" +#include "kudu/util/scoped_cleanup.h" #include "kudu/util/status.h" using std::string; @@ -33,6 +37,9 @@ using std::string; namespace kudu { namespace security { +// This OID is generated via the UUID method. +static const char* kKuduKerberosPrincipalOidStr = "2.25.243346677289068076843480765133256509912"; + string X509NameToString(X509_NAME* name) { CHECK(name); auto bio = ssl_make_unique(BIO_new(BIO_s_mem())); @@ -43,6 +50,16 @@ string X509NameToString(X509_NAME* name) { return string(membuf->data, membuf->length); } +int GetKuduKerberosPrincipalOidNid() { + InitializeOpenSSL(); + + int nid = OBJ_txt2nid(kKuduKerberosPrincipalOidStr); + if (nid != NID_undef) return nid; + nid = OBJ_create(kKuduKerberosPrincipalOidStr, "kuduPrinc", "kuduKerberosPrincipal"); + CHECK_NE(nid, NID_undef) << "could not create kuduPrinc oid"; + return nid; +} + Status Cert::FromString(const std::string& data, DataFormat format) { return ::kudu::security::FromString(data, format, &data_); } @@ -63,6 +80,31 @@ string Cert::IssuerName() const { return X509NameToString(X509_get_issuer_name(data_.get())); } +boost::optional<string> Cert::UserId() const { + X509_NAME* name = X509_get_subject_name(data_.get()); + char buf[1024]; + int len = X509_NAME_get_text_by_NID(name, NID_userId, buf, arraysize(buf)); + if (len < 0) return boost::none; + return string(buf, len); +} + +boost::optional<string> Cert::KuduKerberosPrincipal() const { + int idx = X509_get_ext_by_NID(data_.get(), GetKuduKerberosPrincipalOidNid(), -1); + if (idx < 0) return boost::none; + X509_EXTENSION* ext = X509_get_ext(data_.get(), idx); + ASN1_OCTET_STRING* octet_str = X509_EXTENSION_get_data(ext); + const unsigned char* octet_str_data = octet_str->data; + long len; // NOLINT(runtime/int) + int tag, xclass; + if (ASN1_get_object(&octet_str_data, &len, &tag, &xclass, octet_str->length) != 0 || + tag != V_ASN1_UTF8STRING) { + LOG(DFATAL) << "invalid extension value in cert " << SubjectName(); + return boost::none; + } + + return string(reinterpret_cast<const char*>(octet_str_data), len); +} + Status Cert::CheckKeyMatch(const PrivateKey& key) const { OPENSSL_RET_NOT_OK(X509_check_private_key(data_.get(), key.GetRawData()), "certificate does not match private key"); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/cert.h ---------------------------------------------------------------------- diff --git a/src/kudu/security/cert.h b/src/kudu/security/cert.h index b460c31..929f8b2 100644 --- a/src/kudu/security/cert.h +++ b/src/kudu/security/cert.h @@ -19,6 +19,8 @@ #include <string> +#include <boost/optional/optional_fwd.hpp> + #include "kudu/security/openssl_util.h" namespace kudu { @@ -33,6 +35,10 @@ class PublicKey; // Convert an X509_NAME object to a human-readable string. std::string X509NameToString(X509_NAME* name); +// Return the OpenSSL NID for the custom X509 extension where we store +// our Kerberos principal in IPKI certs. +int GetKuduKerberosPrincipalOidNid(); + class Cert : public RawDataWrapper<X509> { public: Status FromString(const std::string& data, DataFormat format); @@ -42,6 +48,12 @@ class Cert : public RawDataWrapper<X509> { std::string SubjectName() const; std::string IssuerName() const; + // Return the 'userId' extension of this cert, if set. + boost::optional<std::string> UserId() const; + + // Return the Kerberos principal encoded in this certificate, if set. + boost::optional<std::string> KuduKerberosPrincipal() const; + // Check whether the specified private key matches the certificate. // Return Status::OK() if key match the certificate. Status CheckKeyMatch(const PrivateKey& key) const; http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/init.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc index 4ccbc05..fe4659b 100644 --- a/src/kudu/security/init.cc +++ b/src/kudu/security/init.cc @@ -17,10 +17,15 @@ #include "kudu/security/init.h" -#include <algorithm> #include <krb5/krb5.h> + +#include <algorithm> +#include <mutex> #include <random> #include <string> +#include <vector> + +#include <boost/optional.hpp> #include "kudu/gutil/strings/util.h" #include "kudu/util/flags.h" @@ -51,14 +56,27 @@ using std::random_device; using std::string; using std::uniform_int_distribution; using std::uniform_real_distribution; +using std::vector; namespace kudu { namespace security { namespace { -class KinitContext { +class KinitContext; + +// Global context for usage of the Krb5 library. +krb5_context g_krb5_ctx; +// Global instance of the context used by the kinit/renewal thread. +KinitContext* g_kinit_ctx; + +// This lock is used to avoid a race while renewing the kerberos ticket. +// The race can occur between the time we reinitialize the cache and the +// time when we actually store the renewed credential back in the cache. +RWMutex* g_kerberos_reinit_lock; + +class KinitContext { public: KinitContext(); @@ -88,31 +106,35 @@ class KinitContext { // 'num_retries', with a random jitter of +/- 0%-50% of that value. int32_t GetBackedOffRenewInterval(int32_t time_remaining, uint32_t num_retries); - krb5_context* krb5_ctx() { return &krb5_ctx_; } + const string& principal_str() const { return principal_str_; } private: krb5_principal principal_; krb5_keytab keytab_; krb5_ccache ccache_; krb5_get_init_creds_opt* opts_; - krb5_context krb5_ctx_; + + // The stringified principal that we are logged in as. + string principal_str_; // This is the time that the current TGT in use expires. int32_t ticket_end_timestamp_; }; -KinitContext* g_kinit_ctx; - -// This lock is used to avoid a race while renewing the kerberos ticket. -// The race can occur between the time we reinitialize the cache and the -// time when we actually store the renewed credential back in the cache. -RWMutex* g_kerberos_reinit_lock; Status Krb5CallToStatus(krb5_context ctx, krb5_error_code code) { if (code == 0) return Status::OK(); return Status::RuntimeError(krb5_get_error_message(ctx, code)); } #define KRB5_RETURN_NOT_OK_PREPEND(call, prepend) \ - RETURN_NOT_OK_PREPEND(Krb5CallToStatus(*g_kinit_ctx->krb5_ctx(), (call)), (prepend)) + RETURN_NOT_OK_PREPEND(Krb5CallToStatus(g_krb5_ctx, (call)), (prepend)) + + +void InitKrb5Ctx() { + static std::once_flag once; + std::call_once(once, [&]() { + CHECK_EQ(krb5_init_context(&g_krb5_ctx), 0); + }); +} KinitContext::KinitContext() {} @@ -126,6 +148,17 @@ inline int data_eq_string(krb5_data d, const char *s) { return (d.length == strlen(s) && !memcmp(d.data, s, d.length)); } +Status Krb5UnparseName(krb5_principal princ, string* name) { + char* c_name; + KRB5_RETURN_NOT_OK_PREPEND(krb5_unparse_name(g_krb5_ctx, princ, &c_name), + "krb5_unparse_name"); + auto cleanup_name = MakeScopedCleanup([&]() { + krb5_free_unparsed_name(g_krb5_ctx, c_name); + }); + *name = c_name; + return Status::OK(); +} + // Periodically calls DoRenewal(). void RenewThread() { uint32_t failure_retries = 0; @@ -182,20 +215,20 @@ Status KinitContext::DoRenewal() { krb5_cc_cursor cursor; // Setup a cursor to iterate through the credential cache. - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_start_seq_get(krb5_ctx_, ccache_, &cursor), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_start_seq_get(g_krb5_ctx, ccache_, &cursor), "Failed to peek into ccache"); auto cleanup_cursor = MakeScopedCleanup([&]() { - krb5_cc_end_seq_get(krb5_ctx_, ccache_, &cursor); }); + krb5_cc_end_seq_get(g_krb5_ctx, ccache_, &cursor); }); krb5_creds creds; memset(&creds, 0, sizeof(krb5_creds)); krb5_error_code rc; // Iterate through the credential cache. - while (!(rc = krb5_cc_next_cred(krb5_ctx_, ccache_, &cursor, &creds))) { + while (!(rc = krb5_cc_next_cred(g_krb5_ctx, ccache_, &cursor, &creds))) { auto cleanup_creds = MakeScopedCleanup([&]() { - krb5_free_cred_contents(krb5_ctx_, &creds); }); - if (krb5_is_config_principal(krb5_ctx_, creds.server)) continue; + krb5_free_cred_contents(g_krb5_ctx, &creds); }); + if (krb5_is_config_principal(g_krb5_ctx, creds.server)) continue; // We only want to renew the TGT (Ticket Granting Ticket). Ignore all other tickets. // This follows the same format as is_local_tgt() from krb5:src/clients/klist/klist.c @@ -214,7 +247,7 @@ Status KinitContext::DoRenewal() { krb5_creds new_creds; memset(&new_creds, 0, sizeof(krb5_creds)); auto cleanup_new_creds = MakeScopedCleanup([&]() { - krb5_free_cred_contents(krb5_ctx_, &new_creds); }); + krb5_free_cred_contents(g_krb5_ctx, &new_creds); }); // If the ticket has already expired or if there's only a short period before which the // renew window closes, we acquire a new ticket. if (ticket_expiry < now || renew_deadline < now) { @@ -222,7 +255,7 @@ Status KinitContext::DoRenewal() { // credential cache. { std::lock_guard<RWMutex> l(*g_kerberos_reinit_lock); - KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_keytab(krb5_ctx_, &new_creds, principal_, + KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_keytab(g_krb5_ctx, &new_creds, principal_, keytab_, 0 /* valid from now */, nullptr /* TKT service name */, opts_), @@ -230,17 +263,17 @@ Status KinitContext::DoRenewal() { #ifdef __APPLE__ // Heimdal krb5 doesn't have the 'krb5_get_init_creds_opt_set_out_ccache' option, // so use this alternate route. - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_initialize(krb5_ctx_, ccache_, principal_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_initialize(g_krb5_ctx, ccache_, principal_), "Reacquire error: could not init ccache"); - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_store_cred(krb5_ctx_, ccache_, &creds), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_store_cred(g_krb5_ctx, ccache_, &creds), "Reacquire error: could not store creds in cache"); #endif } LOG(INFO) << "Successfully reacquired a new kerberos TGT"; } else { // Renew existing ticket. - KRB5_RETURN_NOT_OK_PREPEND(krb5_get_renewed_creds(krb5_ctx_, &new_creds, principal_, + KRB5_RETURN_NOT_OK_PREPEND(krb5_get_renewed_creds(g_krb5_ctx, &new_creds, principal_, ccache_, nullptr), "Failed to renew ticket"); @@ -249,11 +282,11 @@ Status KinitContext::DoRenewal() { // until the new credentials are placed in the cache. std::lock_guard<RWMutex> l(*g_kerberos_reinit_lock); // Clear existing credentials in cache. - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_initialize(krb5_ctx_, ccache_, principal_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_initialize(g_krb5_ctx, ccache_, principal_), "Failed to re-initialize ccache"); // Store the new credentials in the cache. - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_store_cred(krb5_ctx_, ccache_, &new_creds), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_store_cred(g_krb5_ctx, ccache_, &new_creds), "Failed to store credentials in ccache"); } LOG(INFO) << "Successfully renewed kerberos TGT"; @@ -265,51 +298,58 @@ Status KinitContext::DoRenewal() { } Status KinitContext::Kinit(const string& keytab_path, const string& principal) { - if (krb5_init_context(&krb5_ctx_) != 0) { - return Status::RuntimeError("could not initialize krb5 library"); - } + InitKrb5Ctx(); // Parse the principal - KRB5_RETURN_NOT_OK_PREPEND(krb5_parse_name(krb5_ctx_, principal.c_str(), &principal_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_parse_name(g_krb5_ctx, principal.c_str(), &principal_), "could not parse principal"); - KRB5_RETURN_NOT_OK_PREPEND(krb5_kt_resolve(krb5_ctx_, keytab_path.c_str(), &keytab_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_kt_resolve(g_krb5_ctx, keytab_path.c_str(), &keytab_), "unable to resolve keytab"); - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_default(krb5_ctx_, &ccache_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_default(g_krb5_ctx, &ccache_), "unable to get default credentials cache"); - KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_opt_alloc(krb5_ctx_, &opts_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_opt_alloc(g_krb5_ctx, &opts_), "unable to allocate get_init_creds_opt struct"); #ifndef __APPLE__ - KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_opt_set_out_ccache(krb5_ctx_, opts_, ccache_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_opt_set_out_ccache(g_krb5_ctx, opts_, ccache_), "unable to set init_creds options"); #endif krb5_creds creds; - KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_keytab(krb5_ctx_, &creds, principal_, keytab_, + KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_keytab(g_krb5_ctx, &creds, principal_, keytab_, 0 /* valid from now */, nullptr /* TKT service name */, opts_), "unable to login from keytab"); auto cleanup_creds = MakeScopedCleanup([&]() { - krb5_free_cred_contents(krb5_ctx_, &creds); }); + krb5_free_cred_contents(g_krb5_ctx, &creds); }); ticket_end_timestamp_ = creds.times.endtime; #ifdef __APPLE__ // Heimdal krb5 doesn't have the 'krb5_get_init_creds_opt_set_out_ccache' option, // so use this alternate route. - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_initialize(krb5_ctx_, ccache_, principal_), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_initialize(g_krb5_ctx, ccache_, principal_), "could not init ccache"); - KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_store_cred(krb5_ctx_, ccache_, &creds), + KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_store_cred(g_krb5_ctx, ccache_, &creds), "could not store creds in cache"); #endif + + // Convert the logged-in principal back to a string. This may be different than + // 'principal', since the default realm will be filled in based on the Kerberos + // configuration if not originally specified. + RETURN_NOT_OK_PREPEND(Krb5UnparseName(principal_, &principal_str_), + "could not stringify the logged-in principal"); + + LOG(INFO) << "Logged in from keytab as " << principal_str_; + return Status::OK(); } -Status GetLoginPrincipal(string* principal) { +Status GetConfiguredPrincipal(string* principal) { string p = FLAGS_principal; string hostname; // Try to fill in either the FQDN or hostname. @@ -323,10 +363,61 @@ Status GetLoginPrincipal(string* principal) { } // anonymous namespace + RWMutex* KerberosReinitLock() { return g_kerberos_reinit_lock; } +Status CanonicalizeKrb5Principal(std::string* principal) { + InitKrb5Ctx(); + krb5_principal princ; + KRB5_RETURN_NOT_OK_PREPEND(krb5_parse_name(g_krb5_ctx, principal->c_str(), &princ), + "could not parse principal"); + auto cleanup = MakeScopedCleanup([&]() { + krb5_free_principal(g_krb5_ctx, princ); + }); + RETURN_NOT_OK_PREPEND(Krb5UnparseName(princ, principal), + "failed to convert principal back to string"); + return Status::OK(); +} + +Status MapPrincipalToLocalName(const std::string& principal, std::string* local_name) { + InitKrb5Ctx(); + krb5_principal princ; + KRB5_RETURN_NOT_OK_PREPEND(krb5_parse_name(g_krb5_ctx, principal.c_str(), &princ), + "could not parse principal"); + auto cleanup = MakeScopedCleanup([&]() { + krb5_free_principal(g_krb5_ctx, princ); + }); + char buf[1024]; + krb5_error_code rc = krb5_aname_to_localname(g_krb5_ctx, princ, arraysize(buf), buf); + if (rc == KRB5_LNAME_NOTRANS) { + // No name mapping specified. We fall back to simply taking the first component + // of the principal, for compatibility with the default behavior of Hadoop. + // TODO(todd): we should support custom configured auth-to-local mapping, since + // most Hadoop ecosystem components do not load them from krb5.conf. + if (princ->length > 0) { + local_name->assign(princ->data[0].data, princ->data[0].length); + return Status::OK(); + } + return Status::NotFound("unable to find first component of principal"); + } + if (rc == KRB5_CONFIG_NOTENUFSPACE) { + return Status::InvalidArgument("mapped username too large"); + } + KRB5_RETURN_NOT_OK_PREPEND(rc, "krb5_aname_to_localname"); + if (strlen(buf) == 0) { + return Status::InvalidArgument("principal mapped to empty username"); + } + local_name->assign(buf); + return Status::OK(); +} + +boost::optional<string> GetLoggedInPrincipalFromKeytab() { + if (!g_kinit_ctx) return boost::none; + return g_kinit_ctx->principal_str(); +} + Status InitKerberosForServer() { if (FLAGS_keytab_file.empty()) return Status::OK(); @@ -339,7 +430,7 @@ Status InitKerberosForServer() { g_kinit_ctx = new KinitContext(); string principal; - RETURN_NOT_OK(GetLoginPrincipal(&principal)); + RETURN_NOT_OK(GetConfiguredPrincipal(&principal)); RETURN_NOT_OK_PREPEND(g_kinit_ctx->Kinit(FLAGS_keytab_file, principal), "unable to kinit"); g_kerberos_reinit_lock = new RWMutex(RWMutex::Priority::PREFER_WRITING); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/init.h ---------------------------------------------------------------------- diff --git a/src/kudu/security/init.h b/src/kudu/security/init.h index 51a1460..c62346f 100644 --- a/src/kudu/security/init.h +++ b/src/kudu/security/init.h @@ -16,11 +16,14 @@ // under the License. #pragma once -#include "kudu/util/status.h" +#include <string> + +#include <boost/optional/optional_fwd.hpp> namespace kudu { class RWMutex; +class Status; namespace security { @@ -33,5 +36,28 @@ Status InitKerberosForServer(); // acquired in read mode before using the SASL library which might require a ticket. RWMutex* KerberosReinitLock(); +// Return the full principal (user/host@REALM) that the server has used to +// log in from the keytab. +// +// If the server has not logged in from a keytab, returns boost::none. +boost::optional<std::string> GetLoggedInPrincipalFromKeytab(); + +// Canonicalize the given principal name by adding '@DEFAULT_REALM' in the case that +// the principal has no realm. +// +// TODO(todd): move to kerberos_util.h in the later patch in this series (the file doesn't +// exist yet, and trying to avoid rebase pain). +Status CanonicalizeKrb5Principal(std::string* principal); + +// Map the given Kerberos principal 'principal' to a short username (i.e. with no realm or +// host component). +// +// This respects the "auth-to-local" mappings from the system krb5.conf. However, if no such +// mapping can be found, we fall back to simply taking the first component of the principal. +// +// TODO(todd): move to kerberos_util.h in the later patch in this series (the file doesn't +// exist yet, and trying to avoid rebase pain). +Status MapPrincipalToLocalName(const std::string& principal, std::string* local_name); + } // namespace security } // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/security-test-util.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/security-test-util.cc b/src/kudu/security/security-test-util.cc index dfa9c4c..2043b5c 100644 --- a/src/kudu/security/security-test-util.cc +++ b/src/kudu/security/security-test-util.cc @@ -60,14 +60,14 @@ Status ConfigureTlsContext(PkiConfig config, switch (config) { case PkiConfig::NONE: break; case PkiConfig::SELF_SIGNED: - RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey("test-uuid")); + RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey()); break; case PkiConfig::TRUSTED: RETURN_NOT_OK(tls_context->AddTrustedCertificate(ca_cert)); break; case PkiConfig::SIGNED: { RETURN_NOT_OK(tls_context->AddTrustedCertificate(ca_cert)); - RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey("test-uuid")); + RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey()); Cert cert; RETURN_NOT_OK(CertSigner(&ca_cert, &ca_key).Sign(*tls_context->GetCsrIfNecessary(), &cert)); RETURN_NOT_OK(tls_context->AdoptSignedCert(cert)); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/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 8e90930..e780edc 100644 --- a/src/kudu/security/test/mini_kdc-test.cc +++ b/src/kudu/security/test/mini_kdc-test.cc @@ -17,6 +17,7 @@ #include <string> +#include <boost/optional.hpp> #include <gflags/gflags.h> #include <gtest/gtest.h> @@ -77,6 +78,25 @@ TEST_F(MiniKdcTest, TestBasicOperation) { FLAGS_keytab_file = kt_path; FLAGS_principal = kSPN; ASSERT_OK(security::InitKerberosForServer()); + ASSERT_EQ("kudu/[email protected]", *security::GetLoggedInPrincipalFromKeytab()); + + // Test principal canonicalization. + string princ = "foo"; + ASSERT_OK(security::CanonicalizeKrb5Principal(&princ)); + ASSERT_EQ("[email protected]", princ); + + // Test auth-to-local mapping for a user from the local realm as well as a remote realm. + { + string local_user; + ASSERT_OK(security::MapPrincipalToLocalName("[email protected]", &local_user)); + ASSERT_EQ("foo", local_user); + + ASSERT_OK(security::MapPrincipalToLocalName("foo/[email protected]", &local_user)); + ASSERT_EQ("foo", local_user); + + ASSERT_OK(security::MapPrincipalToLocalName("[email protected]", &local_user)); + ASSERT_EQ("other-foo", local_user); + } } // Regression test to ensure that dropping a stopped MiniKdc doesn't panic. http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/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 09a2088..abbb9d5 100644 --- a/src/kudu/security/test/mini_kdc.cc +++ b/src/kudu/security/test/mini_kdc.cc @@ -240,6 +240,11 @@ Status MiniKdc::CreateKrb5Conf() const { [realms] $1 = { kdc = 127.0.0.1:$0 + # This super-arcane syntax can be found documented in various Hadoop + # vendors' security guides and very briefly in the MIT krb5 docs. + # Basically, this one says to map anyone coming in as [email protected] + # and map them to a local user 'other-foo' + auth_to_local = RULE:[1:other-$$1@$$0](.*@OTHERREALM.COM$$)s/@.*// } )"; string file_contents = strings::Substitute(kFileTemplate, options_.port, options_.realm, http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/tls_context.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/tls_context.cc b/src/kudu/security/tls_context.cc index 0844a70..6106d9e 100644 --- a/src/kudu/security/tls_context.cc +++ b/src/kudu/security/tls_context.cc @@ -30,10 +30,13 @@ #include "kudu/security/ca/cert_management.h" #include "kudu/security/cert.h" #include "kudu/security/crypto.h" +#include "kudu/security/init.h" #include "kudu/security/openssl_util.h" #include "kudu/security/tls_handshake.h" #include "kudu/util/flag_tags.h" +#include "kudu/util/net/net_util.h" #include "kudu/util/status.h" +#include "kudu/util/user.h" using strings::Substitute; using std::string; @@ -233,7 +236,34 @@ Status TlsContext::AddTrustedCertificate(const Cert& cert) { return Status::OK(); } -Status TlsContext::GenerateSelfSignedCertAndKey(const std::string& server_uuid) { +namespace { +Status SetCertAttributes(CertRequestGenerator::Config* config) { + RETURN_NOT_OK_PREPEND(GetFQDN(&config->cn), "could not determine FQDN for CSR"); + + // If the server has logged in from a keytab, then we have a 'real' identity, + // and our desired CN should match the local username mapped from the Kerberos + // principal name. Otherwise, we'll make up a common name based on the hostname. + boost::optional<string> principal = GetLoggedInPrincipalFromKeytab(); + if (!principal) { + string uid; + RETURN_NOT_OK_PREPEND(GetLoggedInUser(&uid), + "couldn't get local username"); + config->user_id = uid; + return Status::OK(); + } + string uid; + RETURN_NOT_OK_PREPEND(security::MapPrincipalToLocalName(*principal, &uid), + "could not get local username for krb5 principal"); + config->user_id = uid; + config->kerberos_principal = *principal; + return Status::OK(); +} +} // anonymous namespace + +Status TlsContext::GenerateSelfSignedCertAndKey() { + CertRequestGenerator::Config config; + RETURN_NOT_OK(SetCertAttributes(&config)); + // Step 1: generate the private key to be self signed. PrivateKey key; RETURN_NOT_OK_PREPEND(GeneratePrivateKey(FLAGS_ipki_server_key_size, @@ -242,8 +272,6 @@ Status TlsContext::GenerateSelfSignedCertAndKey(const std::string& server_uuid) // Step 2: generate a CSR so that the self-signed cert can eventually be // replaced with a CA-signed cert. - const CertRequestGenerator::Config config = { server_uuid }; - CertRequestGenerator gen(config); CertSignRequest csr; RETURN_NOT_OK_PREPEND(gen.Init(), "could not initialize CSR generator"); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/tls_context.h ---------------------------------------------------------------------- diff --git a/src/kudu/security/tls_context.h b/src/kudu/security/tls_context.h index f26afec..f92199d 100644 --- a/src/kudu/security/tls_context.h +++ b/src/kudu/security/tls_context.h @@ -114,7 +114,7 @@ class TlsContext { // CA-signed cert for the generated private key, and 'AdoptSignedCert' can be // used to transition to using the CA-signed cert with subsequent TLS // connections. - Status GenerateSelfSignedCertAndKey(const std::string& server_uuid); + Status GenerateSelfSignedCertAndKey(); // Returns a new certificate signing request (CSR) in DER format, if this // context's cert is self-signed. If the cert is already signed, returns http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/security/tls_handshake-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/tls_handshake-test.cc b/src/kudu/security/tls_handshake-test.cc index 1c196af..8ffc99c 100644 --- a/src/kudu/security/tls_handshake-test.cc +++ b/src/kudu/security/tls_handshake-test.cc @@ -172,7 +172,7 @@ TEST_F(TestTlsHandshake, TestTlsContextCertTransition) { ASSERT_FALSE(server_tls_.has_signed_cert()); ASSERT_EQ(boost::none, server_tls_.GetCsrIfNecessary()); - ASSERT_OK(server_tls_.GenerateSelfSignedCertAndKey("test-uuid")); + ASSERT_OK(server_tls_.GenerateSelfSignedCertAndKey()); ASSERT_TRUE(server_tls_.has_cert()); ASSERT_FALSE(server_tls_.has_signed_cert()); ASSERT_NE(boost::none, server_tls_.GetCsrIfNecessary()); @@ -205,7 +205,7 @@ TEST_F(TestTlsHandshake, TestTlsContextCertTransition) { { TlsContext bogus_tls; ASSERT_OK(bogus_tls.Init()); - ASSERT_OK(bogus_tls.GenerateSelfSignedCertAndKey("test-uuid")); + ASSERT_OK(bogus_tls.GenerateSelfSignedCertAndKey()); ASSERT_OK(CertSigner(&ca_cert, &ca_key).Sign(*bogus_tls.GetCsrIfNecessary(), &bogus_cert)); } ASSERT_STR_MATCHES(server_tls_.AdoptSignedCert(bogus_cert).ToString(), http://git-wip-us.apache.org/repos/asf/kudu/blob/ed9ce952/src/kudu/server/server_base.cc ---------------------------------------------------------------------- diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc index e8f9578..1c981d1 100644 --- a/src/kudu/server/server_base.cc +++ b/src/kudu/server/server_base.cc @@ -195,7 +195,7 @@ Status ServerBase::Init() { .set_max_negotiation_threads(FLAGS_max_negotiation_threads) .set_metric_entity(metric_entity()) // TODO(PKI): make built-in PKI enabled/disabled based on a flag. - .enable_inbound_tls(fs_manager_->uuid()); + .enable_inbound_tls(); RETURN_NOT_OK(builder.Build(&messenger_)); RETURN_NOT_OK(rpc_server_->Init(messenger_));
