AS-REQ using SPN

Andrew Bartlett abartlet at samba.org
Thu Nov 30 04:02:45 UTC 2017


On Thu, 2017-11-16 at 11:20 +1300, Andrew Bartlett via samba-technical
wrote:
> On Wed, 2017-11-15 at 22:18 +0100, Ralph Böhme via samba-technical
> wrote:
> > On Thu, Nov 16, 2017 at 06:51:54AM +1300, Andrew Bartlett wrote:
> > > Can you show me the full LDIF for that account, and if at all possible
> > > a network capture?  
> > 
> > sure.
> > 
> > dn: CN=Foo Foo,CN=Users,DC=riverside,DC=site
> 
> ...
> > sAMAccountName: foo
> > sAMAccountType: 805306368
> > userPrincipalName: foo/win2016.riverside.site at RIVERSIDE.SITE
> > lockoutTime: 0
> > servicePrincipalName: foo/win2016.riverside.site
> > objectCategory: CN=Person,CN=Schema,CN=Configuration,DC=riverside,DC=site
> > dSCorePropagationData: 16010101000000.0Z
> > lastLogonTimestamp: 131552130336033649
> 
> Thanks!  
> 
> So that looks to me like it is using the userPrincipalName, not the
> servicePrincipalName.  I've not seen this work unless the UPN is set
> (and even then there appear to be restrictions based on the principal
> type). 
> 
> I'll lock this down with some more tests, so far they indicate that the
> userPrincipalName is the only reason it works, and only for name type
> KRB5_NT_PRINCIPAL_NAME.

I've been working on this, and I'm pretty sure the patch was wrong. 
With the attached change to krb5.kdc-canon I get a pass against
windows, a failure against Samba master and a pass against Samba if I
revert your patch.

This covers the UPN is not an SPN case.  I'm still finishing off the
UPN is an SPN case, but I would like to revert your KDC change once I
can prove the last combinations against Windows and Samba.  

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
diff --git a/source4/torture/krb5/kdc-canon-heimdal.c b/source4/torture/krb5/kdc-canon-heimdal.c
index 7f806e73e66..33e6eb5515d 100644
--- a/source4/torture/krb5/kdc-canon-heimdal.c
+++ b/source4/torture/krb5/kdc-canon-heimdal.c
@@ -43,7 +43,8 @@
 #define TEST_UPN              0x0000040
 #define TEST_S4U2SELF         0x0000080
 #define TEST_REMOVEDOLLAR     0x0000100
-#define TEST_ALL              0x00001FF
+#define TEST_AS_REQ_SPN       0x0000200
+#define TEST_ALL              0x00003FF
 
 struct test_data {
 	const char *test_name;
@@ -62,6 +63,8 @@ struct test_data {
 	bool other_upn_suffix;
 	bool s4u2self;
 	bool removedollar;
+	bool as_req_spn;
+	bool spn_is_upn;
 	const char *krb5_service;
 	const char *krb5_hostname;
 };
@@ -238,7 +241,8 @@ static bool torture_krb5_pre_send_as_req_test(struct torture_krb5_context *test_
 	torture_assert_int_equal(test_context->tctx, used, send_buf->length, "length mismatch");
 	torture_assert_int_equal(test_context->tctx, test_context->as_req.pvno,
 				 5, "Got wrong as_req->pvno");
-	if (test_context->test_data->canonicalize || test_context->test_data->enterprise) {
+	if (test_context->test_data->canonicalize
+	    || test_context->test_data->enterprise) {
 		torture_assert(test_context->tctx,
 			       test_context->as_req.req_body.kdc_options.canonicalize,
 			       "krb5 libs did not set canonicalize!");
@@ -249,7 +253,23 @@ static bool torture_krb5_pre_send_as_req_test(struct torture_krb5_context *test_
 					 "krb5 libs unexpectedly set canonicalize!");
 	}
 
-	if (test_context->test_data->enterprise) {
+	if (test_context->test_data->as_req_spn) {
+		if (test_context->test_data->upn) {
+			torture_assert_int_equal(test_context->tctx,
+						 test_context->as_req.req_body.cname->name_type,
+						 KRB5_NT_PRINCIPAL,
+						 "krb5 libs unexpectedly "
+						 "did not set principal "
+						 "as NT_SRV_HST!");
+		} else {
+			torture_assert_int_equal(test_context->tctx,
+						 test_context->as_req.req_body.cname->name_type,
+						 KRB5_NT_SRV_HST,
+						 "krb5 libs unexpectedly "
+						 "did not set principal "
+						 "as NT_SRV_HST!");
+		}
+	} else if (test_context->test_data->enterprise) {
 		torture_assert_int_equal(test_context->tctx,
 					 test_context->as_req.req_body.cname->name_type,
 					 KRB5_NT_ENTERPRISE_PRINCIPAL,
@@ -351,6 +371,11 @@ static bool torture_krb5_post_recv_as_req_test(struct torture_krb5_context *test
 						 error.error_code,
 						 KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN - KRB5KDC_ERR_NONE,
 						 "Got wrong error.error_code");
+		} else if (test_context->test_data->as_req_spn && !test_context->test_data->spn_is_upn) {
+			torture_assert_int_equal(test_context->tctx,
+						 error.error_code,
+						 KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN - KRB5KDC_ERR_NONE,
+						 "Got wrong error.error_code");
 		} else {
 			torture_assert_int_equal(test_context->tctx,
 						 error.error_code,
@@ -580,6 +605,14 @@ static bool torture_krb5_pre_send_tgs_req_canon_test(struct torture_krb5_context
 					 test_context->tgs_req.req_body.realm, test_context->test_data->real_realm,
 				 "Mismatch in realm between request and expected request");
 
+	} else if (test_context->test_data->as_req_spn) {
+		torture_assert_int_equal(test_context->tctx,
+					 test_context->tgs_req.req_body.sname->name_type, KRB5_NT_SRV_HST,
+					 "Mismatch in name type between request and expected request, expected  KRB5_NT_SRV_HST");
+		torture_assert_str_equal(test_context->tctx,
+					 test_context->tgs_req.req_body.realm, test_context->test_data->real_realm,
+				 "Mismatch in realm between request and expected request");
+
 	} else if (test_context->test_data->canonicalize) {
 		torture_assert_int_equal(test_context->tctx,
 					 test_context->tgs_req.req_body.sname->name_type, KRB5_NT_PRINCIPAL,
@@ -704,7 +737,22 @@ static bool torture_krb5_pre_send_self_trust_tgs_req_test(struct torture_krb5_co
 				 0, "decode_TGS_REQ for TEST_SELF_TRUST_TGS_REQ test failed");
 	torture_assert_int_equal(test_context->tctx, used, send_buf->length, "length mismatch");
 	torture_assert_int_equal(test_context->tctx, test_context->tgs_req.pvno, 5, "Got wrong as_req->pvno");
-	torture_assert_int_equal(test_context->tctx, test_context->tgs_req.req_body.kdc_options.canonicalize, false, "krb5 libs unexpectedly set canonicalize!");
+
+	if (test_context->test_data->enterprise
+	    || test_context->test_data->spn_is_upn) {
+		torture_assert_int_equal(test_context->tctx,
+					 test_context->tgs_req.req_body.kdc_options.canonicalize,
+					 true,
+					 "krb5 libs unexpectedly"
+					 " did not set canonicalize!");
+	} else {
+		torture_assert_int_equal(test_context->tctx,
+					 test_context->tgs_req.req_body.kdc_options.canonicalize,
+					 false,
+					 "krb5 libs unexpectedly"
+					 " set canonicalize!");
+	}
+	
 
 	if (test_context->test_data->canonicalize) {
 		torture_assert_str_equal(test_context->tctx,
@@ -829,11 +877,22 @@ static bool torture_krb5_pre_send_tgs_req_test(struct torture_krb5_context *test
 	torture_assert_int_equal(test_context->tctx, used, send_buf->length, "length mismatch");
 	torture_assert_int_equal(test_context->tctx, test_context->tgs_req.pvno, 5,
 				 "Got wrong as_req->pvno");
-	torture_assert_int_equal(test_context->tctx,
-				 test_context->tgs_req.req_body.kdc_options.canonicalize,
-				 false,
-				 "krb5 libs unexpectedly set canonicalize!");
 
+	if (test_context->test_data->enterprise
+	    && test_context->test_data->spn_is_upn) {
+		torture_assert_int_equal(test_context->tctx,
+					 test_context->tgs_req.req_body.kdc_options.canonicalize,
+					 true,
+					 "krb5 libs unexpectedly"
+					 " did not set canonicalize!");
+	} else {
+		torture_assert_int_equal(test_context->tctx,
+					 test_context->tgs_req.req_body.kdc_options.canonicalize,
+					 false,
+					 "krb5 libs unexpectedly"
+					 " set canonicalize!");
+	}
+	
 	if (test_context->test_data->enterprise) {
 		torture_assert_int_equal(test_context->tctx,
 					 test_context->tgs_req.req_body.sname->name_type,
@@ -1191,7 +1250,9 @@ static bool torture_krb5_post_recv_as_req_self_test(struct torture_krb5_context
 					 error.pvno, 5,
 					 "Got wrong error.pvno");
 		if ((torture_setting_bool(test_context->tctx, "expect_machine_account", false)
-		     && (test_context->test_data->upn == false))) {
+		     && ((test_context->test_data->upn == false)
+		     || (test_context->test_data->as_req_spn &&
+			 test_context->test_data->spn_is_upn)))) {
 			torture_assert_int_equal(test_context->tctx,
 						 error.error_code,
 						 KRB5KRB_ERR_RESPONSE_TOO_BIG - KRB5KDC_ERR_NONE,
@@ -1415,10 +1476,10 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 	krb5_principal principal;
 	krb5_principal krbtgt_other;
 	krb5_principal expected_principal;
-	char *principal_string;
+	const char *principal_string = NULL;
 	char *krbtgt_other_string;
 	int principal_flags;
-	char *expected_principal_string;
+	const char *expected_principal_string = NULL;
 	char *expected_unparse_principal_string;
 	int expected_principal_flags;
 	char *got_principal_string;
@@ -1436,6 +1497,7 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 	krb5_data in_data, enc_ticket;
 	krb5_get_creds_opt opt;
 
+	const char *spn = NULL;
 	const char *upn = torture_setting_string(tctx, "krb5-upn", "");
 	test_data->krb5_service = torture_setting_string(tctx, "krb5-service", "host");
 	test_data->krb5_hostname = torture_setting_string(tctx, "krb5-hostname", "");
@@ -1448,6 +1510,14 @@ 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 we have not passed a SPN on the command line,
+	 * then skip the SPN tests.
+	 */
+	if (test_data->as_req_spn && test_data->krb5_hostname[0] == '\0') {
+		torture_skip(tctx, "This test needs a hostname specified as --option=torture:krb5-hostname=hostname.example.com and optinally --option=torture:krb5-service=service (defaults to host) to run");
+	}
+
 	if (test_data->removedollar &&
 	    !torture_setting_bool(tctx, "run_removedollar_test", false))
 	{
@@ -1520,8 +1590,29 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 		*p = '\0';
 	}
 
-	principal_string = talloc_asprintf(test_data, "%s@%s", test_data->username, test_data->realm);
+	spn = talloc_asprintf(test_data, "%s/%s@%s",
+			      test_data->krb5_service,
+			      test_data->krb5_hostname,
+			      test_data->realm);
+
+	if (test_data->as_req_spn) {
+		if (test_data->enterprise) {
+			torture_skip(tctx,
+				     "This test combination "
+				     "is skipped intentionally");
+		}
+		principal_string = spn;
+	} else {
+		principal_string = talloc_asprintf(test_data,
+						   "%s@%s",
+						   test_data->username,
+						   test_data->realm);
+		
+	}
 
+	test_data->spn_is_upn
+		= (strcasecmp(upn, spn) == 0);
+				
 	/*
 	 * If we are set to canonicalize, we get back the fixed UPPER
 	 * case realm, and the real username (ie matching LDAP
@@ -1533,7 +1624,9 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 	 * Finally, if we are not set to canonicalize, we get back the
 	 * fixed UPPER case realm, but the as-sent username
 	 */
-	if (test_data->canonicalize) {
+	if (test_data->as_req_spn) {
+		expected_principal_string = spn;
+	} else if (test_data->canonicalize) {
 		expected_principal_string = talloc_asprintf(test_data,
 							    "%s@%s",
 							    test_data->real_username,
@@ -1574,7 +1667,25 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 						       expected_principal_flags,
 						       &expected_principal),
 				 0, "krb5_parse_name_flags failed");
-
+	
+	if (test_data->as_req_spn) {
+		if (test_data->upn) {
+			krb5_principal_set_type(k5_context,
+						principal,
+						KRB5_NT_PRINCIPAL);
+			krb5_principal_set_type(k5_context,
+						expected_principal,
+						KRB5_NT_PRINCIPAL);
+		} else {
+			krb5_principal_set_type(k5_context,
+						principal,
+						KRB5_NT_SRV_HST);
+			krb5_principal_set_type(k5_context,
+						expected_principal,
+						KRB5_NT_SRV_HST);
+		}
+	}
+	
 	torture_assert_int_equal(tctx,
 				 krb5_unparse_name(k5_context,
 						   expected_principal,
@@ -1617,6 +1728,14 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 					 "Got wrong error_code from krb5_get_init_creds_password");
 		/* We can't proceed with more checks */
 		return true;
+	} else if (test_context->test_data->as_req_spn
+		   && !test_context->test_data->spn_is_upn) {
+		torture_assert_int_equal(tctx, k5ret,
+					 KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN,
+					 "Got wrong error_code from "
+					 "krb5_get_init_creds_password");
+		/* We can't proceed with more checks */
+		return true;
 	} else {
 		assertion_message = talloc_asprintf(tctx,
 						    "krb5_get_init_creds_password for %s failed: %s",
@@ -1639,6 +1758,12 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 								 my_creds.client),
 					 KRB5_NT_ENTERPRISE_PRINCIPAL,
 					 "smb_krb5_init_context gave incorrect client->name.name_type");
+	} else if (test_data->canonicalize == false && test_data->as_req_spn) {
+		torture_assert_int_equal(tctx,
+					 krb5_principal_get_type(k5_context,
+								 my_creds.client),
+					 KRB5_NT_SRV_HST,
+					 "smb_krb5_init_context gave incorrect client->name.name_type");
 	} else {
 		torture_assert_int_equal(tctx,
 					 krb5_principal_get_type(k5_context,
@@ -1844,7 +1969,9 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 		 * servicePrincipalName) can expect this test to succeed
 		 */
 		if (torture_setting_bool(tctx, "expect_machine_account", false)
-		    && (test_data->enterprise || test_data->upn == false)) {
+		    && (test_data->enterprise
+			|| test_data->spn_is_upn
+			|| test_data->upn == false)) {
 			torture_assert_int_equal(tctx, k5ret, 0, assertion_message);
 			torture_assert_int_equal(tctx, krb5_cc_store_cred(k5_context,
 									  ccache, server_creds),
@@ -1882,6 +2009,7 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 	 */
 	if (test_context->test_data->canonicalize == false
 	    || test_context->test_data->enterprise
+	    || test_context->test_data->spn_is_upn
 	    || (test_context->test_data->upper_realm
 		&& test_context->test_data->netbios_realm == false)) {
 		test_context->test_stage = TEST_TGS_REQ;
@@ -1909,7 +2037,11 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 	 * Only machine accounts (strictly, accounts with a
 	 * servicePrincipalName) can expect this test to succeed
 	 */
-	if (torture_setting_bool(tctx, "expect_machine_account", false) && (test_data->enterprise || test_data->upn == false)) {
+	if (torture_setting_bool(tctx, "expect_machine_account", false)
+	    && (test_data->enterprise ||
+		(test_context->test_data->as_req_spn 
+		 || test_context->test_data->spn_is_upn)
+		|| test_data->upn == false)) {
 		DATA_BLOB client_to_server;
 		torture_assert_int_equal(tctx, k5ret, 0, assertion_message);
 		client_to_server = data_blob_const(enc_ticket.data, enc_ticket.length);
@@ -1967,25 +2099,31 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 				    &in_data, ccache,
 				    &enc_ticket);
 
-		if (test_data->canonicalize == false && test_data->enterprise == false
+		if (test_data->spn_is_upn == false
+		    && test_data->canonicalize == false
+		    && test_data->enterprise == false
 		    && (test_data->upper_realm == false || test_data->netbios_realm == true)) {
 			torture_assert_int_equal(tctx, k5ret, KRB5_CC_NOTFOUND,
 						 "krb5_get_creds should have failed with KRB5_CC_NOTFOUND");
 		} else {
 			assertion_message = talloc_asprintf(tctx,
 							    "krb5_mk_req for %s/%s failed: %s",
-							    test_data->krb5_hostname,
 							    test_data->krb5_service,
+							    test_data->krb5_hostname,
 							    smb_get_krb5_error_message(k5_context, k5ret, tctx));
 
 			torture_assert_int_equal(tctx, k5ret, 0, assertion_message);
-			/*
-			 * Only in these cases would the above code have needed to
-			 * send packets to the network
-			 */
-			torture_assert(tctx,
-				       test_context->packet_count > 0,
-				       "Expected krb5_get_creds to send packets");
+
+			if (test_data->spn_is_upn == false) {
+				/*
+				 * Only in these cases would the above
+				 * code have needed to send packets to
+				 * the network
+				 */
+				torture_assert(tctx,
+					       test_context->packet_count > 0,
+					       "Expected krb5_get_creds to send packets");
+			}
 		}
 
 
@@ -2118,7 +2256,8 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 					     password, NULL, NULL, 0,
 					     principal_string, krb_options);
 
-	if (torture_setting_bool(test_context->tctx, "expect_machine_account", false) && (test_data->upn == false)) {
+	if (torture_setting_bool(test_context->tctx, "expect_machine_account", false)
+	    && (test_data->upn == false || (test_data->enterprise == false && test_data->upn == true && test_data->spn_is_upn))) {
 		assertion_message = talloc_asprintf(tctx,
 						    "krb5_get_init_creds_password for %s failed: %s",
 						    principal_string,
@@ -2214,19 +2353,31 @@ 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.%s",
+		char *name = talloc_asprintf(suite, "%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",
 					     (i & TEST_UPPER_USERNAME) ? "uc-user" : "lc-user",
 					     (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_REMOVEDOLLAR) ? "removedollar" : "keepdollar");
+					     (i & TEST_UPN) ? "upn" :
+					     ((i & TEST_AS_REQ_SPN) ? "spn" : 
+					      ((i & TEST_REMOVEDOLLAR) ? "removedollar" : "samaccountname")),
+					     (i & TEST_S4U2SELF) ? "s4u2self" : "normal");
+		struct torture_suite *sub_suite = torture_suite_create(mem_ctx, name);
 
 		struct test_data *test_data = talloc_zero(suite, struct test_data);
-
+		if (i & TEST_UPN) {
+			if (i & TEST_AS_REQ_SPN) {
+				continue;
+			}
+		}
+		if ((i & TEST_UPN) || (i & TEST_AS_REQ_SPN)) {
+			if (i & TEST_REMOVEDOLLAR) {
+				continue;
+			}
+		}
+		
 		test_data->test_name = name;
 		test_data->real_realm
 			= strupper_talloc(test_data,
@@ -2247,8 +2398,10 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX *mem_ctx)
 		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->as_req_spn = (i & TEST_AS_REQ_SPN) != 0;
+		torture_suite_add_simple_tcase_const(sub_suite, name, torture_krb5_as_req_canon,
 						     test_data);
+		torture_suite_add_suite(suite, sub_suite);
 
 	}
 	return suite;


More information about the samba-technical mailing list