Add custom extension sanity checks.

Reject attempts to use extensions handled internally.

Add flags to each extension structure to indicate if an extension
has been sent or received. Enforce RFC5246 compliance by rejecting
duplicate extensions and unsolicited extensions and only send a
server extension if we have sent the corresponding client extension.
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 28ea0a0c6a5e4e217c405340fa22a8503c7a17db)

4 files changed, 96 insertions, 8 deletions

diff --git a/ssl/ssl.h b/ssl/ssl.h
index 823bf0625b..65dfe97933 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -1266,7 +1266,7 @@ const char *SSL_get_psk_identity(const SSL *s);
  * 
  * Returns nonzero on success.  You cannot register twice for the same 
  * extension number, and registering for an extension number already 
- * handled by OpenSSL will succeed, but the callbacks will not be invoked.
+ * handled by OpenSSL will fail.
  *
  * NULL can be registered for any callback function.  For the client
  * functions, a NULL custom_cli_ext_first_cb_fn sends an empty ClientHello
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index f9a67b3832..af08597474 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -528,11 +528,26 @@ typedef struct cert_pkey_st
 
 typedef struct {
 	unsigned short ext_type;
+	/* Per-connection flags relating to this extension type: not used 
+	 * if part of an SSL_CTX structure.
+	 */
+	unsigned short ext_flags;
 	custom_ext_add_cb add_cb; 
 	custom_ext_parse_cb parse_cb; 
 	void *arg;
 } custom_ext_method;
 
+/* ext_flags values */
+
+/* Indicates an extension has been received.
+ * Used to check for unsolicited or duplicate extensions.
+ */
+#define SSL_EXT_FLAG_RECEIVED	0x1
+/* Indicates an extension has been sent: used to
+ * enable sending of corresponding ServerHello extension.
+ */
+#define SSL_EXT_FLAG_SENT	0x2
+
 typedef struct {
 	custom_ext_method *meths;
 	size_t meths_count;
@@ -1380,6 +1395,8 @@ int srp_verify_server_param(SSL *s, int *al);
 
 /* t1_ext.c */
 
+void custom_ext_init(custom_ext_methods *meths);
+
 int custom_ext_parse(SSL *s, int server,
 			unsigned short ext_type,
 			const unsigned char *ext_data, 
diff --git a/ssl/t1_ext.c b/ssl/t1_ext.c
index ba6d3ded74..bd14806e6a 100644
--- a/ssl/t1_ext.c
+++ b/ssl/t1_ext.c
@@ -73,6 +73,16 @@ static custom_ext_method *custom_ext_find(custom_ext_methods *exts,
 		}
 	return NULL;
 	}
+/* Initialise custom extensions flags to indicate neither sent nor
+ * received.
+ */
+void custom_ext_init(custom_ext_methods *exts)
+	{
+	size_t i;
+	custom_ext_method *meth = exts->meths;
+	for (i = 0; i < exts->meths_count; i++, meth++)
+		meth->ext_flags = 0;
+	}
 
 /* pass received custom extension data to the application for parsing */
 
@@ -86,7 +96,28 @@ int custom_ext_parse(SSL *s, int server,
 	custom_ext_method *meth;
 	meth = custom_ext_find(exts, ext_type);
 	/* If not found or no parse function set, return success */
-	if (!meth || !meth->parse_cb)
+	/* If not found return success */
+	if (!meth)
+		return 1;
+	if (!server)
+		{
+		/* If it's ServerHello we can't have any extensions not 
+		 * sent in ClientHello.
+		 */
+		if (!(meth->ext_flags & SSL_EXT_FLAG_SENT))
+			{
+			*al = TLS1_AD_UNSUPPORTED_EXTENSION;
+			return 0;
+			}
+		}
+	/* If already present it's a duplicate */
+	if (meth->ext_flags & SSL_EXT_FLAG_RECEIVED)
+		{
+		*al = TLS1_AD_DECODE_ERROR;
+		return 0;
+		}
+	meth->ext_flags |= SSL_EXT_FLAG_RECEIVED;
+	if (!meth->parse_cb)
 		return 1;
 
 	return meth->parse_cb(s, ext_type, ext_data, ext_size, al, meth->arg);
@@ -112,11 +143,17 @@ int custom_ext_add(SSL *s, int server,
 		unsigned short outlen = 0;
 		meth = exts->meths + i;
 
-		/* For servers no callback omits extension,
-		 * For clients it sends empty extension.
-		 */
-		if (server && !meth->add_cb)
-			continue;
+		if (server)
+			{
+			/* For ServerHello only send extensions present
+			 * in ClientHello.
+			 */
+			if (!(meth->ext_flags & SSL_EXT_FLAG_RECEIVED))
+				continue;
+			/* If callback absent for server skip it */
+			if (!meth->add_cb)
+				continue;
+			}
 		if (meth->add_cb)
 			{
 			int cb_retval = 0;
@@ -137,6 +174,14 @@ int custom_ext_add(SSL *s, int server,
 			memcpy(ret, out, outlen);
 			ret += outlen;
 			}
+		/* We can't send duplicates: code logic should prevent this */
+		OPENSSL_assert(!(meth->ext_flags & SSL_EXT_FLAG_SENT));
+		/* Indicate extension has been sent: this is both a sanity
+		 * check to ensure we don't send duplicate extensions
+		 * and indicates to servers that an extension can be
+		 * sent in ServerHello.
+		 */
+		meth->ext_flags |= SSL_EXT_FLAG_SENT;
 		}
 	*pret = ret;
 	return 1;
@@ -170,6 +215,30 @@ static int custom_ext_set(custom_ext_methods *exts,
 			void *arg)
 	{
 	custom_ext_method *meth;
+	/* See if it is a supported internally */
+	switch(ext_type)
+		{
+	case TLSEXT_TYPE_application_layer_protocol_negotiation:
+	case TLSEXT_TYPE_ec_point_formats:
+	case TLSEXT_TYPE_elliptic_curves:
+	case TLSEXT_TYPE_heartbeat:
+	case TLSEXT_TYPE_next_proto_neg:
+	case TLSEXT_TYPE_padding:
+	case TLSEXT_TYPE_renegotiate:
+	case TLSEXT_TYPE_server_name:
+	case TLSEXT_TYPE_session_ticket:
+	case TLSEXT_TYPE_signature_algorithms:
+	case TLSEXT_TYPE_srp:
+	case TLSEXT_TYPE_status_request:
+	case TLSEXT_TYPE_use_srtp:
+#ifdef TLSEXT_TYPE_opaque_prf_input
+	case TLSEXT_TYPE_opaque_prf_input:
+#endif
+#ifdef TLSEXT_TYPE_encrypt_then_mac
+	case TLSEXT_TYPE_encrypt_then_mac:
+#endif
+		return 0;
+		}
 	/* Search for duplicate */
 	if (custom_ext_find(exts, ext_type))
 		return 0;
@@ -183,6 +252,7 @@ static int custom_ext_set(custom_ext_methods *exts,
 		}
 
 	meth = exts->meths + exts->meths_count;
+	memset(meth, 0, sizeof(custom_ext_method));
 	meth->parse_cb = parse_cb;
 	meth->add_cb = add_cb;
 	meth->ext_type = ext_type;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 3b3e0e334f..f13b3762c9 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1444,7 +1444,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, unsigned c
 			}
                 ret += el;
                 }
-
+	custom_ext_init(&s->cert->cli_ext);
 	/* Add custom TLS Extensions to ClientHello */
 	if (!custom_ext_add(s, 0, &ret, limit, al))
 		return NULL;
@@ -2422,6 +2422,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char
 int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n) 
 	{
 	int al = -1;
+	custom_ext_init(&s->cert->srv_ext);
 	if (ssl_scan_clienthello_tlsext(s, p, d, n, &al) <= 0) 
 		{
 		ssl3_send_alert(s,SSL3_AL_FATAL,al);