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>

diff --git a/ssl/ssl.h b/ssl/ssl.h
index 2705c7cf64568996c7f92b2bbfb5bd4c73e875ce..2498c3191d6b696862465cc7be967d22cefd7147 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -1273,7 +1273,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 3bd50dfa4c8001b02287447fbce36e44c0218b63..8e9110a4ceaf4258d38115c3efc27501680deee5 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -534,11 +534,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;
@@ -1410,6 +1425,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 ba6d3ded74bd1d6051242ca89c0c838b4c5e4ee4..bd14806e6a6fe89969bf7e802b97236b3b7aedf2 100644 (file)
--- 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 7d0774d5e7a31004d589a3c1093ec7ff85535499..86fb69cb07d5393eb0e156707a7a711a94227eb4 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1480,7 +1480,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;
@@ -2485,6 +2485,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);