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)

diff --git a/ssl/ssl.h b/ssl/ssl.h
index 823bf0625b5ebd5c6257178d0fe58b9569b6ee47..65dfe979338621801a491ad160d981eda1f58029 100644 (file)
--- 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 f9a67b38326452e028f5b7f1f2becc38f5aaef0d..af08597474f6beeae814f2f6dfeb1ba66788ac09 100644 (file)
--- 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 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 3b3e0e334f6eca390a9e6f0511c1393af51584ed..f13b3762c9a683eafb74d57f4ac68644a79a6e6a 100644 (file)
--- 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);