summaryrefslogtreecommitdiff
path: root/src/lib/libssl/ssl_sess.c
diff options
context:
space:
mode:
authortb <>2020-09-01 17:25:17 +0000
committertb <>2020-09-01 17:25:17 +0000
commita528105b51540c5edf59368d36bf78b914cd2899 (patch)
tree923a83ec6bfc6873fcf70ec3684be8700290615e /src/lib/libssl/ssl_sess.c
parent1e6510105e17f4686509b6cef5e4a607664dd5c0 (diff)
downloadopenbsd-a528105b51540c5edf59368d36bf78b914cd2899.tar.gz
openbsd-a528105b51540c5edf59368d36bf78b914cd2899.tar.bz2
openbsd-a528105b51540c5edf59368d36bf78b914cd2899.zip
Split session retrieval out of ssl_get_prev_session()
In case the session ticket was empty or missing, an attempt is made to retrieve the session from the internal cache or via a callback. This code can easily be flattened a bit and factored into two functions. I decided to wrap those into a third function to make the call from the switch easier on the eye. I could have kept the try_session_cache flag, but it now seems rather pointless and awkwardly named anyway, so I took its negation and named it ticket_decrypted. To top things off, a little bit of polish in the exit path. ok beck inoguchi jsing (with the usual healthy dose of nits)
Diffstat (limited to 'src/lib/libssl/ssl_sess.c')
-rw-r--r--src/lib/libssl/ssl_sess.c170
1 files changed, 92 insertions, 78 deletions
diff --git a/src/lib/libssl/ssl_sess.c b/src/lib/libssl/ssl_sess.c
index 460c5d85f1..327164da25 100644
--- a/src/lib/libssl/ssl_sess.c
+++ b/src/lib/libssl/ssl_sess.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: ssl_sess.c,v 1.92 2020/09/01 12:40:53 tb Exp $ */ 1/* $OpenBSD: ssl_sess.c,v 1.93 2020/09/01 17:25:17 tb Exp $ */
2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) 2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
3 * All rights reserved. 3 * All rights reserved.
4 * 4 *
@@ -413,6 +413,84 @@ ssl_get_new_session(SSL *s, int session)
413 return (1); 413 return (1);
414} 414}
415 415
416static SSL_SESSION *
417ssl_session_from_cache(SSL *s, CBS *session_id)
418{
419 SSL_SESSION *sess;
420 SSL_SESSION data;
421
422 if ((s->session_ctx->internal->session_cache_mode &
423 SSL_SESS_CACHE_NO_INTERNAL_LOOKUP))
424 return NULL;
425
426 data.ssl_version = s->version;
427 data.session_id_length = CBS_len(session_id);
428 memcpy(data.session_id, CBS_data(session_id), CBS_len(session_id));
429
430 CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
431 sess = lh_SSL_SESSION_retrieve(s->session_ctx->internal->sessions, &data);
432 if (sess != NULL)
433 CRYPTO_add(&sess->references, 1, CRYPTO_LOCK_SSL_SESSION);
434 CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX);
435
436 if (sess == NULL)
437 s->session_ctx->internal->stats.sess_miss++;
438
439 return sess;
440}
441
442static SSL_SESSION *
443ssl_session_from_callback(SSL *s, CBS *session_id)
444{
445 SSL_SESSION *sess;
446 int copy;
447
448 if (s->session_ctx->internal->get_session_cb == NULL)
449 return NULL;
450
451 copy = 1;
452 if ((sess = s->session_ctx->internal->get_session_cb(s,
453 CBS_data(session_id), CBS_len(session_id), &copy)) == NULL)
454 return NULL;
455
456 s->session_ctx->internal->stats.sess_cb_hit++;
457
458 /*
459 * The copy handler may have set copy == 0 to indicate that the session
460 * structures are shared between threads and that it handles the
461 * reference count itself. If it didn't set copy to zero, we must
462 * increment the reference count.
463 */
464 if (copy)
465 CRYPTO_add(&sess->references, 1, CRYPTO_LOCK_SSL_SESSION);
466
467 /* Add the externally cached session to the internal cache as well. */
468 if (!(s->session_ctx->internal->session_cache_mode &
469 SSL_SESS_CACHE_NO_INTERNAL_STORE)) {
470 /*
471 * The following should not return 1,
472 * otherwise, things are very strange.
473 */
474 SSL_CTX_add_session(s->session_ctx, sess);
475 }
476
477 return sess;
478}
479
480static SSL_SESSION *
481ssl_session_by_id(SSL *s, CBS *session_id)
482{
483 SSL_SESSION *sess;
484
485 if (CBS_len(session_id) == 0)
486 return NULL;
487
488 if ((sess = ssl_session_from_cache(s, session_id)) == NULL)
489 sess = ssl_session_from_callback(s, session_id);
490
491 return sess;
492}
493
416/* 494/*
417 * ssl_get_prev attempts to find an SSL_SESSION to be used to resume this 495 * ssl_get_prev attempts to find an SSL_SESSION to be used to resume this
418 * connection. It is only called by servers. 496 * connection. It is only called by servers.
@@ -439,16 +517,13 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
439 SSL_SESSION *sess = NULL; 517 SSL_SESSION *sess = NULL;
440 size_t session_id_len; 518 size_t session_id_len;
441 int alert_desc = SSL_AD_INTERNAL_ERROR, fatal = 0; 519 int alert_desc = SSL_AD_INTERNAL_ERROR, fatal = 0;
442 int try_session_cache = 1; 520 int ticket_decrypted = 0;
443 521
444 /* This is used only by servers. */ 522 /* This is used only by servers. */
445 523
446 if (CBS_len(session_id) > SSL_MAX_SSL_SESSION_ID_LENGTH) 524 if (CBS_len(session_id) > SSL_MAX_SSL_SESSION_ID_LENGTH)
447 goto err; 525 goto err;
448 526
449 if (CBS_len(session_id) == 0)
450 try_session_cache = 0;
451
452 /* Sets s->internal->tlsext_ticket_expected. */ 527 /* Sets s->internal->tlsext_ticket_expected. */
453 switch (tls1_process_ticket(s, ext_block, &alert_desc, &sess)) { 528 switch (tls1_process_ticket(s, ext_block, &alert_desc, &sess)) {
454 case TLS1_TICKET_FATAL_ERROR: 529 case TLS1_TICKET_FATAL_ERROR:
@@ -456,12 +531,13 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
456 goto err; 531 goto err;
457 case TLS1_TICKET_NONE: 532 case TLS1_TICKET_NONE:
458 case TLS1_TICKET_EMPTY: 533 case TLS1_TICKET_EMPTY:
459 break; /* Ok to carry on processing session id. */ 534 if ((sess = ssl_session_by_id(s, session_id)) == NULL)
535 goto err;
536 break;
460 case TLS1_TICKET_NOT_DECRYPTED: 537 case TLS1_TICKET_NOT_DECRYPTED:
461 try_session_cache = 0;
462 goto err; 538 goto err;
463 case TLS1_TICKET_DECRYPTED: 539 case TLS1_TICKET_DECRYPTED:
464 try_session_cache = 0; 540 ticket_decrypted = 1;
465 541
466 /* 542 /*
467 * The session ID is used by some clients to detect that the 543 * The session ID is used by some clients to detect that the
@@ -480,66 +556,6 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
480 goto err; 556 goto err;
481 } 557 }
482 558
483 if (try_session_cache && sess == NULL &&
484 !(s->session_ctx->internal->session_cache_mode &
485 SSL_SESS_CACHE_NO_INTERNAL_LOOKUP)) {
486 SSL_SESSION data;
487
488 data.ssl_version = s->version;
489 data.session_id_length = CBS_len(session_id);
490 memcpy(data.session_id, CBS_data(session_id),
491 CBS_len(session_id));
492
493 CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
494 sess = lh_SSL_SESSION_retrieve(s->session_ctx->internal->sessions, &data);
495 if (sess != NULL) {
496 /* Don't allow other threads to steal it. */
497 CRYPTO_add(&sess->references, 1,
498 CRYPTO_LOCK_SSL_SESSION);
499 }
500 CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX);
501
502 if (sess == NULL)
503 s->session_ctx->internal->stats.sess_miss++;
504 }
505
506 if (try_session_cache && sess == NULL &&
507 s->session_ctx->internal->get_session_cb != NULL) {
508 int copy = 1;
509
510 if ((sess = s->session_ctx->internal->get_session_cb(s,
511 CBS_data(session_id), CBS_len(session_id), &copy))) {
512 s->session_ctx->internal->stats.sess_cb_hit++;
513
514 /*
515 * Increment reference count now if the session
516 * callback asks us to do so (note that if the session
517 * structures returned by the callback are shared
518 * between threads, it must handle the reference count
519 * itself [i.e. copy == 0], or things won't be
520 * thread-safe).
521 */
522 if (copy)
523 CRYPTO_add(&sess->references, 1,
524 CRYPTO_LOCK_SSL_SESSION);
525
526 /*
527 * Add the externally cached session to the internal
528 * cache as well if and only if we are supposed to.
529 */
530 if (!(s->session_ctx->internal->session_cache_mode &
531 SSL_SESS_CACHE_NO_INTERNAL_STORE))
532 /*
533 * The following should not return 1,
534 * otherwise, things are very strange.
535 */
536 SSL_CTX_add_session(s->session_ctx, sess);
537 }
538 }
539
540 if (sess == NULL)
541 goto err;
542
543 /* Now sess is non-NULL and we own one of its reference counts. */ 559 /* Now sess is non-NULL and we own one of its reference counts. */
544 560
545 if (sess->sid_ctx_length != s->sid_ctx_length || 561 if (sess->sid_ctx_length != s->sid_ctx_length ||
@@ -576,7 +592,7 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
576 if (sess->timeout < (time(NULL) - sess->time)) { 592 if (sess->timeout < (time(NULL) - sess->time)) {
577 /* timeout */ 593 /* timeout */
578 s->session_ctx->internal->stats.sess_timeout++; 594 s->session_ctx->internal->stats.sess_timeout++;
579 if (try_session_cache) { 595 if (!ticket_decrypted) {
580 /* session was from the cache, so remove it */ 596 /* session was from the cache, so remove it */
581 SSL_CTX_remove_session(s->session_ctx, sess); 597 SSL_CTX_remove_session(s->session_ctx, sess);
582 } 598 }
@@ -591,15 +607,13 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
591 return 1; 607 return 1;
592 608
593 err: 609 err:
594 if (sess != NULL) { 610 SSL_SESSION_free(sess);
595 SSL_SESSION_free(sess); 611 if (ticket_decrypted) {
596 if (!try_session_cache) { 612 /*
597 /* 613 * The session was from a ticket. Issue a ticket for the new
598 * The session was from a ticket, so we should 614 * session.
599 * issue a ticket for the new session. 615 */
600 */ 616 s->internal->tlsext_ticket_expected = 1;
601 s->internal->tlsext_ticket_expected = 1;
602 }
603 } 617 }
604 if (fatal) { 618 if (fatal) {
605 *alert = alert_desc; 619 *alert = alert_desc;