Patches for KDC TGS not finding principal (bug #11130)
Stefan (metze) Metzmacher
metze at samba.org
Tue Jun 23 08:45:51 MDT 2015
Hi,
here're patches to fix https://bugzilla.samba.org/show_bug.cgi?id=11130
KDC TGS not finding principal.
Please review and push.
Thanks!
metze
-------------- next part --------------
From be17e3e10933c84af433cdd5b386b6aa06c9f0fa Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 11 Jun 2015 20:04:09 +0200
Subject: [PATCH 1/5] s4:kdc/db-glue: fix memory leak in
samba_kdc_lookup_server()
We need to free enterprise_principal if generated.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/kdc/db-glue.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 4237c65..7f6ef3d 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -1650,7 +1650,8 @@ static krb5_error_code samba_kdc_lookup_server(krb5_context context,
*/
int lret;
char *short_princ;
- krb5_principal enterprise_prinicpal = NULL;
+ krb5_principal enterprise_principal = NULL;
+ krb5_const_principal used_principal = NULL;
if (smb_krb5_principal_get_type(context, principal) == KRB5_NT_ENTERPRISE_PRINCIPAL) {
char *str = NULL;
@@ -1667,12 +1668,14 @@ static krb5_error_code samba_kdc_lookup_server(krb5_context context,
return KRB5_PARSE_MALFORMED;
}
ret = krb5_parse_name(context, str,
- &enterprise_prinicpal);
+ &enterprise_principal);
talloc_free(str);
if (ret) {
return ret;
}
- principal = enterprise_prinicpal;
+ used_principal = enterprise_principal;
+ } else {
+ used_principal = principal;
}
/* server as client principal case, but we must not lookup userPrincipalNames */
@@ -1680,10 +1683,13 @@ static krb5_error_code samba_kdc_lookup_server(krb5_context context,
/* TODO: Check if it is our realm, otherwise give referral */
- ret = krb5_unparse_name_flags(context, principal,
+ ret = krb5_unparse_name_flags(context, used_principal,
KRB5_PRINCIPAL_UNPARSE_NO_REALM |
KRB5_PRINCIPAL_UNPARSE_DISPLAY,
&short_princ);
+ used_principal = NULL;
+ krb5_free_principal(context, enterprise_principal);
+ enterprise_principal = NULL;
if (ret != 0) {
krb5_set_error_message(context, ret, "samba_kdc_lookup_principal: could not parse principal");
--
1.9.1
From 4c8b9bd7b3cf7dda4af9cd21bcd888be4170e156 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 23 Mar 2015 10:00:51 +0000
Subject: [PATCH 2/5] s4:kdc/db-glue: allow principals in form of
computer at EXAMPLE.COM
This should be translated to computer$@EXAMPLE.COM.
Note the behavior differs between client and server lookup.
In samba_kdc_lookup_client() we need to fallback in case of
NO_SUCH_USER. samba_kdc_lookup_server() needs to do a single search
and only use the result if it's unique.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11130
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/kdc/db-glue.c | 156 ++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 137 insertions(+), 19 deletions(-)
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 7f6ef3d..9272717 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -1370,9 +1370,10 @@ static krb5_error_code samba_kdc_lookup_client(krb5_context context,
krb5_const_principal principal,
const char **attrs,
struct ldb_dn **realm_dn,
- struct ldb_message **msg) {
+ struct ldb_message **msg)
+{
NTSTATUS nt_status;
- char *principal_string;
+ char *principal_string = NULL;
if (smb_krb5_principal_get_type(context, principal) == KRB5_NT_ENTERPRISE_PRINCIPAL) {
principal_string = smb_krb5_principal_get_comp_string(mem_ctx, context,
@@ -1380,21 +1381,111 @@ static krb5_error_code samba_kdc_lookup_client(krb5_context context,
if (principal_string == NULL) {
return ENOMEM;
}
- nt_status = sam_get_results_principal(kdc_db_ctx->samdb,
- mem_ctx, principal_string, attrs,
- realm_dn, msg);
- TALLOC_FREE(principal_string);
} else {
+ char *principal_string_m = NULL;
krb5_error_code ret;
- ret = krb5_unparse_name(context, principal, &principal_string);
+
+ ret = krb5_unparse_name(context, principal, &principal_string_m);
if (ret != 0) {
return ret;
}
- nt_status = sam_get_results_principal(kdc_db_ctx->samdb,
- mem_ctx, principal_string, attrs,
- realm_dn, msg);
- free(principal_string);
+
+ principal_string = talloc_strdup(mem_ctx, principal_string_m);
+ SAFE_FREE(principal_string_m);
+ if (principal_string == NULL) {
+ return ENOMEM;
+ }
+ }
+
+ nt_status = sam_get_results_principal(kdc_db_ctx->samdb,
+ mem_ctx, principal_string, attrs,
+ realm_dn, msg);
+ if (NT_STATUS_EQUAL(nt_status, NT_STATUS_NO_SUCH_USER)) {
+ krb5_principal fallback_principal = NULL;
+ unsigned int num_comp;
+ char *fallback_realm = NULL;
+ char *fallback_account = NULL;
+ krb5_error_code ret;
+
+ ret = krb5_parse_name(context, principal_string,
+ &fallback_principal);
+ TALLOC_FREE(principal_string);
+ if (ret != 0) {
+ return ret;
+ }
+
+ num_comp = krb5_princ_size(context, fallback_principal);
+ fallback_realm = smb_krb5_principal_get_realm(context,
+ fallback_principal);
+ if (fallback_realm == NULL) {
+ krb5_free_principal(context, fallback_principal);
+ return ENOMEM;
+ }
+
+ if (num_comp == 1) {
+ size_t len;
+
+ fallback_account = smb_krb5_principal_get_comp_string(mem_ctx,
+ context, fallback_principal, 0);
+ if (fallback_account == NULL) {
+ krb5_free_principal(context, fallback_principal);
+ SAFE_FREE(fallback_realm);
+ return ENOMEM;
+ }
+
+ len = strlen(fallback_account);
+ if (len >= 2 && fallback_account[len - 1] == '$') {
+ TALLOC_FREE(fallback_account);
+ }
+ }
+ krb5_free_principal(context, fallback_principal);
+ fallback_principal = NULL;
+
+ if (fallback_account != NULL) {
+ char *with_dollar;
+
+ with_dollar = talloc_asprintf(mem_ctx, "%s$",
+ fallback_account);
+ if (with_dollar == NULL) {
+ SAFE_FREE(fallback_realm);
+ return ENOMEM;
+ }
+ TALLOC_FREE(fallback_account);
+
+ ret = smb_krb5_make_principal(context,
+ &fallback_principal,
+ fallback_realm,
+ with_dollar, NULL);
+ TALLOC_FREE(with_dollar);
+ if (ret != 0) {
+ SAFE_FREE(fallback_realm);
+ return ret;
+ }
+ }
+ SAFE_FREE(fallback_realm);
+
+ if (fallback_principal != NULL) {
+ char *fallback_string = NULL;
+
+ ret = krb5_unparse_name(context,
+ fallback_principal,
+ &fallback_string);
+ if (ret != 0) {
+ krb5_free_principal(context, fallback_principal);
+ return ret;
+ }
+
+ nt_status = sam_get_results_principal(kdc_db_ctx->samdb,
+ mem_ctx,
+ fallback_string,
+ attrs,
+ realm_dn, msg);
+ SAFE_FREE(fallback_string);
+ }
+ krb5_free_principal(context, fallback_principal);
+ fallback_principal = NULL;
}
+ TALLOC_FREE(principal_string);
if (NT_STATUS_EQUAL(nt_status, NT_STATUS_NO_SUCH_USER)) {
return HDB_ERR_NOENTRY;
@@ -1652,6 +1743,9 @@ static krb5_error_code samba_kdc_lookup_server(krb5_context context,
char *short_princ;
krb5_principal enterprise_principal = NULL;
krb5_const_principal used_principal = NULL;
+ char *name1 = NULL;
+ size_t len1 = 0;
+ char *filter = NULL;
if (smb_krb5_principal_get_type(context, principal) == KRB5_NT_ENTERPRISE_PRINCIPAL) {
char *str = NULL;
@@ -1697,24 +1791,48 @@ static krb5_error_code samba_kdc_lookup_server(krb5_context context,
return ret;
}
+ name1 = ldb_binary_encode_string(mem_ctx, short_princ);
+ SAFE_FREE(short_princ);
+ if (name1 == NULL) {
+ return ENOMEM;
+ }
+ len1 = strlen(name1);
+ if (len1 >= 1 && name1[len1 - 1] != '$') {
+ filter = talloc_asprintf(mem_ctx,
+ "(&(objectClass=user)(|(samAccountName=%s)(samAccountName=%s$)))",
+ name1, name1);
+ if (filter == NULL) {
+ return ENOMEM;
+ }
+ } else {
+ filter = talloc_asprintf(mem_ctx,
+ "(&(objectClass=user)(samAccountName=%s))",
+ name1);
+ if (filter == NULL) {
+ return ENOMEM;
+ }
+ }
+
lret = dsdb_search_one(kdc_db_ctx->samdb, mem_ctx, msg,
*realm_dn, LDB_SCOPE_SUBTREE,
attrs,
DSDB_SEARCH_SHOW_EXTENDED_DN | DSDB_SEARCH_NO_GLOBAL_CATALOG,
- "(&(objectClass=user)(samAccountName=%s))",
- ldb_binary_encode_string(mem_ctx, short_princ));
+ "%s", filter);
if (lret == LDB_ERR_NO_SUCH_OBJECT) {
- DEBUG(3, ("Failed to find an entry for %s\n", short_princ));
- free(short_princ);
+ DEBUG(10, ("Failed to find an entry for %s filter:%s\n",
+ name1, filter));
+ return HDB_ERR_NOENTRY;
+ }
+ if (lret == LDB_ERR_CONSTRAINT_VIOLATION) {
+ DEBUG(10, ("Failed to find unique entry for %s filter:%s\n",
+ name1, filter));
return HDB_ERR_NOENTRY;
}
if (lret != LDB_SUCCESS) {
- DEBUG(3, ("Failed single search for %s - %s\n",
- short_princ, ldb_errstring(kdc_db_ctx->samdb)));
- free(short_princ);
+ DEBUG(0, ("Failed single search for %s - %s\n",
+ name1, ldb_errstring(kdc_db_ctx->samdb)));
return HDB_ERR_NOENTRY;
}
- free(short_princ);
return 0;
}
return HDB_ERR_NOENTRY;
--
1.9.1
From a3b84be8836d40fe578ad7da61c396af5d045b7a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 23 Mar 2015 22:10:02 +0000
Subject: [PATCH 3/5] s4:selftest: run samba4.rpc.lsa.secrets with more
principal combinations
'dcom/SERVER', 'SERVER$' and 'SERVER' as target principal names.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11130
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/selftest/tests.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index b8d1ff5..e9bba72 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -181,6 +181,9 @@ for env in ["ad_dc_ntvfs", "fl2000dc", "fl2003dc", "fl2008r2dc", "ad_dc"]:
plansmbtorture4testsuite('rpc.pac', env, ["%s:$SERVER[]" % (transport, ), '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN'], "samba4.rpc.pac on %s" % (transport,))
plansmbtorture4testsuite('rpc.lsa.secrets', env, ["%s:$SERVER[]" % (transport, ), '-k', 'yes', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN', '--option=gensec:target_hostname=$NETBIOSNAME', 'rpc.lsa.secrets'], "samba4.rpc.lsa.secrets on %s with Kerberos" % (transport,))
plansmbtorture4testsuite('rpc.lsa.secrets', env, ["%s:$SERVER[]" % (transport, ), '-k', 'yes', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN', "--option=clientusespnegoprincipal=yes", '--option=gensec:target_hostname=$NETBIOSNAME'], "samba4.rpc.lsa.secrets on %s with Kerberos - use target principal" % (transport,))
+ plansmbtorture4testsuite('rpc.lsa.secrets', env, ["%s:$SERVER[target_principal=dcom/$NETBIOSNAME]" % (transport, ), '-k', 'yes', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN'], "samba4.rpc.lsa.secrets on %s with Kerberos - netbios name principal dcom" % (transport,))
+ plansmbtorture4testsuite('rpc.lsa.secrets', env, ["%s:$SERVER[target_principal=$NETBIOSNAME\$]" % (transport, ), '-k', 'yes', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN'], "samba4.rpc.lsa.secrets on %s with Kerberos - netbios name principal dollar" % (transport,))
+ plansmbtorture4testsuite('rpc.lsa.secrets', env, ["%s:$SERVER[target_principal=$NETBIOSNAME]" % (transport, ), '-k', 'yes', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN'], "samba4.rpc.lsa.secrets on %s with Kerberos - netbios name principal" % (transport,))
plansmbtorture4testsuite('rpc.lsa.secrets.none*', env, ["%s:$SERVER" % transport, '-k', 'yes', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN', "--option=gensec:fake_gssapi_krb5=yes", '--option=gensec:gssapi_krb5=no', '--option=gensec:target_hostname=$NETBIOSNAME'], "samba4.rpc.lsa.secrets on %s with Kerberos - use Samba3 style login" % transport)
plansmbtorture4testsuite('rpc.lsa.secrets.none*', env, ["%s:$SERVER" % transport, '-k', 'yes', '-U$USERNAME%$PASSWORD', '--workgroup=$DOMAIN', "--option=clientusespnegoprincipal=yes", '--option=gensec:fake_gssapi_krb5=yes', '--option=gensec:gssapi_krb5=no', '--option=gensec:target_hostname=$NETBIOSNAME'], "samba4.rpc.lsa.secrets on %s with Kerberos - use Samba3 style login, use target principal" % transport)
--
1.9.1
From 4c29f5fe72f0aa5ea02a0ba212f24807905a0001 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 27 Mar 2015 14:41:19 +0100
Subject: [PATCH 4/5] s4:torture/krb5: add a
--option=torture:run_removedollar_test=true option to kdc-conon
With this option a machine account is tested without the trailing '$'
in the account name.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11130
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/torture/krb5/kdc-canon.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/source4/torture/krb5/kdc-canon.c b/source4/torture/krb5/kdc-canon.c
index 20f0cf1..0551484 100644
--- a/source4/torture/krb5/kdc-canon.c
+++ b/source4/torture/krb5/kdc-canon.c
@@ -42,7 +42,8 @@
#define TEST_WIN2K 0x0000020
#define TEST_UPN 0x0000040
#define TEST_S4U2SELF 0x0000080
-#define TEST_ALL 0x00000FF
+#define TEST_REMOVEDOLLAR 0x0000100
+#define TEST_ALL 0x00001FF
struct test_data {
const char *test_name;
@@ -60,6 +61,7 @@ struct test_data {
bool upn;
bool other_upn_suffix;
bool s4u2self;
+ bool removedollar;
const char *krb5_service;
const char *krb5_hostname;
};
@@ -1445,6 +1447,12 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
torture_skip(tctx, "This test needs a UPN specified as --option=torture:krb5-upn=user at example.com to run");
}
+ if (test_data->removedollar &&
+ !torture_setting_bool(tctx, "run_removedollar_test", false))
+ {
+ torture_skip(tctx, "--option=torture:run_removedollar_test=true not specified");
+ }
+
if (test_data->netbios_realm) {
test_data->realm = test_data->real_domain;
} else {
@@ -1501,6 +1509,16 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
test_data->username = talloc_strdup(test_data, test_data->username);
}
+ if (test_data->removedollar) {
+ char *p;
+
+ p = strchr_m(test_data->username, '$');
+ torture_assert(tctx, p != NULL, talloc_asprintf(tctx,
+ "username[%s] contains no '$'\n",
+ test_data->username));
+ *p = '\0';
+ }
+
principal_string = talloc_asprintf(test_data, "%s@%s", test_data->username, test_data->realm);
/*
@@ -2194,7 +2212,7 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX *mem_ctx)
suite->description = talloc_strdup(suite, "Kerberos Canonicalisation tests");
for (i = 0; i < TEST_ALL; i++) {
- char *name = talloc_asprintf(suite, "%s.%s.%s.%s.%s.%s.%s.%s",
+ char *name = talloc_asprintf(suite, "%s.%s.%s.%s.%s.%s.%s.%s.%s",
(i & TEST_CANONICALIZE) ? "canon" : "no-canon",
(i & TEST_ENTERPRISE) ? "enterprise" : "no-enterprise",
(i & TEST_UPPER_REALM) ? "uc-realm" : "lc-realm",
@@ -2202,7 +2220,8 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX *mem_ctx)
(i & TEST_NETBIOS_REALM) ? "netbios-realm" : "krb5-realm",
(i & TEST_WIN2K) ? "win2k" : "no-win2k",
(i & TEST_UPN) ? "upn" : "no-upn",
- (i & TEST_S4U2SELF) ? "s4u2self" : "normal");
+ (i & TEST_S4U2SELF) ? "s4u2self" : "normal",
+ (i & TEST_REMOVEDOLLAR) ? "removedollar" : "keepdollar");
struct test_data *test_data = talloc_zero(suite, struct test_data);
@@ -2220,6 +2239,7 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX *mem_ctx)
test_data->win2k = (i & TEST_WIN2K) != 0;
test_data->upn = (i & TEST_UPN) != 0;
test_data->s4u2self = (i & TEST_S4U2SELF) != 0;
+ test_data->removedollar = (i & TEST_REMOVEDOLLAR) != 0;
torture_suite_add_simple_tcase_const(suite, name, torture_krb5_as_req_canon,
test_data);
--
1.9.1
From a378a37cf6d5959a588b534774ca98665e833aa1 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 27 Mar 2015 14:41:19 +0100
Subject: [PATCH 5/5] s4:selftest: add torture:run_removedollar_test=true to
the machine account kdc tests
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11130
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/selftest/tests.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index e9bba72..070586d 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -583,7 +583,11 @@ for env in ["ad_dc_ntvfs", "rodc", "promoted_dc", "ad_dc", "fl2000dc", "fl2003dc
else:
extra_options = []
- plansmbtorture4testsuite('krb5.kdc', "%s:local" % env, ['ncacn_np:$SERVER_IP', "-k", "yes", '-P', '--workgroup=$DOMAIN', '--realm=$REALM', '--option=torture:krb5-hostname=$SERVER', '--option=torture:expect_machine_account=true'] + extra_options,
+ plansmbtorture4testsuite('krb5.kdc', "%s:local" % env, ['ncacn_np:$SERVER_IP', "-k", "yes", '-P',
+ '--workgroup=$DOMAIN', '--realm=$REALM',
+ '--option=torture:krb5-hostname=$SERVER',
+ '--option=torture:run_removedollar_test=true',
+ '--option=torture:expect_machine_account=true'] + extra_options,
"samba4.krb5.kdc with machine account")
plansmbtorture4testsuite('krb5.kdc', env, ['ncacn_np:$SERVER_IP', "-k", "yes", '-Utestallowed\ account%$PASSWORD',
'--workgroup=$DOMAIN', '--realm=$REALM',
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150623/40f931b3/attachment.pgp>
More information about the samba-technical
mailing list