X-Git-Url: https://git.librecmc.org/?a=blobdiff_plain;f=ssl%2Fssl_sess.c;h=849908141d76e43a01ef53d10d81b7ad290338d4;hb=dd9d233e2aa493fa1398b527afbf6aa5cdb23f23;hp=cac408c38e805689c102f6a7c71accfc2d409fec;hpb=673eadec2c9032e938040d5eba241baed30d2c10;p=oweals%2Fopenssl.git diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index cac408c38e..849908141d 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -65,21 +65,43 @@ static void SSL_SESSION_list_remove(SSL_CTX *ctx, SSL_SESSION *s); static void SSL_SESSION_list_add(SSL_CTX *ctx,SSL_SESSION *s); static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lck); static int ssl_session_num=0; -static STACK *ssl_session_meth=NULL; +static STACK_OF(CRYPTO_EX_DATA_FUNCS) *ssl_session_meth=NULL; +#if 1 /* traditional SSLeay behaviour */ SSL_SESSION *SSL_get_session(SSL *ssl) { return(ssl->session); } +#else /* suggested change: increase reference counter so that a session + * can later be set in a new SSL object. + * Objections: + * -- the modified function should have a new name (or old + * applications, including s_client, leak memory); + * -- the locking seems unnecessary given that SSL structures + * usually cannot be safely shared between threads anyway. */ +SSL_SESSION *SSL_get_session(SSL *ssl) + { + SSL_SESSION *sess; + /* Need to lock this all up rather than just use CRYPTO_add so that + * somebody doesn't free ssl->session between when we check it's + * non-null and when we up the reference count. */ + CRYPTO_r_lock(CRYPTO_LOCK_SSL_SESSION); + sess = ssl->session; + if(sess) + sess->references++; + CRYPTO_r_unlock(CRYPTO_LOCK_SSL_SESSION); + return(sess); + } +#endif -int SSL_SESSION_get_ex_new_index(long argl, char *argp, int (*new_func)(), - int (*dup_func)(), void (*free_func)()) - { - ssl_session_num++; - return(CRYPTO_get_ex_new_index(ssl_session_num-1, +int SSL_SESSION_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func, + CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) + { + ssl_session_num++; + return(CRYPTO_get_ex_new_index(ssl_session_num-1, &ssl_session_meth, - argl,argp,new_func,dup_func,free_func)); - } + argl,argp,new_func,dup_func,free_func)); + } int SSL_SESSION_set_ex_data(SSL_SESSION *s, int idx, void *arg) { @@ -103,13 +125,14 @@ SSL_SESSION *SSL_SESSION_new(void) } memset(ss,0,sizeof(SSL_SESSION)); + ss->verify_result = 1; /* avoid 0 (= X509_V_OK) just in case */ ss->references=1; ss->timeout=60*5+4; /* 5 minute timeout by default */ ss->time=time(NULL); ss->prev=NULL; ss->next=NULL; ss->compress_meth=0; - CRYPTO_new_ex_data(ssl_session_meth,(char *)ss,&ss->ex_data); + CRYPTO_new_ex_data(ssl_session_meth,ss,&ss->ex_data); return(ss); } @@ -161,13 +184,21 @@ int ssl_get_new_session(SSL *s, int session) { SSL_SESSION *r; - RAND_bytes(ss->session_id,ss->session_id_length); + RAND_pseudo_bytes(ss->session_id,ss->session_id_length); CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX); r=(SSL_SESSION *)lh_retrieve(s->ctx->sessions, (char *)ss); CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX); if (r == NULL) break; /* else - woops a session_id match */ + /* XXX We should also check the external cache -- + * but the probability of a collision is negligible, and + * we could not prevent the concurrent creation of sessions + * with identical IDs since we currently don't have means + * to atomically check whether a session ID already exists + * and make a reservation for it if it does not + * (this problem applies to the internal cache as well). + */ } } else @@ -179,6 +210,7 @@ int ssl_get_new_session(SSL *s, int session) ss->sid_ctx_length=s->sid_ctx_length; s->session=ss; ss->ssl_version=s->version; + ss->verify_result = X509_V_OK; return(1); } @@ -188,24 +220,28 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len) /* This is used only by servers. */ SSL_SESSION *ret=NULL,data; - int copy=1; + int fatal = 0; - /* conn_init();*/ data.ssl_version=s->version; data.session_id_length=len; if (len > SSL_MAX_SSL_SESSION_ID_LENGTH) - return(0); + goto err; memcpy(data.session_id,session_id,len); if (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_NO_INTERNAL_LOOKUP)) { CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX); ret=(SSL_SESSION *)lh_retrieve(s->ctx->sessions,(char *)&data); + if (ret != NULL) + /* don't allow other threads to steal it: */ + CRYPTO_add(&ret->references,1,CRYPTO_LOCK_SSL_SESSION); CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX); } if (ret == NULL) { + int copy=1; + s->ctx->stats.sess_miss++; ret=NULL; if (s->ctx->get_session_cb != NULL @@ -214,28 +250,52 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len) { s->ctx->stats.sess_cb_hit++; + /* Increment reference count now if the session callback + * asks us to do so (note that if the session structures + * returned by the callback are shared between threads, + * it must handle the reference count itself [i.e. copy == 0], + * or things won't be thread-safe). */ + if (copy) + CRYPTO_add(&ret->references,1,CRYPTO_LOCK_SSL_SESSION); + /* The following should not return 1, otherwise, * things are very strange */ SSL_CTX_add_session(s->ctx,ret); } - if (ret == NULL) return(0); + if (ret == NULL) + goto err; } + /* Now ret is non-NULL, and we own one of its reference counts. */ + if((s->verify_mode&SSL_VERIFY_PEER) && (!s->sid_ctx_length || ret->sid_ctx_length != s->sid_ctx_length || memcmp(ret->sid_ctx,s->sid_ctx,ret->sid_ctx_length))) { - if (s->sid_ctx_length) - SSLerr(SSL_F_SSL_GET_PREV_SESSION,SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT); - else - /* application should have used SSL[_CTX]_set_session_id_context */ + /* We've found the session named by the client, but we don't + * want to use it in this context. */ + + if (s->sid_ctx_length == 0) + { + /* application should have used SSL[_CTX]_set_session_id_context + * -- we could tolerate this and just pretend we never heard + * of this session, but then applications could effectively + * disable the session cache by accident without anyone noticing */ + SSLerr(SSL_F_SSL_GET_PREV_SESSION,SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED); - return 0; - } + fatal = 1; + goto err; + } + else + { +#if 0 /* The client cannot always know when a session is not appropriate, + * so we shouldn't generate an error message. */ - /* auto free it */ - if (!copy) - SSL_SESSION_free(ret); + SSLerr(SSL_F_SSL_GET_PREV_SESSION,SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT); +#endif + goto err; /* treat like cache miss */ + } + } if (ret->cipher == NULL) { @@ -250,22 +310,25 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len) else ret->cipher=ssl_get_cipher_by_char(s,&(buf[1])); if (ret->cipher == NULL) - return(0); + goto err; } + +#if 0 /* This is way too late. */ + /* If a thread got the session, then 'swaped', and another got * it and then due to a time-out decided to 'Free' it we could * be in trouble. So I'll increment it now, then double decrement * later - am I speaking rubbish?. */ CRYPTO_add(&ret->references,1,CRYPTO_LOCK_SSL_SESSION); +#endif if ((long)(ret->time+ret->timeout) < (long)time(NULL)) /* timeout */ { s->ctx->stats.sess_timeout++; /* remove it from the cache */ SSL_CTX_remove_session(s->ctx,ret); - SSL_SESSION_free(ret); /* again to actually Free it */ - return(0); + goto err; } s->ctx->stats.sess_hit++; @@ -277,7 +340,16 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len) if (s->session != NULL) SSL_SESSION_free(s->session); s->session=ret; + s->verify_result = s->session->verify_result; return(1); + + err: + if (ret != NULL) + SSL_SESSION_free(ret); + if (fatal) + return -1; + else + return 0; } int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) @@ -285,27 +357,47 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) int ret=0; SSL_SESSION *s; - /* conn_init(); */ + /* add just 1 reference count for the SSL_CTX's session cache + * even though it has two ways of access: each session is in a + * doubly linked list and an lhash */ CRYPTO_add(&c->references,1,CRYPTO_LOCK_SSL_SESSION); + /* if session c is in already in cache, we take back the increment later */ CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); s=(SSL_SESSION *)lh_insert(ctx->sessions,(char *)c); - /* Put on the end of the queue unless it is already in the cache */ + /* s != NULL iff we already had a session with the given PID. + * In this case, s == c should hold (then we did not really modify + * ctx->sessions), or we're in trouble. */ + if (s != NULL && s != c) + { + /* We *are* in trouble ... */ + SSL_SESSION_list_remove(ctx,s); + SSL_SESSION_free(s); + /* ... so pretend the other session did not exist in cache + * (we cannot handle two SSL_SESSION structures with identical + * session ID in the same cache, which could happen e.g. when + * two threads concurrently obtain the same session from an external + * cache) */ + s = NULL; + } + + /* Put at the head of the queue unless it is already in the cache */ if (s == NULL) SSL_SESSION_list_add(ctx,c); - /* If the same session if is being 're-added', Free the old - * one when the last person stops using it. - * This will also work if it is alread in the cache. - * The references will go up and then down :-) */ if (s != NULL) { - SSL_SESSION_free(s); + /* existing cache entry -- decrement previously incremented reference + * count because it already takes into account the cache */ + + SSL_SESSION_free(s); /* s == c */ ret=0; } else { + /* new cache entry -- remove old ones if cache has become too large */ + ret=1; if (SSL_CTX_sess_get_cache_size(ctx) > 0) @@ -380,7 +472,7 @@ void SSL_SESSION_free(SSL_SESSION *ss) } #endif - CRYPTO_free_ex_data(ssl_session_meth,(char *)ss,&ss->ex_data); + CRYPTO_free_ex_data(ssl_session_meth,ss,&ss->ex_data); memset(ss->key_arg,0,SSL_MAX_KEY_ARG_LENGTH); memset(ss->master_key,0,SSL_MAX_MASTER_KEY_LENGTH);