From b193d75a1568e1da7f7ea35ade35a796a29796f5 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Fri, 15 Jan 2021 11:57:10 -0600 Subject: [PATCH] hx509: Revamp name handling ahead of new SAN types --- lib/hx509/name.c | 164 +++++++++++++++++++++++++++-------- lib/hx509/print.c | 195 ++++++++++++++++-------------------------- lib/hx509/req.c | 30 ++++--- lib/hx509/test_name.c | 6 +- 4 files changed, 223 insertions(+), 172 deletions(-) diff --git a/lib/hx509/name.c b/lib/hx509/name.c index 8b1904d5a..ec7737693 100644 --- a/lib/hx509/name.c +++ b/lib/hx509/name.c @@ -1020,22 +1020,31 @@ hx509_name_is_null_p(const hx509_name name) * Note that we cannot handle embedded NULs because of Heimdal's representation * of ASN.1 strings as C strings. */ -struct rk_strpool * -_hx509_unparse_KRB5PrincipalName(struct rk_strpool *strpool, heim_any *value) +int +_hx509_unparse_KRB5PrincipalName(hx509_context context, + struct rk_strpool **strpool, + heim_any *value) { KRB5PrincipalName kn; size_t len; int ret; ret = decode_KRB5PrincipalName(value->data, value->length, &kn, &len); - if (ret == 0) - strpool = _hx509_unparse_kerberos_name(strpool, &kn); + if (ret == 0 && + (*strpool = _hx509_unparse_kerberos_name(*strpool, &kn)) == NULL) + ret = hx509_enomem(context); free_KRB5PrincipalName(&kn); - if (ret == 0 && strpool && (value->length != len)) - strpool = rk_strpoolprintf(strpool, " "); - if (ret) - return rk_strpoolprintf(strpool, "length != len) && + (*strpool = rk_strpoolprintf(*strpool, " ")) == NULL) + ret = hx509_enomem(context); + if (ret) { + rk_strpoolfree(*strpool); + *strpool = rk_strpoolprintf(NULL, + "data, value->length, &us, &size)) - return rk_strpoolprintf(strpool, ""); - strpool = rk_strpoolprintf(strpool, "%s", us); + ret = decode_PKIXXmppAddr(value->data, value->length, &us, &size); + if (ret == 0 && + (*strpool = rk_strpoolprintf(*strpool, "%s", us)) == NULL) + ret = hx509_enomem(context); + if (ret) { + rk_strpoolfree(*strpool); + *strpool = rk_strpoolprintf(NULL, + ""); + hx509_set_error_string(context, 0, ret, + "Failed to decode UTF8String SAN"); + } free_PKIXXmppAddr(&us); - return strpool; + return ret; } +int +_hx509_unparse_ia5_string_name(hx509_context context, + struct rk_strpool **strpool, + heim_any *value) +{ + SRVName us; + size_t size; + int ret; + + ret = decode_SRVName(value->data, value->length, &us, &size); + if (ret == 0) { + rk_strpoolfree(*strpool); + *strpool = rk_strpoolprintf(NULL, + ""); + hx509_set_error_string(context, 0, ret, + "Failed to decode UTF8String SAN"); + return ret; + } + *strpool = rk_strpoolprintf(*strpool, "%.*s", + (int)us.length, (char *)us.data); + free_SRVName(&us); + return ret; +} + +typedef int (*other_unparser_f)(hx509_context, + struct rk_strpool **, + heim_any *); + +struct { + const heim_oid *oid; + const char *friendly_name; + other_unparser_f f; +} o_unparsers[] = { + { &asn1_oid_id_pkinit_san, + "KerberosPrincipalName", + _hx509_unparse_KRB5PrincipalName }, + { &asn1_oid_id_pkix_on_xmppAddr, + "XMPPName", + _hx509_unparse_utf8_string_name }, + { &asn1_oid_id_pkinit_ms_san, + "MSFTKerberosPrincipalName", + _hx509_unparse_utf8_string_name }, +}; + /** * Unparse the hx509 name in name into a string. * @@ -1123,35 +1187,62 @@ hx509_unparse_utf8_string_name(struct rk_strpool *strpool, heim_any *value) HX509_LIB_FUNCTION int HX509_LIB_CALL hx509_general_name_unparse(GeneralName *name, char **str) +{ + hx509_context context; + int ret; + + if ((ret = hx509_context_init(&context))) + return ret; + return hx509_general_name_unparse2(context, name, str); +} + +/** + * Unparse the hx509 name in name into a string. + * + * @param context hx509 library context + * @param name the name to print + * @param str an allocated string returns the name in string form + * + * @return An hx509 error code, see hx509_get_error_string(). + * + * @ingroup hx509_name + */ + +HX509_LIB_FUNCTION int HX509_LIB_CALL +hx509_general_name_unparse2(hx509_context context, + GeneralName *name, + char **str) { struct rk_strpool *strpool = NULL; + int ret = 0; *str = NULL; switch (name->element) { case choice_GeneralName_otherName: { + size_t i; char *oid; - hx509_oid_sprint(&name->u.otherName.type_id, &oid); - if (oid == NULL) - return ENOMEM; - strpool = rk_strpoolprintf(strpool, "otherName: %s ", oid); - if (der_heim_oid_cmp(&name->u.otherName.type_id, - &asn1_oid_id_pkinit_san) == 0) { - strpool = - _hx509_unparse_KRB5PrincipalName(strpool, - &name->u.otherName.value); - } else if (der_heim_oid_cmp(&name->u.otherName.type_id, - &asn1_oid_id_pkix_on_xmppAddr) == 0) { - strpool = rk_strpoolprintf(strpool, "xmppAddr "); - strpool = hx509_unparse_utf8_string_name(strpool, - &name->u.otherName.value); - } else if (der_heim_oid_cmp(&name->u.otherName.type_id, - &asn1_oid_id_pkinit_ms_san) == 0) { - strpool = rk_strpoolprintf(strpool, "pkinitMsSan "); - strpool = hx509_unparse_utf8_string_name(strpool, - &name->u.otherName.value); - } else { - strpool = rk_strpoolprintf(strpool, "u.otherName.type_id, &oid); + if (ret == 0) + strpool = rk_strpoolprintf(strpool, "otherName: %s ", oid); + if (strpool == NULL) + ret = ENOMEM; + + for (i = 0; ret == 0 && i < sizeof(o_unparsers)/sizeof(o_unparsers[0]); i++) { + if (der_heim_oid_cmp(&name->u.otherName.type_id, + o_unparsers[i].oid)) + continue; + strpool = rk_strpoolprintf(strpool, "%s ",o_unparsers[i].friendly_name); + if (strpool == NULL) + ret = ENOMEM; + if (ret == 0) + ret = o_unparsers[i].f(context, &strpool, &name->u.otherName.value); + break; + } + if (ret == 0 && i == sizeof(o_unparsers)/sizeof(o_unparsers[0])) { + strpool = rk_strpoolprintf(strpool, ""); + ret = ENOTSUP; } free(oid); break; @@ -1169,7 +1260,6 @@ hx509_general_name_unparse(GeneralName *name, char **str) case choice_GeneralName_directoryName: { Name dir; char *s; - int ret; memset(&dir, 0, sizeof(dir)); dir.element = (enum Name_enum)name->u.directoryName.element; dir.u.rdnSequence = name->u.directoryName.u.rdnSequence; diff --git a/lib/hx509/print.c b/lib/hx509/print.c index 65aa57ad0..a5b90d793 100644 --- a/lib/hx509/print.c +++ b/lib/hx509/print.c @@ -32,6 +32,7 @@ */ #include "hx_locl.h" +#include /** * @page page_print Hx509 printing functions @@ -40,6 +41,7 @@ */ struct hx509_validate_ctx_data { + hx509_context context; int flags; hx509_vprint_func vprint_func; void *ctx; @@ -412,67 +414,6 @@ check_extKeyUsage(hx509_validate_ctx ctx, return 0; } -static int -check_pkinit_san(hx509_validate_ctx ctx, heim_any *a) -{ - KRB5PrincipalName kn; - unsigned i; - size_t size; - int ret; - - ret = decode_KRB5PrincipalName(a->data, a->length, &kn, &size); - if (ret) { - validate_print(ctx, HX509_VALIDATE_F_VALIDATE, - "Decoding kerberos name in SAN failed: %d", ret); - return 1; - } - - if (size != a->length) { - validate_print(ctx, HX509_VALIDATE_F_VALIDATE, - "Decoding kerberos name have extra bits on the end"); - return 1; - } - - /* print kerberos principal, add code to quote / within components */ - for (i = 0; i < kn.principalName.name_string.len; i++) { - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "%s", - kn.principalName.name_string.val[i]); - if (i + 1 < kn.principalName.name_string.len) - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "/"); - } - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "@"); - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "%s", kn.realm); - - free_KRB5PrincipalName(&kn); - return 0; -} - -static int -check_utf8_string_san(hx509_validate_ctx ctx, heim_any *a) -{ - PKIXXmppAddr jid; - size_t size; - int ret; - - ret = decode_PKIXXmppAddr(a->data, a->length, &jid, &size); - if (ret) { - validate_print(ctx, HX509_VALIDATE_F_VALIDATE, - "Decoding JID in SAN failed: %d", ret); - return 1; - } - - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "%s", jid); - free_PKIXXmppAddr(&jid); - - return 0; -} - -static int -check_altnull(hx509_validate_ctx ctx, heim_any *a) -{ - return 0; -} - static int check_CRLDistributionPoints(hx509_validate_ctx ctx, struct cert_status *status, @@ -518,8 +459,13 @@ check_CRLDistributionPoints(hx509_validate_ctx ctx, char *s; GeneralName *name = &dpname.u.fullName.val[j]; - ret = hx509_general_name_unparse(name, &s); - if (ret == 0 && s != NULL) { + ret = hx509_general_name_unparse2(ctx->context, name, &s); + if (ret) { + s = hx509_get_error_string(ctx->context, ret); + validate_print(ctx, HX509_VALIDATE_F_VALIDATE, + "Unknown DistributionPointName: %s", s); + hx509_free_error_string(s); + } else { validate_print(ctx, HX509_VALIDATE_F_VERBOSE, " %s\n", s); free(s); } @@ -544,19 +490,6 @@ check_CRLDistributionPoints(hx509_validate_ctx ctx, return 0; } - -struct { - const char *name; - const heim_oid *oid; - int (*func)(hx509_validate_ctx, heim_any *); -} altname_types[] = { - { "pk-init", &asn1_oid_id_pkinit_san, check_pkinit_san }, - { "jabber", &asn1_oid_id_pkix_on_xmppAddr, check_utf8_string_san }, - { "dns-srv", &asn1_oid_id_pkix_on_dnsSRV, check_altnull }, - { "card-id", &asn1_oid_id_uspkicommon_card_id, check_altnull }, - { "Microsoft NT-PRINCIPAL-NAME", &asn1_oid_id_pkinit_ms_san, check_utf8_string_san } -}; - static int check_altName(hx509_validate_ctx ctx, struct cert_status *status, @@ -591,48 +524,21 @@ check_altName(hx509_validate_ctx ctx, } for (i = 0; i < gn.len; i++) { - switch (gn.val[i].element) { - case choice_GeneralName_otherName: { - unsigned j; + char *s; - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, - "%sAltName otherName ", name); - - for (j = 0; j < sizeof(altname_types)/sizeof(altname_types[0]); j++) { - if (der_heim_oid_cmp(altname_types[j].oid, - &gn.val[i].u.otherName.type_id) != 0) - continue; - - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "%s: ", - altname_types[j].name); - (*altname_types[j].func)(ctx, &gn.val[i].u.otherName.value); - break; - } - if (j == sizeof(altname_types)/sizeof(altname_types[0])) { - hx509_oid_print(&gn.val[i].u.otherName.type_id, - validate_vprint, ctx); - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, " unknown"); - } - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "\n"); - break; - } - default: { - char *s; - ret = hx509_general_name_unparse(&gn.val[i], &s); - if (ret) { - validate_print(ctx, HX509_VALIDATE_F_VALIDATE, - "ret = %d unparsing GeneralName\n", ret); - return 1; - } - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "%s\n", s); - free(s); - break; - } - } + ret = hx509_general_name_unparse2(ctx->context, &gn.val[i], &s); + if (ret) { + s = hx509_get_error_string(ctx->context, ret); + validate_print(ctx, HX509_VALIDATE_F_VALIDATE, + "Error unparsing GeneralName: %s\n", s); + hx509_free_error_string(s); + return 1; + } + validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "\t%s\n", s); + free(s); } free_GeneralNames(&gn); - return 0; } @@ -737,14 +643,59 @@ check_authorityInfoAccess(hx509_validate_ctx ctx, validate_print(ctx, HX509_VALIDATE_F_VERBOSE, "\ttype: "); hx509_oid_print(&aia.val[i].accessMethod, validate_vprint, ctx); - hx509_general_name_unparse(&aia.val[i].accessLocation, &str); - validate_print(ctx, HX509_VALIDATE_F_VERBOSE, - "\n\tdirname: %s\n", str); - free(str); + ret = hx509_general_name_unparse2(ctx->context, + &aia.val[i].accessLocation, &str); + if (ret) { + str = hx509_get_error_string(ctx->context, ret); + validate_print(ctx, HX509_VALIDATE_F_VALIDATE, + "Error unparsing AuthorityInfoAccessSyntax " + "accessLocation: %s", str); + hx509_free_error_string(str); + } else { + validate_print(ctx, HX509_VALIDATE_F_VERBOSE, + "\n\tdirname: %s\n", str); + free(str); + } } free_AuthorityInfoAccessSyntax(&aia); - return 0; + return ret; +} + +static int +get_display_text(DisplayText *dt, char **out) +{ + int r = -1; + + *out = NULL; + + /* + * XXX We're cheating with various string types here. + * + * Proper support for IA5String is a real pain, and we don't have it. + * + * We also don't have support for BMPString. + */ + switch (dt->element) { + case choice_DisplayText_ia5String: + r = rk_strasvisx(out, dt->u.ia5String.data, dt->u.ia5String.length, + VIS_CSTYLE | VIS_TAB | VIS_NL, ""); + break; + case choice_DisplayText_visibleString: + r = rk_strasvis(out, dt->u.visibleString, + VIS_CSTYLE | VIS_TAB | VIS_NL, ""); + break; + case choice_DisplayText_bmpString: + errno = ENOTSUP; /* XXX Need a UTF-16 -> UTF-8 conversion */ + break; + case choice_DisplayText_utf8String: + r = rk_strasvis(out, dt->u.visibleString, + VIS_CSTYLE | VIS_TAB | VIS_NL, ""); + break; + default: + errno = EINVAL; + } + return r < 0 ? errno : 0; } /* @@ -810,10 +761,10 @@ struct { HX509_LIB_FUNCTION int HX509_LIB_CALL hx509_validate_ctx_init(hx509_context context, hx509_validate_ctx *ctx) { - *ctx = malloc(sizeof(**ctx)); + *ctx = calloc(1, sizeof(**ctx)); if (*ctx == NULL) - return ENOMEM; - memset(*ctx, 0, sizeof(**ctx)); + return hx509_enomem(context); + (*ctx)->context = context; return 0; } diff --git a/lib/hx509/req.c b/lib/hx509/req.c index c0c88984b..3946da031 100644 --- a/lib/hx509/req.c +++ b/lib/hx509/req.c @@ -40,6 +40,7 @@ typedef struct abitstring_s { } *abitstring; struct hx509_request_data { + hx509_context context; hx509_name name; SubjectPublicKeyInfo key; KeyUsage ku; @@ -70,6 +71,7 @@ hx509_request_init(hx509_context context, hx509_request *req) if (*req == NULL) return ENOMEM; + (*req)->context = context; return 0; } @@ -1228,7 +1230,7 @@ hx509_request_get_san(hx509_request req, hx509_san_type *type, char **out) { - struct rk_strpool *pool; + struct rk_strpool *pool = NULL; GeneralName *san; *out = NULL; @@ -1262,18 +1264,26 @@ hx509_request_get_san(hx509_request req, return der_print_heim_oid(&san->u.registeredID, '.', out); case HX509_SAN_TYPE_XMPP: /*fallthrough*/ - case HX509_SAN_TYPE_MS_UPN: - pool = hx509_unparse_utf8_string_name(NULL, &san->u.otherName.value); - if (pool == NULL || + case HX509_SAN_TYPE_MS_UPN: { + int ret; + + ret = _hx509_unparse_utf8_string_name(req->context, &pool, + &san->u.otherName.value); + if (ret == 0 && (*out = rk_strpoolcollect(pool)) == NULL) - return ENOMEM; - return 0; - case HX509_SAN_TYPE_PKINIT: - pool = _hx509_unparse_KRB5PrincipalName(NULL, &san->u.otherName.value); - if (pool == NULL || + return hx509_enomem(req->context); + return ret; + } + case HX509_SAN_TYPE_PKINIT: { + int ret; + + ret = _hx509_unparse_KRB5PrincipalName(req->context, &pool, + &san->u.otherName.value); + if (ret == 0 && (*out = rk_strpoolcollect(pool)) == NULL) - return ENOMEM; + return hx509_enomem(req->context); return 0; + } default: *type = HX509_SAN_TYPE_UNSUPPORTED; return 0; diff --git a/lib/hx509/test_name.c b/lib/hx509/test_name.c index 7fd6c22b8..cc023caba 100644 --- a/lib/hx509/test_name.c +++ b/lib/hx509/test_name.c @@ -390,10 +390,10 @@ test_pkinit_san(hx509_context context, const char *p, const char *realm, ...) ret = hx509_general_name_unparse(&gn, &round_trip); if (ret) return 1; - if (strncmp(round_trip, "otherName: 1.3.6.1.5.2.2 ", - sizeof("otherName: 1.3.6.1.5.2.2 ") - 1)) + if (strncmp(round_trip, "otherName: 1.3.6.1.5.2.2 KerberosPrincipalName ", + sizeof("otherName: 1.3.6.1.5.2.2 KerberosPrincipalName ") - 1)) return 1; - if (ret || strcmp(round_trip + sizeof("otherName: 1.3.6.1.5.2.2 ") - 1, p)) + if (ret || strcmp(round_trip + sizeof("otherName: 1.3.6.1.5.2.2 KerberosPrincipalName ") - 1, p)) return 1; free_KRB5PrincipalName(&kn); free_GeneralName(&gn);