From 805ea5e0a0dba18c573bff5edd118764ec9f3f31 Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Thu, 23 Dec 2021 19:45:32 +1100 Subject: [PATCH] kdc: use rep/et/ek fields in astgs_request_t Use rep/et/ek fields in astgs_request_t that were previously present but not globally used. --- kdc/kerberos5.c | 55 +++++++-------- kdc/krb5tgs.c | 173 ++++++++++++++++++++++++------------------------ 2 files changed, 113 insertions(+), 115 deletions(-) diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 1dffc3dc5..aad13f6d0 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -1082,7 +1082,6 @@ krb5_error_code _kdc_encode_reply(krb5_context context, krb5_kdc_configuration *config, astgs_request_t r, uint32_t nonce, - KDC_REP *rep, EncTicketPart *et, EncKDCRepPart *ek, krb5_enctype etype, int skvno, const EncryptionKey *skey, int ckvno, @@ -1095,6 +1094,9 @@ _kdc_encode_reply(krb5_context context, size_t len = 0; krb5_error_code ret; krb5_crypto crypto; + KDC_REP *rep = &r->rep; + EncTicketPart *et = &r->et; + EncKDCRepPart *ek = &r->ek; ASN1_MALLOC_ENCODE(EncTicketPart, buf, buf_size, et, &len, ret); if(ret) { @@ -2085,7 +2087,7 @@ _kdc_as_rep(astgs_request_t r) KDC_REQ *req = &r->req; const char *from = r->from; KDC_REQ_BODY *b = NULL; - AS_REP rep; + KDC_REP *rep = &r->rep; KDCOptions f; krb5_enctype setype; krb5_error_code ret = 0; @@ -2098,7 +2100,7 @@ _kdc_as_rep(astgs_request_t r) hdb_entry_ex *krbtgt = NULL; Key *krbtgt_key; - memset(&rep, 0, sizeof(rep)); + memset(rep, 0, sizeof(*rep)); /* * Look for FAST armor and unwrap @@ -2439,46 +2441,46 @@ _kdc_as_rep(astgs_request_t r) * Build reply */ - rep.pvno = 5; - rep.msg_type = krb_as_rep; + rep->pvno = 5; + rep->msg_type = krb_as_rep; if (!config->historical_anon_realm && _kdc_is_anonymous(r->context, r->client_princ)) { Realm anon_realm = KRB5_ANON_REALM; - ret = copy_Realm(&anon_realm, &rep.crealm); + ret = copy_Realm(&anon_realm, &rep->crealm); } else if (f.canonicalize || r->client->entry.flags.force_canonicalize) - ret = copy_Realm(&r->client->entry.principal->realm, &rep.crealm); + ret = copy_Realm(&r->client->entry.principal->realm, &rep->crealm); else - ret = copy_Realm(&r->client_princ->realm, &rep.crealm); + ret = copy_Realm(&r->client_princ->realm, &rep->crealm); if (ret) goto out; if (r->et.flags.anonymous) - ret = _kdc_make_anonymous_principalname(&rep.cname); + ret = _kdc_make_anonymous_principalname(&rep->cname); else if (f.canonicalize || r->client->entry.flags.force_canonicalize) - ret = _krb5_principal2principalname(&rep.cname, r->client->entry.principal); + ret = _krb5_principal2principalname(&rep->cname, r->client->entry.principal); else - ret = _krb5_principal2principalname(&rep.cname, r->client_princ); + ret = _krb5_principal2principalname(&rep->cname, r->client_princ); if (ret) goto out; - rep.ticket.tkt_vno = 5; + rep->ticket.tkt_vno = 5; if (f.canonicalize || r->server->entry.flags.force_canonicalize) - ret = copy_Realm(&r->server->entry.principal->realm, &rep.ticket.realm); + ret = copy_Realm(&r->server->entry.principal->realm, &rep->ticket.realm); else - ret = copy_Realm(&r->server_princ->realm, &rep.ticket.realm); + ret = copy_Realm(&r->server_princ->realm, &rep->ticket.realm); if (ret) goto out; if (f.canonicalize || r->server->entry.flags.force_canonicalize) - _krb5_principal2principalname(&rep.ticket.sname, + _krb5_principal2principalname(&rep->ticket.sname, r->server->entry.principal); else - _krb5_principal2principalname(&rep.ticket.sname, + _krb5_principal2principalname(&rep->ticket.sname, r->server_princ); /* java 1.6 expects the name to be the same type, lets allow that * uncomplicated name-types. */ #define CNT(sp,t) (((sp)->sname->name_type) == KRB5_NT_##t) if (CNT(b, UNKNOWN) || CNT(b, PRINCIPAL) || CNT(b, SRV_INST) || CNT(b, SRV_HST) || CNT(b, SRV_XHST)) - rep.ticket.sname.name_type = b->sname->name_type; + rep->ticket.sname.name_type = b->sname->name_type; #undef CNT r->et.flags.initial = 1; @@ -2513,10 +2515,10 @@ _kdc_as_rep(astgs_request_t r) } } - ret = copy_PrincipalName(&rep.cname, &r->et.cname); + ret = copy_PrincipalName(&rep->cname, &r->et.cname); if (ret) goto out; - ret = copy_Realm(&rep.crealm, &r->et.crealm); + ret = copy_Realm(&rep->crealm, &r->et.crealm); if (ret) goto out; @@ -2648,10 +2650,10 @@ _kdc_as_rep(astgs_request_t r) ALLOC(r->ek.renew_till); *r->ek.renew_till = *r->et.renew_till; } - ret = copy_Realm(&rep.ticket.realm, &r->ek.srealm); + ret = copy_Realm(&rep->ticket.realm, &r->ek.srealm); if (ret) goto out; - ret = copy_PrincipalName(&rep.ticket.sname, &r->ek.sname); + ret = copy_PrincipalName(&rep->ticket.sname, &r->ek.sname); if (ret) goto out; if(r->et.caddr){ @@ -2685,12 +2687,12 @@ _kdc_as_rep(astgs_request_t r) if (r->outpadata.len) { - ALLOC(rep.padata); - if (rep.padata == NULL) { + ALLOC(rep->padata); + if (rep->padata == NULL) { ret = ENOMEM; goto out; } - ret = copy_METHOD_DATA(&r->outpadata, rep.padata); + ret = copy_METHOD_DATA(&r->outpadata, rep->padata); if (ret) goto out; } @@ -2749,8 +2751,7 @@ _kdc_as_rep(astgs_request_t r) */ ret = _kdc_encode_reply(r->context, config, - r, req->req_body.nonce, - &rep, &r->et, &r->ek, setype, + r, req->req_body.nonce, setype, r->server->entry.kvno, &skey->key, pa_used_flag_isset(r, PA_REPLACE_REPLY_KEY) ? 0 : r->client->entry.kvno, 0, &r->e_text, r->reply); @@ -2767,7 +2768,7 @@ _kdc_as_rep(astgs_request_t r) } out: - free_AS_REP(&rep); + free_AS_REP(&r->rep); /* * In case of a non proxy error, build an error message. diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index 3dd29b1e0..598855231 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -606,27 +606,23 @@ tgs_make_reply(astgs_request_t r, KDC_REQ_BODY *b = &r->req.req_body; const char **e_text = &r->e_text; krb5_data *reply = r->reply; - KDC_REP rep; - EncKDCRepPart ek; - EncTicketPart et; + KDC_REP *rep = &r->rep; + EncTicketPart *et = &r->et; + EncKDCRepPart *ek = &r->ek; KDCOptions f = b->kdc_options; krb5_error_code ret; int is_weak = 0; - memset(&rep, 0, sizeof(rep)); - memset(&et, 0, sizeof(et)); - memset(&ek, 0, sizeof(ek)); + rep->pvno = 5; + rep->msg_type = krb_tgs_rep; - rep.pvno = 5; - rep.msg_type = krb_tgs_rep; - - et.authtime = tgt->authtime; + et->authtime = tgt->authtime; _kdc_fix_time(&b->till); - et.endtime = min(tgt->endtime, *b->till); - ALLOC(et.starttime); - *et.starttime = kdc_time; + et->endtime = min(tgt->endtime, *b->till); + ALLOC(et->starttime); + *et->starttime = kdc_time; - ret = check_tgs_flags(r, b, tgt_name, tgt, &et); + ret = check_tgs_flags(r, b, tgt_name, tgt, et); if(ret) goto out; @@ -656,18 +652,18 @@ tgs_make_reply(astgs_request_t r, !((GLOBAL_ALLOW_PER_PRINCIPAL && PRINCIPAL_ALLOW_DISABLE_TRANSITED_CHECK(server)) || GLOBAL_ALLOW_DISABLE_TRANSITED_CHECK), - &tgt->transited, &et, + &tgt->transited, et, krb5_principal_get_realm(r->context, client_principal), krb5_principal_get_realm(r->context, server->entry.principal), tgt_realm); if(ret) goto out; - ret = copy_Realm(&server_principal->realm, &rep.ticket.realm); + ret = copy_Realm(&server_principal->realm, &rep->ticket.realm); if (ret) goto out; - _krb5_principal2principalname(&rep.ticket.sname, server_principal); - ret = copy_Realm(&tgt_name->realm, &rep.crealm); + _krb5_principal2principalname(&rep->ticket.sname, server_principal); + ret = copy_Realm(&tgt_name->realm, &rep->crealm); if (ret) goto out; @@ -677,86 +673,86 @@ tgs_make_reply(astgs_request_t r, * whilst the TGT flag check below is superfluous, it is included in * order to follow the specification to its letter. */ - if (et.flags.anonymous && !tgt->flags.anonymous) - _kdc_make_anonymous_principalname(&rep.cname); + if (et->flags.anonymous && !tgt->flags.anonymous) + _kdc_make_anonymous_principalname(&rep->cname); else - ret = copy_PrincipalName(&tgt_name->name, &rep.cname); + ret = copy_PrincipalName(&tgt_name->name, &rep->cname); if (ret) goto out; - rep.ticket.tkt_vno = 5; + rep->ticket.tkt_vno = 5; - ek.caddr = et.caddr; + ek->caddr = et->caddr; { time_t life; - life = et.endtime - *et.starttime; + life = et->endtime - *et->starttime; if(client && client->entry.max_life) life = min(life, *client->entry.max_life); if(server->entry.max_life) life = min(life, *server->entry.max_life); - et.endtime = *et.starttime + life; + et->endtime = *et->starttime + life; } if(f.renewable_ok && tgt->flags.renewable && - et.renew_till == NULL && et.endtime < *b->till && + et->renew_till == NULL && et->endtime < *b->till && tgt->renew_till != NULL) { - et.flags.renewable = 1; - ALLOC(et.renew_till); - *et.renew_till = *b->till; + et->flags.renewable = 1; + ALLOC(et->renew_till); + *et->renew_till = *b->till; } - if(et.renew_till){ + if(et->renew_till){ time_t renew; - renew = *et.renew_till - *et.starttime; + renew = *et->renew_till - *et->starttime; if(client && client->entry.max_renew) renew = min(renew, *client->entry.max_renew); if(server->entry.max_renew) renew = min(renew, *server->entry.max_renew); - *et.renew_till = *et.starttime + renew; + *et->renew_till = *et->starttime + renew; } - if(et.renew_till){ - *et.renew_till = min(*et.renew_till, *tgt->renew_till); - *et.starttime = min(*et.starttime, *et.renew_till); - et.endtime = min(et.endtime, *et.renew_till); + if(et->renew_till){ + *et->renew_till = min(*et->renew_till, *tgt->renew_till); + *et->starttime = min(*et->starttime, *et->renew_till); + et->endtime = min(et->endtime, *et->renew_till); } - *et.starttime = min(*et.starttime, et.endtime); + *et->starttime = min(*et->starttime, et->endtime); - if(*et.starttime == et.endtime){ + if(*et->starttime == et->endtime){ ret = KRB5KDC_ERR_NEVER_VALID; goto out; } - if(et.renew_till && et.endtime == *et.renew_till){ - free(et.renew_till); - et.renew_till = NULL; - et.flags.renewable = 0; + if(et->renew_till && et->endtime == *et->renew_till){ + free(et->renew_till); + et->renew_till = NULL; + et->flags.renewable = 0; } - et.flags.pre_authent = tgt->flags.pre_authent; - et.flags.hw_authent = tgt->flags.hw_authent; - et.flags.ok_as_delegate = server->entry.flags.ok_as_delegate; + et->flags.pre_authent = tgt->flags.pre_authent; + et->flags.hw_authent = tgt->flags.hw_authent; + et->flags.ok_as_delegate = server->entry.flags.ok_as_delegate; /* See MS-KILE 3.3.5.1 */ if (!server->entry.flags.forwardable) - et.flags.forwardable = 0; + et->flags.forwardable = 0; if (!server->entry.flags.proxiable) - et.flags.proxiable = 0; + et->flags.proxiable = 0; if (auth_data) { unsigned int i = 0; /* XXX check authdata */ - if (et.authorization_data == NULL) { - et.authorization_data = calloc(1, sizeof(*et.authorization_data)); - if (et.authorization_data == NULL) { + if (et->authorization_data == NULL) { + et->authorization_data = calloc(1, sizeof(*et->authorization_data)); + if (et->authorization_data == NULL) { ret = ENOMEM; krb5_set_error_message(r->context, ret, "malloc: out of memory"); goto out; } } for(i = 0; i < auth_data->len ; i++) { - ret = add_AuthorizationData(et.authorization_data, &auth_data->val[i]); + ret = add_AuthorizationData(et->authorization_data, &auth_data->val[i]); if (ret) { krb5_set_error_message(r->context, ret, "malloc: out of memory"); goto out; @@ -764,39 +760,39 @@ tgs_make_reply(astgs_request_t r, } } - ret = krb5_copy_keyblock_contents(r->context, sessionkey, &et.key); + ret = krb5_copy_keyblock_contents(r->context, sessionkey, &et->key); if (ret) goto out; - et.crealm = rep.crealm; - et.cname = rep.cname; + et->crealm = rep->crealm; + et->cname = rep->cname; - ek.key = et.key; + ek->key = et->key; /* MIT must have at least one last_req */ - ek.last_req.val = calloc(1, sizeof(*ek.last_req.val)); - if (ek.last_req.val == NULL) { + ek->last_req.val = calloc(1, sizeof(*ek->last_req.val)); + if (ek->last_req.val == NULL) { ret = ENOMEM; goto out; } - ek.last_req.len = 1; /* set after alloc to avoid null deref on cleanup */ - ek.nonce = b->nonce; - ek.flags = et.flags; - ek.authtime = et.authtime; - ek.starttime = et.starttime; - ek.endtime = et.endtime; - ek.renew_till = et.renew_till; - ek.srealm = rep.ticket.realm; - ek.sname = rep.ticket.sname; + ek->last_req.len = 1; /* set after alloc to avoid null deref on cleanup */ + ek->nonce = b->nonce; + ek->flags = et->flags; + ek->authtime = et->authtime; + ek->starttime = et->starttime; + ek->endtime = et->endtime; + ek->renew_till = et->renew_till; + ek->srealm = rep->ticket.realm; + ek->sname = rep->ticket.sname; - _kdc_log_timestamp(r, "TGS-REQ", et.authtime, et.starttime, - et.endtime, et.renew_till); + _kdc_log_timestamp(r, "TGS-REQ", et->authtime, et->starttime, + et->endtime, et->renew_till); if (enc_pa_data->len) { - rep.padata = calloc(1, sizeof(*rep.padata)); - if (rep.padata == NULL) { + rep->padata = calloc(1, sizeof(*rep->padata)); + if (rep->padata == NULL) { ret = ENOMEM; goto out; } - ret = copy_METHOD_DATA(enc_pa_data, rep.padata); + ret = copy_METHOD_DATA(enc_pa_data, rep->padata); if (ret) goto out; } @@ -823,7 +819,7 @@ tgs_make_reply(astgs_request_t r, * restrictive authorization data. Policy for unknown authorization types * is implementation dependent. */ - if (r->pac && !et.flags.anonymous) { + if (r->pac && !et->flags.anonymous) { _kdc_audit_addkv((kdc_request_t)r, 0, "pac_attributes", "%lx", (long)r->pac_attributes); @@ -838,7 +834,7 @@ tgs_make_reply(astgs_request_t r, ret = _krb5_kdc_pac_sign_ticket(r->context, r->pac, tgt_name, serverkey, krbtgtkey, rodc_id, NULL, r->client_princ, - add_ticket_sig, &et, + add_ticket_sig, et, is_tgs ? &r->pac_attributes : NULL); if (ret) goto out; @@ -856,8 +852,7 @@ tgs_make_reply(astgs_request_t r, etype list, even if we don't want a session key with DES3? */ ret = _kdc_encode_reply(r->context, r->config, r, b->nonce, - &rep, &et, &ek, serverkey->keytype, - kvno, + serverkey->keytype, kvno, serverkey, 0, r->rk_is_subkey, e_text, reply); if (is_weak) @@ -866,19 +861,6 @@ tgs_make_reply(astgs_request_t r, _log_astgs_req(r, serverkey->keytype); out: - free_TGS_REP(&rep); - free_TransitedEncoding(&et.transited); - if(et.starttime) - free(et.starttime); - if(et.renew_till) - free(et.renew_till); - if(et.authorization_data) { - free_AuthorizationData(et.authorization_data); - free(et.authorization_data); - } - free_LastReq(&ek.last_req); - memset(et.key.keyvalue.data, 0, et.key.keyvalue.length); - free_EncryptionKey(&et.key); return ret; } @@ -2622,6 +2604,21 @@ out: free(csec); free(cusec); + free_TGS_REP(&r->rep); + free_TransitedEncoding(&r->et.transited); + free(r->et.starttime); + free(r->et.renew_till); + if(r->et.authorization_data) { + free_AuthorizationData(r->et.authorization_data); + free(r->et.authorization_data); + } + free_LastReq(&r->ek.last_req); + if (r->et.key.keyvalue.data) { + memset_s(r->et.key.keyvalue.data, 0, r->et.key.keyvalue.length, + r->et.key.keyvalue.length); + } + free_EncryptionKey(&r->et.key); + if (r->client_princ) { krb5_free_principal(r->context, r->client_princ); r->client_princ = NULL;