From 54977822e0a9194362fa7041c7e85a67e2ec0d11 Mon Sep 17 00:00:00 2001 From: markus Date: Wed, 7 May 2014 12:57:13 +0000 Subject: [PATCH] make authentication work with X509 certificates that don't have a subject-altname, i.e. support IKEV2_ID_ASN1_DN correctly; feedback & ok mikeb@ --- sbin/iked/ca.c | 154 ++++++++++++++++++++++++++++++++++++++++------ sbin/iked/iked.h | 3 +- sbin/iked/ikev2.c | 19 +++++- 3 files changed, 154 insertions(+), 22 deletions(-) diff --git a/sbin/iked/ca.c b/sbin/iked/ca.c index cb20531c71b..37ae7336ec2 100644 --- a/sbin/iked/ca.c +++ b/sbin/iked/ca.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ca.c,v 1.29 2014/05/05 18:56:42 markus Exp $ */ +/* $OpenBSD: ca.c,v 1.30 2014/05/07 12:57:13 markus Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -56,6 +56,7 @@ int ca_getauth(struct iked *, struct imsg *); X509 *ca_by_subjectpubkey(X509_STORE *, u_int8_t *, size_t); X509 *ca_by_issuer(X509_STORE *, X509_NAME *, struct iked_static_id *); int ca_subjectpubkey_digest(X509 *, u_int8_t *, u_int *); +int ca_x509_subject_cmp(X509 *, struct iked_static_id *); int ca_validate_pubkey(struct iked *, struct iked_static_id *, void *, size_t); int ca_validate_cert(struct iked *, struct iked_static_id *, @@ -731,9 +732,16 @@ ca_by_issuer(X509_STORE *ctx, X509_NAME *subject, struct iked_static_id *id) if ((issuer = X509_get_issuer_name(cert)) == NULL) continue; else if (X509_NAME_cmp(subject, issuer) == 0) { - if (ca_x509_subjectaltname_cmp(cert, id) != 0) - continue; - return (cert); + switch (id->id_type) { + case IKEV2_ID_ASN1_DN: + if (ca_x509_subject_cmp(cert, id) == 0) + return (cert); + break; + default: + if (ca_x509_subjectaltname_cmp(cert, id) == 0) + return (cert); + break; + } } } @@ -906,6 +914,101 @@ ca_x509_name(void *ptr) return (strdup(buf)); } +/* + * Copy 'src' to 'dst' until 'marker' is found while unescaping '\' + * characters. The return value tells the caller where to continue + * parsing (might be the end of the string) or NULL on error. + */ +static char * +ca_x509_name_unescape(char *src, char *dst, char marker) +{ + while (*src) { + if (*src == marker) { + src++; + break; + } + if (*src == '\\') { + src++; + if (!*src) { + log_warnx("%s: '\\' at end of string", + __func__); + *dst = '\0'; + return (NULL); + } + } + *dst++ = *src++; + } + *dst = '\0'; + return (src); +} +/* + * Parse an X509 subject name where 'subject' is in the format + * /type0=value0/type1=value1/type2=... + * where characters may be escaped by '\'. + * See lib/libssl/src/apps/apps.c:parse_name() + */ +void * +ca_x509_name_parse(char *subject) +{ + char *cp, *value = NULL, *type = NULL; + size_t maxlen; + X509_NAME *name = NULL; + + if (*subject != '/') { + log_warnx("%s: leading '/' missing in '%s'", __func__, subject); + goto err; + } + + /* length of subject is upper bound for unescaped type/value */ + maxlen = strlen(subject) + 1; + + if ((type = calloc(1, maxlen)) == NULL || + (value = calloc(1, maxlen)) == NULL || + (name = X509_NAME_new()) == NULL) + goto err; + + cp = subject + 1; + while (*cp) { + /* unescape type, terminated by '=' */ + cp = ca_x509_name_unescape(cp, type, '='); + if (cp == NULL) { + log_warnx("%s: could not parse type", __func__); + goto err; + } + if (!*cp) { + log_warnx("%s: missing value", __func__); + goto err; + } + /* unescape value, terminated by '/' */ + cp = ca_x509_name_unescape(cp, value, '/'); + if (cp == NULL) { + log_warnx("%s: could not parse value", __func__); + goto err; + } + if (!*type || !*value) { + log_warnx("%s: empty type or value", __func__); + goto err; + } + log_debug("%s: setting '%s' to '%s'", __func__, type, value); + if (!X509_NAME_add_entry_by_txt(name, type, MBSTRING_ASC, + value, -1, -1, 0)) { + log_warnx("%s: setting '%s' to '%s' failed", __func__, + type, value); + ca_sslerror(__func__); + goto err; + } + } + free(type); + free(value); + return (name); + +err: + X509_NAME_free(name); + free(type); + free(value); + return (NULL); +} + int ca_validate_pubkey(struct iked *env, struct iked_static_id *id, void *data, size_t len) @@ -1016,9 +1119,7 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id, BIO *rawcert = NULL; X509 *cert = NULL; int ret = -1, result, error; - size_t idlen, idoff; - const u_int8_t *idptr; - X509_NAME *idname = NULL, *subject; + X509_NAME *subject; const char *errstr = "failed"; if (len == 0) { @@ -1047,17 +1148,7 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id, switch (id->id_type) { case IKEV2_ID_ASN1_DN: - idoff = id->id_offset; - if (id->id_length <= idoff) { - errstr = "invalid ASN1_DN id length"; - goto done; - } - idlen = id->id_length - idoff; - idptr = id->id_data + idoff; - - if ((idname = d2i_X509_NAME(NULL, - &idptr, idlen)) == NULL || - X509_NAME_cmp(subject, idname) != 0) { + if (ca_x509_subject_cmp(cert, id) < 0) { errstr = "ASN1_DN identifier mismatch"; goto done; } @@ -1100,8 +1191,6 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id, if (cert != NULL) log_debug("%s: %s %.100s", __func__, cert->name, errstr); - if (idname != NULL) - X509_NAME_free(idname); if (rawcert != NULL) { BIO_free(rawcert); if (cert != NULL) @@ -1111,6 +1200,31 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id, return (ret); } +/* check if subject from cert matches the id */ +int +ca_x509_subject_cmp(X509 *cert, struct iked_static_id *id) +{ + X509_NAME *subject, *idname = NULL; + const u_int8_t *idptr; + size_t idlen; + int ret = -1; + + if (id->id_type != IKEV2_ID_ASN1_DN) + return (-1); + if ((subject = X509_get_subject_name(cert)) == NULL) + return (-1); + if (id->id_length <= id->id_offset) + return (-1); + idlen = id->id_length - id->id_offset; + idptr = id->id_data + id->id_offset; + if ((idname = d2i_X509_NAME(NULL, &idptr, idlen)) == NULL) + return (-1); + if (X509_NAME_cmp(subject, idname) == 0) + ret = 0; + X509_NAME_free(idname); + return (ret); +} + int ca_x509_subjectaltname_cmp(X509 *cert, struct iked_static_id *id) { diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h index d89d19dba38..172cc68cea7 100644 --- a/sbin/iked/iked.h +++ b/sbin/iked/iked.h @@ -1,4 +1,4 @@ -/* $OpenBSD: iked.h,v 1.77 2014/05/06 14:10:53 markus Exp $ */ +/* $OpenBSD: iked.h,v 1.78 2014/05/07 12:57:13 markus Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -848,6 +848,7 @@ void ca_sslinit(void); void ca_sslerror(const char *); char *ca_asn1_name(u_int8_t *, size_t); char *ca_x509_name(void *); +void *ca_x509_name_parse(char *); /* timer.c */ void timer_set(struct iked *, struct iked_timer *, diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c index 552ca7227de..888b09bbe8a 100644 --- a/sbin/iked/ikev2.c +++ b/sbin/iked/ikev2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ikev2.c,v 1.109 2014/05/07 10:52:47 markus Exp $ */ +/* $OpenBSD: ikev2.c,v 1.110 2014/05/07 12:57:13 markus Exp $ */ /* * Copyright (c) 2010-2013 Reyk Floeter @@ -39,6 +39,7 @@ #include #include +#include #include "iked.h" #include "ikev2.h" @@ -1109,6 +1110,9 @@ ikev2_policy2id(struct iked_static_id *polid, struct iked_id *id, int srcid) char idstr[IKED_ID_SIZE]; struct in_addr in4; struct in6_addr in6; + X509_NAME *name = NULL; + u_int8_t *p; + size_t len; /* Fixup the local Id if not specified */ if (srcid && polid->id_type == 0) { @@ -1151,6 +1155,19 @@ ikev2_policy2id(struct iked_static_id *polid, struct iked_id *id, int srcid) return (-1); } break; + case IKEV2_ID_ASN1_DN: + /* policy has ID in string-format, convert to ASN1 */ + if ((name = ca_x509_name_parse(polid->id_data)) == NULL || + (len = i2d_X509_NAME(name, NULL)) < 0 || + (p = ibuf_reserve(id->id_buf, len)) == NULL || + (i2d_X509_NAME(name, &p)) < 0) { + if (name) + X509_NAME_free(name); + ibuf_release(id->id_buf); + return (-1); + } + X509_NAME_free(name); + break; default: if (ibuf_add(id->id_buf, polid->id_data, polid->id_length) != 0) { -- 2.20.1