Remove SSL_set_session_secret_cb (EAP-FAST)
This is only used for EAP-FAST which we apparently don't need to support.
Remove it outright. We broke it in 9eaeef81fa2d4fd6246dc02b6203fa936a5eaf67 by
failing to account for session misses.
If this changes and we need it later, we can resurrect it. Preferably
implemented differently: the current implementation is bolted badly onto the
handshake. Ideally use the supplied callbacks to fabricate an appropriate
SSL_SESSION and resume that with as much of the normal session ticket flow as
possible.
The one difference is that EAP-FAST seems to require the probing mechanism for
session tickets rather than the sane session ID echoing version. We can
reimplement that by asking the record layer to probe ahead for one byte.
Change-Id: I38304953cc36b2020611556a91e8ac091691edac
Reviewed-on: https://boringssl-review.googlesource.com/2360
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index bcc9501..a833e63 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -280,7 +280,6 @@
* 'struct ssl_st *' function parameters used to prototype callbacks
* in SSL_CTX. */
typedef struct ssl_st *ssl_crock_st;
-typedef struct tls_session_ticket_ext_st TLS_SESSION_TICKET_EXT;
typedef struct ssl_method_st SSL_METHOD;
typedef struct ssl_cipher_st SSL_CIPHER;
typedef struct ssl_session_st SSL_SESSION;
@@ -298,9 +297,6 @@
DECLARE_STACK_OF(SRTP_PROTECTION_PROFILE)
-typedef int (*tls_session_ticket_ext_cb_fn)(SSL *s, const unsigned char *data, int len, void *arg);
-typedef int (*tls_session_secret_cb_fn)(SSL *s, void *secret, int *secret_len, STACK_OF(SSL_CIPHER) *peer_ciphers, const SSL_CIPHER **cipher, void *arg);
-
#ifndef OPENSSL_NO_SSL_INTERN
/* used to hold info on the particular ciphers used */
@@ -1329,17 +1325,6 @@
size_t tlsext_ellipticcurvelist_length;
uint16_t *tlsext_ellipticcurvelist; /* our list */
- /* TLS Session Ticket extension override */
- TLS_SESSION_TICKET_EXT *tlsext_session_ticket;
-
- /* TLS Session Ticket extension callback */
- tls_session_ticket_ext_cb_fn tls_session_ticket_ext_cb;
- void *tls_session_ticket_ext_cb_arg;
-
- /* TLS pre-shared secret session resumption */
- tls_session_secret_cb_fn tls_session_secret_cb;
- void *tls_session_secret_cb_arg;
-
SSL_CTX * initial_ctx; /* initial ctx, used to store sessions */
/* Next protocol negotiation. For the client, this is the protocol that
@@ -2213,14 +2198,6 @@
OPENSSL_EXPORT void *SSL_COMP_get_compression_methods(void);
OPENSSL_EXPORT int SSL_COMP_add_compression_method(int id,void *cm);
-/* TLS extensions functions */
-OPENSSL_EXPORT int SSL_set_session_ticket_ext(SSL *s, void *ext_data, int ext_len);
-
-OPENSSL_EXPORT int SSL_set_session_ticket_ext_cb(SSL *s, tls_session_ticket_ext_cb_fn cb, void *arg);
-
-/* Pre-shared secret session resumption functions */
-OPENSSL_EXPORT int SSL_set_session_secret_cb(SSL *s, tls_session_secret_cb_fn tls_session_secret_cb, void *arg);
-
OPENSSL_EXPORT void SSL_set_debug(SSL *s, int debug);
OPENSSL_EXPORT int SSL_cache_hit(SSL *s);
OPENSSL_EXPORT int SSL_is_server(SSL *s);
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index dc106d2..d448ce2 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -708,14 +708,6 @@
#define TLS_MD_EXTENDED_MASTER_SECRET_CONST "extended master secret"
#define TLS_MD_EXTENDED_MASTER_SECRET_CONST_SIZE 22
-
-/* TLS Session Ticket extension struct */
-struct tls_session_ticket_ext_st
- {
- unsigned short length;
- void *data;
- };
-
#ifdef __cplusplus
}
#endif
diff --git a/ssl/s23_clnt.c b/ssl/s23_clnt.c
index ebbd810..a65b216 100644
--- a/ssl/s23_clnt.c
+++ b/ssl/s23_clnt.c
@@ -244,7 +244,7 @@
return(ret);
}
-/* Fill a ClientRandom or ServerRandom field of length len. Returns <= 0
+/* Fill a ClientRandom or ServerRandom field of length len. Returns 0
* on failure, 1 on success. */
int ssl_fill_hello_random(SSL *s, int server, unsigned char *result, int len)
{
@@ -260,10 +260,10 @@
unsigned long Time = (unsigned long)time(NULL);
unsigned char *p = result;
l2n(Time, p);
- return RAND_pseudo_bytes(p, len-4);
+ return RAND_bytes(p, len-4);
}
else
- return RAND_pseudo_bytes(result, len);
+ return RAND_bytes(result, len);
}
static int ssl23_client_hello(SSL *s)
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 54352d9..700d0cb 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -881,26 +881,7 @@
/* Copy over the server random. */
memcpy(s->s3->server_random, CBS_data(&server_random), SSL3_RANDOM_SIZE);
- s->hit = 0;
-
- /* check if we want to resume the session based on external pre-shared secret */
- if (s->version >= TLS1_VERSION && s->tls_session_secret_cb)
- {
- const SSL_CIPHER *pref_cipher=NULL;
- s->session->master_key_length=sizeof(s->session->master_key);
- if (s->tls_session_secret_cb(s, s->session->master_key,
- &s->session->master_key_length,
- NULL, &pref_cipher,
- s->tls_session_secret_cb_arg))
- {
- s->session->cipher = pref_cipher ?
- pref_cipher :
- ssl3_get_cipher_by_value(cipher_suite);
- s->hit = 1;
- }
- }
-
- if (!s->hit && CBS_len(&session_id) != 0 &&
+ if (CBS_len(&session_id) != 0 &&
CBS_mem_equal(&session_id,
s->session->session_id, s->session->session_id_length))
{
@@ -914,10 +895,10 @@
}
s->hit = 1;
}
-
- /* a miss or crap from the other end */
- if (!s->hit)
+ else
{
+ /* The session wasn't resumed. */
+ s->hit = 0;
/* If we were trying for session-id reuse, make a new
* SSL_SESSION so we don't stuff up other people */
if (s->session->session_id_length > 0)
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index c7bc58b..e166735 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1064,54 +1064,7 @@
goto f_err;
}
- /* Check if we want to use external pre-shared secret for this
- * handshake for not reused session only. We need to generate
- * server_random before calling tls_session_secret_cb in order to allow
- * SessionTicket processing to use it in key derivation. */
- {
- unsigned char *pos;
- pos=s->s3->server_random;
- if (ssl_fill_hello_random(s, 1, pos, SSL3_RANDOM_SIZE) <= 0)
- {
- goto f_err;
- }
- }
-
- if (!s->hit && s->version >= TLS1_VERSION && s->tls_session_secret_cb)
- {
- const SSL_CIPHER *pref_cipher=NULL;
-
- s->session->master_key_length=sizeof(s->session->master_key);
- if(s->tls_session_secret_cb(s, s->session->master_key, &s->session->master_key_length,
- ciphers, &pref_cipher, s->tls_session_secret_cb_arg))
- {
- s->hit=1;
- s->session->verify_result=X509_V_OK;
-
- /* check if some cipher was preferred by call back */
- pref_cipher=pref_cipher ? pref_cipher : ssl3_choose_cipher(s, ciphers, ssl_get_cipher_preferences(s));
- if (pref_cipher == NULL)
- {
- al=SSL_AD_HANDSHAKE_FAILURE;
- OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_NO_SHARED_CIPHER);
- goto f_err;
- }
-
- s->session->cipher=pref_cipher;
-
- if (s->cipher_list)
- ssl_cipher_preference_list_free(s->cipher_list);
-
- if (s->cipher_list_by_id)
- sk_SSL_CIPHER_free(s->cipher_list_by_id);
-
- s->cipher_list = ssl_cipher_preference_list_from_ciphers(ciphers);
- s->cipher_list_by_id = sk_SSL_CIPHER_dup(ciphers);
- }
- }
-
/* Given ciphers and SSL_get_ciphers, we must pick a cipher */
-
if (!s->hit)
{
if (ciphers == NULL)
@@ -1213,6 +1166,11 @@
*(p++)=s->version&0xff;
/* Random stuff */
+ if (!ssl_fill_hello_random(s, 1, s->s3->server_random, SSL3_RANDOM_SIZE))
+ {
+ OPENSSL_PUT_ERROR(SSL, ssl3_send_server_hello, ERR_R_INTERNAL_ERROR);
+ return -1;
+ }
memcpy(p,s->s3->server_random,SSL3_RANDOM_SIZE);
p+=SSL3_RANDOM_SIZE;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 40ea252..5fb13da 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -682,11 +682,6 @@
if (s->srtp_profiles)
sk_SRTP_PROTECTION_PROFILE_free(s->srtp_profiles);
- if (s->tlsext_session_ticket)
- {
- OPENSSL_free(s->tlsext_session_ticket);
- }
-
OPENSSL_free(s);
}
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index ee3daa9..c0cb1d3 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -789,59 +789,6 @@
return(s->session_timeout);
}
-int SSL_set_session_secret_cb(SSL *s, int (*tls_session_secret_cb)(SSL *s, void *secret, int *secret_len,
- STACK_OF(SSL_CIPHER) *peer_ciphers, const SSL_CIPHER **cipher, void *arg), void *arg)
- {
- if (s == NULL) return(0);
- s->tls_session_secret_cb = tls_session_secret_cb;
- s->tls_session_secret_cb_arg = arg;
- return(1);
- }
-
-int SSL_set_session_ticket_ext_cb(SSL *s, tls_session_ticket_ext_cb_fn cb,
- void *arg)
- {
- if (s == NULL) return(0);
- s->tls_session_ticket_ext_cb = cb;
- s->tls_session_ticket_ext_cb_arg = arg;
- return(1);
- }
-
-int SSL_set_session_ticket_ext(SSL *s, void *ext_data, int ext_len)
- {
- if (s->version >= TLS1_VERSION)
- {
- if (s->tlsext_session_ticket)
- {
- OPENSSL_free(s->tlsext_session_ticket);
- s->tlsext_session_ticket = NULL;
- }
-
- s->tlsext_session_ticket = OPENSSL_malloc(sizeof(TLS_SESSION_TICKET_EXT) + ext_len);
- if (!s->tlsext_session_ticket)
- {
- OPENSSL_PUT_ERROR(SSL, SSL_set_session_ticket_ext, ERR_R_MALLOC_FAILURE);
- return 0;
- }
-
- if (ext_data)
- {
- s->tlsext_session_ticket->length = ext_len;
- s->tlsext_session_ticket->data = s->tlsext_session_ticket + 1;
- memcpy(s->tlsext_session_ticket->data, ext_data, ext_len);
- }
- else
- {
- s->tlsext_session_ticket->length = 0;
- s->tlsext_session_ticket->data = NULL;
- }
-
- return 1;
- }
-
- return 0;
- }
-
typedef struct timeout_param_st
{
SSL_CTX *ctx;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 6ba8585..fa506bd 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -980,25 +980,10 @@
if (!(SSL_get_options(s) & SSL_OP_NO_TICKET))
{
- int ticklen;
+ int ticklen = 0;
if (!s->new_session && s->session && s->session->tlsext_tick)
ticklen = s->session->tlsext_ticklen;
- else if (s->session && s->tlsext_session_ticket &&
- s->tlsext_session_ticket->data)
- {
- s->session->tlsext_tick = BUF_memdup(
- s->tlsext_session_ticket->data,
- s->tlsext_session_ticket->length);
- if (!s->session->tlsext_tick)
- return NULL;
- ticklen = s->tlsext_session_ticket->length;
- s->session->tlsext_ticklen = ticklen;
- }
- else
- ticklen = 0;
- if (ticklen == 0 && s->tlsext_session_ticket &&
- s->tlsext_session_ticket->data == NULL)
- goto skip_ext;
+
/* Check for enough room 2 for extension type, 2 for len
* rest for ticket
*/
@@ -1011,7 +996,6 @@
ret += ticklen;
}
}
- skip_ext:
if (SSL_USE_SIGALGS(s))
{
@@ -1659,15 +1643,6 @@
}
s->s3->tmp.peer_ellipticcurvelist_length = num_curves;
}
- else if (type == TLSEXT_TYPE_session_ticket)
- {
- if (s->tls_session_ticket_ext_cb &&
- !s->tls_session_ticket_ext_cb(s, CBS_data(&extension), CBS_len(&extension), s->tls_session_ticket_ext_cb_arg))
- {
- *out_alert = SSL_AD_INTERNAL_ERROR;
- return 0;
- }
- }
else if (type == TLSEXT_TYPE_renegotiate)
{
if (!ssl_parse_clienthello_renegotiate_ext(s, &extension, out_alert))
@@ -1948,14 +1923,6 @@
}
else if (type == TLSEXT_TYPE_session_ticket)
{
- if (s->tls_session_ticket_ext_cb &&
- !s->tls_session_ticket_ext_cb(s, CBS_data(&extension), CBS_len(&extension),
- s->tls_session_ticket_ext_cb_arg))
- {
- *out_alert = SSL_AD_INTERNAL_ERROR;
- return 0;
- }
-
if ((SSL_get_options(s) & SSL_OP_NO_TICKET) || CBS_len(&extension) > 0)
{
*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
@@ -2272,26 +2239,21 @@
* ret: (output) on return, if a ticket was decrypted, then this is set to
* point to the resulting session.
*
- * If s->tls_session_secret_cb is set then we are expecting a pre-shared key
- * ciphersuite, in which case we have no use for session tickets and one will
- * never be decrypted, nor will s->tlsext_ticket_expected be set to 1.
- *
* Returns:
* -1: fatal error, either from parsing or decrypting the ticket.
* 0: no ticket was found (or was ignored, based on settings).
* 1: a zero length extension was found, indicating that the client supports
* session tickets but doesn't currently have one to offer.
- * 2: either s->tls_session_secret_cb was set, or a ticket was offered but
- * couldn't be decrypted because of a non-fatal error.
+ * 2: a ticket was offered but couldn't be decrypted because of a non-fatal
+ * error.
* 3: a ticket was successfully decrypted and *ret was set.
*
* Side effects:
* Sets s->tlsext_ticket_expected to 1 if the server will have to issue
* a new session ticket to the client because the client indicated support
- * (and s->tls_session_secret_cb is NULL) but the client either doesn't have
- * a session ticket or we couldn't use the one it gave us, or if
- * s->ctx->tlsext_ticket_key_cb asked to renew the client's ticket.
- * Otherwise, s->tlsext_ticket_expected is set to 0.
+ * but the client either doesn't have a session ticket or we couldn't use
+ * the one it gave us, or if s->ctx->tlsext_ticket_key_cb asked to renew
+ * the client's ticket. Otherwise, s->tlsext_ticket_expected is set to 0.
*/
int tls1_process_ticket(SSL *s, const struct ssl_early_callback_ctx *ctx,
SSL_SESSION **ret)
@@ -2321,15 +2283,6 @@
s->tlsext_ticket_expected = 1;
return 1;
}
- if (s->tls_session_secret_cb)
- {
- /* Indicate that the ticket couldn't be
- * decrypted rather than generating the session
- * from ticket now, trigger abbreviated
- * handshake based on external mechanism to
- * calculate the master secret later. */
- return 2;
- }
r = tls_decrypt_ticket(s, data, len, ctx->session_id,
ctx->session_id_len, ret);
switch (r)
