diff --git a/lib/hx509/ChangeLog b/lib/hx509/ChangeLog index 4b9545ddf..55c01a536 100644 --- a/lib/hx509/ChangeLog +++ b/lib/hx509/ChangeLog @@ -1,3 +1,7 @@ +2008-02-11 Love Hörnquist Åstrand + + * Use ldap-prep (with libwind) to compare names + 2008-01-27 Love Hörnquist Åstrand * cert.c (hx509_query_match_eku): update to support the NULL diff --git a/lib/hx509/Makefile.am b/lib/hx509/Makefile.am index b081e397c..b8c4a55a7 100644 --- a/lib/hx509/Makefile.am +++ b/lib/hx509/Makefile.am @@ -85,6 +85,7 @@ libhx509_la_LIBADD = \ $(LIB_com_err) \ $(LIB_hcrypto) \ $(top_builddir)/lib/asn1/libasn1.la \ + $(top_builddir)/lib/wind/libwind.la \ $(LIBADD_roken) \ $(LIB_dlopen) diff --git a/lib/hx509/cert.c b/lib/hx509/cert.c index a7d6e0024..b16e515f0 100644 --- a/lib/hx509/cert.c +++ b/lib/hx509/cert.c @@ -893,10 +893,13 @@ _hx509_cert_is_parent_cmp(const Certificate *subject, int diff; AuthorityKeyIdentifier ai; SubjectKeyIdentifier si; - int ret_ai, ret_si; + int ret_ai, ret_si, ret; - diff = _hx509_name_cmp(&issuer->tbsCertificate.subject, - &subject->tbsCertificate.issuer); + ret = _hx509_name_cmp(&issuer->tbsCertificate.subject, + &subject->tbsCertificate.issuer, + &diff); + if (ret) + return ret; if (diff) return diff; @@ -951,8 +954,11 @@ _hx509_cert_is_parent_cmp(const Certificate *subject, name.u.rdnSequence = ai.authorityCertIssuer->val[0].u.directoryName.u.rdnSequence; - diff = _hx509_name_cmp(&issuer->tbsCertificate.subject, - &name); + ret = _hx509_name_cmp(&issuer->tbsCertificate.subject, + &name, + &diff); + if (ret) + return ret; if (diff) return diff; diff = 0; @@ -991,10 +997,18 @@ certificate_is_anchor(hx509_context context, } static int -certificate_is_self_signed(const Certificate *cert) +certificate_is_self_signed(hx509_context context, + const Certificate *cert, + int *self_signed) { - return _hx509_name_cmp(&cert->tbsCertificate.subject, - &cert->tbsCertificate.issuer) == 0; + int ret, diff; + ret = _hx509_name_cmp(&cert->tbsCertificate.subject, + &cert->tbsCertificate.issuer, &diff); + *self_signed = (diff == 0); + if (ret) + hx509_set_error_string(context, 0, ret, + "Failed to check if self signed"); + return ret; } /* @@ -1605,9 +1619,14 @@ match_RDN(const RelativeDistinguishedName *c, return HX509_NAME_CONSTRAINT_ERROR; for (i = 0; i < n->len; i++) { + int diff, ret; + if (der_heim_oid_cmp(&c->val[i].type, &n->val[i].type) != 0) return HX509_NAME_CONSTRAINT_ERROR; - if (_hx509_name_ds_cmp(&c->val[i].value, &n->val[i].value) != 0) + ret = _hx509_name_ds_cmp(&c->val[i].value, &n->val[i].value, &diff); + if (ret) + return ret; + if (diff != 0) return HX509_NAME_CONSTRAINT_ERROR; } return 0; @@ -1867,10 +1886,7 @@ hx509_verify_path(hx509_context context, { hx509_name_constraints nc; hx509_path path; -#if 0 - const AlgorithmIdentifier *alg_id; -#endif - int ret, i, proxy_cert_depth, selfsigned_depth; + int ret, i, proxy_cert_depth, selfsigned_depth, diff; enum certtype type; Name proxy_issuer; hx509_certs anchors = NULL; @@ -1910,10 +1926,6 @@ hx509_verify_path(hx509_context context, if (ret) goto out; -#if 0 - alg_id = path.val[path->len - 1]->data->tbsCertificate.signature; -#endif - /* * Check CA and proxy certificate chain from the top of the * certificate chain. Also check certificate is valid with respect @@ -1943,6 +1955,7 @@ hx509_verify_path(hx509_context context, switch (type) { case CA_CERT: + /* XXX make constants for keyusage */ ret = check_key_usage(context, c, 1 << 5, REQUIRE_RFC3280(ctx) ? TRUE : FALSE); @@ -1952,8 +1965,16 @@ hx509_verify_path(hx509_context context, goto out; } - if (i + 1 != path.len && certificate_is_self_signed(c)) - selfsigned_depth++; + /* self signed cert doesn't add to path length */ + if (i + 1 != path.len) { + int selfsigned; + + ret = certificate_is_self_signed(context, c, &selfsigned); + if (ret) + goto out; + if (selfsigned) + selfsigned_depth++; + } break; case PROXY_CERT: { @@ -2001,8 +2022,12 @@ hx509_verify_path(hx509_context context, */ if (proxy_cert_depth) { - ret = _hx509_name_cmp(&proxy_issuer, &c->tbsCertificate.subject); + ret = _hx509_name_cmp(&proxy_issuer, &c->tbsCertificate.subject, &diff); if (ret) { + hx509_set_error_string(context, 0, ret, "Out of memory"); + goto out; + } + if (diff) { ret = HX509_PROXY_CERT_NAME_WRONG; hx509_set_error_string(context, 0, ret, "Base proxy name not right"); @@ -2035,8 +2060,12 @@ hx509_verify_path(hx509_context context, free_RelativeDistinguishedName(&proxy_issuer.u.rdnSequence.val[j - 1]); proxy_issuer.u.rdnSequence.len -= 1; - ret = _hx509_name_cmp(&proxy_issuer, &c->tbsCertificate.issuer); - if (ret != 0) { + ret = _hx509_name_cmp(&proxy_issuer, &c->tbsCertificate.issuer, &diff); + if (ret) { + hx509_set_error_string(context, 0, ret, "Out of memory"); + goto out; + } + if (diff != 0) { ret = HX509_PROXY_CERT_NAME_WRONG; hx509_set_error_string(context, 0, ret, "Proxy issuer name not as expected"); @@ -2062,9 +2091,13 @@ hx509_verify_path(hx509_context context, */ if (proxy_cert_depth) { - ret = _hx509_name_cmp(&proxy_issuer, - &c->tbsCertificate.subject); + ret = _hx509_name_cmp(&proxy_issuer, + &c->tbsCertificate.subject, &diff); if (ret) { + hx509_set_error_string(context, 0, ret, "out of memory"); + goto out; + } + if (diff) { ret = HX509_PROXY_CERT_NAME_WRONG; hx509_clear_error_string(context); goto out; @@ -2120,11 +2153,16 @@ hx509_verify_path(hx509_context context, for (ret = 0, i = path.len - 1; i >= 0; i--) { Certificate *c; + int selfsigned; c = _hx509_get_cert(path.val[i]); + ret = certificate_is_self_signed(context, c, &selfsigned); + if (ret) + goto out; + /* verify name constraints, not for selfsigned and anchor */ - if (!certificate_is_self_signed(c) || i + 1 != path.len) { + if (!selfsigned || i + 1 != path.len) { ret = check_name_constraints(context, &nc, c); if (ret) { goto out; @@ -2192,10 +2230,16 @@ hx509_verify_path(hx509_context context, /* is last in chain (trust anchor) */ if (i + 1 == path.len) { + int selfsigned; + signer = path.val[i]->data; + ret = certificate_is_self_signed(context, signer, &selfsigned); + if (ret) + goto out; + /* if trust anchor is not self signed, don't check sig */ - if (!certificate_is_self_signed(signer)) + if (!selfsigned) continue; } else { /* take next certificate in chain */ @@ -2717,6 +2761,7 @@ int _hx509_query_match_cert(hx509_context context, const hx509_query *q, hx509_cert cert) { Certificate *c = _hx509_get_cert(cert); + int ret, diff; _hx509_query_statistic(context, 1, q); @@ -2732,17 +2777,20 @@ _hx509_query_match_cert(hx509_context context, const hx509_query *q, hx509_cert && der_heim_integer_cmp(&c->tbsCertificate.serialNumber, q->serial) != 0) return 0; - if ((q->match & HX509_QUERY_MATCH_ISSUER_NAME) - && _hx509_name_cmp(&c->tbsCertificate.issuer, q->issuer_name) != 0) - return 0; + if (q->match & HX509_QUERY_MATCH_ISSUER_NAME) { + ret = _hx509_name_cmp(&c->tbsCertificate.issuer, q->issuer_name, &diff); + if (ret || diff) + return 0; + } - if ((q->match & HX509_QUERY_MATCH_SUBJECT_NAME) - && _hx509_name_cmp(&c->tbsCertificate.subject, q->subject_name) != 0) - return 0; + if (q->match & HX509_QUERY_MATCH_SUBJECT_NAME) { + ret = _hx509_name_cmp(&c->tbsCertificate.subject, q->subject_name, &diff); + if (ret || diff) + return 0; + } if (q->match & HX509_QUERY_MATCH_SUBJECT_KEY_ID) { SubjectKeyIdentifier si; - int ret; ret = _hx509_find_extension_subject_key_id(c, &si); if (ret == 0) { @@ -2806,14 +2854,13 @@ _hx509_query_match_cert(hx509_context context, const hx509_query *q, hx509_cert return 0; } if (q->match & HX509_QUERY_MATCH_FUNCTION) { - int ret = (*q->cmp_func)(q->cmp_func_ctx, cert); + ret = (*q->cmp_func)(q->cmp_func_ctx, cert); if (ret != 0) return 0; } if (q->match & HX509_QUERY_MATCH_KEY_HASH_SHA1) { heim_octet_string os; - int ret; os.data = c->tbsCertificate.subjectPublicKeyInfo.subjectPublicKey.data; os.length = diff --git a/lib/hx509/name.c b/lib/hx509/name.c index e3735efbc..3c0b256ce 100644 --- a/lib/hx509/name.c +++ b/lib/hx509/name.c @@ -32,6 +32,7 @@ */ #include "hx_locl.h" +#include RCSID("$Id$"); /** @@ -63,6 +64,7 @@ RCSID("$Id$"); static const struct { const char *n; const heim_oid *(*o)(void); + wind_profile_flags flags; } no[] = { { "C", oid_id_at_countryName }, { "CN", oid_id_at_commonName }, @@ -279,95 +281,162 @@ _hx509_Name_to_string(const Name *n, char **str) return 0; } -/* - * XXX this function is broken, it needs to compare code points, not - * bytes. - */ +#define COPYCHARARRAY(_ds,_el,_l,_n) \ + (_l) = strlen(_ds->u._el); \ + (_n) = malloc((_l) * sizeof((_n)[0])); \ + if ((_n) == NULL) \ + return ENOMEM; \ + for (i = 0; i < (_l); i++) \ + (_n)[i] = _ds->u._el[i] -static void -prune_space(const unsigned char **s) + +#define COPYVALARRAY(_ds,_el,_l,_n) \ + (_l) = _ds->u._el.length; \ + (_n) = malloc((_l) * sizeof((_n)[0])); \ + if ((_n) == NULL) \ + return ENOMEM; \ + for (i = 0; i < (_l); i++) \ + (_n)[i] = _ds->u._el.data[i] + +#define COPYVOIDARRAY(_ds,_el,_l,_n) \ + (_l) = _ds->u._el.length; \ + (_n) = malloc((_l) * sizeof((_n)[0])); \ + if ((_n) == NULL) \ + return ENOMEM; \ + for (i = 0; i < (_l); i++) \ + (_n)[i] = ((unsigned char *)_ds->u._el.data)[i] + + + +static int +dsstringprep(const DirectoryString *ds, uint32_t **rname, size_t *rlen) { - while (**s == ' ') - (*s)++; -} + wind_profile_flags flags = 0; + size_t i, len; + int ret; + uint32_t *name; -int -_hx509_name_ds_cmp(const DirectoryString *ds1, const DirectoryString *ds2) -{ - int c; + *rname = NULL; + *rlen = 0; - c = ds1->element - ds2->element; - if (c) - return c; - - switch(ds1->element) { + switch(ds->element) { case choice_DirectoryString_ia5String: - c = strcmp(ds1->u.ia5String, ds2->u.ia5String); + COPYCHARARRAY(ds, ia5String, len, name); + break; + case choice_DirectoryString_printableString: + flags = WIND_PROFILE_LDAP_CASE_EXACT_ATTRIBUTE; + COPYCHARARRAY(ds, printableString, len, name); break; case choice_DirectoryString_teletexString: - c = der_heim_octet_string_cmp(&ds1->u.teletexString, - &ds2->u.teletexString); - break; - case choice_DirectoryString_printableString: { - const unsigned char *s1 = (unsigned char*)ds1->u.printableString; - const unsigned char *s2 = (unsigned char*)ds2->u.printableString; - prune_space(&s1); prune_space(&s2); - while (*s1 && *s2) { - if (toupper(*s1) != toupper(*s2)) { - c = toupper(*s1) - toupper(*s2); - break; - } - if (*s1 == ' ') { prune_space(&s1); prune_space(&s2); } - else { s1++; s2++; } - } - prune_space(&s1); prune_space(&s2); - c = *s1 - *s2; - break; - } - case choice_DirectoryString_utf8String: - c = strcmp(ds1->u.utf8String, ds2->u.utf8String); - break; - case choice_DirectoryString_universalString: - c = der_heim_universal_string_cmp(&ds1->u.universalString, - &ds2->u.universalString); + COPYVOIDARRAY(ds, teletexString, len, name); break; case choice_DirectoryString_bmpString: - c = der_heim_bmp_string_cmp(&ds1->u.bmpString, - &ds2->u.bmpString); + COPYVALARRAY(ds, bmpString, len, name); + break; + case choice_DirectoryString_universalString: + COPYVALARRAY(ds, universalString, len, name); + break; + case choice_DirectoryString_utf8String: + ret = wind_utf8ucs4_length(ds->u.utf8String, &len); + if (ret) + return ret; + name = malloc(len * sizeof(name[0])); + if (name == NULL) + return ENOMEM; + ret = wind_utf8ucs4(ds->u.utf8String, name, &len); + if (ret) + return ret; break; default: - c = 1; - break; + _hx509_abort("unknown directory type: %d", ds->element); } - return c; + + *rlen = len; + /* try a couple of times to get the length right, XXX gross */ + for (i = 0; i < 4; i++) { + *rlen = *rlen * 2; + *rname = malloc(*rlen * sizeof((*rname)[0])); + + ret = wind_stringprep(name, len, *rname, rlen, + WIND_PROFILE_LDAP|flags); + if (ret == WIND_ERR_OVERRUN) { + free(*rname); + *rname = NULL; + continue; + } else + break; + } + free(name); + if (ret) { + if (*rname) + free(*rname); + *rname = NULL; + *rlen = 0; + return ret; + } + + return 0; } int -_hx509_name_cmp(const Name *n1, const Name *n2) +_hx509_name_ds_cmp(const DirectoryString *ds1, + const DirectoryString *ds2, + int *diff) { - int i, j, c; + uint32_t *ds1lp, *ds2lp; + size_t ds1len, ds2len; + int ret; - c = n1->u.rdnSequence.len - n2->u.rdnSequence.len; - if (c) - return c; + ret = dsstringprep(ds1, &ds1lp, &ds1len); + if (ret) + return ret; + ret = dsstringprep(ds2, &ds2lp, &ds2len); + if (ret) { + free(ds1lp); + return ret; + } + + if (ds1len != ds2len) + *diff = ds1len - ds2len; + else + *diff = memcmp(ds1lp, ds2lp, ds1len * sizeof(ds1lp[0])); + + free(ds1lp); + free(ds2lp); + + return 0; +} + +int +_hx509_name_cmp(const Name *n1, const Name *n2, int *c) +{ + int ret, i, j; + + *c = n1->u.rdnSequence.len - n2->u.rdnSequence.len; + if (*c) + return 0; for (i = 0 ; i < n1->u.rdnSequence.len; i++) { - c = n1->u.rdnSequence.val[i].len - n2->u.rdnSequence.val[i].len; - if (c) - return c; + *c = n1->u.rdnSequence.val[i].len - n2->u.rdnSequence.val[i].len; + if (*c) + return 0; for (j = 0; j < n1->u.rdnSequence.val[i].len; j++) { - c = der_heim_oid_cmp(&n1->u.rdnSequence.val[i].val[j].type, - &n1->u.rdnSequence.val[i].val[j].type); - if (c) - return c; + *c = der_heim_oid_cmp(&n1->u.rdnSequence.val[i].val[j].type, + &n1->u.rdnSequence.val[i].val[j].type); + if (*c) + return 0; - c = _hx509_name_ds_cmp(&n1->u.rdnSequence.val[i].val[j].value, - &n2->u.rdnSequence.val[i].val[j].value); - if (c) - return c; + ret = _hx509_name_ds_cmp(&n1->u.rdnSequence.val[i].val[j].value, + &n2->u.rdnSequence.val[i].val[j].value, + c); + if (ret) + return ret; + if (*c) + return 0; } } + *c = 0; return 0; } @@ -386,7 +455,11 @@ _hx509_name_cmp(const Name *n1, const Name *n2) int hx509_name_cmp(hx509_name n1, hx509_name n2) { - return _hx509_name_cmp(&n1->der_name, &n2->der_name); + int ret, diff; + ret = _hx509_name_cmp(&n1->der_name, &n2->der_name, &diff); + if (ret) + return ret; + return diff; } diff --git a/lib/hx509/revoke.c b/lib/hx509/revoke.c index 25d274fac..8f584fdef 100644 --- a/lib/hx509/revoke.c +++ b/lib/hx509/revoke.c @@ -763,11 +763,12 @@ hx509_revoke_verify(hx509_context context, for (i = 0; i < ctx->crls.len; i++) { struct revoke_crl *crl = &ctx->crls.val[i]; struct stat sb; + int diff; /* check if cert.issuer == crls.val[i].crl.issuer */ ret = _hx509_name_cmp(&c->tbsCertificate.issuer, - &crl->crl.tbsCertList.issuer); - if (ret) + &crl->crl.tbsCertList.issuer, &diff); + if (ret || diff) continue; ret = stat(crl->path, &sb);