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>

6 files changed, 73 insertions, 80 deletions

diff --git a/ssl/ssl.h b/ssl/ssl.h
index 8c59ed3520..2a0d9283c0 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -389,36 +389,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
- *   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.
- */
+
+/* Typedefs for handling custom extensions */
 
 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
 
@@ -1264,30 +1251,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.
- * 
- * 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.
- */
+/* Register callbacks to handle custom TLS Extensions for client or server. */
+
 int SSL_CTX_set_custom_cli_ext(SSL_CTX *ctx, unsigned int ext_type,
-			       custom_ext_add_cb add_cb, 
-			       custom_ext_parse_cb parse_cb, void *arg);
+			       custom_ext_add_cb add_cb,
+			       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, void *arg);
-
+			       custom_ext_add_cb add_cb,
+			       custom_ext_free_cb free_cb,
+                               void *add_arg,
+			       custom_ext_parse_cb parse_cb, void *parse_arg);
 #endif
 
 #define SSL_NOTHING	1
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index d76269d608..3f87da7d53 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -539,8 +539,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 */
diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index d4f0e28a16..6e3d44c79c 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -921,8 +921,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, NULL))
+						       serverinfo_srv_add_cb,
+						       NULL, NULL,
+						       serverinfo_srv_parse_cb, 
+						       NULL))
 			return 0;
 
 		serverinfo += 2;
diff --git a/ssl/ssltest.c b/ssl/ssltest.c
index 5837abb24f..09400a1855 100644
--- a/ssl/ssltest.c
+++ b/ssl/ssltest.c
@@ -558,7 +558,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,
@@ -650,7 +650,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,
@@ -672,7 +672,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,
@@ -1584,10 +1584,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)
@@ -1600,31 +1602,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);
+					   custom_ext_0_srv_add_cb, NULL, 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);
+					   custom_ext_1_srv_add_cb, NULL, 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);
+					   custom_ext_2_srv_add_cb, NULL, 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);
+					   custom_ext_3_srv_add_cb, NULL, NULL,
+					   custom_ext_3_srv_parse_cb, NULL);
 		}
 
 	if (alpn_server)
diff --git a/ssl/t1_ext.c b/ssl/t1_ext.c
index 8b6c170ef6..115e4345ea 100644
--- a/ssl/t1_ext.c
+++ b/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);
-			if (cb_retval == 0)
+							meth->add_arg);
+			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_parse_cb parse_cb, void *arg)
+			       custom_ext_add_cb add_cb,
+			       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,
-									arg);
+	return custom_ext_set(&ctx->cert->cli_ext, ext_type,
+				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, void *arg)
+			       custom_ext_add_cb add_cb,
+			       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,
-									arg);
+	return custom_ext_set(&ctx->cert->srv_ext, ext_type,
+				add_cb, free_cb, add_arg,
+				parse_cb, parse_arg);
 	}
 #endif
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index f94a4c0b8a..f46279dbb3 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2442,7 +2442,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;
 			}
 #ifdef TLSEXT_TYPE_encrypt_then_mac
@@ -2777,7 +2777,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;
 #ifdef TLSEXT_TYPE_encrypt_then_mac
 		else if (type == TLSEXT_TYPE_encrypt_then_mac)