krb5 commit: Infer name type when creating principals

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/457ce648b654990cb101f66255b4a87b7339efc1
commit 457ce648b654990cb101f66255b4a87b7339efc1
Author: Fraser Tweedale <[email protected]>
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().
    
    [[email protected]: 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@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@B\\n\\t\\\\GAG \
 		parse_name tytso/\\n/\\b\\t@B\\0hacky-test \
 		parse_name \\/slash/\\@atsign/octa\\/thorpe@\\/slash\\@at\\/sign \
+		name_type host/[email protected] \
+		name_type krbtg/[email protected] \
+		name_type krbtgt/[email protected] \
+		name_type WELLKNOWN/[email protected] \
 		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@FO
 parsed (and unparsed) principal(tytso\\0/\0@B\n\t\\GAG): MATCH
 parsed (and unparsed) principal(tytso/\n/\b\t@B\0hacky-test): MATCH
 parsed (and unparsed) principal(\/slash/\@atsign/octa\/thorpe@\/slash\@at\/sign): MATCH
+name_type principal(host/[email protected]): 1
+name_type principal(krbtg/[email protected]): 1
+name_type principal(krbtgt/[email protected]): 2
+name_type principal(WELLKNOWN/[email protected]): 11
 425_converted principal(rcmd, e40-po, ATHENA.MIT.EDU): 'host/[email protected]'
 425_converted principal(rcmd, mit, ATHENA.MIT.EDU): 'host/[email protected]'
 425_converted principal(rcmd, lithium, ATHENA.MIT.EDU): 'host/[email protected]'
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));
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.