Verify PAC client name independently of name-type

In krb5_pac_verify(), unparse the provided principal name and compare
using strcmp(), instead of parsing pac principal, in order to avoid
relying on the provided name type.

This change is needed for tickets issued with cross-realm S4U2Proxy
(with resource-based constrained delegation), because the final
request uses a cross-TGT as the evidence ticket, so the ticket client
name is taken from the PAC and does not preserve the name type.
Microsoft KDCs use NT-MS-PRINCIPAL as the ticket client name type in
this case, regardless of the original name type.

[ghudson@mit.edu: rewrote commit message; made minor style edits]

ticket: 8815 (new)

diff --git a/src/lib/krb5/krb/pac.c b/src/lib/krb5/krb/pac.c
index cc74f378b0b2a4169c5061b7c1fe245b7fffc39f..5efc91eeb4838bc576114f79a5f7338b7b051b85 100644 (file)
--- a/src/lib/krb5/krb/pac.c
+++ b/src/lib/krb5/krb/pac.c
@@ -408,12 +408,11 @@ k5_pac_validate_client(krb5_context context,
 {
     krb5_error_code ret;
     krb5_data client_info;
-    char *pac_princname;
+    char *pac_princname, *princname;
     unsigned char *p;
     krb5_timestamp pac_authtime;
     krb5_ui_2 pac_princname_length;
     int64_t pac_nt_authtime;
-    krb5_principal pac_principal;
     int flags = 0;
 
     ret = k5_pac_locate_buffer(context, pac, KRB5_PAC_CLIENT_INFO,
@@ -442,33 +441,21 @@ k5_pac_validate_client(krb5_context context,
     if (ret != 0)
         return ret;
 
-    /* Parse the UTF-8 name as an enterprise principal if we are matching
-     * against one; otherwise parse it as a regular principal. */
-    if (principal->type == KRB5_NT_ENTERPRISE_PRINCIPAL)
-        flags |= KRB5_PRINCIPAL_PARSE_ENTERPRISE;
+    flags = KRB5_PRINCIPAL_UNPARSE_DISPLAY;
+    if (!with_realm)
+        flags |= KRB5_PRINCIPAL_UNPARSE_NO_REALM;
 
-    if (with_realm)
-        flags |= KRB5_PRINCIPAL_PARSE_REQUIRE_REALM;
-    else
-        flags |= KRB5_PRINCIPAL_PARSE_NO_REALM;
-
-    ret = krb5_parse_name_flags(context, pac_princname, flags, &pac_principal);
+    ret = krb5_unparse_name_flags(context, principal, flags, &princname);
     if (ret != 0) {
         free(pac_princname);
         return ret;
     }
 
-    free(pac_princname);
-
-    if (pac_authtime != authtime ||
-        !krb5_principal_compare_flags(context,
-                                      pac_principal,
-                                      principal,
-                                      with_realm ? 0 :
-                                      KRB5_PRINCIPAL_COMPARE_IGNORE_REALM))
+    if (pac_authtime != authtime || strcmp(pac_princname, princname) != 0)
         ret = KRB5KRB_AP_WRONG_PRINC;
 
-    krb5_free_principal(context, pac_principal);
+    free(pac_princname);
+    krb5_free_unparsed_name(context, princname);
 
     return ret;
 }

diff --git a/src/lib/krb5/krb/t_pac.c b/src/lib/krb5/krb/t_pac.c
index 7b756a2a5a5b73e4743ec0b30e22790c6601c629..ee47152ee44d7df6bc72a88d2e0f1934439c8be9 100644 (file)
--- a/src/lib/krb5/krb/t_pac.c
+++ b/src/lib/krb5/krb/t_pac.c
@@ -733,13 +733,18 @@ main(int argc, char **argv)
     }
 
     {
-        krb5_principal ep;
+        krb5_principal ep, np;
 
         ret = krb5_parse_name_flags(context, user,
                                     KRB5_PRINCIPAL_PARSE_ENTERPRISE, &ep);
         if (ret)
             err(context, ret, "krb5_parse_name_flags");
 
+        ret = krb5_copy_principal(context, ep, &np);
+        if (ret)
+            err(context, ret, "krb5_copy_principal");
+        np->type = KRB5_NT_MS_PRINCIPAL;
+
         /* Try to verify as enterprise. */
         ret = krb5_pac_verify(context, pac, authtime, ep, &member_keyblock,
                               &kdc_keyblock);
@@ -788,6 +793,47 @@ main(int argc, char **argv)
         if (ret)
             err(context, ret, "krb5_pac_verify enterprise failed");
 
+        /* Also verify enterprise as KRB5_NT_MS_PRINCIPAL. */
+        ret = krb5_pac_verify(context, pac, authtime, np, &member_keyblock,
+                              &kdc_keyblock);
+        if (ret)
+            err(context, ret, "krb5_pac_verify enterprise as nt-ms failed");
+
+        ret = krb5_pac_verify(context, pac, authtime, p, &member_keyblock,
+                              &kdc_keyblock);
+        if (!ret)
+            err(context, ret, "krb5_pac_verify should have failed");
+
+        krb5_pac_free(context, pac);
+
+        /* Test nt-ms-principal. */
+        ret = krb5_pac_init(context, &pac);
+        if (ret)
+            err(context, ret, "krb5_pac_init");
+
+        ret = krb5_pac_sign(context, pac, authtime, np, &member_keyblock,
+                            &kdc_keyblock, &data);
+        if (ret)
+            err(context, ret, "krb5_pac_sign enterprise failed");
+
+        krb5_pac_free(context, pac);
+
+        ret = krb5_pac_parse(context, data.data, data.length, &pac);
+        krb5_free_data_contents(context, &data);
+        if (ret)
+            err(context, ret, "krb5_pac_parse failed");
+
+        ret = krb5_pac_verify(context, pac, authtime, np, &member_keyblock,
+                              &kdc_keyblock);
+        if (ret)
+            err(context, ret, "krb5_pac_verify enterprise failed");
+
+        /* Also verify as enterprise principal. */
+        ret = krb5_pac_verify(context, pac, authtime, ep, &member_keyblock,
+                              &kdc_keyblock);
+        if (ret)
+            err(context, ret, "krb5_pac_verify nt-ms as enterprise failed");
+
         ret = krb5_pac_verify(context, pac, authtime, p, &member_keyblock,
                               &kdc_keyblock);
         if (!ret)
@@ -862,6 +908,7 @@ main(int argc, char **argv)
             err(context, ret, "krb5_pac_verify_ext should have failed");
 
         krb5_free_principal(context, ep);
+        krb5_free_principal(context, np);
     }
 
     krb5_pac_free(context, pac);