diff options
author | Trevor <unsafe@trevp.net> | 2013-06-13 22:36:45 -0700 |
---|---|---|
committer | Ben Laurie <ben@links.org> | 2013-06-18 16:13:08 +0100 |
commit | 9cd50f738ff55eae2a64f872492d3a7ce115f18d (patch) | |
tree | ff3b9868dd02aa7b632d82a8f9ff9f7c0d8ebc18 /ssl | |
parent | 8ee3c7e676c5edb1d5fbe0d66b7ce307a4f92899 (diff) |
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.
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/ssl.h | 8 | ||||
-rw-r--r-- | ssl/ssl_rsa.c | 62 | ||||
-rw-r--r-- | ssl/ssltest.c | 273 | ||||
-rw-r--r-- | ssl/t1_lib.c | 32 |
4 files changed, 331 insertions, 44 deletions
@@ -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); |