From 867457871a9c76128424b9848b70c86c7bdcfac5 Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Sat, 8 Jan 2022 17:35:53 +1100 Subject: [PATCH] krb5: make krb5_pac a heim_object_t Make krb5_pac a heim_object_t and use heim_retain() (i.e. reference count +1) as the copy constructor for the type decoration. Note: this assumes that PACs included in naming attributes are immutable. --- lib/asn1/krb5.opt | 2 +- lib/base/heimbasepriv.h | 1 + lib/krb5/Makefile.am | 2 +- lib/krb5/NTMakefile | 2 + lib/krb5/init_creds_pw.c | 3 +- lib/krb5/pac.c | 94 +++++++++++++++++----------------------- lib/krb5/principal.c | 24 ++-------- 7 files changed, 51 insertions(+), 77 deletions(-) diff --git a/lib/asn1/krb5.opt b/lib/asn1/krb5.opt index 29fc22482..998d5734c 100644 --- a/lib/asn1/krb5.opt +++ b/lib/asn1/krb5.opt @@ -5,5 +5,5 @@ --sequence=ETYPE-INFO --sequence=ETYPE-INFO2 --preserve-binary=KDC-REQ-BODY ---decorate=PrincipalNameAttrs:void:pac?::: +--decorate=PrincipalNameAttrs:heim_object_t:pac --decorate=Principal:PrincipalNameAttrs:nameattrs? diff --git a/lib/base/heimbasepriv.h b/lib/base/heimbasepriv.h index 8c2b722cd..b9f63e56b 100644 --- a/lib/base/heimbasepriv.h +++ b/lib/base/heimbasepriv.h @@ -65,6 +65,7 @@ enum { HEIM_TID_DATA = 134, HEIM_TID_DB = 135, HEIM_TID_PA_AUTH_MECH = 136, + HEIM_TID_PAC = 137, HEIM_TID_USER = 255 }; diff --git a/lib/krb5/Makefile.am b/lib/krb5/Makefile.am index 2981e3e2a..c1345c28a 100644 --- a/lib/krb5/Makefile.am +++ b/lib/krb5/Makefile.am @@ -4,7 +4,7 @@ include $(top_srcdir)/Makefile.am.common WFLAGS += $(WFLAGS_ENUM_CONV) -AM_CPPFLAGS += -I../com_err -I$(srcdir)/../com_err $(INCLUDE_sqlite3) $(INCLUDE_libintl) $(INCLUDE_openssl_crypto) +AM_CPPFLAGS += -I../com_err -I$(srcdir)/../com_err -I../base -I$(srcdir)/../base $(INCLUDE_sqlite3) $(INCLUDE_libintl) $(INCLUDE_openssl_crypto) bin_PROGRAMS = verify_krb5_conf diff --git a/lib/krb5/NTMakefile b/lib/krb5/NTMakefile index d32025130..40ca0fb0b 100644 --- a/lib/krb5/NTMakefile +++ b/lib/krb5/NTMakefile @@ -31,6 +31,8 @@ RELDIR=lib\krb5 +intcflags=-I$(SRCDIR) -I$(SRCDIR)\..\com_err -I$(SRCDIR)\..\base + !include ../../windows/NTMakefile.w32 libkrb5_OBJS = \ diff --git a/lib/krb5/init_creds_pw.c b/lib/krb5/init_creds_pw.c index 0ed47e798..8de87bd75 100644 --- a/lib/krb5/init_creds_pw.c +++ b/lib/krb5/init_creds_pw.c @@ -35,7 +35,8 @@ */ #include "krb5_locl.h" -#include "../base/heimbasepriv.h" /* XXX */ + +#include struct pa_info_data { krb5_enctype etype; diff --git a/lib/krb5/pac.c b/lib/krb5/pac.c index bfb15a686..fded19204 100644 --- a/lib/krb5/pac.c +++ b/lib/krb5/pac.c @@ -32,6 +32,8 @@ */ #include "krb5_locl.h" + +#include #include struct PAC_INFO_BUFFER { @@ -98,6 +100,38 @@ struct krb5_pac_data { static const char zeros[PAC_ALIGNMENT] = { 0 }; +static void +pac_dealloc(void *ctx) +{ + krb5_pac pac = (krb5_pac)ctx; + + krb5_data_free(&pac->data); + krb5_data_free(&pac->ticket_sign_data); + + if (pac->upn_princ) { + free_Principal(pac->upn_princ); + free(pac->upn_princ); + } + if (pac->canon_princ) { + free_Principal(pac->canon_princ); + free(pac->canon_princ); + } + krb5_data_free(&pac->sid); + + free(pac->pac); +} + +struct heim_type_data pac_object = { + HEIM_TID_PAC, + "heim-pac", + NULL, + pac_dealloc, + NULL, + NULL, + NULL, + NULL +}; + /* * HMAC-MD5 checksum over any key (needed for the PAC routines) */ @@ -154,7 +188,7 @@ krb5_pac_parse(krb5_context context, const void *ptr, size_t len, krb5_storage *sp = NULL; uint32_t i, tmp, tmp2, header_end; - p = calloc(1, sizeof(*p)); + p = _heim_alloc_object(&pac_object, sizeof(*p)); if (p == NULL) { ret = krb5_enomem(context); goto out; @@ -304,7 +338,7 @@ out: if (p) { if (p->pac) free(p->pac); - free(p); + krb5_pac_free(context, p); } *pac = NULL; @@ -317,21 +351,21 @@ krb5_pac_init(krb5_context context, krb5_pac *pac) krb5_error_code ret; krb5_pac p; - p = calloc(1, sizeof(*p)); + p = _heim_alloc_object(&pac_object, sizeof(*p)); if (p == NULL) { return krb5_enomem(context); } p->pac = calloc(1, sizeof(*p->pac)); if (p->pac == NULL) { - free(p); + krb5_pac_free(context, p); return krb5_enomem(context); } ret = krb5_data_alloc(&p->data, PACTYPE_SIZE); if (ret) { free (p->pac); - free(p); + krb5_pac_free(context, p); return krb5_enomem(context); } @@ -519,17 +553,7 @@ krb5_pac_get_types(krb5_context context, KRB5_LIB_FUNCTION void KRB5_LIB_CALL krb5_pac_free(krb5_context context, krb5_pac pac) { - if (pac == NULL) - return; - krb5_data_free(&pac->data); - krb5_data_free(&pac->ticket_sign_data); - - krb5_free_principal(context, pac->upn_princ); - krb5_free_principal(context, pac->canon_princ); - krb5_data_free(&pac->sid); - - free(pac->pac); - free(pac); + heim_release(pac); } /* @@ -1998,41 +2022,3 @@ _krb5_kdc_pac_sign_ticket(krb5_context context, krb5_data_free(&rspac); return ret; } - -/* - * Helper function for krb5_copy_principal(), because the krb5_pac - * in nameattrs lacks a copy constructor (not being an ASN.1 type) - */ - -KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL -_krb5_pac_copy(krb5_context context, krb5_pac in, krb5_pac *out) -{ - krb5_error_code ret; - krb5_pac p; - - *out = NULL; - - ret = krb5_pac_parse(context, in->data.data, in->data.length, &p); - if (ret == 0 && in->ticket_sign_data.data) - ret = krb5_data_copy(&p->ticket_sign_data, in->ticket_sign_data.data, - in->ticket_sign_data.length); - - if (ret == 0 && in->upn_princ) - ret = krb5_copy_principal(context, in->upn_princ, &p->upn_princ); - p->upn_flags = in->upn_flags; - if (ret == 0 && in->canon_princ) - ret = krb5_copy_principal(context, in->canon_princ, &p->canon_princ); - if (ret == 0 && in->sid.data) - ret = krb5_data_copy(&p->sid, in->sid.data, in->sid.length); - - p->pac_attributes = in->pac_attributes; - - if (ret) { - krb5_pac_free(context, p); - return ret; - } - - *out = p; - - return 0; -} diff --git a/lib/krb5/principal.c b/lib/krb5/principal.c index 9dfeba02f..dc6692ff2 100644 --- a/lib/krb5/principal.c +++ b/lib/krb5/principal.c @@ -102,13 +102,10 @@ KRB5_LIB_FUNCTION void KRB5_LIB_CALL krb5_free_principal(krb5_context context, krb5_principal p) { - if (p == NULL) - return; - - if (p->nameattrs) - krb5_pac_free(context, p->nameattrs->pac); - free_Principal(p); - free(p); + if(p){ + free_Principal(p); + free(p); + } } /** @@ -929,25 +926,12 @@ krb5_copy_principal(krb5_context context, krb5_principal *outprinc) { krb5_principal p = malloc(sizeof(*p)); - krb5_error_code ret; - if (p == NULL) return krb5_enomem(context); if(copy_Principal(inprinc, p)) { free(p); return krb5_enomem(context); } - if (inprinc->nameattrs && inprinc->nameattrs->pac) { - krb5_pac pac; - - ret = _krb5_pac_copy(context, inprinc->nameattrs->pac, &pac); - if (ret) { - krb5_free_principal(context, p); - return ret; - } - heim_assert(p->nameattrs, "nameattrs uninitialized"); - p->nameattrs->pac = pac; - } *outprinc = p; return 0; }