Re: iked: support multiple subjectAltNames

2017-10-26 Thread Markus Friedl 
ok

2017-10-19 15:40 GMT+02:00 Patrick Wildt :
> Hi,
>
> so far, even if we look for our own cert, we only match the id against
> the first subjectAltName.  This means we cannot use certificates where
> we actually need a different one.  This diff changes the behaviour so
> that we check all subjectAltNames of a given certificate.
>
> ok?
>
> Patrick
>
> diff --git a/sbin/iked/ca.c b/sbin/iked/ca.c
> index a8034411e77..543bd0b8725 100644
> --- a/sbin/iked/ca.c
> +++ b/sbin/iked/ca.c
> @@ -65,7 +65,7 @@ intca_privkey_to_method(struct iked_id *);
>  struct ibuf *
>  ca_x509_serialize(X509 *);
>  int ca_x509_subjectaltname_cmp(X509 *, struct iked_static_id *);
> -int ca_x509_subjectaltname(X509 *cert, struct iked_id *);
> +int ca_x509_subjectaltname(X509 *cert, struct iked_id *, int);
>  int ca_dispatch_parent(int, struct privsep_proc *, struct imsg *);
>  int ca_dispatch_ikev2(int, struct privsep_proc *, struct imsg *);
>
> @@ -1400,34 +1400,31 @@ ca_x509_subjectaltname_cmp(X509 *cert, struct 
> iked_static_id *id)
>  {
> struct iked_id   sanid;
> char idstr[IKED_ID_SIZE];
> -   int  ret = -1;
> -
> -   bzero(&sanid, sizeof(sanid));
> -
> -   if (ca_x509_subjectaltname(cert, &sanid) != 0)
> -   return (-1);
> -
> -   ikev2_print_id(&sanid, idstr, sizeof(idstr));
> -
> -   /* Compare id types, length and data */
> -   if ((id->id_type != sanid.id_type) ||
> -   ((ssize_t)ibuf_size(sanid.id_buf) !=
> -   (id->id_length - id->id_offset)) ||
> -   (memcmp(id->id_data + id->id_offset,
> -   ibuf_data(sanid.id_buf),
> -   ibuf_size(sanid.id_buf)) != 0)) {
> +   int  ret = -1, lastpos = -1;
> +
> +   while (ca_x509_subjectaltname(cert, &sanid, lastpos++) == 0) {
> +   ikev2_print_id(&sanid, idstr, sizeof(idstr));
> +
> +   /* Compare id types, length and data */
> +   if ((id->id_type == sanid.id_type) &&
> +   ((ssize_t)ibuf_size(sanid.id_buf) ==
> +   (id->id_length - id->id_offset)) &&
> +   (memcmp(id->id_data + id->id_offset,
> +   ibuf_data(sanid.id_buf),
> +   ibuf_size(sanid.id_buf)) == 0)) {
> +   ret = 0;
> +   break;
> +   }
> log_debug("%s: %s mismatched", __func__, idstr);
> -   goto done;
> +   bzero(&sanid, sizeof(sanid));
> }
>
> -   ret = 0;
> - done:
> ibuf_release(sanid.id_buf);
> return (ret);
>  }
>
>  int
> -ca_x509_subjectaltname(X509 *cert, struct iked_id *id)
> +ca_x509_subjectaltname(X509 *cert, struct iked_id *id, int lastpos)
>  {
> X509_EXTENSION  *san;
> uint8_t  sanhdr[4], *data;
> @@ -1435,7 +1432,7 @@ ca_x509_subjectaltname(X509 *cert, struct iked_id *id)
> char idstr[IKED_ID_SIZE];
>
> if ((ext = X509_get_ext_by_NID(cert,
> -   NID_subject_alt_name, -1)) == -1 ||
> +   NID_subject_alt_name, lastpos)) == -1 ||
> ((san = X509_get_ext(cert, ext)) == NULL)) {
> log_debug("%s: did not find subjectAltName in certificate",
> __func__);
>

iked: support multiple subjectAltNames

2017-10-19 Thread Patrick Wildt 
Hi,

so far, even if we look for our own cert, we only match the id against
the first subjectAltName.  This means we cannot use certificates where
we actually need a different one.  This diff changes the behaviour so
that we check all subjectAltNames of a given certificate.

ok?

Patrick

diff --git a/sbin/iked/ca.c b/sbin/iked/ca.c
index a8034411e77..543bd0b8725 100644
--- a/sbin/iked/ca.c
+++ b/sbin/iked/ca.c
@@ -65,7 +65,7 @@ intca_privkey_to_method(struct iked_id *);
 struct ibuf *
 ca_x509_serialize(X509 *);
 int ca_x509_subjectaltname_cmp(X509 *, struct iked_static_id *);
-int ca_x509_subjectaltname(X509 *cert, struct iked_id *);
+int ca_x509_subjectaltname(X509 *cert, struct iked_id *, int);
 int ca_dispatch_parent(int, struct privsep_proc *, struct imsg *);
 int ca_dispatch_ikev2(int, struct privsep_proc *, struct imsg *);
 
@@ -1400,34 +1400,31 @@ ca_x509_subjectaltname_cmp(X509 *cert, struct 
iked_static_id *id)
 {
struct iked_id   sanid;
char idstr[IKED_ID_SIZE];
-   int  ret = -1;
-
-   bzero(&sanid, sizeof(sanid));
-
-   if (ca_x509_subjectaltname(cert, &sanid) != 0)
-   return (-1);
-
-   ikev2_print_id(&sanid, idstr, sizeof(idstr));
-
-   /* Compare id types, length and data */
-   if ((id->id_type != sanid.id_type) ||
-   ((ssize_t)ibuf_size(sanid.id_buf) !=
-   (id->id_length - id->id_offset)) ||
-   (memcmp(id->id_data + id->id_offset,
-   ibuf_data(sanid.id_buf),
-   ibuf_size(sanid.id_buf)) != 0)) {
+   int  ret = -1, lastpos = -1;
+
+   while (ca_x509_subjectaltname(cert, &sanid, lastpos++) == 0) {
+   ikev2_print_id(&sanid, idstr, sizeof(idstr));
+
+   /* Compare id types, length and data */
+   if ((id->id_type == sanid.id_type) &&
+   ((ssize_t)ibuf_size(sanid.id_buf) ==
+   (id->id_length - id->id_offset)) &&
+   (memcmp(id->id_data + id->id_offset,
+   ibuf_data(sanid.id_buf),
+   ibuf_size(sanid.id_buf)) == 0)) {
+   ret = 0;
+   break;
+   }
log_debug("%s: %s mismatched", __func__, idstr);
-   goto done;
+   bzero(&sanid, sizeof(sanid));
}
 
-   ret = 0;
- done:
ibuf_release(sanid.id_buf);
return (ret);
 }
 
 int
-ca_x509_subjectaltname(X509 *cert, struct iked_id *id)
+ca_x509_subjectaltname(X509 *cert, struct iked_id *id, int lastpos)
 {
X509_EXTENSION  *san;
uint8_t  sanhdr[4], *data;
@@ -1435,7 +1432,7 @@ ca_x509_subjectaltname(X509 *cert, struct iked_id *id)
char idstr[IKED_ID_SIZE];
 
if ((ext = X509_get_ext_by_NID(cert,
-   NID_subject_alt_name, -1)) == -1 ||
+   NID_subject_alt_name, lastpos)) == -1 ||
((san = X509_get_ext(cert, ext)) == NULL)) {
log_debug("%s: did not find subjectAltName in certificate",
__func__);