[Patches] s3:gse_krb5: make use of precalculated krb5 keys in fill_mem_keytab_from_secrets()
Stefan Metzmacher
metze at samba.org
Thu Aug 17 20:32:08 UTC 2017
Hi,
here're patches to avoid a lot of CPU processing
for each SMB connection. As we now already store
the precalculated kerberos keys in secrets.tdb
with https://git.samba.org/?p=samba.git;a=commitdiff;h=5f0038fba612afd7
That means we can avoid constantly recalculating the kerberos keys
from a the passwords.
Please review and push:-)
Thanks!
metze
-------------- next part --------------
From a3fa5c1c4d2955186052ff8c644e532df8a0d1dd Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 17 Aug 2017 21:42:34 +0200
Subject: [PATCH 1/2] s3:secrets: allow secrets_fetch_or_upgrade_domain_info()
on an AD DC
The reason for the check is for write access as secrets.ldb is the
master database.
But secrets_fetch_or_upgrade_domain_info() just syncs the values
we got from if they got overwritten by secrets_store_machine_pw_sync().
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12973
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/passdb/machine_account_secrets.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
index 3d1cb5b..5a0f7a8 100644
--- a/source3/passdb/machine_account_secrets.c
+++ b/source3/passdb/machine_account_secrets.c
@@ -832,7 +832,8 @@ static NTSTATUS secrets_store_domain_info1_by_key(const char *key,
return NT_STATUS_OK;
}
-static NTSTATUS secrets_store_domain_info(const struct secrets_domain_info1 *info)
+static NTSTATUS secrets_store_domain_info(const struct secrets_domain_info1 *info,
+ bool upgrade)
{
TALLOC_CTX *frame = talloc_stackframe();
const char *domain = info->domain_info.name.string;
@@ -853,7 +854,7 @@ static NTSTATUS secrets_store_domain_info(const struct secrets_domain_info1 *inf
switch (info->secure_channel_type) {
case SEC_CHAN_WKSTA:
case SEC_CHAN_BDC:
- if (role >= ROLE_ACTIVE_DIRECTORY_DC) {
+ if (!upgrade && role >= ROLE_ACTIVE_DIRECTORY_DC) {
DBG_ERR("AD_DC not supported for %s\n",
domain);
TALLOC_FREE(frame);
@@ -1490,7 +1491,7 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
secrets_debug_domain_info(DBGLVL_INFO, info, "upgrade");
- status = secrets_store_domain_info(info);
+ status = secrets_store_domain_info(info, true /* upgrade */);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("secrets_store_domain_info() failed "
"for %s - %s\n", domain, nt_errstr(status));
@@ -1647,7 +1648,7 @@ NTSTATUS secrets_store_JoinCtx(const struct libnet_JoinCtx *r)
secrets_debug_domain_info(DBGLVL_INFO, info, "join");
- status = secrets_store_domain_info(info);
+ status = secrets_store_domain_info(info, false /* upgrade */);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("secrets_store_domain_info() failed "
"for %s - %s\n", domain, nt_errstr(status));
@@ -1739,7 +1740,7 @@ NTSTATUS secrets_prepare_password_change(const char *domain, const char *dcname,
secrets_debug_domain_info(DBGLVL_INFO, info, "prepare_change");
- status = secrets_store_domain_info(info);
+ status = secrets_store_domain_info(info, false /* upgrade */);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("secrets_store_domain_info() failed "
"for %s - %s\n", domain, nt_errstr(status));
@@ -1963,7 +1964,7 @@ static NTSTATUS secrets_abort_password_change(const char *change_server,
secrets_debug_domain_info(DBGLVL_WARNING, info, reason);
- status = secrets_store_domain_info(info);
+ status = secrets_store_domain_info(info, false /* upgrade */);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("secrets_store_domain_info() failed "
"for %s - %s\n", domain, nt_errstr(status));
@@ -2057,7 +2058,7 @@ NTSTATUS secrets_finish_password_change(const char *change_server,
secrets_debug_domain_info(DBGLVL_WARNING, info, "finish_change");
- status = secrets_store_domain_info(info);
+ status = secrets_store_domain_info(info, false /* upgrade */);
if (!NT_STATUS_IS_OK(status)) {
DBG_ERR("secrets_store_domain_info() failed "
"for %s - %s\n", domain, nt_errstr(status));
--
1.9.1
From 248561cd8d2764a48380ba3ff8b62775b3e05332 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 17 Aug 2017 17:45:21 +0200
Subject: [PATCH 2/2] s3:gse_krb5: make use of precalculated krb5 keys in
fill_mem_keytab_from_secrets()
This avoids a lot of cpu cycles, which were wasted for each single smb
connection, even if the client didn't use kerberos.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12973
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/librpc/crypto/gse_krb5.c | 180 ++++++++++++++++++---------------------
1 file changed, 85 insertions(+), 95 deletions(-)
diff --git a/source3/librpc/crypto/gse_krb5.c b/source3/librpc/crypto/gse_krb5.c
index 2c9fc03..cc8cb90 100644
--- a/source3/librpc/crypto/gse_krb5.c
+++ b/source3/librpc/crypto/gse_krb5.c
@@ -20,6 +20,7 @@
#include "includes.h"
#include "smb_krb5.h"
#include "secrets.h"
+#include "librpc/gen_ndr/secrets.h"
#include "gse_krb5.h"
#include "lib/param/loadparm.h"
#include "libads/kerberos_proto.h"
@@ -85,45 +86,15 @@ out:
return ret;
}
-static krb5_error_code get_host_principal(krb5_context krbctx,
- krb5_principal *host_princ)
-{
- krb5_error_code ret;
- char *host_princ_s = NULL;
- int err;
-
- err = asprintf(&host_princ_s, "%s$@%s", lp_netbios_name(), lp_realm());
- if (err == -1) {
- return -1;
- }
-
- if (!strlower_m(host_princ_s)) {
- SAFE_FREE(host_princ_s);
- return -1;
- }
- ret = smb_krb5_parse_name(krbctx, host_princ_s, host_princ);
- if (ret) {
- DEBUG(1, (__location__ ": smb_krb5_parse_name(%s) "
- "failed (%s)\n",
- host_princ_s, error_message(ret)));
- }
-
- SAFE_FREE(host_princ_s);
- return ret;
-}
-
static krb5_error_code fill_keytab_from_password(krb5_context krbctx,
krb5_keytab keytab,
krb5_principal princ,
krb5_kvno vno,
- krb5_data *password)
+ struct secrets_domain_info1_password *pw)
{
krb5_error_code ret;
krb5_enctype *enctypes;
- krb5_keytab_entry kt_entry;
- unsigned int i;
- krb5_principal salt_princ = NULL;
- char *salt_princ_s = NULL;
+ uint16_t i;
ret = smb_krb5_get_allowed_etypes(krbctx, &enctypes);
if (ret) {
@@ -132,61 +103,47 @@ static krb5_error_code fill_keytab_from_password(krb5_context krbctx,
return ret;
}
- salt_princ_s = kerberos_secrets_fetch_salt_princ();
- if (salt_princ_s == NULL) {
- ret = ENOMEM;
- goto out;
- }
- ret = krb5_parse_name(krbctx, salt_princ_s, &salt_princ);
- SAFE_FREE(salt_princ_s);
- if (ret != 0) {
- goto out;
- }
-
- for (i = 0; enctypes[i]; i++) {
+ for (i = 0; i < pw->num_keys; i++) {
+ krb5_keytab_entry kt_entry;
krb5_keyblock *key = NULL;
- int rc;
+ unsigned int ei;
+ bool found_etype = false;
- if (!(key = SMB_MALLOC_P(krb5_keyblock))) {
- ret = ENOMEM;
- goto out;
+ for (ei=0; enctypes[ei] != 0; ei++) {
+ if ((uint32_t)enctypes[ei] != pw->keys[i].keytype) {
+ continue;
+ }
+
+ found_etype = true;
+ break;
}
- rc = create_kerberos_key_from_string(krbctx,
- princ,
- salt_princ,
- password,
- key,
- enctypes[i],
- false);
- if (rc != 0) {
- DEBUG(10, ("Failed to create key for enctype %d "
- "(error: %s)\n",
- enctypes[i], error_message(ret)));
- SAFE_FREE(key);
+ if (!found_etype) {
continue;
}
+ ZERO_STRUCT(kt_entry);
kt_entry.principal = princ;
kt_entry.vno = vno;
- *(KRB5_KT_KEY(&kt_entry)) = *key;
+
+ key = KRB5_KT_KEY(&kt_entry);
+ KRB5_KEY_TYPE(key) = pw->keys[i].keytype;
+ KRB5_KEY_DATA(key) = pw->keys[i].value.data;
+ KRB5_KEY_LENGTH(key) = pw->keys[i].value.length;
ret = krb5_kt_add_entry(krbctx, keytab, &kt_entry);
if (ret) {
DEBUG(1, (__location__ ": Failed to add entry to "
"keytab for enctype %d (error: %s)\n",
- enctypes[i], error_message(ret)));
- krb5_free_keyblock(krbctx, key);
+ (unsigned)pw->keys[i].keytype,
+ error_message(ret)));
goto out;
}
-
- krb5_free_keyblock(krbctx, key);
}
ret = 0;
out:
- krb5_free_principal(krbctx, salt_princ);
SAFE_FREE(enctypes);
return ret;
}
@@ -197,27 +154,43 @@ out:
static krb5_error_code fill_mem_keytab_from_secrets(krb5_context krbctx,
krb5_keytab *keytab)
{
+ TALLOC_CTX *frame = talloc_stackframe();
krb5_error_code ret;
- char *pwd = NULL;
- size_t pwd_len;
+ const char *domain = lp_workgroup();
+ struct secrets_domain_info1 *info = NULL;
+ const char *realm = NULL;
+ const DATA_BLOB *ct = NULL;
krb5_kt_cursor kt_cursor;
krb5_keytab_entry kt_entry;
- krb5_data password;
krb5_principal princ = NULL;
krb5_kvno kvno = 0; /* FIXME: fetch current vno from KDC ? */
- char *pwd_old = NULL;
+ NTSTATUS status;
if (!secrets_init()) {
DEBUG(1, (__location__ ": secrets_init failed\n"));
+ TALLOC_FREE(frame);
return KRB5_CONFIG_CANTOPEN;
}
- pwd = secrets_fetch_machine_password(lp_workgroup(), NULL, NULL);
- if (!pwd) {
- DEBUG(2, (__location__ ": failed to fetch machine password\n"));
+ status = secrets_fetch_or_upgrade_domain_info(domain,
+ frame,
+ &info);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_WARNING("secrets_fetch_or_upgrade_domain_info(%s) - %s\n",
+ domain, nt_errstr(status));
+ TALLOC_FREE(frame);
return KRB5_LIBOS_CANTREADPWD;
}
- pwd_len = strlen(pwd);
+ ct = &info->password->cleartext_blob;
+
+ if (info->domain_info.dns_domain.string != NULL) {
+ realm = strupper_talloc(frame,
+ info->domain_info.dns_domain.string);
+ if (realm == NULL) {
+ TALLOC_FREE(frame);
+ return ENOMEM;
+ }
+ }
ZERO_STRUCT(kt_entry);
ZERO_STRUCT(kt_cursor);
@@ -249,9 +222,9 @@ static krb5_error_code fill_mem_keytab_from_secrets(krb5_context krbctx,
/* found private entry,
* check if keytab is up to date */
- if ((pwd_len == KRB5_KEY_LENGTH(KRB5_KT_KEY(&kt_entry))) &&
+ if ((ct->length == KRB5_KEY_LENGTH(KRB5_KT_KEY(&kt_entry))) &&
(memcmp(KRB5_KEY_DATA(KRB5_KT_KEY(&kt_entry)),
- pwd, pwd_len) == 0)) {
+ ct->data, ct->length) == 0)) {
/* keytab is already up to date, return */
smb_krb5_kt_free_entry(krbctx, &kt_entry);
goto out;
@@ -277,32 +250,51 @@ static krb5_error_code fill_mem_keytab_from_secrets(krb5_context krbctx,
/* keytab is not up to date, fill it up */
- ret = get_host_principal(krbctx, &princ);
+ ret = smb_krb5_make_principal(krbctx, &princ, realm,
+ info->account_name, NULL);
if (ret) {
DEBUG(1, (__location__ ": Failed to get host principal!\n"));
goto out;
}
- password.data = pwd;
- password.length = pwd_len;
ret = fill_keytab_from_password(krbctx, *keytab,
- princ, kvno, &password);
+ princ, kvno,
+ info->password);
if (ret) {
- DEBUG(1, (__location__ ": Failed to fill memory keytab!\n"));
+ DBG_WARNING("fill_keytab_from_password() failed for "
+ "info->password.\n.");
goto out;
}
- pwd_old = secrets_fetch_prev_machine_password(lp_workgroup());
- if (!pwd_old) {
- DEBUG(10, (__location__ ": no prev machine password\n"));
- } else {
- password.data = pwd_old;
- password.length = strlen(pwd_old);
+ if (info->old_password != NULL) {
+ ret = fill_keytab_from_password(krbctx, *keytab,
+ princ, kvno - 1,
+ info->old_password);
+ if (ret) {
+ DBG_WARNING("fill_keytab_from_password() failed for "
+ "info->old_password.\n.");
+ goto out;
+ }
+ }
+
+ if (info->older_password != NULL) {
ret = fill_keytab_from_password(krbctx, *keytab,
- princ, kvno -1, &password);
+ princ, kvno - 2,
+ info->older_password);
if (ret) {
- DEBUG(1, (__location__
- ": Failed to fill memory keytab!\n"));
+ DBG_WARNING("fill_keytab_from_password() failed for "
+ "info->older_password.\n.");
+ goto out;
+ }
+ }
+
+ if (info->next_change != NULL) {
+ ret = fill_keytab_from_password(krbctx, *keytab,
+ princ, kvno - 3,
+ info->next_change->password);
+ if (ret) {
+ DBG_WARNING("fill_keytab_from_password() failed for "
+ "info->next_change->password.\n.");
goto out;
}
}
@@ -314,8 +306,8 @@ static krb5_error_code fill_mem_keytab_from_secrets(krb5_context krbctx,
kt_entry.vno = 0;
KRB5_KEY_TYPE(KRB5_KT_KEY(&kt_entry)) = CLEARTEXT_PRIV_ENCTYPE;
- KRB5_KEY_LENGTH(KRB5_KT_KEY(&kt_entry)) = pwd_len;
- KRB5_KEY_DATA(KRB5_KT_KEY(&kt_entry)) = (uint8_t *)pwd;
+ KRB5_KEY_LENGTH(KRB5_KT_KEY(&kt_entry)) = ct->length;
+ KRB5_KEY_DATA(KRB5_KT_KEY(&kt_entry)) = ct->data;
ret = krb5_kt_add_entry(krbctx, *keytab, &kt_entry);
if (ret) {
@@ -328,9 +320,6 @@ static krb5_error_code fill_mem_keytab_from_secrets(krb5_context krbctx,
ret = 0;
out:
- SAFE_FREE(pwd);
- SAFE_FREE(pwd_old);
-
if (!all_zero((uint8_t *)&kt_cursor, sizeof(kt_cursor)) && *keytab) {
krb5_kt_end_seq_get(krbctx, *keytab, &kt_cursor);
}
@@ -339,6 +328,7 @@ out:
krb5_free_principal(krbctx, princ);
}
+ TALLOC_FREE(frame);
return ret;
}
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170817/85d2f8b6/signature.sig>
More information about the samba-technical
mailing list