Update custom TLS extension and supplemental data 'generate' callbacks to support sending an alert. If multiple TLS extensions are expected but not received, the TLS extension and supplemental data 'generate' callbacks are the only chance for the receive-side to trigger a specific TLS alert during the handshake. Removed logic which no-op'd TLS extension generate callbacks (as the generate callbacks need to always be called in order to trigger alerts), and updated the serverinfo-specific custom TLS extension callbacks to track which custom TLS extensions were received by the client, where no-ops for 'generate' callbacks are appropriate.
diff --git a/ssl/s23_clnt.c b/ssl/s23_clnt.c index fa20c1c..452a19c 100644 --- a/ssl/s23_clnt.c +++ b/ssl/s23_clnt.c
@@ -553,8 +553,9 @@ SSLerr(SSL_F_SSL23_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT); return -1; } - if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH)) == NULL) + if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH, &al)) == NULL) { + ssl3_send_alert(s,SSL3_AL_FATAL,al); SSLerr(SSL_F_SSL23_CLIENT_HELLO,ERR_R_INTERNAL_ERROR); return -1; }
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 68d3131..eccfcc3 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c
@@ -891,8 +891,9 @@ SSLerr(SSL_F_SSL3_CLIENT_HELLO,SSL_R_CLIENTHELLO_TLSEXT); goto err; } - if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH)) == NULL) + if ((p = ssl_add_clienthello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH, &al)) == NULL) { + ssl3_send_alert(s,SSL3_AL_FATAL,al); SSLerr(SSL_F_SSL3_CLIENT_HELLO,ERR_R_INTERNAL_ERROR); goto err; } @@ -3622,6 +3623,7 @@ #ifndef OPENSSL_NO_TLSEXT int tls1_send_client_supplemental_data(SSL *s, int *skip) { + int al = 0; if (s->ctx->cli_supp_data_records_count) { unsigned char *p = NULL; @@ -3641,14 +3643,14 @@ if (!record->fn2) continue; cb_retval = record->fn2(s, record->supp_data_type, - &out, &outlen, + &out, &outlen, &al, record->arg); if (cb_retval == -1) continue; /* skip this supp data entry */ if (cb_retval == 0) { SSLerr(SSL_F_TLS1_SEND_CLIENT_SUPPLEMENTAL_DATA,ERR_R_BUF_LIB); - return 0; + goto f_err; } if (outlen == 0 || TLSEXT_MAXLEN_supplemental_data < outlen + 4 + length) { @@ -3703,7 +3705,11 @@ s->init_num = 0; s->init_off = 0; return 1; - } + +f_err: + ssl3_send_alert(s,SSL3_AL_FATAL,al); + return 0; +} int tls1_get_server_supplemental_data(SSL *s) {
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 82f715b..31074f2 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c
@@ -3029,8 +3029,8 @@ SSL_SRP_CTX_free(s); #endif #ifndef OPENSSL_NO_TLSEXT - if (s->s3->tlsext_custom_types != NULL) - OPENSSL_free(s->s3->tlsext_custom_types); + if (s->s3->serverinfo_client_tlsext_custom_types != NULL) + OPENSSL_free(s->s3->serverinfo_client_tlsext_custom_types); #endif OPENSSL_cleanse(s->s3,sizeof *s->s3); OPENSSL_free(s->s3); @@ -3076,12 +3076,12 @@ } #endif #ifndef OPENSSL_NO_TLSEXT - if (s->s3->tlsext_custom_types != NULL) + if (s->s3->serverinfo_client_tlsext_custom_types != NULL) { - OPENSSL_free(s->s3->tlsext_custom_types); - s->s3->tlsext_custom_types = NULL; + OPENSSL_free(s->s3->serverinfo_client_tlsext_custom_types); + s->s3->serverinfo_client_tlsext_custom_types = NULL; } - s->s3->tlsext_custom_types_count = 0; + s->s3->serverinfo_client_tlsext_custom_types_count = 0; #ifndef OPENSSL_NO_EC s->s3->is_probably_safari = 0; #endif /* !OPENSSL_NO_EC */
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 822d9b5..54266fb 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c
@@ -1504,7 +1504,7 @@ { unsigned char *buf; unsigned char *p,*d; - int i,sl; + int i,sl,al; unsigned long l; if (s->state == SSL3_ST_SW_SRVR_HELLO_A) @@ -1574,8 +1574,9 @@ SSLerr(SSL_F_SSL3_SEND_SERVER_HELLO,SSL_R_SERVERHELLO_TLSEXT); return -1; } - if ((p = ssl_add_serverhello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH)) == NULL) + if ((p = ssl_add_serverhello_tlsext(s, p, buf+SSL3_RT_MAX_PLAIN_LENGTH, &al)) == NULL) { + ssl3_send_alert(s, SSL3_AL_FATAL, al); SSLerr(SSL_F_SSL3_SEND_SERVER_HELLO,ERR_R_INTERNAL_ERROR); return -1; } @@ -3703,6 +3704,7 @@ int tls1_send_server_supplemental_data(SSL *s, int *skip) { + int al = 0; if (s->ctx->srv_supp_data_records_count) { unsigned char *p = NULL; @@ -3722,14 +3724,14 @@ if (!record->fn1) continue; cb_retval = record->fn1(s, record->supp_data_type, - &out, &outlen, + &out, &outlen, &al, record->arg); if (cb_retval == -1) continue; /* skip this supp data entry */ if (cb_retval == 0) { SSLerr(SSL_F_TLS1_SEND_SERVER_SUPPLEMENTAL_DATA,ERR_R_BUF_LIB); - return 0; + goto f_err; } if (outlen == 0 || TLSEXT_MAXLEN_supplemental_data < outlen + 4 + length) { @@ -3791,6 +3793,9 @@ s->init_num = 0; s->init_off = 0; return 1; +f_err: + ssl3_send_alert(s,SSL3_AL_FATAL,al); + return 0; } int tls1_get_client_supplemental_data(SSL *s)
diff --git a/ssl/ssl.h b/ssl/ssl.h index 296c166..ede930e 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h
@@ -410,7 +410,7 @@ */ typedef int (*custom_cli_ext_first_cb_fn)(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg); + unsigned short *outlen, int *al, void *arg); typedef int (*custom_cli_ext_second_cb_fn)(SSL *s, unsigned short ext_type, const unsigned char *in, unsigned short inlen, int *al, @@ -422,7 +422,7 @@ void *arg); typedef int (*custom_srv_ext_second_cb_fn)(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg); + unsigned short *outlen, int *al, void *arg); typedef struct { unsigned short ext_type; @@ -461,7 +461,7 @@ */ typedef int (*srv_supp_data_first_cb_fn)(SSL *s, unsigned short supp_data_type, const unsigned char **out, - unsigned short *outlen, void *arg); + unsigned short *outlen, int *al, void *arg); typedef int (*srv_supp_data_second_cb_fn)(SSL *s, unsigned short supp_data_type, const unsigned char *in, unsigned short inlen, int *al, @@ -473,7 +473,7 @@ void *arg); typedef int (*cli_supp_data_second_cb_fn)(SSL *s, unsigned short supp_data_type, const unsigned char **out, - unsigned short *outlen, void *arg); + unsigned short *outlen, int *al, void *arg); typedef struct { unsigned short supp_data_type;
diff --git a/ssl/ssl3.h b/ssl/ssl3.h index 8fe6949..8bd201e 100644 --- a/ssl/ssl3.h +++ b/ssl/ssl3.h
@@ -583,14 +583,12 @@ #endif #ifndef OPENSSL_NO_TLSEXT - /* tlsext_custom_types contains an array of TLS Extension types which - * were advertised by the client in its ClientHello, which were not - * otherwise handled by OpenSSL, and which the server has registered - * a custom_srv_ext_record to handle. + /* serverinfo_client_tlsext_custom_types contains an array of TLS Extension types which + * were advertised by the client in its ClientHello and leveraged by ServerInfo TLS extension callbacks. * The array does not contain any duplicates, and is in the same order * as the types were received in the client hello. */ - unsigned short *tlsext_custom_types; - size_t tlsext_custom_types_count; /* how many tlsext_custom_types */ + unsigned short *serverinfo_client_tlsext_custom_types; + size_t serverinfo_client_tlsext_custom_types_count; /* how many serverinfo_client_tlsext_custom_types */ /* ALPN information * (we are in the process of transitioning from NPN to ALPN.) */
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index fd6b883..1a2aef7 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h
@@ -1269,8 +1269,8 @@ const unsigned char *l1, size_t l1len, const unsigned char *l2, size_t l2len, int nmatch); -unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit); -unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit); +unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit, int *al); +unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit, int *al); int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n); int ssl_check_clienthello_tlsext_late(SSL *s); int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data, unsigned char *d, int n);
diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index 7fcd846..063eea5 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c
@@ -848,20 +848,59 @@ unsigned short inlen, int *al, void *arg) { + size_t i = 0; if (inlen != 0) { *al = SSL_AD_DECODE_ERROR; return 0; } + //if already in list, error out + for (i = 0; i < s->s3->serverinfo_client_tlsext_custom_types_count; i++) + { + if (s->s3->serverinfo_client_tlsext_custom_types[i] == ext_type) + { + *al = SSL_AD_DECODE_ERROR; + return 0; + } + } + s->s3->serverinfo_client_tlsext_custom_types_count++; + s->s3->serverinfo_client_tlsext_custom_types = OPENSSL_realloc( + s->s3->serverinfo_client_tlsext_custom_types, + s->s3->serverinfo_client_tlsext_custom_types_count * 2); + if (s->s3->serverinfo_client_tlsext_custom_types == NULL) + { + s->s3->serverinfo_client_tlsext_custom_types_count = 0; + *al = TLS1_AD_INTERNAL_ERROR; + return 0; + } + s->s3->serverinfo_client_tlsext_custom_types[ + s->s3->serverinfo_client_tlsext_custom_types_count - 1] = ext_type; + return 1; } static int serverinfo_srv_second_cb(SSL *s, unsigned short ext_type, const unsigned char **out, unsigned short *outlen, - void *arg) + int *al, void *arg) { const unsigned char *serverinfo = NULL; size_t serverinfo_length = 0; + size_t i = 0; + unsigned int match = 0; + /* Did the client send a TLS extension for this type? */ + for (i = 0; i < s->s3->serverinfo_client_tlsext_custom_types_count; i++) + { + if (s->s3->serverinfo_client_tlsext_custom_types[i] == ext_type) + { + match = 1; + break; + } + } + if (!match) + { + //extension not sent by client...don't send extension + return -1; + } /* Is there serverinfo data for the chosen server cert? */ if ((ssl_get_server_cert_serverinfo(s, &serverinfo,
diff --git a/ssl/ssltest.c b/ssl/ssltest.c index 5e2fed8..abdb1b8 100644 --- a/ssl/ssltest.c +++ b/ssl/ssltest.c
@@ -564,7 +564,7 @@ static int custom_ext_0_cli_first_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_0) custom_ext_error = 1; @@ -582,7 +582,7 @@ static int custom_ext_1_cli_first_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_1) custom_ext_error = 1; @@ -602,7 +602,7 @@ static int custom_ext_2_cli_first_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_2) custom_ext_error = 1; @@ -625,7 +625,7 @@ static int custom_ext_3_cli_first_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { if (ext_type != CUSTOM_EXT_TYPE_3) custom_ext_error = 1; @@ -648,7 +648,7 @@ return 1; } - +//custom_ext_0_cli_first_cb returns -1 - the server won't receive a callback for this extension static int custom_ext_0_srv_first_cb(SSL *s, unsigned short ext_type, const unsigned char *in, unsigned short inlen, int *al, @@ -658,12 +658,12 @@ return 0; /* Shouldn't be called */ } +//'generate' callbacks are always called, even if the 'receive' callback isn't called static int custom_ext_0_srv_second_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { - custom_ext_error = 1; - return 0; /* Shouldn't be called */ + return -1; /* Don't send an extension */ } static int custom_ext_1_srv_first_cb(SSL *s, unsigned short ext_type, @@ -683,7 +683,7 @@ static int custom_ext_1_srv_second_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { return -1; /* Don't send an extension */ } @@ -705,7 +705,7 @@ static int custom_ext_2_srv_second_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { *out = NULL; *outlen = 0; @@ -729,7 +729,7 @@ static int custom_ext_3_srv_second_cb(SSL *s, unsigned short ext_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { *out = (const unsigned char*)custom_ext_srv_string; *outlen = strlen(custom_ext_srv_string); @@ -738,7 +738,7 @@ static int supp_data_0_srv_first_cb(SSL *s, unsigned short supp_data_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { *out = (const unsigned char*)supp_data_0_string; *outlen = strlen(supp_data_0_string); @@ -765,7 +765,7 @@ static int supp_data_1_srv_first_cb(SSL *s, unsigned short supp_data_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { return -1; } @@ -806,7 +806,7 @@ static int supp_data_0_cli_second_cb(SSL *s, unsigned short supp_data_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { *out = (const unsigned char*)supp_data_0_string; *outlen = strlen(supp_data_0_string); @@ -826,7 +826,7 @@ static int supp_data_1_cli_second_cb(SSL *s, unsigned short supp_data_type, const unsigned char **out, - unsigned short *outlen, void *arg) + unsigned short *outlen, int *al, void *arg) { return -1; }
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 44e4c45..0354e70 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c
@@ -1089,7 +1089,7 @@ c->valid = 1; } -unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit) +unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit, int *al) { int extdatalen=0; unsigned char *ret = p; @@ -1453,7 +1453,7 @@ { int cb_retval = 0; cb_retval = record->fn1(s, record->ext_type, - &out, &outlen, + &out, &outlen, al, record->arg); if (cb_retval == 0) return NULL; /* error */ @@ -1509,10 +1509,12 @@ return ret; } -unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit) +unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit, int *al) { int extdatalen=0; unsigned char *ret = p; + size_t i; + custom_srv_ext_record *record; #ifndef OPENSSL_NO_NEXTPROTONEG int next_proto_neg_seen; #endif @@ -1696,45 +1698,29 @@ } #endif - /* If custom types were sent in ClientHello, add ServerHello responses */ - if (s->s3->tlsext_custom_types_count) + for (i = 0; i < s->ctx->custom_srv_ext_records_count; i++) { - size_t i; + record = &s->ctx->custom_srv_ext_records[i]; + const unsigned char *out = NULL; + unsigned short outlen = 0; + int cb_retval = 0; - for (i = 0; i < s->s3->tlsext_custom_types_count; i++) - { - size_t j; - custom_srv_ext_record *record; - - for (j = 0; j < s->ctx->custom_srv_ext_records_count; j++) - { - record = &s->ctx->custom_srv_ext_records[j]; - if (s->s3->tlsext_custom_types[i] == record->ext_type) - { - const unsigned char *out = NULL; - unsigned short outlen = 0; - 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); - s2n(outlen, ret); - memcpy(ret, out, outlen); - ret += outlen; - break; - } - } - } + /* NULL callback or -1 omits extension */ + if (!record->fn2) + break; + cb_retval = record->fn2(s, record->ext_type, + &out, &outlen, al, + 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); + s2n(outlen, ret); + memcpy(ret, out, outlen); + ret += outlen; } #ifdef TLSEXT_TYPE_encrypt_then_mac if (s->s3->flags & TLS1_FLAGS_ENCRYPT_THEN_MAC) @@ -1949,11 +1935,11 @@ } /* Clear observed custom extensions */ - s->s3->tlsext_custom_types_count = 0; - if (s->s3->tlsext_custom_types != NULL) + s->s3->serverinfo_client_tlsext_custom_types_count = 0; + if (s->s3->serverinfo_client_tlsext_custom_types != NULL) { - OPENSSL_free(s->s3->tlsext_custom_types); - s->s3->tlsext_custom_types = NULL; + OPENSSL_free(s->s3->serverinfo_client_tlsext_custom_types); + s->s3->serverinfo_client_tlsext_custom_types = NULL; } #ifndef OPENSSL_NO_HEARTBEATS @@ -2476,35 +2462,8 @@ record = &s->ctx->custom_srv_ext_records[i]; if (type == record->ext_type) { - size_t j; - - /* Error on duplicate TLS Extensions */ - for (j = 0; j < s->s3->tlsext_custom_types_count; j++) - { - if (type == s->s3->tlsext_custom_types[j]) - { - *al = TLS1_AD_DECODE_ERROR; - return 0; - } - } - - /* NULL callback still notes the extension */ if (record->fn1 && !record->fn1(s, type, data, size, al, record->arg)) return 0; - - /* Add the (non-duplicated) entry */ - s->s3->tlsext_custom_types_count++; - s->s3->tlsext_custom_types = OPENSSL_realloc( - s->s3->tlsext_custom_types, - s->s3->tlsext_custom_types_count * 2); - if (s->s3->tlsext_custom_types == NULL) - { - s->s3->tlsext_custom_types = 0; - *al = TLS1_AD_INTERNAL_ERROR; - return 0; - } - s->s3->tlsext_custom_types[ - s->s3->tlsext_custom_types_count - 1] = type; } } }