This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 50f872986 [security] a small style-related code cleanup
50f872986 is described below
commit 50f8729860e6e8e3081ea3de5fcc5c016d4b2215
Author: Alexey Serbin <[email protected]>
AuthorDate: Fri Dec 1 19:35:01 2023 -0800
[security] a small style-related code cleanup
Since I'm working on adding new functionality into MiniKdc, I separated
this code-style only part of recent updates into a separate patch.
This patch doesn't contain any functional modifications.
Change-Id: Icfd68a9f7a34235095b4d0f94f57b698a7a71c1d
Reviewed-on: http://gerrit.cloudera.org:8080/20745
Reviewed-by: Mahesh Reddy <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/security/init.cc | 64 +++++++++++++++++++++-----------------
src/kudu/security/test/mini_kdc.cc | 48 +++++++++++++---------------
src/kudu/security/test/mini_kdc.h | 5 ++-
3 files changed, 58 insertions(+), 59 deletions(-)
diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc
index 33b0b8e95..dfadb4e85 100644
--- a/src/kudu/security/init.cc
+++ b/src/kudu/security/init.cc
@@ -103,15 +103,17 @@ KinitContext* g_kinit_ctx = nullptr;
namespace {
// Global context for usage of the Krb5 library.
-krb5_context g_krb5_ctx;
+krb5_context g_krb5_ctx = nullptr;
// This lock is used to avoid a race while reacquiring the kerberos ticket.
// The race can occur between the time we reinitialize the cache and the
// time when we actually store the new credentials back in the cache.
-RWMutex* g_kerberos_reinit_lock;
+RWMutex* g_kerberos_reinit_lock = nullptr;
Status Krb5CallToStatus(krb5_context ctx, krb5_error_code code) {
- if (code == 0) return Status::OK();
+ if (code == 0) {
+ return Status::OK();
+ }
std::unique_ptr<const char, std::function<void(const char*)>> err_msg(
krb5_get_error_message(ctx, code),
@@ -145,12 +147,12 @@ void InitKrb5Ctx() {
// Port of the data_eq() implementation from krb5/k5-int.h
inline int data_eq(krb5_data d1, krb5_data d2) {
- return (d1.length == d2.length && !memcmp(d1.data, d2.data, d1.length));
+ return (d1.length == d2.length && !memcmp(d1.data, d2.data, d1.length));
}
// Port of the data_eq_string() implementation from krb5/k5-int.h
-inline int data_eq_string(krb5_data d, const char *s) {
- return (d.length == strlen(s) && !memcmp(d.data, s, d.length));
+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) {
@@ -312,12 +314,13 @@ Status KinitContext::Kinit(const string& keytab_path,
const string& principal) {
KRB5_RETURN_NOT_OK_PREPEND(krb5_get_init_creds_opt_alloc(g_krb5_ctx, &opts_),
"unable to allocate get_init_creds_opt struct");
- RETURN_NOT_OK(KinitInternal());
+ RETURN_NOT_OK_PREPEND(KinitInternal(), "kinit failed");
// Start the thread to renew and reacquire Kerberos tickets.
- RETURN_NOT_OK(Thread::Create("kerberos", "reacquire thread",
- [this]() { this->RenewThread(); },
&reacquire_thread_));
- return Status::OK();
+ return Thread::Create("kerberos",
+ "reacquire thread",
+ [this]() { this->RenewThread(); },
+ &reacquire_thread_);
}
void KinitContext::Kdestroy() {
@@ -329,17 +332,22 @@ void KinitContext::Kdestroy() {
Status KinitContext::KinitInternal() {
#if defined(HAVE_KRB5_GET_INIT_CREDS_OPT_SET_OUT_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");
+ 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(g_krb5_ctx, &creds,
principal_, keytab_,
- 0 /* valid from now */,
- nullptr /* TKT service
name */, opts_),
- "unable to login from keytab");
- SCOPED_CLEANUP({
- krb5_free_cred_contents(g_krb5_ctx, &creds); });
+ 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");
+ SCOPED_CLEANUP({ krb5_free_cred_contents(g_krb5_ctx, &creds); });
ticket_end_timestamp_ = creds.times.endtime;
@@ -361,9 +369,8 @@ Status KinitContext::KinitInternal() {
RETURN_NOT_OK_PREPEND(MapPrincipalToLocalName(principal_str_,
&username_str_),
"could not map own logged-in principal to a short
username");
- LOG(INFO) << "Logged in from keytab as " << principal_str_
- << " (short username " << username_str_ << ")";
-
+ LOG(INFO) << Substitute("Logged in from keytab as $0 (short username $1)",
+ principal_str_, username_str_);
return Status::OK();
}
@@ -516,17 +523,16 @@ Status InitKerberosForServer(const std::string&
raw_principal,
PCHECK(setenv("KRB5RCACHETYPE", "none", 1) == 0);
}
- g_kinit_ctx = new KinitContext();
- string configured_principal;
- RETURN_NOT_OK(GetConfiguredPrincipal(raw_principal, &configured_principal));
- RETURN_NOT_OK_PREPEND(g_kinit_ctx->Kinit(
- keytab_file, configured_principal), "unable to kinit");
-
- return Status::OK();
+ g_kinit_ctx = new KinitContext;
+ string principal;
+ RETURN_NOT_OK(GetConfiguredPrincipal(raw_principal, &principal));
+ return g_kinit_ctx->Kinit(keytab_file, principal);
}
void DestroyKerberosForServer() {
- if (g_kinit_ctx == nullptr) return;
+ if (g_kinit_ctx == nullptr) {
+ return;
+ }
delete g_kinit_ctx;
g_kinit_ctx = nullptr;
diff --git a/src/kudu/security/test/mini_kdc.cc
b/src/kudu/security/test/mini_kdc.cc
index c7bcee333..a9cbe5ed4 100644
--- a/src/kudu/security/test/mini_kdc.cc
+++ b/src/kudu/security/test/mini_kdc.cc
@@ -144,8 +144,7 @@ Status MiniKdc::Start() {
string krb5kdc_bin;
RETURN_NOT_OK(GetBinaryPath("krb5kdc", &krb5kdc_bin));
- kdc_process_.reset(new Subprocess(
- MakeArgv({
+ kdc_process_.reset(new Subprocess(MakeArgv({
krb5kdc_bin,
"-n", // Do not daemonize.
})));
@@ -175,9 +174,7 @@ Status MiniKdc::Stop() {
VLOG(1) << "Stopping KDC";
unique_ptr<Subprocess> proc(kdc_process_.release());
RETURN_NOT_OK(proc->Kill(SIGKILL));
- RETURN_NOT_OK(proc->Wait());
-
- return Status::OK();
+ return proc->Wait();
}
// Creates a kdc.conf file according to the provided options.
@@ -255,9 +252,8 @@ Status MiniKdc::CreateUserPrincipal(const string& username)
{
SCOPED_LOG_SLOW_EXECUTION(WARNING, 100, Substitute("creating user principal
$0", username));
string kadmin;
RETURN_NOT_OK(GetBinaryPath("kadmin.local", &kadmin));
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({
- kadmin, "-q", Substitute("add_principal -pw $0 $0", username)})));
- return Status::OK();
+ return Subprocess::Call(MakeArgv(
+ { kadmin, "-q", Substitute("add_principal -pw $0 $0", username) }));
}
Status MiniKdc::CreateServiceKeytab(const string& spn,
@@ -269,10 +265,10 @@ Status MiniKdc::CreateServiceKeytab(const string& spn,
string kadmin;
RETURN_NOT_OK(GetBinaryPath("kadmin.local", &kadmin));
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({
- kadmin, "-q", Substitute("add_principal -randkey $0", spn)})));
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({
- kadmin, "-q", Substitute("ktadd -k $0 $1", kt_path, spn)})));
+ RETURN_NOT_OK(Subprocess::Call(MakeArgv(
+ { kadmin, "-q", Substitute("add_principal -randkey $0", spn) })));
+ RETURN_NOT_OK(Subprocess::Call(MakeArgv(
+ { kadmin, "-q", Substitute("ktadd -k $0 $1", kt_path, spn) })));
*path = kt_path;
return Status::OK();
}
@@ -281,9 +277,8 @@ Status MiniKdc::RandomizePrincipalKey(const string& spn) {
SCOPED_LOG_SLOW_EXECUTION(WARNING, 100, Substitute("randomizing key for $0",
spn));
string kadmin;
RETURN_NOT_OK(GetBinaryPath("kadmin.local", &kadmin));
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({
- kadmin, "-q", Substitute("change_password -randkey $0", spn)})));
- return Status::OK();
+ return Subprocess::Call(MakeArgv(
+ { kadmin, "-q", Substitute("change_password -randkey $0", spn) }));
}
Status MiniKdc::CreateKeytabForExistingPrincipal(const string& spn) {
@@ -291,9 +286,8 @@ Status MiniKdc::CreateKeytabForExistingPrincipal(const
string& spn) {
string kt_path = GetKeytabPathForPrincipal(spn);
string kadmin;
RETURN_NOT_OK(GetBinaryPath("kadmin.local", &kadmin));
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({
- kadmin, "-q", Substitute("xst -norandkey -k $0 $1", kt_path,
spn)})));
- return Status::OK();
+ return Subprocess::Call(MakeArgv(
+ { kadmin, "-q", Substitute("xst -norandkey -k $0 $1", kt_path, spn) }));
}
string MiniKdc::GetKeytabPathForPrincipal(const string& spn) const {
@@ -314,15 +308,17 @@ Status MiniKdc::Kinit(const string& username) {
WritableFileOptions opts;
opts.is_sensitive = false;
RETURN_NOT_OK_PREPEND(Env::Default()->NewTempWritableFile(
- opts,
- JoinPathSegments(options_.data_root, tmp_template),
- &tmp_cc_path, &tmp_cc_file),
- "could not create temporary file");
+ opts,
+ JoinPathSegments(options_.data_root, tmp_template),
+ &tmp_cc_path,
+ &tmp_cc_file),
+ "could not create temporary file");
auto delete_tmp_cc = MakeScopedCleanup([&]() {
WARN_NOT_OK(Env::Default()->DeleteFile(tmp_cc_path),
"could not delete file " + tmp_cc_path);
});
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({ kinit, "-c", tmp_cc_path, username
}), username));
+ RETURN_NOT_OK(Subprocess::Call(MakeArgv(
+ { kinit, "-c", tmp_cc_path, username }), username));
const auto env_vars_map = GetEnvVars();
const auto& ccache_path = FindOrDie(env_vars_map, "KRB5CCNAME");
RETURN_NOT_OK_PREPEND(Env::Default()->RenameFile(tmp_cc_path, ccache_path),
@@ -341,15 +337,13 @@ Status MiniKdc::Kdestroy() {
Status MiniKdc::Klist(string* output) {
string klist;
RETURN_NOT_OK(GetBinaryPath("klist", &klist));
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({ klist, "-A" }), "", output));
- return Status::OK();
+ return Subprocess::Call(MakeArgv({ klist, "-A" }), "", output);
}
Status MiniKdc::KlistKeytab(const string& keytab_path, string* output) {
string klist;
RETURN_NOT_OK(GetBinaryPath("klist", &klist));
- RETURN_NOT_OK(Subprocess::Call(MakeArgv({ klist, "-k", keytab_path }), "",
output));
- return Status::OK();
+ return Subprocess::Call(MakeArgv({ klist, "-k", keytab_path }), "", output);
}
Status MiniKdc::SetKrb5Environment() const {
diff --git a/src/kudu/security/test/mini_kdc.h
b/src/kudu/security/test/mini_kdc.h
index 10dc66986..ecbabe082 100644
--- a/src/kudu/security/test/mini_kdc.h
+++ b/src/kudu/security/test/mini_kdc.h
@@ -128,14 +128,13 @@ class MiniKdc {
std::map<std::string, std::string> GetEnvVars() const;
private:
-
// Prepends required Kerberos environment variables to the process arguments.
std::vector<std::string> MakeArgv(const std::vector<std::string>& in_argv);
- // Creates a kdc.conf in the data root.
+ // Creates a krb5.conf in the data root.
Status CreateKrb5Conf() const WARN_UNUSED_RESULT;
- // Creates a krb5.conf in the data root.
+ // Creates a kdc.conf in the data root.
Status CreateKdcConf() const WARN_UNUSED_RESULT;
std::unique_ptr<Subprocess> kdc_process_;
