From 33f653adf3bff5b0795e22de1f54b7c5472252d0 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Sat, 16 Aug 2014 18:16:26 +0100 Subject: [PATCH] New extension callback features. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- apps/s_client.c | 2 +- ssl/ssl.h | 62 +++++++++++++++---------------------------------- ssl/ssl_locl.h | 4 +++- ssl/ssl_rsa.c | 6 +++-- ssl/ssltest.c | 36 ++++++++++++++-------------- ssl/t1_ext.c | 41 ++++++++++++++++++++------------ ssl/t1_lib.c | 4 ++-- 7 files changed, 74 insertions(+), 81 deletions(-) diff --git a/apps/s_client.c b/apps/s_client.c index 6a377743a1..e4007c2905 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -1363,7 +1363,7 @@ bad: { SSL_CTX_set_custom_cli_ext(ctx, serverinfo_types[i], - NULL, + NULL, NULL, NULL, serverinfo_cli_cb, NULL); } 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)