Fix an oversight - when checking a potential session ID for conflicts with
an SSL_CTX's session cache, it is necessary to compare the ssl_version at the same time (a conflict is defined, courtesy of SSL_SESSION_cmp(), as a matching id/id_length pair and a matching ssl_version). However, the SSL_SESSION that will result from the current negotiation does not necessarily have the same ssl version as the "SSL_METHOD" in use by the SSL_CTX - part of the work in a handshake is to agree on an ssl version! This is fixed by having the check function accept an SSL pointer rather than the SSL_CTX it belongs to. [Thanks to Lutz for illuminating the full extent of my stupidity]
This commit is contained in:
parent
48bf4aae24
commit
f85c9904c6
3 changed files with 8 additions and 8 deletions
|
@ -391,7 +391,7 @@ typedef struct ssl_session_st
|
|||
* callbacks should themselves check if the id they generate is unique otherwise
|
||||
* the SSL handshake will fail with an error - callbacks can do this using the
|
||||
* 'ssl' value they're passed by;
|
||||
* SSL_CTX_has_matching_session_id(ssl->ctx, id, *id_len)
|
||||
* SSL_has_matching_session_id(ssl, id, *id_len)
|
||||
* The length value passed in is set at the maximum size the session ID can be.
|
||||
* In SSLv2 this is 16 bytes, whereas SSLv3/TLSv1 it is 32 bytes. The callback
|
||||
* can alter this length to be less if desired, but under SSLv2 session IDs are
|
||||
|
@ -1054,7 +1054,7 @@ int SSL_CTX_add_session(SSL_CTX *s, SSL_SESSION *c);
|
|||
int SSL_CTX_remove_session(SSL_CTX *,SSL_SESSION *c);
|
||||
int SSL_CTX_set_generate_session_id(SSL_CTX *, GEN_SESSION_CB);
|
||||
int SSL_set_generate_session_id(SSL *, GEN_SESSION_CB);
|
||||
int SSL_CTX_has_matching_session_id(const SSL_CTX *ctx, const unsigned char *id,
|
||||
int SSL_has_matching_session_id(const SSL *ssl, const unsigned char *id,
|
||||
unsigned int id_len);
|
||||
SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a,unsigned char **pp,long length);
|
||||
|
||||
|
|
|
@ -299,16 +299,16 @@ int SSL_set_generate_session_id(SSL *ssl, GEN_SESSION_CB cb)
|
|||
return 1;
|
||||
}
|
||||
|
||||
int SSL_CTX_has_matching_session_id(const SSL_CTX *ctx, const unsigned char *id,
|
||||
int SSL_has_matching_session_id(const SSL *ssl, const unsigned char *id,
|
||||
unsigned int id_len)
|
||||
{
|
||||
/* A quick examination of SSL_SESSION_hash and SSL_SESSION_cmp shows how
|
||||
* we can "construct" a session to give us the desired check - ie. to
|
||||
* find if there's a session in the hash table that would conflict with
|
||||
* any new session built out of this id/id_len and the ssl_version in
|
||||
* use by this SSL_CTX. */
|
||||
* use by this SSL. */
|
||||
SSL_SESSION r, *p;
|
||||
r.ssl_version = ctx->method->version;
|
||||
r.ssl_version = ssl->version;
|
||||
r.session_id_length = id_len;
|
||||
memcpy(r.session_id, id, id_len);
|
||||
/* NB: SSLv2 always uses a fixed 16-byte session ID, so even if a
|
||||
|
@ -324,7 +324,7 @@ int SSL_CTX_has_matching_session_id(const SSL_CTX *ctx, const unsigned char *id,
|
|||
}
|
||||
|
||||
CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
|
||||
p = (SSL_SESSION *)lh_retrieve(ctx->sessions, &r);
|
||||
p = (SSL_SESSION *)lh_retrieve(ssl->ctx->sessions, &r);
|
||||
CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX);
|
||||
return (p != NULL);
|
||||
}
|
||||
|
|
|
@ -146,7 +146,7 @@ static int def_generate_session_id(const SSL *ssl, unsigned char *id,
|
|||
unsigned int retry = 0;
|
||||
do
|
||||
RAND_pseudo_bytes(id, *id_len);
|
||||
while(SSL_CTX_has_matching_session_id(ssl->ctx, id, *id_len) &&
|
||||
while(SSL_has_matching_session_id(ssl, id, *id_len) &&
|
||||
(++retry < MAX_SESS_ID_ATTEMPTS));
|
||||
if(retry < MAX_SESS_ID_ATTEMPTS)
|
||||
return 1;
|
||||
|
@ -240,7 +240,7 @@ int ssl_get_new_session(SSL *s, int session)
|
|||
else
|
||||
ss->session_id_length = tmp;
|
||||
/* Finally, check for a conflict */
|
||||
if(SSL_CTX_has_matching_session_id(s->ctx, ss->session_id,
|
||||
if(SSL_has_matching_session_id(s, ss->session_id,
|
||||
ss->session_id_length))
|
||||
{
|
||||
SSLerr(SSL_F_SSL_GET_NEW_SESSION,
|
||||
|
|
Loading…
Reference in a new issue