From 1cede09a0b772e99beac6fcc440a917c9e8b183a Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Wed, 29 Dec 2021 11:59:59 -0600 Subject: [PATCH] krb5: Add support for AD-KDC-ISSUED --- lib/asn1/krb5.asn1 | 8 ++++-- lib/krb5/rd_req.c | 6 ++++ lib/krb5/ticket.c | 70 +++++++++++++++++++++++++++++++++++----------- 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/lib/asn1/krb5.asn1 b/lib/asn1/krb5.asn1 index e604eb027..f442d3739 100644 --- a/lib/asn1/krb5.asn1 +++ b/lib/asn1/krb5.asn1 @@ -462,8 +462,9 @@ PrincipalNameAttrSrc ::= CHOICE { enc-ticket-part [1] EncTicketPart -- minus session key } PrincipalNameAttrs ::= SEQUENCE { + -- True if this name was authenticated via an AP-REQ or a KDC-REP authenticated [0] BOOLEAN, - -- These are compiled from the Ticket and Authenticator: + -- These are compiled from the Ticket, KDC-REP, and/or Authenticator source [1] PrincipalNameAttrSrc OPTIONAL, authenticator-ad [2] AuthorizationData OPTIONAL, -- For the server on the client side we should keep track of the @@ -472,7 +473,10 @@ PrincipalNameAttrs ::= SEQUENCE { -- We don't learn much more about the server from the KDC. peer-realm [3] Realm OPTIONAL, transited [4] TransitedEncoding OPTIONAL, - pac-verified [5] BOOLEAN + -- True if the PAC was verified + pac-verified [5] BOOLEAN, + -- True if any AD-KDC-ISSUEDs in the Ticket were validated + kdc-issued-verified [6] BOOLEAN -- TODO: Add requested attributes, for gss_set_name_attribute(), which -- should cause corresponding authz-data elements to be added to -- any TGS-REQ or to the AP-REQ's Authenticator as appropriate. diff --git a/lib/krb5/rd_req.c b/lib/krb5/rd_req.c index ce18d715a..2bd45a369 100644 --- a/lib/krb5/rd_req.c +++ b/lib/krb5/rd_req.c @@ -1037,6 +1037,12 @@ krb5_rd_req_ctx(krb5_context context, goto out; } + ret = krb5_ticket_get_authorization_data_type(context, o->ticket, + KRB5_AUTHDATA_KDC_ISSUED, + NULL); + if (ret == 0) + o->ticket->client->nameattrs->kdc_issued_verified = 1; + /* If there is a PAC, verify its server signature */ if (inctx == NULL || inctx->check_pac) { krb5_pac pac; diff --git a/lib/krb5/ticket.c b/lib/krb5/ticket.c index 631fb9480..e2f2ab208 100644 --- a/lib/krb5/ticket.c +++ b/lib/krb5/ticket.c @@ -204,13 +204,38 @@ krb5_ticket_get_flags(krb5_context context, return TicketFlags2int(ticket->ticket.flags); } +/* + * Find an authz-data element in the given `ad'. If `failp', then validate any + * containing AD-KDC-ISSUED's keyed checksum with the `sessionkey' (if given). + * + * All AD-KDC-ISSUED will be validated (if requested) even when `type' is + * `KRB5_AUTHDATA_KDC_ISSUED'. + * + * Only the first matching element will be output (via `data'). + * + * Note that all AD-KDC-ISSUEDs found while traversing the authz-data will be + * validated, though only the first one will be returned. + * + * XXX We really need a better interface though. First, forget AD-AND-OR -- + * just remove it. Second, probably forget AD-KDC-ISSUED, but still, between + * that, the PAC, and the CAMMAC, we need an interface that can: + * + * a) take the derived keys instead of the service key or the session key, + * b) can indicate whether the element was marked critical, + * c) can indicate whether the element was authenticated to the KDC, + * d) can iterate over all the instances found (if more than one is found). + * + * Also, we need to know here if the authz-data is from a Ticket or from an + * Authenticator -- if the latter then we must refuse to find AD-KDC-ISSUED / + * PAC / CAMMAC or anything of the sort, ever. + */ static int find_type_in_ad(krb5_context context, int type, - krb5_data *data, + krb5_data *data, /* optional */ krb5_boolean *found, - krb5_boolean failp, - krb5_keyblock *sessionkey, + krb5_boolean failp, /* validate AD-KDC-ISSUED */ + krb5_keyblock *sessionkey, /* ticket session key */ const AuthorizationData *ad, int level) { @@ -233,14 +258,19 @@ find_type_in_ad(krb5_context context, */ for (i = 0; i < ad->len; i++) { if (!*found && ad->val[i].ad_type == type) { - ret = der_copy_octet_string(&ad->val[i].ad_data, data); - if (ret) { - krb5_set_error_message(context, ret, - N_("malloc: out of memory", "")); - goto out; - } + if (data) { + ret = der_copy_octet_string(&ad->val[i].ad_data, data); + if (ret) { + krb5_set_error_message(context, ret, + N_("malloc: out of memory", "")); + goto out; + } + } *found = TRUE; - continue; + if (type != KRB5_AUTHDATA_KDC_ISSUED || + !failp || !sessionkey || !sessionkey->keyvalue.length) + continue; + /* else go on to validate the AD-KDC-ISSUED's keyed checksum */ } switch (ad->val[i].ad_type) { case KRB5_AUTHDATA_IF_RELEVANT: { @@ -263,7 +293,6 @@ find_type_in_ad(krb5_context context, goto out; break; } -#if 0 /* XXX test */ case KRB5_AUTHDATA_KDC_ISSUED: { AD_KDCIssued child; @@ -278,7 +307,7 @@ find_type_in_ad(krb5_context context, ret); goto out; } - if (failp) { + if (failp && sessionkey && sessionkey->keyvalue.length) { krb5_boolean valid; krb5_data buf; size_t len; @@ -306,7 +335,12 @@ find_type_in_ad(krb5_context context, free_AD_KDCIssued(&child); goto out; } - } + } else if (failp) { + krb5_clear_error_message(context); + ret = ENOENT; + free_AD_KDCIssued(&child); + goto out; + } ret = find_type_in_ad(context, type, data, found, failp, sessionkey, &child.elements, level + 1); free_AD_KDCIssued(&child); @@ -314,7 +348,6 @@ find_type_in_ad(krb5_context context, goto out; break; } -#endif case KRB5_AUTHDATA_AND_OR: if (!failp) break; @@ -338,7 +371,8 @@ find_type_in_ad(krb5_context context, out: if (ret) { if (*found) { - krb5_data_free(data); + if (data) + krb5_data_free(data); *found = 0; } } @@ -355,7 +389,8 @@ _krb5_get_ad(krb5_context context, krb5_boolean found = FALSE; krb5_error_code ret; - krb5_data_zero(data); + if (data) + krb5_data_zero(data); if (ad == NULL) { krb5_set_error_message(context, ENOENT, @@ -399,7 +434,8 @@ krb5_ticket_get_authorization_data_type(krb5_context context, krb5_error_code ret; krb5_boolean found = FALSE; - krb5_data_zero(data); + if (data) + krb5_data_zero(data); ad = ticket->ticket.authorization_data; if (ticket->ticket.authorization_data == NULL) {