krb5 commit: Infer name type when creating principals

Greg Hudson ghudson at mit.edu
Thu Feb 11 15:18:57 EST 2021


https://github.com/krb5/krb5/commit/457ce648b654990cb101f66255b4a87b7339efc1
commit 457ce648b654990cb101f66255b4a87b7339efc1
Author: Fraser Tweedale <ftweedal at redhat.com>
Date:   Fri Feb 5 11:00:24 2021 +1100

    Infer name type when creating principals
    
    In the krb5_parse_name() and krb5_build_principal() families, infer
    the name type if the principal is a TGS name or has first component
    WELLKNOWN.
    
    Revert commit 0d5df56ea6d4a05c31b7e513ee9ec1542a4b5dce and part of
    commit 5994d8928b8ff88751b14bc60c7d7bfce8b30e57 since that
    responsibility now lies with krb5_build_principal_ext().
    
    [ghudson at mit.edu: made editorial changes; added removal of
    no-longer-needed code; added documentation; rewrote commit message]
    
    ticket: 8983 (new)

 src/include/krb5/krb5.hin                    |   12 ++++++++++++
 src/lib/krb5/krb/Makefile.in                 |    9 ++++++---
 src/lib/krb5/krb/bld_pr_ext.c                |    3 ++-
 src/lib/krb5/krb/bld_princ.c                 |   18 +++++++++++++++++-
 src/lib/krb5/krb/deps                        |    6 +++---
 src/lib/krb5/krb/get_in_tkt.c                |    7 +------
 src/lib/krb5/krb/int-proto.h                 |    4 ++++
 src/lib/krb5/krb/parse.c                     |    4 +++-
 src/lib/krb5/krb/t_kerb.c                    |   24 +++++++++++++++++++++++-
 src/lib/krb5/krb/t_ref_kerb.out              |    4 ++++
 src/lib/krb5/krb/tgtname.c                   |   19 ++++---------------
 src/plugins/preauth/pkinit/pkinit_kdf_test.c |    4 ++++
 12 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/src/include/krb5/krb5.hin b/src/include/krb5/krb5.hin
index 6a01ed1..4af545f 100644
--- a/src/include/krb5/krb5.hin
+++ b/src/include/krb5/krb5.hin
@@ -3450,6 +3450,10 @@ krb5_rd_priv(krb5_context context, krb5_auth_context auth_context,
  * @note The realm in a Kerberos @a name cannot contain slash, colon,
  * or NULL characters.
  *
+ * Beginning with release 1.20, the name type of the principal will be inferred
+ * as @c KRB5_NT_SRV_INST or @c KRB5_NT_WELLKNOWN based on the principal name.
+ * The type will be @c KRB5_NT_PRINCIPAL if a type cannot be inferred.
+ *
  * Use krb5_free_principal() to free @a principal_out when it is no longer
  * needed.
  *
@@ -4013,6 +4017,10 @@ krb5_get_server_rcache(krb5_context context, const krb5_data *piece,
  * empty component with this function).  Call krb5_free_principal() to free
  * allocated memory for principal when it is no longer needed.
  *
+ * Beginning with release 1.20, the name type of the principal will be inferred
+ * as @c KRB5_NT_SRV_INST or @c KRB5_NT_WELLKNOWN based on the principal name.
+ * The type will be @c KRB5_NT_PRINCIPAL if a type cannot be inferred.
+ *
  * @code
  * Example of how to build principal WELLKNOWN/ANONYMOUS at R
  *     krb5_build_principal_ext(context, &principal, strlen("R"), "R",
@@ -4042,6 +4050,10 @@ krb5_build_principal_ext(krb5_context context,  krb5_principal * princ,
  *
  * Call krb5_free_principal() to free @a princ when it is no longer needed.
  *
+ * Beginning with release 1.20, the name type of the principal will be inferred
+ * as @c KRB5_NT_SRV_INST or @c KRB5_NT_WELLKNOWN based on the principal name.
+ * The type will be @c KRB5_NT_PRINCIPAL if a type cannot be inferred.
+ *
  * @note krb5_build_principal() and krb5_build_principal_alloc_va() perform the
  * same task.  krb5_build_principal() takes variadic arguments.
  * krb5_build_principal_alloc_va() takes a pre-computed @a varargs pointer.
diff --git a/src/lib/krb5/krb/Makefile.in b/src/lib/krb5/krb/Makefile.in
index f63de8a..5a163ab 100644
--- a/src/lib/krb5/krb/Makefile.in
+++ b/src/lib/krb5/krb/Makefile.in
@@ -390,8 +390,7 @@ clean-unix:: clean-libobjs
 
 COMERRLIB=$(TOPLIBD)/libcom_err.a
 
-T_WALK_RTREE_OBJS= t_walk_rtree.o walk_rtree.o tgtname.o unparse.o \
-	bld_pr_ext.o copy_data.o
+T_WALK_RTREE_OBJS= t_walk_rtree.o
 
 T_KERB_OBJS= t_kerb.o conv_princ.o unparse.o set_realm.o str_conv.o
 
@@ -403,7 +402,7 @@ T_DELTAT_OBJS= t_deltat.o deltat.o
 
 T_PAC_OBJS= t_pac.o pac.o pac_sign.o copy_data.o
 
-T_PRINC_OBJS= t_princ.o parse.o unparse.o
+T_PRINC_OBJS= t_princ.o
 
 T_ETYPES_OBJS= t_etypes.o init_ctx.o etype_list.o plugin.o
 
@@ -486,6 +485,10 @@ check-unix: $(TEST_PROGS) runenv.sh
 		parse_name tytso\\\\0/\\0 at B\\n\\t\\\\GAG \
 		parse_name tytso/\\n/\\b\\t at B\\0hacky-test \
 		parse_name \\/slash/\\@atsign/octa\\/thorpe@\\/slash\\@at\\/sign \
+		name_type host/www.krb5.test at KRB5.TEST \
+		name_type krbtg/KRB5.TEST at KRB5.TEST \
+		name_type krbtgt/KRB5.TEST at KRB5.TEST \
+		name_type WELLKNOWN/ANONYMOUS at KRB5.TEST \
 		425_conv_principal rcmd e40-po ATHENA.MIT.EDU \
 		425_conv_principal rcmd mit ATHENA.MIT.EDU \
 		425_conv_principal rcmd lithium ATHENA.MIT.EDU \
diff --git a/src/lib/krb5/krb/bld_pr_ext.c b/src/lib/krb5/krb/bld_pr_ext.c
index 10268a0..4da6f9f 100644
--- a/src/lib/krb5/krb/bld_pr_ext.c
+++ b/src/lib/krb5/krb/bld_pr_ext.c
@@ -30,6 +30,7 @@
  */
 
 #include "k5-int.h"
+#include "int-proto.h"
 
 #include <stdarg.h>
 
@@ -83,7 +84,7 @@ krb5_build_principal_ext(krb5_context context,  krb5_principal * princ,
     }
     va_end(ap);
     *princ = princ_ret;
-    princ_ret->type = KRB5_NT_UNKNOWN;
+    princ_ret->type = k5_infer_principal_type(princ_ret);
     return 0;
 
 free_out:
diff --git a/src/lib/krb5/krb/bld_princ.c b/src/lib/krb5/krb/bld_princ.c
index 8604268..ff8265a 100644
--- a/src/lib/krb5/krb/bld_princ.c
+++ b/src/lib/krb5/krb/bld_princ.c
@@ -25,6 +25,22 @@
  */
 
 #include "k5-int.h"
+#include "int-proto.h"
+
+krb5_int32
+k5_infer_principal_type(krb5_principal princ)
+{
+    /* RFC 4120 section 7.3 */
+    if (princ->length == 2 && data_eq_string(princ->data[0], KRB5_TGS_NAME))
+        return KRB5_NT_SRV_INST;
+
+    /* RFC 6111 section 3.1 */
+    if (princ->length >= 2 &&
+        data_eq_string(princ->data[0], KRB5_WELLKNOWN_NAMESTR))
+        return KRB5_NT_WELLKNOWN;
+
+    return KRB5_NT_PRINCIPAL;
+}
 
 static krb5_error_code
 build_principal_va(krb5_context context, krb5_principal princ,
@@ -65,11 +81,11 @@ build_principal_va(krb5_context context, krb5_principal princ,
     }
 
     if (!retval) {
-        princ->type = KRB5_NT_UNKNOWN;
         princ->magic = KV5M_PRINCIPAL;
         princ->realm = make_data(r, rlen);
         princ->data = data;
         princ->length = count;
+        princ->type = k5_infer_principal_type(princ);
         r = NULL;    /* take ownership */
         data = NULL; /* take ownership */
     }
diff --git a/src/lib/krb5/krb/deps b/src/lib/krb5/krb/deps
index ab3cc3e..6197a98 100644
--- a/src/lib/krb5/krb/deps
+++ b/src/lib/krb5/krb/deps
@@ -136,7 +136,7 @@ bld_pr_ext.so bld_pr_ext.po $(OUTPRE)bld_pr_ext.$(OBJEXT): \
   $(top_srcdir)/include/k5-trace.h $(top_srcdir)/include/krb5.h \
   $(top_srcdir)/include/krb5/authdata_plugin.h $(top_srcdir)/include/krb5/plugin.h \
   $(top_srcdir)/include/port-sockets.h $(top_srcdir)/include/socket-utils.h \
-  bld_pr_ext.c
+  bld_pr_ext.c int-proto.h
 bld_princ.so bld_princ.po $(OUTPRE)bld_princ.$(OBJEXT): \
   $(BUILDTOP)/include/autoconf.h $(BUILDTOP)/include/krb5/krb5.h \
   $(BUILDTOP)/include/osconf.h $(BUILDTOP)/include/profile.h \
@@ -147,7 +147,7 @@ bld_princ.so bld_princ.po $(OUTPRE)bld_princ.$(OBJEXT): \
   $(top_srcdir)/include/k5-trace.h $(top_srcdir)/include/krb5.h \
   $(top_srcdir)/include/krb5/authdata_plugin.h $(top_srcdir)/include/krb5/plugin.h \
   $(top_srcdir)/include/port-sockets.h $(top_srcdir)/include/socket-utils.h \
-  bld_princ.c
+  bld_princ.c int-proto.h
 brand.so brand.po $(OUTPRE)brand.$(OBJEXT): $(top_srcdir)/patchlevel.h \
   brand.c
 chk_trans.so chk_trans.po $(OUTPRE)chk_trans.$(OBJEXT): \
@@ -741,7 +741,7 @@ parse.so parse.po $(OUTPRE)parse.$(OBJEXT): $(BUILDTOP)/include/autoconf.h \
   $(top_srcdir)/include/k5-thread.h $(top_srcdir)/include/k5-trace.h \
   $(top_srcdir)/include/krb5.h $(top_srcdir)/include/krb5/authdata_plugin.h \
   $(top_srcdir)/include/krb5/plugin.h $(top_srcdir)/include/port-sockets.h \
-  $(top_srcdir)/include/socket-utils.h parse.c
+  $(top_srcdir)/include/socket-utils.h int-proto.h parse.c
 parse_host_string.so parse_host_string.po $(OUTPRE)parse_host_string.$(OBJEXT): \
   $(BUILDTOP)/include/autoconf.h $(BUILDTOP)/include/krb5/krb5.h \
   $(BUILDTOP)/include/osconf.h $(BUILDTOP)/include/profile.h \
diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c
index 5695187..b28b38a 100644
--- a/src/lib/krb5/krb/get_in_tkt.c
+++ b/src/lib/krb5/krb/get_in_tkt.c
@@ -506,12 +506,7 @@ build_in_tkt_name(krb5_context context,
         if (ret)
             return ret;
     }
-    /*
-     * Windows Server 2008 R2 RODC insists on TGS principal names having the
-     * right name type.
-     */
-    if (server->length == 2 && data_eq_string(server->data[0], KRB5_TGS_NAME))
-        server->type = KRB5_NT_SRV_INST;
+
     *server_out = server;
     return 0;
 }
diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h
index 2fde08d..15f4adf 100644
--- a/src/lib/krb5/krb/int-proto.h
+++ b/src/lib/krb5/krb/int-proto.h
@@ -391,4 +391,8 @@ k5_get_proxy_cred_from_kdc(krb5_context context, krb5_flags options,
 krb5_boolean
 k5_sname_wildcard_host(krb5_context context, krb5_const_principal mprinc);
 
+/* Guess the appropriate name-type for a principal based on the name. */
+krb5_int32
+k5_infer_principal_type(krb5_principal princ);
+
 #endif /* KRB5_INT_FUNC_PROTO__ */
diff --git a/src/lib/krb5/krb/parse.c b/src/lib/krb5/krb/parse.c
index 00c0ec1..953389b 100644
--- a/src/lib/krb5/krb/parse.c
+++ b/src/lib/krb5/krb/parse.c
@@ -25,6 +25,7 @@
  */
 
 #include "k5-int.h"
+#include "int-proto.h"
 
 /*
  * Scan name and allocate a shell principal with enough space in each field.
@@ -218,7 +219,8 @@ krb5_parse_name_flags(krb5_context context, const char *name,
     }
 
     princ->type = (enterprise) ? KRB5_NT_ENTERPRISE_PRINCIPAL :
-        KRB5_NT_PRINCIPAL;
+        k5_infer_principal_type(princ);
+
     princ->magic = KV5M_PRINCIPAL;
     *principal_out = princ;
     princ = NULL;
diff --git a/src/lib/krb5/krb/t_kerb.c b/src/lib/krb5/krb/t_kerb.c
index 74ac14d..e8c42ab 100644
--- a/src/lib/krb5/krb/t_kerb.c
+++ b/src/lib/krb5/krb/t_kerb.c
@@ -14,6 +14,7 @@ void test_string_to_timestamp (krb5_context, char *);
 void test_425_conv_principal (krb5_context, char *, char*, char *);
 void test_524_conv_principal (krb5_context, char *);
 void test_parse_name (krb5_context, const char *);
+void test_name_type (krb5_context, const char *);
 void test_set_realm (krb5_context, const char *, const char *);
 void usage (char *);
 
@@ -121,6 +122,21 @@ fail:
 }
 
 void
+test_name_type(krb5_context ctx, const char *name)
+{
+    krb5_error_code retval;
+    krb5_principal  princ = 0;
+
+    retval = krb5_parse_name(ctx, name, &princ);
+    if (retval) {
+        com_err("krb5_parse_name", retval, 0);
+        return;
+    }
+    printf("name_type principal(%s): %d\n", name, krb5_princ_type(ctx, princ));
+    krb5_free_principal(ctx, princ);
+}
+
+void
 test_set_realm(krb5_context ctx, const char *name, const char *realm)
 {
     krb5_error_code retval;
@@ -154,10 +170,11 @@ fail:
 void
 usage(char *progname)
 {
-    fprintf(stderr, "%s: Usage: %s 425_conv_principal <name> <inst> <realm\n",
+    fprintf(stderr, "%s: Usage: %s 425_conv_principal <name> <inst> <realm>\n",
             progname, progname);
     fprintf(stderr, "\t%s 524_conv_principal <name>\n", progname);
     fprintf(stderr, "\t%s parse_name <name>\n", progname);
+    fprintf(stderr, "\t%s name_type <name>\n", progname);
     fprintf(stderr, "\t%s set_realm <name> <realm>\n", progname);
     fprintf(stderr, "\t%s string_to_timestamp <time>\n", progname);
     exit(1);
@@ -198,6 +215,11 @@ main(int argc, char **argv)
             if (!argc) usage(progname);
             name = *argv;
             test_parse_name(ctx, name);
+        } else if (strcmp(*argv, "name_type") == 0) {
+            argc--; argv++;
+            if (!argc) usage(progname);
+            name = *argv;
+            test_name_type(ctx, name);
         } else if (strcmp(*argv, "set_realm") == 0) {
             argc--; argv++;
             if (!argc) usage(progname);
diff --git a/src/lib/krb5/krb/t_ref_kerb.out b/src/lib/krb5/krb/t_ref_kerb.out
index 08a5334..8daff9e 100644
--- a/src/lib/krb5/krb/t_ref_kerb.out
+++ b/src/lib/krb5/krb/t_ref_kerb.out
@@ -7,6 +7,10 @@ parsed (and unparsed) principal(tytso/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t at FO
 parsed (and unparsed) principal(tytso\\0/\0 at B\n\t\\GAG): MATCH
 parsed (and unparsed) principal(tytso/\n/\b\t at B\0hacky-test): MATCH
 parsed (and unparsed) principal(\/slash/\@atsign/octa\/thorpe@\/slash\@at\/sign): MATCH
+name_type principal(host/www.krb5.test at KRB5.TEST): 1
+name_type principal(krbtg/KRB5.TEST at KRB5.TEST): 1
+name_type principal(krbtgt/KRB5.TEST at KRB5.TEST): 2
+name_type principal(WELLKNOWN/ANONYMOUS at KRB5.TEST): 11
 425_converted principal(rcmd, e40-po, ATHENA.MIT.EDU): 'host/e40-po.mit.edu at ATHENA.MIT.EDU'
 425_converted principal(rcmd, mit, ATHENA.MIT.EDU): 'host/mit.edu at ATHENA.MIT.EDU'
 425_converted principal(rcmd, lithium, ATHENA.MIT.EDU): 'host/lithium.lcs.mit.edu at ATHENA.MIT.EDU'
diff --git a/src/lib/krb5/krb/tgtname.c b/src/lib/krb5/krb/tgtname.c
index 0ffeb17..6fc2fae 100644
--- a/src/lib/krb5/krb/tgtname.c
+++ b/src/lib/krb5/krb/tgtname.c
@@ -30,19 +30,8 @@
 krb5_error_code
 krb5int_tgtname(krb5_context context, const krb5_data *server, const krb5_data *client, krb5_principal *tgtprinc)
 {
-    krb5_error_code ret;
-
-    ret = krb5_build_principal_ext(context, tgtprinc, client->length, client->data,
-                                   KRB5_TGS_NAME_SIZE, KRB5_TGS_NAME,
-                                   server->length, server->data,
-                                   0);
-    if (ret)
-        return ret;
-    /*
-     * Windows Server 2008 R2 RODC insists on TGS principal names having the
-     * right name type.
-     */
-    (*tgtprinc)->type = KRB5_NT_SRV_INST;
-
-    return ret;
+    return krb5_build_principal_ext(context, tgtprinc,
+                                    client->length, client->data,
+                                    KRB5_TGS_NAME_SIZE, KRB5_TGS_NAME,
+                                    server->length, server->data, 0);
 }
diff --git a/src/plugins/preauth/pkinit/pkinit_kdf_test.c b/src/plugins/preauth/pkinit/pkinit_kdf_test.c
index 7acbd0d..5f60de9 100644
--- a/src/plugins/preauth/pkinit/pkinit_kdf_test.c
+++ b/src/plugins/preauth/pkinit/pkinit_kdf_test.c
@@ -112,6 +112,10 @@ main(int argc, char **argv)
         goto cleanup;
     }
 
+    /* The test vectors in RFC 8636 implicitly use NT-PRINCIPAL names. */
+    u_principal->type = KRB5_NT_PRINCIPAL;
+    v_principal->type = KRB5_NT_PRINCIPAL;
+
     /* set-up the as_req and and pk_as_rep data */
     memset(twenty_as, 0xaa, sizeof(twenty_as));
     memset(eighteen_bs, 0xbb, sizeof(eighteen_bs));


More information about the cvs-krb5 mailing list