From: Trevor <unsafe@trevp.net>
Date: Fri, 14 Jun 2013 05:36:45 +0000 (-0700)
Subject: Cleanup of custom extension stuff.
X-Git-Tag: master-post-reformat~1284
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=9cd50f738ff55eae2a64f872492d3a7ce115f18d;p=oweals%2Fopenssl.git

Cleanup of custom extension stuff.

serverinfo rejects non-empty extensions.

Omit extension if no relevant serverinfo data.

Improve error-handling in serverinfo callback.

Cosmetic cleanups.

s_client documentation.

s_server documentation.

SSL_CTX_serverinfo documentation.

Cleaup -1 and NULL callback handling for custom extensions, add tests.

Cleanup ssl_rsa.c serverinfo code.

Whitespace cleanup.

Improve comments in ssl.h for serverinfo.

Whitespace.

Cosmetic cleanup.

Reject non-zero-len serverinfo extensions.

Whitespace.

Make it build.
---

diff --git a/doc/apps/s_client.pod b/doc/apps/s_client.pod
index 32476acfc3..e8cc8712d2 100644
--- a/doc/apps/s_client.pod
+++ b/doc/apps/s_client.pod
@@ -43,6 +43,7 @@ B<openssl> B<s_client>
 [B<-sess_out filename>]
 [B<-sess_in filename>]
 [B<-rand file(s)>]
+[B<-serverinfo types>]
 
 =head1 DESCRIPTION
 
@@ -256,6 +257,13 @@ Multiple files can be specified separated by a OS-dependent character.
 The separator is B<;> for MS-Windows, B<,> for OpenVMS, and B<:> for
 all others.
 
+=item B<-serverinfo types>
+
+a list of comma-separated TLS Extension Types (numbers between 0 and 
+65535).  Each type will be sent as an empty ClientHello TLS Extension.
+The server's response (if any) will be encoded and displayed as a PEM
+file.
+
 =back
 
 =head1 CONNECTED COMMANDS
diff --git a/doc/apps/s_server.pod b/doc/apps/s_server.pod
index 2a08ee25e0..cd167d11bc 100644
--- a/doc/apps/s_server.pod
+++ b/doc/apps/s_server.pod
@@ -56,6 +56,7 @@ B<openssl> B<s_server>
 [B<-no_ticket>]
 [B<-id_prefix arg>]
 [B<-rand file(s)>]
+[B<-serverinfo file>]
 
 =head1 DESCRIPTION
 
@@ -306,6 +307,14 @@ Multiple files can be specified separated by a OS-dependent character.
 The separator is B<;> for MS-Windows, B<,> for OpenVMS, and B<:> for
 all others.
 
+=item B<-serverinfo file>
+
+a file containing one or more blocks of PEM data.  Each PEM block
+must encode a TLS ServerHello extension (2 bytes type, 2 bytes length,
+followed by "length" bytes of extension data).  If the client sends
+an empty TLS ClientHello extension matching the type, the corresponding
+ServerHello extension will be returned.
+
 =back
 
 =head1 CONNECTED COMMANDS
diff --git a/doc/ssl/SSL_CTX_use_serverinfo.pod b/doc/ssl/SSL_CTX_use_serverinfo.pod
new file mode 100644
index 0000000000..485b813b83
--- /dev/null
+++ b/doc/ssl/SSL_CTX_use_serverinfo.pod
@@ -0,0 +1,45 @@
+=pod
+
+=head1 NAME
+
+SSL_CTX_use_serverinfo, SSL_CTX_use_serverinfo_file
+
+=head1 SYNOPSIS
+
+ #include <openssl/ssl.h>
+
+ int SSL_CTX_use_serverinfo(SSL_CTX *ctx, const unsigned char *serverinfo,
+                            size_t serverinfo_length);
+
+ int SSL_CTX_use_serverinfo_file(SSL_CTX *ctx, const char *file);
+
+=head1 DESCRIPTION
+
+These functions load "serverinfo" TLS ServerHello Extensions into the SSL_CTX.
+A "serverinfo" extension is returned in response to an empty ClientHello
+Extension.
+
+SSL_CTX_use_serverinfo_file() loads one or more serverinfo extensions from
+a byte array into B<ctx>. The extensions must be concatenated into a 
+sequence of bytes.  Each extension must consist of a 2-byte Extension Type, 
+a 2-byte length, and then length bytes of extension_data.
+
+SSL_CTX_use_serverinfo_file() loads one or more serverinfo extensions from
+B<file> into B<ctx>. The extensions must be in PEM format.  Each extension
+must consist of a 2-byte Extension Type, a 2-byte length, and then length
+bytes of extension_data.
+
+=head1 NOTES
+
+=head1 RETURN VALUES
+
+On success, the functions return 1.
+On failure, the functions return 0.  Check out the error stack to find out
+the reason.
+
+=head1 SEE ALSO
+
+=head1 HISTORY
+
+
+=cut
diff --git a/ssl/ssl.h b/ssl/ssl.h
index 845308e2ef..86975f22e6 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -392,7 +392,8 @@ typedef int (*tls_session_secret_cb_fn)(SSL *s, void *secret, int *secret_len, S
  *
  *   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).
+ *   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.
@@ -1240,9 +1241,8 @@ const char *SSL_get_psk_identity(const SSL *s);
  *
  * For the server functions, a NULL custom_srv_ext_first_cb_fn means the
  * ClientHello extension's data will be ignored, but the extension will still
- * be noted and custom_srv_ext_second_cb_fn will still be invoked.  If 
- * custom_srv_ext_second_cb_fn is NULL, an empty ServerHello extension is 
- * sent.
+ * be noted and custom_srv_ext_second_cb_fn 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 short ext_type,
 			       custom_cli_ext_first_cb_fn fn1, 
diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index ac16b8506d..507a3d1773 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -880,58 +880,79 @@ static int serverinfo_find_extension(const unsigned char *serverinfo,
 
 		/* end of serverinfo */
 		if (serverinfo_length == 0)
-			return 0;
+			return -1; /* Extension not found */
 
 		/* read 2-byte type field */
 		if (serverinfo_length < 2)
-			return 0;	/* error */
+			return 0; /* Error */
 		type = (serverinfo[0] << 8) + serverinfo[1];
 		serverinfo += 2;
 		serverinfo_length -= 2;
 
 		/* read 2-byte len field */
 		if (serverinfo_length < 2)
-			return 0;	/* error */
+			return 0; /* Error */
 		len = (serverinfo[0] << 8) + serverinfo[1];
 		serverinfo += 2;
 		serverinfo_length -= 2;
 
 		if (len > serverinfo_length)
-			return 0;	/* error */
+			return 0; /* Error */
 
 		if (type == extension_type)
 			{
 			*extension_data = serverinfo;
 			*extension_length = len;
-			return 1;
+			return 1; /* Success */
 			}
 
 		serverinfo += len;
 		serverinfo_length -= len;
 		}
-	return 0;
+	return 0; /* Error */
 	}
 
-static int serverinfo_srv_cb(SSL *s, unsigned short ext_type,
-			     const unsigned char **out, unsigned short *outlen, 
-			     void *arg)
+static int serverinfo_srv_first_cb(SSL *s, unsigned short ext_type,
+				   const unsigned char *in,
+				   unsigned short inlen, int *al,
+				   void *arg)
+	{
+	if (inlen != 0)
+		{
+		*al = SSL_AD_DECODE_ERROR;
+		return 0;
+		}
+	return 1;
+	}
+
+static int serverinfo_srv_second_cb(SSL *s, unsigned short ext_type,
+			            const unsigned char **out, unsigned short *outlen, 
+			            void *arg)
 	{
 	const unsigned char *serverinfo = NULL;
 	size_t serverinfo_length = 0;
 
-	/* Is there a serverinfo for the chosen server cert? */
+	/* Is there serverinfo data for the chosen server cert? */
 	if ((ssl_get_server_cert_serverinfo(s, &serverinfo,
 					    &serverinfo_length)) != 0)
 		{
 		/* Find the relevant extension from the serverinfo */
-		serverinfo_find_extension(serverinfo, serverinfo_length,
-					  ext_type, out, outlen);
-		}
-		return 1;
+		int retval = serverinfo_find_extension(serverinfo, serverinfo_length,
+					      	       ext_type, out, outlen);
+		if (retval == 0)
+			return 0; /* Error */
+		if (retval == -1)
+			return -1; /* No extension found, don't send extension */
+		return 1; /* Send extension */
+		}
+	return -1; /* No serverinfo data found, don't send extension */
 	}
 
-static int serverinfo_validate(const unsigned char *serverinfo, 
-			       size_t serverinfo_length, SSL_CTX *ctx)
+/* With a NULL context, this function just checks that the serverinfo data
+   parses correctly.  With a non-NULL context, it registers callbacks for 
+   the included extensions. */
+static int serverinfo_process_buffer(const unsigned char *serverinfo, 
+			    	     size_t serverinfo_length, SSL_CTX *ctx)
 	{
 	if (serverinfo == NULL || serverinfo_length == 0)
 		return 0;
@@ -951,8 +972,9 @@ static int serverinfo_validate(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, NULL,
-						       serverinfo_srv_cb, NULL))
+		if (ctx && !SSL_CTX_set_custom_srv_ext(ctx, ext_type, 
+						       serverinfo_srv_first_cb,
+						       serverinfo_srv_second_cb, NULL))
 			return 0;
 
 		serverinfo += 2;
@@ -1058,7 +1080,7 @@ int SSL_CTX_use_serverinfo(SSL_CTX *ctx, const unsigned char *serverinfo,
 		SSLerr(SSL_F_SSL_CTX_USE_SERVERINFO,ERR_R_PASSED_NULL_PARAMETER);
 		return 0;
 		}
-	if (!serverinfo_validate(serverinfo, serverinfo_length, NULL))
+	if (!serverinfo_process_buffer(serverinfo, serverinfo_length, NULL))
 		{
 		SSLerr(SSL_F_SSL_CTX_USE_SERVERINFO,SSL_R_INVALID_SERVERINFO_DATA);
 		return(0);
@@ -1085,7 +1107,7 @@ int SSL_CTX_use_serverinfo(SSL_CTX *ctx, const unsigned char *serverinfo,
 
 	/* Now that the serverinfo is validated and stored, go ahead and 
 	 * register callbacks. */
-	if (!serverinfo_validate(serverinfo, serverinfo_length, ctx))
+	if (!serverinfo_process_buffer(serverinfo, serverinfo_length, ctx))
 		{
 		SSLerr(SSL_F_SSL_CTX_USE_SERVERINFO,SSL_R_INVALID_SERVERINFO_DATA);
 		return(0);
diff --git a/ssl/ssltest.c b/ssl/ssltest.c
index f67c7eab8e..1be60c06a8 100644
--- a/ssl/ssltest.c
+++ b/ssl/ssltest.c
@@ -371,27 +371,46 @@ static int verify_npn(SSL *client, SSL *server)
 #endif
 
 #define SCT_EXT_TYPE 18
+
+/* WARNING : below extension types are *NOT* IETF assigned, and 
+   could conflict if these types are reassigned and handled 
+   specially by OpenSSL in the future */
 #define TACK_EXT_TYPE 62208
+#define CUSTOM_EXT_TYPE_0 1000
+#define CUSTOM_EXT_TYPE_1 1001
+#define CUSTOM_EXT_TYPE_2 1002
+#define CUSTOM_EXT_TYPE_3 1003
+
+const char custom_ext_cli_string[] = "abc";
+const char custom_ext_srv_string[] = "defg";
 
 /* These set from cmdline */
 char* serverinfo_file = NULL;
 int serverinfo_sct = 0;
 int serverinfo_tack = 0;
+
+/* These set based on extension callbacks */
 int serverinfo_sct_seen = 0;
 int serverinfo_tack_seen = 0;
 int serverinfo_other_seen = 0;
 
+/* This set from cmdline */
+int custom_ext = 0;
+
+/* This set based on extension callbacks */
+int custom_ext_error = 0;
+
 static int serverinfo_cli_cb(SSL* s, unsigned short ext_type,
-												     const unsigned char* in, unsigned short inlen, 
-												     int* al, void* arg)
+			     const unsigned char* in, unsigned short inlen, 
+			     int* al, void* arg)
 	{
-		if (ext_type == SCT_EXT_TYPE)
-			serverinfo_sct_seen++;
-		else if (ext_type == TACK_EXT_TYPE)
-			serverinfo_tack_seen++;
-		else
-			serverinfo_other_seen++;
-		return 1;
+	if (ext_type == SCT_EXT_TYPE)
+		serverinfo_sct_seen++;
+	else if (ext_type == TACK_EXT_TYPE)
+		serverinfo_tack_seen++;
+	else
+		serverinfo_other_seen++;
+	return 1;
 	}
 
 static int verify_serverinfo()
@@ -405,6 +424,188 @@ static int verify_serverinfo()
 	return 0;
 	}
 
+/* Four test cases for custom extensions:
+ * 0 - no ClientHello extension or ServerHello response
+ * 1 - ClientHello with "abc", no response
+ * 2 - ClientHello with "abc", empty response
+ * 3 - ClientHello with "abc", "defg" response
+ */
+
+static int custom_ext_0_cli_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char **out,
+				     unsigned short *outlen, void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_0)
+		custom_ext_error = 1;
+	return -1;  /* Don't send an extension */
+	}
+
+static int custom_ext_0_cli_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char *in,
+				      unsigned short inlen, int *al,
+				      void *arg)
+	{
+	custom_ext_error = 1; /* Shouldn't be called */
+	return 0;
+	}
+
+static int custom_ext_1_cli_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char **out,
+				     unsigned short *outlen, void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_1)
+		custom_ext_error = 1;
+	*out = (const unsigned char*)custom_ext_cli_string;
+	*outlen = strlen(custom_ext_cli_string);
+	return 1; /* Send "abc" */
+	}
+
+static int custom_ext_1_cli_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char *in,
+				      unsigned short inlen, int *al,
+				      void *arg)
+	{
+	custom_ext_error = 1; /* Shouldn't be called */
+	return 0;
+	}
+
+static int custom_ext_2_cli_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char **out,
+				     unsigned short *outlen, void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_2)
+		custom_ext_error = 1;
+	*out = (const unsigned char*)custom_ext_cli_string;
+	*outlen = strlen(custom_ext_cli_string);
+	return 1; /* Send "abc" */
+	}
+
+static int custom_ext_2_cli_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char *in,
+				      unsigned short inlen, int *al,
+				      void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_2)
+		custom_ext_error = 1;
+	if (inlen != 0)
+		custom_ext_error = 1; /* Should be empty response */
+	return 1;
+	}
+
+static int custom_ext_3_cli_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char **out,
+				     unsigned short *outlen, void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_3)
+		custom_ext_error = 1;
+	*out = (const unsigned char*)custom_ext_cli_string;
+	*outlen = strlen(custom_ext_cli_string);
+	return 1; /* Send "abc" */
+	}
+
+static int custom_ext_3_cli_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char *in,
+				      unsigned short inlen, int *al,
+				      void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_3)
+		custom_ext_error = 1;
+	if (inlen != strlen(custom_ext_srv_string))
+		custom_ext_error = 1;
+	if (memcmp(custom_ext_srv_string, in, inlen) != 0)
+		custom_ext_error = 1; /* Check for "defg" */
+	return 1;
+	}
+
+
+static int custom_ext_0_srv_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char *in,
+				     unsigned short inlen, int *al,
+				     void *arg)
+	{
+	custom_ext_error = 1;
+	return 0; /* Shouldn't be called */
+	}
+
+static int custom_ext_0_srv_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char **out,
+				      unsigned short *outlen, void *arg)
+	{
+	custom_ext_error = 1;
+	return 0; /* Shouldn't be called */
+	}
+
+static int custom_ext_1_srv_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char *in,
+				     unsigned short inlen, int *al,
+				     void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_1)
+		custom_ext_error = 1;		
+	 /* Check for "abc" */
+	if (inlen != strlen(custom_ext_cli_string))
+		custom_ext_error = 1;
+	if (memcmp(in, custom_ext_cli_string, inlen) != 0)
+		custom_ext_error = 1;
+	return 1;
+	}
+
+static int custom_ext_1_srv_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char **out,
+				      unsigned short *outlen, void *arg)
+	{
+	return -1; /* Don't send an extension */
+	}
+
+static int custom_ext_2_srv_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char *in,
+				     unsigned short inlen, int *al,
+				     void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_2)
+		custom_ext_error = 1;		
+	 /* Check for "abc" */
+	if (inlen != strlen(custom_ext_cli_string))
+		custom_ext_error = 1;
+	if (memcmp(in, custom_ext_cli_string, inlen) != 0)
+		custom_ext_error = 1;
+	return 1;
+	}
+
+static int custom_ext_2_srv_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char **out,
+				      unsigned short *outlen, void *arg)
+	{
+	*out = NULL;
+	*outlen = 0;
+	return 1; /* Send empty extension */
+	}
+
+static int custom_ext_3_srv_first_cb(SSL *s, unsigned short ext_type,
+				     const unsigned char *in,
+				     unsigned short inlen, int *al,
+				     void *arg)
+	{
+	if (ext_type != CUSTOM_EXT_TYPE_3)
+		custom_ext_error = 1;		
+	 /* Check for "abc" */	
+	if (inlen != strlen(custom_ext_cli_string))
+		custom_ext_error = 1;
+	if (memcmp(in, custom_ext_cli_string, inlen) != 0)
+		custom_ext_error = 1;
+	return 1;
+	}
+
+static int custom_ext_3_srv_second_cb(SSL *s, unsigned short ext_type,
+				      const unsigned char **out,
+				      unsigned short *outlen, void *arg)
+	{
+	*out = (const unsigned char*)custom_ext_srv_string;
+	*outlen = strlen(custom_ext_srv_string);
+	return 1; /* Send "defg" */
+	}
+
+
 static char *cipher=NULL;
 static int verbose=0;
 static int debug=0;
@@ -484,9 +685,10 @@ static void sv_usage(void)
 	fprintf(stderr," -npn_server - have server side offer NPN\n");
 	fprintf(stderr," -npn_server_reject - have server reject NPN\n");
 #endif
-	fprintf(stderr," -serverinfo_file - have server use this file\n");
+	fprintf(stderr," -serverinfo_file file - have server use this file\n");
 	fprintf(stderr," -serverinfo_sct  - have client offer and expect SCT\n");
 	fprintf(stderr," -serverinfo_tack - have client offer and expect TACK\n");
+	fprintf(stderr," -custom_ext - try various custom extension callbacks\n");
 	}
 
 static void print_details(SSL *c_ssl, const char *prefix)
@@ -912,6 +1114,10 @@ int main(int argc, char *argv[])
 			if (--argc < 1) goto bad;
 			serverinfo_file = *(++argv);
 			}
+		else if (strcmp(*argv,"-custom_ext") == 0)
+			{
+			custom_ext = 1;
+			}
 		else
 			{
 			fprintf(stderr,"unknown option %s\n",*argv);
@@ -1238,9 +1444,11 @@ bad:
 #endif
 
 	if (serverinfo_sct)
-		SSL_CTX_set_custom_cli_ext(c_ctx, SCT_EXT_TYPE, NULL, serverinfo_cli_cb, NULL);
+		SSL_CTX_set_custom_cli_ext(c_ctx, SCT_EXT_TYPE, NULL, 
+					   serverinfo_cli_cb, NULL);
 	if (serverinfo_tack)
-		SSL_CTX_set_custom_cli_ext(c_ctx, TACK_EXT_TYPE, NULL, serverinfo_cli_cb, NULL);
+		SSL_CTX_set_custom_cli_ext(c_ctx, TACK_EXT_TYPE, NULL,
+					   serverinfo_cli_cb, NULL);
 
 	if (serverinfo_file)
 		if (!SSL_CTX_use_serverinfo_file(s_ctx, serverinfo_file))
@@ -1249,6 +1457,36 @@ bad:
 			goto end;
 			}
 
+	if (custom_ext)
+		{
+		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_0, 
+					   custom_ext_0_cli_first_cb, 
+					   custom_ext_0_cli_second_cb, NULL);
+		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_1, 
+					   custom_ext_1_cli_first_cb, 
+					   custom_ext_1_cli_second_cb, NULL);
+		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_2, 
+					   custom_ext_2_cli_first_cb, 
+					   custom_ext_2_cli_second_cb, NULL);
+		SSL_CTX_set_custom_cli_ext(c_ctx, CUSTOM_EXT_TYPE_3, 
+					   custom_ext_3_cli_first_cb, 
+					   custom_ext_3_cli_second_cb, NULL);
+
+
+		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_0, 
+					   custom_ext_0_srv_first_cb, 
+					   custom_ext_0_srv_second_cb, NULL);
+		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_1, 
+					   custom_ext_1_srv_first_cb, 
+					   custom_ext_1_srv_second_cb, NULL);
+		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_2, 
+					   custom_ext_2_srv_first_cb, 
+					   custom_ext_2_srv_second_cb, NULL);
+		SSL_CTX_set_custom_srv_ext(s_ctx, CUSTOM_EXT_TYPE_3, 
+					   custom_ext_3_srv_first_cb, 
+					   custom_ext_3_srv_second_cb, NULL);
+		}
+
 	c_ssl=SSL_new(c_ctx);
 	s_ssl=SSL_new(s_ctx);
 
@@ -1712,6 +1950,12 @@ int doit_biopair(SSL *s_ssl, SSL *c_ssl, long count,
 		goto err;
 		}
 
+	if (custom_ext_error)
+		{
+		ret = 1;
+		goto err;
+		}
+
 end:
 	ret = 0;
 
@@ -2019,6 +2263,11 @@ int doit(SSL *s_ssl, SSL *c_ssl, long count)
 		ret = 1;
 		goto err;
 		}
+	if (custom_ext_error)
+		{
+		ret = 1;
+		goto err;
+		}
 	ret=0;
 err:
 	/* We have to set the BIO's to NULL otherwise they will be
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 19769c5888..8bed38d6d4 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1455,10 +1455,19 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned cha
 			unsigned short outlen = 0;
 
 			record = &s->ctx->custom_cli_ext_records[i];
-			if (record->fn1 && !record->fn1(s, record->ext_type,
+			/* NULL callback sends empty extension */ 
+			/* -1 from callback omits extension */
+			if (record->fn1)
+				{
+				int cb_retval = 0;
+				cb_retval = record->fn1(s, record->ext_type,
 							&out, &outlen,
-							record->arg))
-				return NULL;
+							record->arg);
+				if (cb_retval == 0)
+					return NULL; /* error */
+				if (cb_retval == -1)
+					continue; /* skip this extension */
+				}
 			if (limit < ret + 4 + outlen)
 				return NULL;
 			s2n(record->ext_type, ret);
@@ -1751,11 +1760,18 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned cha
 					{
 					const unsigned char *out = NULL;
 					unsigned short outlen = 0;
-					if (record->fn2
-					    && !record->fn2(s, record->ext_type,
-							    &out, &outlen,
-							    record->arg))
-						return NULL;
+					int cb_retval = 0;
+
+					/* NULL callback or -1 omits extension */
+					if (!record->fn2)
+						break;
+					cb_retval = record->fn2(s, record->ext_type,
+						    		&out, &outlen,
+						    		record->arg);
+					if (cb_retval == 0)
+						return NULL; /* error */
+					if (cb_retval == -1)
+						break; /* skip this extension */
 					if (limit < ret + 4 + outlen)
 						return NULL;
 					s2n(record->ext_type, ret);
diff --git a/test/testssl b/test/testssl
index 61416885b1..71524a0663 100644
--- a/test/testssl
+++ b/test/testssl
@@ -178,13 +178,22 @@ $ssltest -bio_pair -tls1 -npn_client -npn_server || exit 1
 $ssltest -bio_pair -tls1 -npn_client -npn_server -num 2 || exit 1
 $ssltest -bio_pair -tls1 -npn_client -npn_server -num 2 -reuse || exit 1
 
+#############################################################################
+# Custom Extension tests
+
+echo test tls1 with custom extensions
+$ssltest -bio_pair -tls1 -custom_ext || exit 1
+
 #############################################################################
 # Serverinfo tests
 
+echo test tls1 with serverinfo
 $ssltest -bio_pair -tls1 -serverinfo_file $serverinfo || exit 1
 $ssltest -bio_pair -tls1 -serverinfo_file $serverinfo -serverinfo_sct || exit 1
 $ssltest -bio_pair -tls1 -serverinfo_file $serverinfo -serverinfo_tack || exit 1
 $ssltest -bio_pair -tls1 -serverinfo_file $serverinfo -serverinfo_sct -serverinfo_tack || exit 1
+$ssltest -bio_pair -tls1 -custom_ext -serverinfo_file $serverinfo -serverinfo_sct -serverinfo_tack || exit 1
+
 
 if ../util/shlib_wrap.sh ../apps/openssl no-srp; then
   echo skipping SRP tests