New extension callback features.

Support separate parse and add callback arguments.
Add new callback so an application can free extension data.
Change return value for send functions so < 0 is an error 0
omits extension and > 0 includes it. This is more consistent
with the behaviour of other functions in OpenSSL.

Modify parse_cb handling so <= 0 is an error.

Make SSL_CTX_set_custom_cli_ext and SSL_CTX_set_custom_cli_ext argument
order consistent.

NOTE: these changes WILL break existing code.

Remove (now inaccurate) in line documentation.
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit 33f653adf3bff5b0795e22de1f54b7c5472252d0)

apps/s_client.c

@@ -1340,7 +1340,7 @@ bad:
 				{
 				SSL_CTX_set_custom_cli_ext(ctx,
 							   serverinfo_types[i],
-							   NULL, 
+							   NULL, NULL, NULL,
 							   serverinfo_cli_cb,
 							   NULL);
 				}

ssl/ssl.h

@@ -385,36 +385,23 @@ typedef int (*tls_session_ticket_ext_cb_fn)(SSL *s, const unsigned char *data, i
 typedef int (*tls_session_secret_cb_fn)(SSL *s, void *secret, int *secret_len, STACK_OF(SSL_CIPHER) *peer_ciphers, SSL_CIPHER **cipher, void *arg);
 
 #ifndef OPENSSL_NO_TLSEXT
-/* Callbacks and structures for handling custom TLS Extensions: 
+
- *   cli_ext_add_cb   - sends data for ClientHello TLS Extension
+/* Typedefs for handling custom extensions */
- *   cli_ext_parse_cb - receives data from ServerHello TLS Extension
- *   srv_ext_parse_cb - receives data from ClientHello TLS Extension
- *   srv_ext_add_cb   - sends data for ServerHello TLS Extension
- *
- *   All these functions return nonzero on success.  Zero will terminate
- *   the handshake (and return a specific TLS Fatal alert, if the function
- *   declaration has an "al" parameter).  -1 for the "sending" functions
- *   will cause the TLS Extension to be omitted.
- * 
- *   "ext_type" is a TLS "ExtensionType" from 0-65535.
- *   "in" is a pointer to TLS "extension_data" being provided to the cb.
- *   "out" is used by the callback to return a pointer to "extension data"
- *     which OpenSSL will later copy into the TLS handshake.  The contents
- *     of this buffer should not be changed until the handshake is complete.
- *   "inlen" and "outlen" are TLS Extension lengths from 0-65535.
- *   "al" is a TLS "AlertDescription" from 0-255 which WILL be sent as a 
- *     fatal TLS alert, if the callback returns zero.
- */
 
 typedef int (*custom_ext_add_cb)(SSL *s, unsigned int ext_type,
 					  const unsigned char **out,
 					  size_t *outlen, int *al,
-					   void *arg);
+					   void *add_arg);
+
+typedef void (*custom_ext_free_cb)(SSL *s, unsigned int ext_type,
+					   const unsigned char *out,
+					   void *add_arg);
 
 typedef int (*custom_ext_parse_cb)(SSL *s, unsigned int ext_type,
 					   const unsigned char *in,
 					   size_t inlen, int *al,
-					   void *arg);
+					   void *parse_arg);
+
 
 #endif
 
@@ -1257,30 +1244,19 @@ const char *SSL_get_psk_identity(const SSL *s);
 #endif
 
 #ifndef OPENSSL_NO_TLSEXT
-/* Register callbacks to handle custom TLS Extensions as client or server.
+/* Register callbacks to handle custom TLS Extensions for client or server. */
- * 
+
- * Returns nonzero on success.  You cannot register twice for the same 
- * extension number, and registering for an extension number already 
- * handled by OpenSSL will fail.
- *
- * NULL can be registered for any callback function.  For the client
- * functions, a NULL custom_ext_add_cb sends an empty ClientHello
- * Extension, and a NULL custom_ext_parse_cb ignores the ServerHello
- * response (if any).
- *
- * For the server functions, a NULL custom_ext_parse means the
- * ClientHello extension's data will be ignored, but the extension will still
- * be noted and custom_ext_add_cb will still be invoked.  A NULL
- * custom_srv_ext_second_cb doesn't send a ServerHello extension.
- */
 int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned int ext_type,
-			       custom_ext_add_cb add_cb, 
+			       custom_ext_add_cb add_cb,
-			       custom_ext_parse_cb parse_cb, void *arg);
+			       custom_ext_free_cb free_cb,
+                               void *add_arg,
+			       custom_ext_parse_cb parse_cb, void *parse_arg);
 
 int SSL_CTX_set_custom_srv_ext(SSL_CTX *ctx, unsigned int ext_type,
-			       custom_ext_parse_cb parse_cb, 
+			       custom_ext_add_cb add_cb,
-			       custom_ext_add_cb add_cb, void *arg);
+			       custom_ext_free_cb free_cb,
-
+                               void *add_arg,
+			       custom_ext_parse_cb parse_cb, void *parse_arg);
 #endif
 
 #define SSL_NOTHING	1

ssl/ssl_locl.h

@@ -533,8 +533,10 @@ typedef struct {
 	 */
 	unsigned short ext_flags;
 	custom_ext_add_cb add_cb; 
+	custom_ext_free_cb free_cb; 
+	void *add_arg;
 	custom_ext_parse_cb parse_cb; 
-	void *arg;
+	void *parse_arg;
 } custom_ext_method;
 
 /* ext_flags values */

ssl/ssl_rsa.c

@@ -906,8 +906,10 @@ static int serverinfo_process_buffer(const unsigned char *serverinfo,
 		/* Register callbacks for extensions */
 		ext_type = (serverinfo[0] << 8) + serverinfo[1];
 		if (ctx && !SSL_CTX_set_custom_srv_ext(ctx, ext_type, 
-						       serverinfo_srv_parse_cb,
+						       serverinfo_srv_add_cb,
-						       serverinfo_srv_add_cb, NULL))
+						       NULL, NULL,
+						       serverinfo_srv_parse_cb, 
+						       NULL))
 			return 0;
 
 		serverinfo += 2;

ssl/ssltest.c

@@ -483,7 +483,7 @@ static int custom_ext_0_cli_add_cb(SSL *s, unsigned int ext_type,
 	{
 	if (ext_type != CUSTOM_EXT_TYPE_0)
 		custom_ext_error = 1;
-	return -1;  /* Don't send an extension */
+	return 0;  /* Don't send an extension */
 	}
 
 static int custom_ext_0_cli_parse_cb(SSL *s, unsigned int ext_type,
@@ -575,7 +575,7 @@ static int custom_ext_0_srv_add_cb(SSL *s, unsigned int ext_type,
 				      const unsigned char **out,
 				      size_t *outlen, int *al, void *arg)
 	{
-        return -1; /* Don't send an extension */
+        return 0; /* Don't send an extension */
 	}
 
 static int custom_ext_1_srv_parse_cb(SSL *s, unsigned int ext_type,
@@ -597,7 +597,7 @@ static int custom_ext_1_srv_add_cb(SSL *s, unsigned int ext_type,
 				      const unsigned char **out,
 				      size_t *outlen, int *al, void *arg)
 	{
-	return -1; /* Don't send an extension */
+	return 0; /* Don't send an extension */
 	}
 
 static int custom_ext_2_srv_parse_cb(SSL *s, unsigned int ext_type,
@@ -1465,10 +1465,12 @@ bad:
 #endif
 
 	if (serverinfo_sct)
-		SSL_CTX_set_custom_cli_ext(c_ctx, SCT_EXT_TYPE, NULL, 
+		SSL_CTX_set_custom_cli_ext(c_ctx, SCT_EXT_TYPE,
+					   NULL, NULL, NULL,
 					   serverinfo_cli_cb, NULL);
 	if (serverinfo_tack)
-		SSL_CTX_set_custom_cli_ext(c_ctx, TACK_EXT_TYPE, NULL,
+		SSL_CTX_set_custom_cli_ext(c_ctx, TACK_EXT_TYPE,
+					   NULL, NULL, NULL,
 					   serverinfo_cli_cb, NULL);
 
 	if (serverinfo_file)
@@ -1481,31 +1483,31 @@ bad:
 	if (custom_ext)
 		{
 		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_0, 
-					   custom_ext_0_cli_add_cb, 
+					   custom_ext_0_cli_add_cb, NULL, NULL,
 					   custom_ext_0_cli_parse_cb, NULL);
 		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_1, 
-					   custom_ext_1_cli_add_cb, 
+					   custom_ext_1_cli_add_cb, NULL, NULL,
 					   custom_ext_1_cli_parse_cb, NULL);
 		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_2, 
-					   custom_ext_2_cli_add_cb, 
+					   custom_ext_2_cli_add_cb, NULL, NULL,
 					   custom_ext_2_cli_parse_cb, NULL);
 		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_3, 
-					   custom_ext_3_cli_add_cb, 
+					   custom_ext_3_cli_add_cb, NULL, NULL,
 					   custom_ext_3_cli_parse_cb, NULL);
 
 
 		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_0, 
-					   custom_ext_0_srv_parse_cb, 
+					   custom_ext_0_srv_add_cb, NULL, NULL,
-					   custom_ext_0_srv_add_cb, NULL);
+					   custom_ext_0_srv_parse_cb, NULL);
 		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_1, 
-					   custom_ext_1_srv_parse_cb, 
+					   custom_ext_1_srv_add_cb, NULL, NULL,
-					   custom_ext_1_srv_add_cb, NULL);
+					   custom_ext_1_srv_parse_cb, NULL);
 		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_2, 
-					   custom_ext_2_srv_parse_cb, 
+					   custom_ext_2_srv_add_cb, NULL, NULL,
-					   custom_ext_2_srv_add_cb, NULL);
+					   custom_ext_2_srv_parse_cb, NULL);
 		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_3, 
-					   custom_ext_3_srv_parse_cb, 
+					   custom_ext_3_srv_add_cb, NULL, NULL,
-					   custom_ext_3_srv_add_cb, NULL);
+					   custom_ext_3_srv_parse_cb, NULL);
 		}
 
 	if (alpn_server)

ssl/t1_ext.c

@@ -120,7 +120,7 @@ int custom_ext_parse(SSL *s, int server,
 	if (!meth->parse_cb)
 		return 1;
 
-	return meth->parse_cb(s, ext_type, ext_data, ext_size, al, meth->arg);
+	return meth->parse_cb(s, ext_type, ext_data, ext_size, al, meth->parse_arg);
 	}
 
 /* request custom extension data from the application and add to the
@@ -159,10 +159,10 @@ int custom_ext_add(SSL *s, int server,
 			int cb_retval = 0;
 			cb_retval = meth->add_cb(s, meth->ext_type,
 							&out, &outlen, al,
-							meth->arg);
+							meth->add_arg);
-			if (cb_retval == 0)
+			if (cb_retval < 0)
 				return 0; /* error */
-			if (cb_retval == -1)
+			if (cb_retval == 0)
 					continue; /* skip this extension */
 			}
 		if (4 > limit - ret || outlen > (size_t)(limit - ret - 4))
@@ -182,6 +182,8 @@ int custom_ext_add(SSL *s, int server,
 		 * sent in ServerHello.
 		 */
 		meth->ext_flags |= SSL_EXT_FLAG_SENT;
+		if (meth->free_cb)
+			meth->free_cb(s, meth->ext_type, out, meth->add_arg);
 		}
 	*pret = ret;
 	return 1;
@@ -210,9 +212,10 @@ void custom_exts_free(custom_ext_methods *exts)
 /* Set callbacks for a custom extension */
 static int custom_ext_set(custom_ext_methods *exts,
 			unsigned int ext_type,
-			custom_ext_parse_cb parse_cb,
 			custom_ext_add_cb add_cb,
-			void *arg)
+			custom_ext_free_cb free_cb,
+			void *add_arg,
+			custom_ext_parse_cb parse_cb, void *parse_arg)
 	{
 	custom_ext_method *meth;
 	/* See if it is a supported internally */
@@ -258,8 +261,10 @@ static int custom_ext_set(custom_ext_methods *exts,
 	memset(meth, 0, sizeof(custom_ext_method));
 	meth->parse_cb = parse_cb;
 	meth->add_cb = add_cb;
+	meth->free_cb = free_cb;
 	meth->ext_type = ext_type;
-	meth->arg = arg;
+	meth->add_arg = add_arg;
+	meth->parse_arg = parse_arg;
 	exts->meths_count++;
 	return 1;
 	}
@@ -267,19 +272,25 @@ static int custom_ext_set(custom_ext_methods *exts,
 /* Application level functions to add custom extension callbacks */
 
 int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned int ext_type,
-			       custom_ext_add_cb add_cb, 
+			       custom_ext_add_cb add_cb,
-			       custom_ext_parse_cb parse_cb, void *arg)
+			       custom_ext_free_cb free_cb,
+                               void *add_arg,
+			       custom_ext_parse_cb parse_cb, void *parse_arg)
 
 	{
-	return custom_ext_set(&ctx->cert->cli_ext, ext_type, parse_cb, add_cb,
+	return custom_ext_set(&ctx->cert->cli_ext, ext_type,
-									arg);
+				add_cb, free_cb, add_arg,
+				parse_cb, parse_arg);
 	}
 
 int SSL_CTX_set_custom_srv_ext(SSL_CTX *ctx, unsigned int ext_type,
-			       custom_ext_parse_cb parse_cb, 
+			       custom_ext_add_cb add_cb,
-			       custom_ext_add_cb add_cb, void *arg)
+			       custom_ext_free_cb free_cb,
+                               void *add_arg,
+			       custom_ext_parse_cb parse_cb, void *parse_arg)
 	{
-	return custom_ext_set(&ctx->cert->srv_ext, ext_type, parse_cb, add_cb,
+	return custom_ext_set(&ctx->cert->srv_ext, ext_type,
-									arg);
+				add_cb, free_cb, add_arg,
+				parse_cb, parse_arg);
 	}
 #endif

ssl/t1_lib.c

@@ -2383,7 +2383,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char
 		 */
 		else if (!s->hit)
 			{
-			if (!custom_ext_parse(s, 1, type, data, size, al))
+			if (custom_ext_parse(s, 1, type, data, size, al) <= 0)
 				return 0;
 			}
 
@@ -2711,7 +2711,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char
 		/* If this extension type was not otherwise handled, but 
 		 * matches a custom_cli_ext_record, then send it to the c
 		 * callback */
-		else if (!custom_ext_parse(s, 0, type, data, size, al))
+		else if (custom_ext_parse(s, 0, type, data, size, al) <= 0)
 				return 0;
  
 		data += size;