Attachment 14167 Details for Bug 13415 – proposed patch

[patch] proposed patch

keytab_invalidation.patch (text/plain), 8.04 KB, created by Aaron Haslett (dead mail address) on 2018-05-01 01:04:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Aaron Haslett (dead mail address)

Created: 2018-05-01 01:04:06 UTC

Size: 8.04 KB

patch

obsolete

>From bfc821a147ffef63c893d67d7ff30fc4e42cf985 Mon Sep 17 00:00:00 2001
>From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Date: Tue, 1 May 2018 11:10:24 +1200
>Subject: [PATCH 1/2] auth: keytab invalidation test
>
>chgtdcpass should add a new DC password and delete the old ones but the bug
>exposed by this test causes the tool to remove only a single record from
>the old entries, leaving the old passwords functional.  Since the tool is
>used by administrators who may have disclosed their domain join password and
>want to invalidate it, this is a security concern.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13415
>
>Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>---
> selftest/knownfail.d/keytab   |   1 +
> selftest/tests.py             |   2 +
> source4/auth/tests/kerberos.c | 104 ++++++++++++++++++++++++++++++++++++++++++
> source4/auth/wscript_build    |   6 +++
> 4 files changed, 113 insertions(+)
> create mode 100644 selftest/knownfail.d/keytab
> create mode 100644 source4/auth/tests/kerberos.c
>
>diff --git a/selftest/knownfail.d/keytab b/selftest/knownfail.d/keytab
>new file mode 100644
>index 0000000..6777d98
>--- /dev/null
>+++ b/selftest/knownfail.d/keytab
>@@ -0,0 +1 @@
>+^samba.unittests.kerberos.test_krb5_remove_obsolete_keytab_entries_many
>diff --git a/selftest/tests.py b/selftest/tests.py
>index 185ad37..f354bb5 100644
>--- a/selftest/tests.py
>+++ b/selftest/tests.py
>@@ -187,5 +187,7 @@ plantestsuite("samba.unittests.tldap", "none",
>               [os.path.join(bindir(), "default/source3/test_tldap")])
> plantestsuite("samba.unittests.rfc1738", "none",
>               [os.path.join(bindir(), "default/lib/util/test_rfc1738")])
>+plantestsuite("samba.unittests.kerberos", "none",
>+              [os.path.join(bindir(), "test_kerberos")])
> plantestsuite("samba.unittests.ms_fnmatch", "none",
>               [os.path.join(bindir(), "default/lib/util/test_ms_fnmatch")])
>diff --git a/source4/auth/tests/kerberos.c b/source4/auth/tests/kerberos.c
>new file mode 100644
>index 0000000..7a4441f
>--- /dev/null
>+++ b/source4/auth/tests/kerberos.c
>@@ -0,0 +1,104 @@
>+#include <time.h>
>+#include <stdlib.h>
>+#include <stdarg.h>
>+#include <stddef.h>
>+#include <setjmp.h>
>+#include <stdint.h>
>+#include <cmocka.h>
>+
>+#include "includes.h"
>+#include "system/kerberos.h"
>+#include "auth/kerberos/kerberos.h"
>+#include "auth/credentials/credentials.h"
>+#include "auth/credentials/credentials_proto.h"
>+#include "auth/credentials/credentials_krb5.h"
>+#include "auth/kerberos/kerberos_credentials.h"
>+#include "auth/kerberos/kerberos_util.h"
>+
>+static void internal_obsolete_keytab_test(int num_principals, int num_kvnos,
>+                                          krb5_kvno kvno, const char *kt_name)
>+{
>+	krb5_context krb5_ctx;
>+	krb5_keytab keytab;
>+	krb5_keytab_entry kt_entry;
>+	krb5_kt_cursor cursor;
>+	krb5_error_code code;
>+
>+        int i,j;
>+        char princ_name[6] = "user0";
>+        bool found_previous;
>+        const char *error_str;
>+
>+	TALLOC_CTX *tmp_ctx = talloc_new(NULL);
>+	krb5_principal *principals = talloc_zero_array(tmp_ctx,
>+                                                       krb5_principal,
>+                                                       num_principals);
>+        krb5_init_context(&krb5_ctx);
>+        krb5_kt_resolve(krb5_ctx, kt_name, &keytab);
>+        ZERO_STRUCT(kt_entry);
>+
>+        for(i=0; i<num_principals; i++){
>+                princ_name[4] = (char)i+48;
>+                krb5_make_principal(krb5_ctx, &(principals[i]),
>+                                    "samba.example.com", princ_name, NULL);
>+                kt_entry.principal = principals[i];
>+                for(j=0; j<num_kvnos; j++){
>+                        kt_entry.vno = j+1;
>+                        krb5_kt_add_entry(krb5_ctx, keytab, &kt_entry);
>+                }
>+        }
>+
>+	code = krb5_kt_start_seq_get(krb5_ctx, keytab, &cursor);
>+        assert_int_equal(code, 0);
>+        for(i=0; i<num_principals; i++){
>+                princ_name[4] = (char)i+48;
>+                for(j=0; j<num_kvnos; j++){
>+                        code = krb5_kt_next_entry(krb5_ctx, keytab,
>+                                                  &kt_entry, &cursor);
>+                        assert_int_equal(code, 0);
>+                        assert_string_equal(princ_name,
>+                                   *(kt_entry.principal->name.name_string.val));
>+                        assert_int_equal(kt_entry.vno, j+1);
>+                }
>+        }
>+
>+        smb_krb5_remove_obsolete_keytab_entries(tmp_ctx, krb5_ctx, keytab,
>+                                                num_principals, principals,
>+                                                kvno, &found_previous,
>+                                                &error_str);
>+
>+	code = krb5_kt_start_seq_get(krb5_ctx, keytab, &cursor);
>+        assert_int_equal(code, 0);
>+        for(i=0; i<num_principals; i++){
>+                princ_name[4] = (char)i+48;
>+                code = krb5_kt_next_entry(krb5_ctx, keytab, &kt_entry, &cursor);
>+                assert_int_equal(code, 0);
>+                assert_string_equal(princ_name,
>+                        *(kt_entry.principal->name.name_string.val));
>+                assert_int_equal(kt_entry.vno, kvno-1);
>+        }
>+        code = krb5_kt_next_entry(krb5_ctx, keytab, &kt_entry, &cursor);
>+        assert_int_not_equal(code, 0);
>+}
>+
>+static void test_krb5_remove_obsolete_keytab_entries_many(void **state)
>+{
>+        internal_obsolete_keytab_test(5, 4, (krb5_kvno)5, "MEMORY:LOL2");
>+}
>+
>+static void test_krb5_remove_obsolete_keytab_entries_one(void **state)
>+{
>+        internal_obsolete_keytab_test(1, 2, (krb5_kvno)3, "MEMORY:LOL");
>+}
>+
>+int main(int argc, const char **argv)
>+{
>+	const struct CMUnitTest tests[] = {
>+		cmocka_unit_test(test_krb5_remove_obsolete_keytab_entries_one),
>+		cmocka_unit_test(test_krb5_remove_obsolete_keytab_entries_many),
>+	};
>+
>+        samba_start_debugger();
>+	cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
>+	return cmocka_run_group_tests(tests, NULL, NULL);
>+}
>diff --git a/source4/auth/wscript_build b/source4/auth/wscript_build
>index f750861..d3452d2 100644
>--- a/source4/auth/wscript_build
>+++ b/source4/auth/wscript_build
>@@ -42,6 +42,12 @@ bld.SAMBA_SUBSYSTEM('auth4_sam',
> 	deps=''
> 	)
> 
>+bld.SAMBA_BINARY('test_kerberos',
>+        source='tests/kerberos.c',
>+        deps='cmocka authkrb5 krb5samba com_err CREDENTIALS_KRB5',
>+        local_include=False,
>+        install=False
>+        )
> 
> for env in bld.gen_python_environments():
> 	pytalloc_util = bld.pyembed_libname('pytalloc-util')
>-- 
>2.7.4
>
>
>From 4f8e4fae87aa2b2586cf12a97287799f83a04842 Mon Sep 17 00:00:00 2001
>From: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>Date: Tue, 1 May 2018 11:10:50 +1200
>Subject: [PATCH 2/2] auth: keytab invalidation fix
>
>chgtdcpass should add a new DC password and delete the old ones but the bug
>exposed by this test causes the tool to remove only a single record from
>the old entries, leaving the old passwords functional.  Since the tool is
>used by administrators who may have disclosed their domain join password and
>want to invalidate it, this is a security concern.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13415
>
>Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
>---
> selftest/knownfail.d/keytab           | 1 -
> source4/auth/kerberos/kerberos_util.c | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)
> delete mode 100644 selftest/knownfail.d/keytab
>
>diff --git a/selftest/knownfail.d/keytab b/selftest/knownfail.d/keytab
>deleted file mode 100644
>index 6777d98..0000000
>--- a/selftest/knownfail.d/keytab
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba.unittests.kerberos.test_krb5_remove_obsolete_keytab_entries_many
>diff --git a/source4/auth/kerberos/kerberos_util.c b/source4/auth/kerberos/kerberos_util.c
>index 618da62..50bf8fe 100644
>--- a/source4/auth/kerberos/kerberos_util.c
>+++ b/source4/auth/kerberos/kerberos_util.c
>@@ -633,7 +633,7 @@ krb5_error_code smb_krb5_remove_obsolete_keytab_entries(TALLOC_CTX *mem_ctx,
> 		krb5_kt_free_entry(context, &entry);
> 		/* Make sure we do not double free */
> 		ZERO_STRUCT(entry);
>-	} while (code != 0);
>+	} while (code == 0);
> 
> 	krb5_kt_end_seq_get(context, keytab, &cursor);
> 
>-- 
>2.7.4
>

Actions: View

Attachments on bug 13415: 14167