From 457ce648b654990cb101f66255b4a87b7339efc1 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Fri, 5 Feb 2021 11:00:24 +1100 Subject: [PATCH] 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@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 6a01ed10a9..4af545fc0e 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 f63de8ab83..5a163ab622 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/www.krb5.test@KRB5.TEST \ + name_type krbtg/KRB5.TEST@KRB5.TEST \ + name_type krbtgt/KRB5.TEST@KRB5.TEST \ + name_type WELLKNOWN/ANONYMOUS@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 10268a0ff0..4da6f9fb5b 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 @@ -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 8604268ceb..ff8265acf1 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 ab3cc3e5a4..6197a9839e 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 569518722d..b28b38a159 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 2fde08d1ae..15f4adf285 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 00c0ec1b17..953389b69d 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 74ac14d9ab..e8c42ab5d3 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 *); @@ -120,6 +121,21 @@ fail: krb5_free_principal(ctx, princ2); } +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) { @@ -154,10 +170,11 @@ fail: void usage(char *progname) { - fprintf(stderr, "%s: Usage: %s 425_conv_principal \n", progname, progname); fprintf(stderr, "\t%s 524_conv_principal \n", progname); fprintf(stderr, "\t%s parse_name \n", progname); + fprintf(stderr, "\t%s name_type \n", progname); fprintf(stderr, "\t%s set_realm \n", progname); fprintf(stderr, "\t%s string_to_timestamp