From 8e7c7209e83086dc851b5da2fe42b2944d41c0df Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Tue, 23 Mar 2021 12:07:41 -0500 Subject: [PATCH] kdc: Add param to derive max_life from client cert This adds a KDC configuration parameter that can be used to indicate that a PKINIT client's certificate's notAfter overrides the client principal's HDB entry's max_life. This parameter is a relative time parameter, and it enables this only if set to a non-zero value (defaults to zero). The value of this parameter caps the max_life inferred from the certificate. --- kdc/kdc_locl.h | 2 ++ kdc/kerberos5.c | 28 ++++++++++++++++++++++++++-- kdc/pkinit.c | 6 ++++++ lib/krb5/krb5.conf.5 | 14 ++++++++++++++ tests/kdc/Makefile.am | 8 ++++++++ tests/kdc/check-pkinit.in | 25 +++++++++++++++++++++++++ tests/kdc/krb5-pkinit.conf.in | 1 + 7 files changed, 82 insertions(+), 2 deletions(-) diff --git a/kdc/kdc_locl.h b/kdc/kdc_locl.h index faa89f9b9..d4427e36c 100644 --- a/kdc/kdc_locl.h +++ b/kdc/kdc_locl.h @@ -89,6 +89,8 @@ struct astgs_request_desc { krb5_principal server_princ; hdb_entry_ex *server; + krb5_timestamp pa_endtime; + krb5_timestamp pa_max_life; krb5_crypto armor_crypto; diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 9decc9dd2..9d38d8524 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -464,6 +464,7 @@ pa_pkinit_validate(astgs_request_t r, const PA_DATA *pa) pk_client_params *pkp = NULL; char *client_cert = NULL; krb5_error_code ret; + krb5_timestamp max_life; ret = _kdc_pk_rd_padata(r, pa, &pkp); if (ret || pkp == NULL) { @@ -480,6 +481,14 @@ pa_pkinit_validate(astgs_request_t r, const PA_DATA *pa) goto out; } + max_life = krb5_config_get_time_default(r->context, NULL, 0, "kdc", + "pkinit_ticket_max_life_from_cert", + NULL); + if (max_life > 0) { + r->pa_max_life = max_life; + r->pa_endtime = _kdc_pk_endtime(pkp); + } + _kdc_r_log(r, 4, "PKINIT pre-authentication succeeded -- %s using %s", r->cname, client_cert); free(client_cert); @@ -1938,6 +1947,12 @@ _kdc_as_rep(astgs_request_t r) goto out; } default: + /* + * We could have an option to synthetically construct an HDB entry for + * the client from its certificate, if it used PKINIT and its cert has + * the PKINIT SAN. We could have a default HDB entry for this case to + * provide default field values. + */ msg = krb5_get_error_message(context, ret); kdc_log(context, config, 4, "UNKNOWN -- %s: %s", r->cname, msg); krb5_free_error_message(context, msg); @@ -2216,9 +2231,18 @@ _kdc_as_rep(astgs_request_t r) /* be careful not overflowing */ - if(r->client->entry.max_life) + if (r->client->entry.max_life) { t = start + min(t - start, *r->client->entry.max_life); - if(r->server->entry.max_life) + if (r->pa_max_life > 0 && + r->pa_endtime > 0 && + t < r->pa_endtime && + r->pa_max_life > *r->client->entry.max_life) + t = start + min(r->pa_endtime - start, r->pa_max_life); + } else if (r->pa_max_life > 0 && + r->pa_endtime > 0 && + t < r->pa_endtime) + t = start + min(r->pa_endtime - start, r->pa_max_life); + if (r->server->entry.max_life) t = start + min(t - start, *r->server->entry.max_life); #if 0 t = min(t, start + realm->max_life); diff --git a/kdc/pkinit.c b/kdc/pkinit.c index 9fe316f46..1d065fa46 100644 --- a/kdc/pkinit.c +++ b/kdc/pkinit.c @@ -799,6 +799,12 @@ out: return ret; } +krb5_timestamp +_kdc_pk_endtime(pk_client_params *pkp) +{ + return hx509_cert_get_notAfter(pkp->cert); +} + /* * */ diff --git a/lib/krb5/krb5.conf.5 b/lib/krb5/krb5.conf.5 index e9f1021fa..c9fc7a66c 100644 --- a/lib/krb5/krb5.conf.5 +++ b/lib/krb5/krb5.conf.5 @@ -840,6 +840,20 @@ Defaults to .It Li pkinit_dh_min_bits = Va NUMBER Minimum acceptable modular Diffie-Hellman public key size in bits. +.It Li pkinit_ticket_max_life_from_cert = Va TIME +If set, this will override the +.Va max_life +attribute of the client principal's HDB record with the +.Va notAfter +of the client's certificate minus the current time, bounded to +the given relative +.Va TIME . +As usual, +.Va TIME +can be given as a number followed by a unit, such as +.Dq 2d +for +.Dq two days . .It Li historical_anon_realm = Va boolean Enables pre-7.0 non-RFC-comformant KDC behavior. With this option set to diff --git a/tests/kdc/Makefile.am b/tests/kdc/Makefile.am index 4cc8032cc..4510f0f9e 100644 --- a/tests/kdc/Makefile.am +++ b/tests/kdc/Makefile.am @@ -53,6 +53,7 @@ restport = 49192 restport2 = 49193 ipropport = 49194 ipropport2 = 49195 +pkinit_ticket_max_life_from_cert = 0 if HAVE_DLOPEN do_dlopen = -e 's,[@]DLOPEN[@],true,g' @@ -76,6 +77,7 @@ do_subst = $(heim_verbose)sed $(do_dlopen) \ -e 's,[@]objdir[@],$(top_builddir)/tests/kdc,g' \ -e 's,[@]top_builddir[@],$(top_builddir),g' \ -e 's,[@]db_type[@],$(db_type),g' \ + -e 's,[@]max_life_from_cert[@],$(pkinit_ticket_max_life_from_cert),g' \ -e 's,[@]ENABLE_AFS_STRING_TO_KEY[@],$(ENABLE_AFS_STRING_TO_KEY),' \ -e 's,[@]EGREP[@],$(EGREP),g' @@ -282,6 +284,12 @@ krb5-pkinit.conf: krb5-pkinit.conf.in Makefile $(do_subst) -e 's,[@]w2k[@],no,g' < $(srcdir)/krb5-pkinit.conf.in > krb5-pkinit.conf.tmp && \ mv krb5-pkinit.conf.tmp krb5-pkinit.conf +krb5-pkinit2.conf : pkinit_ticket_max_life_from_cert = 30d + +krb5-pkinit2.conf: krb5-pkinit.conf.in Makefile + $(do_subst) -e 's,[@]w2k[@],no,g' < $(srcdir)/krb5-pkinit.conf.in > krb5-pkinit2.conf.tmp && \ + mv krb5-pkinit2.conf.tmp krb5-pkinit2.conf + krb5-bx509.conf: krb5-bx509.conf.in Makefile $(do_subst) -e 's,[@]w2k[@],no,g' < $(srcdir)/krb5-bx509.conf.in > krb5-bx509.conf.tmp && \ mv krb5-bx509.conf.tmp krb5-bx509.conf diff --git a/tests/kdc/check-pkinit.in b/tests/kdc/check-pkinit.in index 8191544aa..55d7073ba 100644 --- a/tests/kdc/check-pkinit.in +++ b/tests/kdc/check-pkinit.in @@ -55,6 +55,8 @@ keyfile="${hx509_data}/key.der" keyfile2="${hx509_data}/key2.der" kinit="${kinit} -c $cache ${afs_no_afslog}" +klistjson="${klist} --json -c $cache" +klistplain="${klist} -c $cache" klist="${klist} --hidden -v -c $cache" kgetcred="${kgetcred} -c $cache" kdestroy="${kdestroy} -c $cache ${afs_no_unlog}" @@ -95,6 +97,7 @@ ${kadmin} \ --realm-max-renewable-life=1month \ ${R} || exit 1 +${kadmin} modify --max-ticket-life=5d krbtgt/${R}@${R} || exit 1 ${kadmin} add -p foo --use-defaults foo@${R} || exit 1 ${kadmin} add -p bar --use-defaults bar@${R} || exit 1 ${kadmin} add -p baz --use-defaults baz@${R} || exit 1 @@ -150,6 +153,7 @@ ${hxtool} issue-certificate \ --type="pkinit-client" \ --pk-init-principal="bar@TEST.H5L.SE" \ --req="PKCS10:req-pkinit.der" \ + --lifetime=7d \ --certificate="FILE:pkinit.crt" || exit 1 echo "issue user 2 certificate (no san)" @@ -198,6 +202,27 @@ ${kinit} -C FILE:${base}/pkinit.crt,${keyfile2} bar@${R} || \ { ec=1 ; eval "${testfailed}"; } ${kgetcred} ${server}@${R} || { ec=1 ; eval "${testfailed}"; } ${klist} +${kdestroy} + +echo "Restarting kdc (${kdcpid}) for longer max_life test" +sh ${leaks_kill} kdc $kdcpid || ec=1 +KRB5_CONFIG="${objdir}/krb5-pkinit2.conf" +${kdc} --detach --testing || { echo "kdc failed to start"; exit 1; } +kdcpid=`getpid kdc` + +echo "Trying pk-init (principal in cert; longer max_life)"; > messages.log +base="${objdir}" +${kinit} --lifetime=5d -C FILE:${base}/pkinit.crt,${keyfile2} bar@${R} || \ + { ec=1 ; eval "${testfailed}"; } +${kgetcred} ${server}@${R} || { ec=1 ; eval "${testfailed}"; } +${klist} +if jq --version >/dev/null 2>&1 && jq -ne true >/dev/null 2>&1; then + ${klistjson} | + jq -e '(((.tickets[0].Expires| + strptime("%b %d %H:%M:%S %Y")|mktime) - now) / 86400) | + (floor < 4)' >/dev/null && + { ec=1 ; eval "${testfailed}"; } +fi echo "Check kx509 certificate acquisition" ${kx509} -s || { ec=1 ; eval "${testfailed}"; } diff --git a/tests/kdc/krb5-pkinit.conf.in b/tests/kdc/krb5-pkinit.conf.in index 6e9fa0266..1795ce524 100644 --- a/tests/kdc/krb5-pkinit.conf.in +++ b/tests/kdc/krb5-pkinit.conf.in @@ -19,6 +19,7 @@ pkinit_identity = FILE:@objdir@/kdc.crt,@srcdir@/../../lib/hx509/data/key2.der pkinit_anchors = FILE:@objdir@/ca.crt pkinit_mappings_file = @srcdir@/pki-mapping + pkinit_ticket_max_life_from_cert = @max_life_from_cert@ plugin_dir = @objdir@/../../kdc/.libs