From cc0874d4102a78ce6f731420dcaf33727fd15c53 Mon Sep 17 00:00:00 2001 From: Luke Howard Date: Sat, 1 Jan 2022 16:50:58 +1100 Subject: [PATCH] kdc: preserve value types in auditing Preserve integer/boolean audit values as their native types; convert to strings when logging only. This commit goes some way towards unifying the two auditing APIs. --- kdc/bx509d.c | 10 +++---- kdc/httpkadmind.c | 6 ++--- kdc/kerberos5.c | 17 +++++------- kdc/krb5tgs.c | 4 +-- kdc/kx509.c | 8 +++--- kdc/libkdc-exports.def | 2 ++ kdc/process.c | 12 +++++++++ kdc/version-script.map | 2 ++ lib/base/heimbase.h | 3 ++- lib/base/log.c | 52 +++++++++++++++++++++++++++++++------ lib/base/number.c | 16 +++++++++--- lib/base/version-script.map | 3 +++ 12 files changed, 98 insertions(+), 37 deletions(-) diff --git a/kdc/bx509d.c b/kdc/bx509d.c index caafc5a7b..90840b031 100644 --- a/kdc/bx509d.c +++ b/kdc/bx509d.c @@ -485,8 +485,8 @@ bad_reqv(struct bx509_request_desc *r, char *formatted = NULL; char *msg = NULL; - heim_audit_addkv((heim_svc_req_desc)r, 0, "http-status-code", "%d", - http_status_code); + heim_audit_addkv_number((heim_svc_req_desc)r, "http-status-code", + http_status_code); (void) gettimeofday(&r->tv_end, NULL); if (code == ENOMEM) { if (r->context) @@ -669,13 +669,13 @@ bx509_param_cb(void *d, &oid); der_free_oid(&oid); } else if (strcmp(key, "csr") == 0 && val) { - heim_audit_addkv((heim_svc_req_desc)r, 0, "requested_csr", "true"); + heim_audit_addkv_bool((heim_svc_req_desc)r, "requested_csr", TRUE); r->ret = 0; /* Handled upstairs */ } else if (strcmp(key, "lifetime") == 0 && val) { r->req_life = parse_time(val, "day"); } else { /* Produce error for unknown params */ - heim_audit_addkv((heim_svc_req_desc)r, 0, "requested_unknown", "true"); + heim_audit_addkv_bool((heim_svc_req_desc)r, "requested_unknown", TRUE); krb5_set_error_message(r->context, r->ret = ENOTSUP, "Query parameter %s not supported", key); } @@ -1738,7 +1738,7 @@ get_tgt_param_cb(void *d, r->req_life = parse_time(val, "day"); } else { /* Produce error for unknown params */ - heim_audit_addkv((heim_svc_req_desc)r, 0, "requested_unknown", "true"); + heim_audit_addkv_bool((heim_svc_req_desc)r, "requested_unknown", TRUE); krb5_set_error_message(r->context, r->ret = ENOTSUP, "Query parameter %s not supported", key); } diff --git a/kdc/httpkadmind.c b/kdc/httpkadmind.c index 3dfb36518..b155c39e3 100644 --- a/kdc/httpkadmind.c +++ b/kdc/httpkadmind.c @@ -700,8 +700,8 @@ bad_reqv(kadmin_request_desc r, if (r && r->context) context = r->context; if (r && r->hcontext && r->kv) - heim_audit_addkv((heim_svc_req_desc)r, 0, "http-status-code", "%d", - http_status_code); + heim_audit_addkv_number((heim_svc_req_desc)r, "http-status-code", + http_status_code); (void) gettimeofday(&r->tv_end, NULL); if (code == ENOMEM) { if (context) @@ -1046,7 +1046,7 @@ param_cb(void *d, #endif } else { /* Produce error for unknown params */ - heim_audit_addkv((heim_svc_req_desc)r, 0, "requested_unknown", "true"); + heim_audit_addkv_bool((heim_svc_req_desc)r, "requested_unknown", TRUE); krb5_set_error_message(r->context, ret = ENOTSUP, "Query parameter %s not supported", key); } diff --git a/kdc/kerberos5.c b/kdc/kerberos5.c index 81c979087..db7287d7f 100644 --- a/kdc/kerberos5.c +++ b/kdc/kerberos5.c @@ -498,15 +498,13 @@ _kdc_log_timestamp(astgs_request_t r, const char *type, endtime_str[100], renewtime_str[100]; if (authtime) - _kdc_audit_addkv((kdc_request_t)r, 0, "auth", "%ld", (long)authtime); + _kdc_audit_addkv_number((kdc_request_t)r, "auth", authtime); if (starttime && *starttime) - _kdc_audit_addkv((kdc_request_t)r, 0, "start", "%ld", - (long)*starttime); + _kdc_audit_addkv_number((kdc_request_t)r, "start", *starttime); if (endtime) - _kdc_audit_addkv((kdc_request_t)r, 0, "end", "%ld", (long)endtime); + _kdc_audit_addkv_number((kdc_request_t)r, "end", endtime); if (renew_till && *renew_till) - _kdc_audit_addkv((kdc_request_t)r, 0, "renew", "%ld", - (long)*renew_till); + _kdc_audit_addkv_number((kdc_request_t)r, "renew", *renew_till); krb5_format_time(r->context, authtime, authtime_str, sizeof(authtime_str), TRUE); @@ -984,8 +982,7 @@ pa_enc_ts_validate(astgs_request_t r, const PA_DATA *pa) str = NULL; _kdc_r_log(r, 4, "ENC-TS Pre-authentication succeeded -- %s using %s", r->cname, str ? str : "unknown enctype"); - _kdc_audit_addkv((kdc_request_t)r, 0, "pa-etype", "%d", - (int)pa_key->key.keytype); + _kdc_audit_addkv_number((kdc_request_t)r, "pa-etype", (int64_t)pa_key->key.keytype); audit_auth_event(r, HDB_AUTH_EVENT_LTK_PREAUTH_SUCCEEDED, str ? str : "unknown enctype"); @@ -1888,8 +1885,8 @@ generate_pac(astgs_request_t r, const Key *skey, const Key *tkey, krb5_const_principal canon_princ = NULL; r->pac_attributes = get_pac_attributes(r->context, &r->req); - _kdc_audit_addkv((kdc_request_t)r, 0, "pac_attributes", "%lx", - (long)r->pac_attributes); + _kdc_audit_addkv_number((kdc_request_t)r, "pac_attributes", + r->pac_attributes); if (!_kdc_include_pac_p(r)) return 0; diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index 1f02a7e64..14db53ab4 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -807,8 +807,8 @@ tgs_make_reply(astgs_request_t r, * is implementation dependent. */ if (r->pac && !et->flags.anonymous) { - _kdc_audit_addkv((kdc_request_t)r, 0, "pac_attributes", "%lx", - (long)r->pac_attributes); + _kdc_audit_addkv_number((kdc_request_t)r, "pac_attributes", + r->pac_attributes); /* * PACs are included when issuing TGTs, if there is no PAC_ATTRIBUTES diff --git a/kdc/kx509.c b/kdc/kx509.c index bc3ca9dec..3bd8ae8d9 100644 --- a/kdc/kx509.c +++ b/kdc/kx509.c @@ -668,7 +668,7 @@ check_authz(krb5_context context, ret = kdc_authorize_csr(context, reqctx->config->app, reqctx->csr, cprincipal); if (ret == 0) { - _kdc_audit_addkv((kdc_request_t)reqctx, 0, "authorized", "true"); + _kdc_audit_addkv_bool((kdc_request_t)reqctx, "authorized", TRUE); ret = hx509_request_get_san(reqctx->csr, 0, &san_type, &s); if (ret == 0) { @@ -785,7 +785,7 @@ check_authz(krb5_context context, if (KeyUsage2int(ku) != (KeyUsage2int(ku) & KeyUsage2int(ku_allowed))) goto eacces; - _kdc_audit_addkv((kdc_request_t)reqctx, 0, "authorized", "true"); + _kdc_audit_addkv_bool((kdc_request_t)reqctx, "authorized", TRUE); return 0; eacces: @@ -1046,9 +1046,9 @@ _kdc_do_kx509(kx509_req_context r) out: hx509_certs_free(&certs); if (ret == 0 && !is_probe) - _kdc_audit_addkv((kdc_request_t)r, 0, "cert_issued", "true"); + _kdc_audit_addkv_bool((kdc_request_t)r, "cert_issued", TRUE); else - _kdc_audit_addkv((kdc_request_t)r, 0, "cert_issued", "false"); + _kdc_audit_addkv_bool((kdc_request_t)r, "cert_issued", FALSE); if (r->ac) krb5_auth_con_free(r->context, r->ac); if (ticket) diff --git a/kdc/libkdc-exports.def b/kdc/libkdc-exports.def index 245e4900d..973e1c891 100644 --- a/kdc/libkdc-exports.def +++ b/kdc/libkdc-exports.def @@ -17,6 +17,8 @@ EXPORTS krb5_kdc_update_time krb5_kdc_pk_initialize _kdc_audit_addkv + _kdc_audit_addkv_bool + _kdc_audit_addkv_number _kdc_audit_addkv_object _kdc_audit_delkv _kdc_audit_getkv diff --git a/kdc/process.c b/kdc/process.c index 9169aab36..c332f3d72 100644 --- a/kdc/process.c +++ b/kdc/process.c @@ -94,6 +94,18 @@ _kdc_audit_addkv_timediff(kdc_request_t r, const char *k, heim_audit_addkv_timediff((heim_svc_req_desc)r,k, start, end); } +void +_kdc_audit_addkv_bool(kdc_request_t r, const char *k, krb5_boolean v) +{ + heim_audit_addkv_number((heim_svc_req_desc)r, k, (int)v); +} + +void +_kdc_audit_addkv_number(kdc_request_t r, const char *k, int64_t v) +{ + heim_audit_addkv_number((heim_svc_req_desc)r, k, v); +} + void _kdc_audit_addkv_object(kdc_request_t r, const char *k, heim_object_t obj) { diff --git a/kdc/version-script.map b/kdc/version-script.map index b79ac7c1a..3e412ff58 100644 --- a/kdc/version-script.map +++ b/kdc/version-script.map @@ -21,6 +21,8 @@ HEIMDAL_KDC_1.0 { krb5_kdc_update_time; krb5_kdc_pk_initialize; _kdc_audit_addkv; + _kdc_audit_addkv_bool; + _kdc_audit_addkv_number; _kdc_audit_addkv_object; _kdc_audit_delkv; _kdc_audit_getkv; diff --git a/lib/base/heimbase.h b/lib/base/heimbase.h index c0c94e264..3a36d3eca 100644 --- a/lib/base/heimbase.h +++ b/lib/base/heimbase.h @@ -436,9 +436,10 @@ void heim_db_iterate(heim_db_t, heim_string_t, typedef struct heim_number_data *heim_number_t; -heim_number_t heim_number_create(int); +heim_number_t heim_number_create(int64_t); heim_tid_t heim_number_get_type_id(void); int heim_number_get_int(heim_number_t); +int64_t heim_number_get_long(heim_number_t); /* * diff --git a/lib/base/log.c b/lib/base/log.c index a7f3b6da4..ee39cce6d 100644 --- a/lib/base/log.c +++ b/lib/base/log.c @@ -817,23 +817,59 @@ heim_audit_addkv_timediff(heim_svc_req_desc r, const char *k, } void -heim_audit_addkv_object(heim_svc_req_desc r, const char *k, heim_object_t obj) +heim_audit_addkv_bool(heim_svc_req_desc r, const char *k, int v) { heim_string_t key = heim_string_create(k); - heim_string_t value; + heim_number_t value; if (key == NULL) return; - value = heim_json_copy_serialize(obj, 0, NULL); - heim_log(r->hcontext, r->logf, 7, "heim_audit_addkv_object(): " - "adding kv pair %s=%s", - k, value ? heim_string_get_utf8(value) : ""); - heim_dict_set_value(r->kv, key, obj); + heim_log(r->hcontext, r->logf, 7, "heim_audit_addkv_bool(): " + "adding kv pair %s=%s", k, v ? "true" : "false"); + + value = heim_bool_create(v); + heim_dict_set_value(r->kv, key, value); heim_release(key); heim_release(value); } +void +heim_audit_addkv_number(heim_svc_req_desc r, const char *k, intptr_t v) +{ + heim_string_t key = heim_string_create(k); + heim_number_t value; + + if (key == NULL) + return; + + heim_log(r->hcontext, r->logf, 7, "heim_audit_addkv_number(): " + "adding kv pair %s=%ld", k, v); + + value = heim_number_create(v); + heim_dict_set_value(r->kv, key, value); + heim_release(key); + heim_release(value); +} + +void +heim_audit_addkv_object(heim_svc_req_desc r, const char *k, heim_object_t value) +{ + heim_string_t key = heim_string_create(k); + heim_string_t descr; + + if (key == NULL) + return; + + descr = heim_json_copy_serialize(value, 0, NULL); + heim_log(r->hcontext, r->logf, 7, "heim_audit_addkv_object(): " + "adding kv pair %s=%s", + k, descr ? heim_string_get_utf8(descr) : ""); + heim_dict_set_value(r->kv, key, value); + heim_release(key); + heim_release(descr); +} + void heim_audit_delkv(heim_svc_req_desc r, const char *k) { @@ -883,7 +919,7 @@ audit_trail_iterator(heim_object_t key, heim_object_t value, void *arg) v = heim_string_get_utf8(value); break; case HEIM_TID_NUMBER: - snprintf(num, sizeof(num), "%d", heim_number_get_int(value)); + snprintf(num, sizeof(num), "%lld", (long long)heim_number_get_long(value)); v = num; break; case HEIM_TID_NULL: diff --git a/lib/base/number.c b/lib/base/number.c index c259f6997..f763c5085 100644 --- a/lib/base/number.c +++ b/lib/base/number.c @@ -86,16 +86,16 @@ struct heim_type_data _heim_number_object = { */ heim_number_t -heim_number_create(int number) +heim_number_create(int64_t number) { heim_number_t n; if (number < 0xffffff && number >= 0) return heim_base_make_tagged_object(number, HEIM_TID_NUMBER); - n = _heim_alloc_object(&_heim_number_object, sizeof(int)); + n = _heim_alloc_object(&_heim_number_object, sizeof(int64_t)); if (n) - *((int *)n) = number; + *((int64_t *)n) = number; return n; } @@ -124,5 +124,13 @@ heim_number_get_int(heim_number_t number) { if (heim_base_is_tagged_object(number)) return heim_base_tagged_object_value(number); - return *(int *)number; + return (int)(*(int64_t *)number); +} + +int64_t +heim_number_get_long(heim_number_t number) +{ + if (heim_base_is_tagged_object(number)) + return heim_base_tagged_object_value(number); + return *(int64_t *)number; } diff --git a/lib/base/version-script.map b/lib/base/version-script.map index 06bed70ac..4b20a8e03 100644 --- a/lib/base/version-script.map +++ b/lib/base/version-script.map @@ -29,6 +29,8 @@ HEIMDAL_BASE_1.0 { heim_array_iterate_reverse_f; heim_array_set_value; heim_audit_addkv; + heim_audit_addkv_bool; + heim_audit_addkv_number; heim_audit_addkv_object; heim_audit_addkv_timediff; heim_audit_addreason; @@ -150,6 +152,7 @@ HEIMDAL_BASE_1.0 { heim_null_create; heim_number_create; heim_number_get_int; + heim_number_get_long; heim_number_get_type_id; heim_openlog; heim_path_copy;