Allow key exchange with RSA signature authentication to work with

Cisco IOS and other initiators that only send their certs in response
to CERT_REQUEST.

With input and help from cloder@, Stuart Henderson, mpf@, and several
others who did lots of testing - thanks to all.

ok hshoexer@

9 files changed, 136 insertions, 32 deletions

diff --git a/sbin/isakmpd/cert.c b/sbin/isakmpd/cert.c
index b3e263e8cf3..d534336dc34 100644
--- a/sbin/isakmpd/cert.c
+++ b/sbin/isakmpd/cert.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: cert.c,v 1.31 2005/04/08 22:32:09 cloder Exp $	 */
+/* $OpenBSD: cert.c,v 1.32 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: cert.c,v 1.18 2000/09/28 12:53:27 niklas Exp $	 */
 
 /*
@@ -49,7 +49,8 @@ struct cert_handler cert_handler[] = {
 	x509_cert_insert, x509_cert_free,
 	x509_certreq_validate, x509_certreq_decode, x509_free_aca,
 	x509_cert_obtain, x509_cert_get_key, x509_cert_get_subjects,
-	x509_cert_dup, x509_serialize, x509_printable, x509_from_printable
+	x509_cert_dup, x509_serialize, x509_printable, x509_from_printable,
+	x509_ca_count
     },
     {
 	ISAKMP_CERTENC_KEYNOTE,
@@ -58,7 +59,7 @@ struct cert_handler cert_handler[] = {
 	keynote_certreq_validate, keynote_certreq_decode, keynote_free_aca,
 	keynote_cert_obtain, keynote_cert_get_key, keynote_cert_get_subjects,
 	keynote_cert_dup, keynote_serialize, keynote_printable,
-	keynote_from_printable
+	keynote_from_printable, keynote_ca_count
     },
 };
 
@@ -118,18 +119,32 @@ certreq_decode(u_int16_t type, u_int8_t *data, u_int32_t datalen)
 
 	aca.id = type;
 	aca.handler = handler;
+	aca.data = aca.raw_ca = NULL;
 
 	if (datalen > 0) {
-		aca.data = handler->certreq_decode(data, datalen);
-		if (!aca.data)
+		int rc;
+
+		rc = handler->certreq_decode(&aca.data, data, datalen);
+		if (!rc)
+			return 0;
+
+		aca.raw_ca = malloc(datalen);
+		if (aca.raw_ca == NULL) {
+			log_error("certreq_decode: malloc (%lu) failed",
+			    (unsigned long)datalen);
+			handler->free_aca(aca.data);
 			return 0;
-	} else
-		aca.data = 0;
+		}
+
+		memcpy(aca.raw_ca, data, datalen);
+	}
+	aca.raw_ca_len = datalen;
 
 	ret = malloc(sizeof aca);
 	if (!ret) {
 		log_error("certreq_decode: malloc (%lu) failed",
 		    (unsigned long)sizeof aca);
+		free(aca.raw_ca);
 		handler->free_aca(aca.data);
 		return 0;
 	}
diff --git a/sbin/isakmpd/cert.h b/sbin/isakmpd/cert.h
index 151b2f941e1..d805f4b7a44 100644
--- a/sbin/isakmpd/cert.h
+++ b/sbin/isakmpd/cert.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: cert.h,v 1.14 2004/05/14 08:42:56 hshoexer Exp $	 */
+/* $OpenBSD: cert.h,v 1.15 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: cert.h,v 1.8 2000/09/28 12:53:27 niklas Exp $	 */
 
 /*
@@ -52,6 +52,7 @@
  * cert_printable - for X509, the hex representation of the serialized form;
  *                  for KeyNote, itself.
  * cert_from_printable - the reverse of cert_printable
+ * ca_count - how many CAs we have in our store (for CERT_REQ processing)
  */
 
 struct cert_handler {
@@ -63,7 +64,7 @@ struct cert_handler {
 	int	 (*cert_insert)(int, void *);
 	void	 (*cert_free)(void *);
 	int	 (*certreq_validate)(u_int8_t *, u_int32_t);
-	void	*(*certreq_decode)(u_int8_t *, u_int32_t);
+	int	 (*certreq_decode)(void **, u_int8_t *, u_int32_t);
 	void	 (*free_aca)(void *);
 	int	 (*cert_obtain)(u_int8_t *, size_t, void *, u_int8_t **,
 		     u_int32_t *);
@@ -74,6 +75,7 @@ struct cert_handler {
 	void	 (*cert_serialize) (void *, u_int8_t **, u_int32_t *);
 	char	*(*cert_printable) (void *);
 	void	*(*cert_from_printable) (char *);
+	int	 (*ca_count)(void);
 };
 
 /* The acceptable authority of cert request.  */
@@ -85,6 +87,10 @@ struct certreq_aca {
 
 	/* If data is a null pointer, everything is acceptable.  */
 	void	*data;
+
+	/* Copy of raw CA value received */
+	u_int32_t raw_ca_len;
+	void	*raw_ca;
 };
 
 struct certreq_aca *certreq_decode(u_int16_t, u_int8_t *, u_int32_t);
diff --git a/sbin/isakmpd/exchange.c b/sbin/isakmpd/exchange.c
index 1cce615e439..39d6242d790 100644
--- a/sbin/isakmpd/exchange.c
+++ b/sbin/isakmpd/exchange.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: exchange.c,v 1.130 2007/04/16 13:01:39 moritz Exp $	 */
+/* $OpenBSD: exchange.c,v 1.131 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: exchange.c,v 1.143 2000/12/04 00:02:25 angelos Exp $	 */
 
 /*
@@ -1593,6 +1593,8 @@ exchange_free_aca_list(struct exchange *exchange)
 
 	for (aca = TAILQ_FIRST(&exchange->aca_list); aca;
 	    aca = TAILQ_FIRST(&exchange->aca_list)) {
+		if (aca->raw_ca)
+			free(aca->raw_ca);
 		if (aca->data) {
 			if (aca->handler)
 				aca->handler->free_aca(aca->data);
@@ -1603,6 +1605,58 @@ exchange_free_aca_list(struct exchange *exchange)
 	}
 }
 
+/* Add any CERTREQs we should send.  */
+int
+exchange_add_certreqs(struct message *msg)
+{
+	struct exchange *exchange = msg->exchange;
+	struct certreq_aca *aca;
+	u_int8_t *buf;
+
+	/*
+	 * Some peers (e.g. Cisco IOS) won't send their cert unless we
+	 * specifically ask beforehand with CERTREQ.  We reflect any
+	 * CERTREQs we receive from the initiator in order to do this.
+	 * This avoids leaking information about which CAs we trust,
+	 * and works in the most common case where both ends trust the
+	 * same CA.
+	 */
+	for (aca = TAILQ_FIRST(&exchange->aca_list); aca;
+	    aca = TAILQ_NEXT(aca, link)) {
+
+		/* But only do this if we have at least one CA */
+		if (aca->handler != NULL && aca->handler->ca_count() == 0) {
+			LOG_DBG((LOG_EXCHANGE, 10,
+			    "exchange_add_certreqs: no CA, so not "
+			    "sending a CERTREQ"));
+			continue;
+		}
+
+		if (aca->raw_ca_len) {
+			buf = malloc(ISAKMP_CERTREQ_SZ + aca->raw_ca_len);
+			if (buf == NULL) {
+				log_error("exchange_add_certreqs: "
+				    "malloc (%lu) failed",
+				    ISAKMP_CERTREQ_SZ +
+				    (unsigned long)aca->raw_ca_len);
+				return -1;
+			}
+
+			buf[ISAKMP_CERTREQ_TYPE_OFF] = aca->id;
+			memcpy(buf + ISAKMP_CERTREQ_AUTHORITY_OFF,
+			    aca->raw_ca, aca->raw_ca_len);
+
+			if (message_add_payload(msg, ISAKMP_PAYLOAD_CERT_REQ,
+			    buf, ISAKMP_CERTREQ_SZ + aca->raw_ca_len, 1)) {
+				free(buf);
+				return -1;
+			}
+		}
+	}
+
+	return 0;
+}
+
 /* Obtain certificates from acceptable certification authority.  */
 int
 exchange_add_certs(struct message *msg)
diff --git a/sbin/isakmpd/exchange.h b/sbin/isakmpd/exchange.h
index 56fb1efc8b1..5a06f4a23f4 100644
--- a/sbin/isakmpd/exchange.h
+++ b/sbin/isakmpd/exchange.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: exchange.h,v 1.32 2006/07/02 13:19:00 hshoexer Exp $	 */
+/* $OpenBSD: exchange.h,v 1.33 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: exchange.h,v 1.28 2000/09/28 12:54:28 niklas Exp $	 */
 
 /*
@@ -224,6 +224,7 @@ struct exchange {
 #define EXCHANGE_FLAG_OPENBSD		0x0200	/* Peer is OpenBSD */
 
 extern int      exchange_add_certs(struct message *);
+extern int      exchange_add_certreqs(struct message *);
 extern void     exchange_finalize(struct message *);
 extern void     exchange_free(struct exchange *);
 extern void     exchange_free_aca_list(struct exchange *);
diff --git a/sbin/isakmpd/ike_phase_1.c b/sbin/isakmpd/ike_phase_1.c
index 198d48c84d1..d793d13ddbd 100644
--- a/sbin/isakmpd/ike_phase_1.c
+++ b/sbin/isakmpd/ike_phase_1.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ike_phase_1.c,v 1.69 2007/05/07 18:19:56 cloder Exp $	 */
+/* $OpenBSD: ike_phase_1.c,v 1.70 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: ike_phase_1.c,v 1.31 2000/12/11 23:47:56 niklas Exp $	 */
 
 /*
@@ -548,6 +548,11 @@ ike_phase_1_send_KE_NONCE(struct message *msg, size_t nonce_sz)
 		/* XXX Log?  */
 		return -1;
 	}
+	/* Are there any CERTREQs to send? */
+	if (exchange_add_certreqs(msg)) {
+		/* XXX Log? */
+		return -1;
+	}
 	/* Try to add certificates which are acceptable for the CERTREQs */
 	if (exchange_add_certs(msg)) {
 		/* XXX Log? */
diff --git a/sbin/isakmpd/policy.c b/sbin/isakmpd/policy.c
index f525efc47af..2c25faedf26 100644
--- a/sbin/isakmpd/policy.c
+++ b/sbin/isakmpd/policy.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: policy.c,v 1.90 2007/04/16 13:01:39 moritz Exp $	 */
+/* $OpenBSD: policy.c,v 1.91 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: policy.c,v 1.49 2000/10/24 13:33:39 niklas Exp $ */
 
 /*
@@ -2096,11 +2096,11 @@ keynote_certreq_validate(u_int8_t *data, u_int32_t len)
 }
 
 /* Beats me what we should be doing with this.  */
-void *
-keynote_certreq_decode(u_int8_t *data, u_int32_t len)
+int
+keynote_certreq_decode(void **pdata, u_int8_t *data, u_int32_t len)
 {
 	/* XXX */
-	return NULL;
+	return 0;
 }
 
 void
@@ -2302,3 +2302,10 @@ keynote_from_printable(char *cert)
 {
 	return strdup(cert);
 }
+
+/* Number of CAs we trust (currently this is x509 only) */
+int
+keynote_ca_count(void)
+{
+	return 0;
+}
diff --git a/sbin/isakmpd/policy.h b/sbin/isakmpd/policy.h
index a4c2c9dcf69..ed4b1f530d9 100644
--- a/sbin/isakmpd/policy.h
+++ b/sbin/isakmpd/policy.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: policy.h,v 1.16 2005/04/05 22:53:50 cloder Exp $	 */
+/* $OpenBSD: policy.h,v 1.17 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: policy.h,v 1.12 2000/09/28 12:53:27 niklas Exp $ */
 
 /*
@@ -54,7 +54,7 @@ extern int      keynote_cert_validate(void *);
 extern int      keynote_cert_insert(int, void *);
 extern void     keynote_cert_free(void *);
 extern int      keynote_certreq_validate(u_int8_t *, u_int32_t);
-extern void    *keynote_certreq_decode(u_int8_t *, u_int32_t);
+extern int      keynote_certreq_decode(void **, u_int8_t *, u_int32_t);
 extern void     keynote_free_aca(void *);
 extern int	keynote_cert_obtain(u_int8_t *, size_t, void *,
 		    u_int8_t **, u_int32_t *);
@@ -65,4 +65,5 @@ extern void    *keynote_cert_dup(void *);
 extern void     keynote_serialize(void *, u_int8_t **, u_int32_t *);
 extern char    *keynote_printable(void *);
 extern void    *keynote_from_printable(char *);
+extern int	keynote_ca_count(void);
 #endif	/* _POLICY_H_ */
diff --git a/sbin/isakmpd/x509.c b/sbin/isakmpd/x509.c
index 41cf05a69d2..5fc0f91dcef 100644
--- a/sbin/isakmpd/x509.c
+++ b/sbin/isakmpd/x509.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509.c,v 1.109 2007/04/16 13:01:39 moritz Exp $	 */
+/* $OpenBSD: x509.c,v 1.110 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: x509.c,v 1.54 2001/01/16 18:42:16 ho Exp $	 */
 
 /*
@@ -78,6 +78,8 @@ static int	 x509_hash_enter(X509 *);
 static X509_STORE *x509_certs = 0;
 static X509_STORE *x509_cas = 0;
 
+static int n_x509_cas = 0;
+
 /* Initial number of bits used as hash.  */
 #define INITIAL_BUCKET_BITS 6
 
@@ -571,7 +573,7 @@ x509_hash_enter(X509 *cert)
 /* X509 Certificate Handling functions.  */
 
 int
-x509_read_from_dir(X509_STORE *ctx, char *name, int hash)
+x509_read_from_dir(X509_STORE *ctx, char *name, int hash, int *pcount)
 {
 	FILE		*certfp;
 	X509		*cert;
@@ -628,6 +630,10 @@ x509_read_from_dir(X509_STORE *ctx, char *name, int hash)
 			    "failed for %s", file);
 			continue;
 		}
+
+		if (pcount != NULL)
+			(*pcount)++;
+
 		if (!X509_STORE_add_cert(ctx, cert)) {
 			/*
 			 * This is actually expected if we have several
@@ -752,7 +758,7 @@ x509_cert_init(void)
 		log_print("x509_cert_init: creating new X509_STORE failed");
 		return 0;
 	}
-	if (!x509_read_from_dir(x509_cas, dirname, 0)) {
+	if (!x509_read_from_dir(x509_cas, dirname, 0, &n_x509_cas)) {
 		log_print("x509_cert_init: x509_read_from_dir failed");
 		return 0;
 	}
@@ -771,7 +777,7 @@ x509_cert_init(void)
 		log_print("x509_cert_init: creating new X509_STORE failed");
 		return 0;
 	}
-	if (!x509_read_from_dir(x509_certs, dirname, 1)) {
+	if (!x509_read_from_dir(x509_certs, dirname, 1, NULL)) {
 		log_print("x509_cert_init: x509_read_from_dir failed");
 		return 0;
 	}
@@ -949,8 +955,8 @@ x509_certreq_validate(u_int8_t *asn, u_int32_t len)
 }
 
 /* Decode the BER Encoding of a RDNSequence in the CERT_REQ payload.  */
-void *
-x509_certreq_decode(u_int8_t *asn, u_int32_t len)
+int
+x509_certreq_decode(void **pdata, u_int8_t *asn, u_int32_t len)
 {
 #if 0
 	/* XXX This needs to be done later.  */
@@ -993,7 +999,7 @@ x509_certreq_decode(u_int8_t *asn, u_int32_t len)
 fail:
 	asn_free(&aca);
 #endif
-	return 0;
+	return 1;
 }
 
 void
@@ -1001,11 +1007,13 @@ x509_free_aca(void *blob)
 {
 	struct x509_aca *aca = blob;
 
-	free(aca->name1.type);
-	free(aca->name1.val);
+	if (aca != NULL) {
+		free(aca->name1.type);
+		free(aca->name1.val);
 
-	free(aca->name2.type);
-	free(aca->name2.val);
+		free(aca->name2.type);
+		free(aca->name2.val);
+	}
 }
 
 X509 *
@@ -1338,3 +1346,9 @@ x509_DN_string(u_int8_t *asn1, size_t sz)
 	return strdup(buf);
 }
 
+/* Number of CAs we trust (to decide whether we can send CERT_REQ) */
+int
+x509_ca_count(void)
+{
+	return n_x509_cas;
+}
diff --git a/sbin/isakmpd/x509.h b/sbin/isakmpd/x509.h
index adba74e8c0b..3c33a3eb78c 100644
--- a/sbin/isakmpd/x509.h
+++ b/sbin/isakmpd/x509.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509.h,v 1.21 2004/05/23 18:17:56 hshoexer Exp $	 */
+/* $OpenBSD: x509.h,v 1.22 2007/08/05 09:43:09 tom Exp $	 */
 /* $EOM: x509.h,v 1.11 2000/09/28 12:53:27 niklas Exp $	 */
 
 /*
@@ -61,7 +61,7 @@ struct X509_STORE;
 /* Functions provided by cert handler.  */
 
 int             x509_certreq_validate(u_int8_t *, u_int32_t);
-void           *x509_certreq_decode(u_int8_t *, u_int32_t);
+int             x509_certreq_decode(void **, u_int8_t *, u_int32_t);
 void            x509_cert_free(void *);
 void           *x509_cert_get(u_int8_t *, u_int32_t);
 int             x509_cert_get_key(void *, void *);
@@ -76,6 +76,7 @@ void           *x509_cert_dup(void *);
 void            x509_serialize(void *, u_int8_t **, u_int32_t *);
 char           *x509_printable(void *);
 void           *x509_from_printable(char *);
+int		x509_ca_count(void);
 
 /* Misc. X509 certificate functions.  */
 
@@ -84,7 +85,7 @@ int             x509_cert_insert(int, void *);
 int             x509_cert_subjectaltname(X509 * cert, u_char **, u_int *);
 X509           *x509_from_asn(u_char *, u_int);
 int             x509_generate_kn(int, X509 *);
-int             x509_read_from_dir(X509_STORE *, char *, int);
+int             x509_read_from_dir(X509_STORE *, char *, int, int *);
 int             x509_read_crls_from_dir(X509_STORE *, char *);
 
 #endif				/* _X509_H_ */