]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
s4-auth/kerberos: Do not add true duplicates to exported keytab
authorAndrew Bartlett <abartlet@samba.org>
Wed, 6 Mar 2024 04:43:47 +0000 (17:43 +1300)
committerAndrew Bartlett <abartlet@samba.org>
Thu, 14 Mar 2024 22:06:39 +0000 (22:06 +0000)
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jo Sutton <josutton@catalyst.net.nz>
source4/auth/kerberos/kerberos_util.c
source4/auth/kerberos/srv_keytab.c
source4/libnet/libnet_export_keytab.c

index ba8d822f638a480f0f5387df03f906782041cd2f..30d9400bffbc04ce5121a7a42749c56ed2765248 100644 (file)
@@ -719,3 +719,106 @@ done:
        talloc_free(tmp_ctx);
        return code;
 }
+
+/*
+ * Walk the keytab, looking for entries of this principal name,
+ * with KVNO and key equal
+ *
+ * These entries do not need to be replaced, so we want to tell the caller not to add them again
+ *
+ * Inspired by the code in Samba3 for 'use kerberos keytab'.
+ */
+krb5_error_code smb_krb5_is_exact_entry_in_keytab(TALLOC_CTX *mem_ctx,
+                                                 krb5_context context,
+                                                 krb5_keytab keytab,
+                                                 krb5_keytab_entry *to_match,
+                                                 bool *found,
+                                                 const char **error_string)
+{
+       TALLOC_CTX *tmp_ctx;
+       krb5_error_code code;
+       krb5_kt_cursor cursor;
+
+       tmp_ctx = talloc_new(mem_ctx);
+       if (tmp_ctx == NULL) {
+               *error_string = "Cannot allocate tmp_ctx";
+               return ENOMEM;
+       }
+
+       *found = false;
+
+       code = krb5_kt_start_seq_get(context, keytab, &cursor);
+       switch (code) {
+       case 0:
+               break;
+#ifdef HEIM_ERR_OPNOTSUPP
+       case HEIM_ERR_OPNOTSUPP:
+#endif
+       case ENOENT:
+       case KRB5_KT_END:
+               /* no point enumerating if there isn't anything here */
+               code = 0;
+               goto done;
+       default:
+               *error_string = talloc_asprintf(mem_ctx,
+                                               "failed to open keytab for read of existing entries: %s\n",
+                                               smb_get_krb5_error_message(context, code, mem_ctx));
+               goto done;
+       }
+
+       do {
+               krb5_keytab_entry entry;
+               bool matched = false;
+               krb5_boolean ok;
+
+               code = krb5_kt_next_entry(context, keytab, &entry, &cursor);
+               if (code) {
+                       break;
+               }
+
+               ok = smb_krb5_kt_compare(context,
+                                        &entry,
+                                        to_match->principal,
+                                        to_match->vno,
+                                        KRB5_KEY_TYPE(KRB5_KT_KEY(to_match)));
+               if (ok) {
+                       /* This is not a security check, constant time is not required */
+                       if ((KRB5_KEY_LENGTH(KRB5_KT_KEY(&entry)) == KRB5_KEY_LENGTH(KRB5_KT_KEY(to_match)))
+                           && memcmp(KRB5_KEY_DATA(KRB5_KT_KEY(&entry)), KRB5_KEY_DATA(KRB5_KT_KEY(to_match)),
+                                     KRB5_KEY_LENGTH(KRB5_KT_KEY(&entry))) == 0) {
+                               matched = true;
+                       }
+               }
+
+               /* Free the entry, we don't need it any more */
+               krb5_kt_free_entry(context, &entry);
+               /* Make sure we do not double free */
+               ZERO_STRUCT(entry);
+               if (matched) {
+                       *found = true;
+                       break;
+               }
+       } while (code == 0);
+
+       krb5_kt_end_seq_get(context, keytab, &cursor);
+
+       switch (code) {
+       case 0:
+               break;
+       case ENOENT:
+       case KRB5_KT_END:
+               break;
+       default:
+               *error_string = talloc_asprintf(mem_ctx,
+                                               "failed in checking old entries for principal: %s\n",
+                                               smb_get_krb5_error_message(context,
+                                                                          code,
+                                                                          mem_ctx));
+               goto done;
+       }
+
+       code = 0;
+done:
+       talloc_free(tmp_ctx);
+       return code;
+}
index 899a677bc619aefe58d17b06a3a6c76e229ec8c6..fdc2fb4012f3f6e4e0610d31b163ba26d82f446e 100644 (file)
@@ -84,11 +84,36 @@ static krb5_error_code keytab_add_keys(TALLOC_CTX *parent_ctx,
                        return ret;
                }
 
-                entry.vno = kvno;
+               entry.vno = kvno;
 
                for (p = 0; p < num_principals; p++) {
+                       bool found = false;
+
                        unparsed = NULL;
                        entry.principal = principals[p];
+
+                       ret = smb_krb5_is_exact_entry_in_keytab(parent_ctx,
+                                                               context,
+                                                               keytab,
+                                                               &entry,
+                                                               &found,
+                                                               error_string);
+                       if (ret != 0) {
+                               krb5_free_keyblock_contents(context,
+                                                           KRB5_KT_KEY(&entry));
+                               return ret;
+                       }
+
+                       /*
+                        * Do not add the exact same key twice, this
+                        * will allow "samba-tool domain exportkeytab"
+                        * to refresh a keytab rather than infinitely
+                        * extend it
+                        */
+                       if (found) {
+                               continue;
+                       }
+
                        ret = krb5_kt_add_entry(context, keytab, &entry);
                        if (ret != 0) {
                                char *k5_error_string =
@@ -206,7 +231,6 @@ NTSTATUS smb_krb5_fill_keytab_gmsa_keys(TALLOC_CTX *mem_ctx,
                                        krb5_principal principal,
                                        struct ldb_context *samdb,
                                        struct ldb_dn *dn,
-                                       bool include_previous,
                                        const char **error_string)
 {
        const char *gmsa_attrs[] = {
@@ -378,7 +402,7 @@ NTSTATUS smb_krb5_fill_keytab_gmsa_keys(TALLOC_CTX *mem_ctx,
                                   &principal,
                                   context,
                                   keytab,
-                                  include_previous,
+                                  true,
                                   error_string);
        if (ret) {
                *error_string = talloc_asprintf(mem_ctx,
index 6374f2b0e5625451e094c7de0fb188b6c3488247..2d3b0ba7c3a20106a6e02d11a903eb5d4d364e4b 100644 (file)
@@ -148,7 +148,6 @@ static NTSTATUS sdb_kt_copy(TALLOC_CTX *mem_ctx,
                                                                sentry.principal,
                                                                db_ctx->samdb,
                                                                dn,
-                                                               found_previous ? false : true,
                                                                error_string);
                        if (NT_STATUS_IS_OK(status)) {
                                keys_exported = true;