Skip to content

ssl_api_ext: lock ticket key context in the public key APIs #11403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ssl_api_ext: lock ticket key context in the public key APIs #11403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions src/ssl_api_ext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2287,15 +2287,15 @@ int wolfSSL_CTX_set_tlsext_ticket_key_cb(WOLFSSL_CTX *ctx, ticketCompatCb cb)
OPENSSL_EXTRA || HAVE_LIGHTY */

#if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && \
!defined(NO_WOLFSSL_SERVER)
!defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
/* Serialize the session ticket encryption keys.
*
* @param [in] ctx SSL/TLS context object.
* @param [out] keys Buffer to hold session ticket keys.
* @param [in] keylen Length of buffer.
* @return WOLFSSL_SUCCESS on success.
* @return WOLFSSL_FAILURE when ctx is NULL, keys is NULL or keylen is not the
* correct length.
* @return WOLFSSL_FAILURE when ctx is NULL, keys is NULL, keylen is not the
* correct length or the key context mutex cannot be locked.
*/
long wolfSSL_CTX_get_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
unsigned char *keys, int keylen)
Expand All @@ -2306,6 +2306,12 @@ long wolfSSL_CTX_get_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
(keylen != WOLFSSL_TICKET_KEYS_SZ)) {
ret = WOLFSSL_FAILURE;
}
#ifndef SINGLE_THREADED
else if (wc_LockMutex(&ctx->ticketKeyCtx.mutex) != 0) {
WOLFSSL_MSG("Couldn't lock key context mutex");
ret = WOLFSSL_FAILURE;
}
#endif
else {
XMEMCPY(keys, ctx->ticketKeyCtx.name, WOLFSSL_TICKET_NAME_SZ);
keys += WOLFSSL_TICKET_NAME_SZ;
Expand All @@ -2316,6 +2322,9 @@ long wolfSSL_CTX_get_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
c32toa(ctx->ticketKeyCtx.expirary[0], keys);
keys += OPAQUE32_LEN;
c32toa(ctx->ticketKeyCtx.expirary[1], keys);
#ifndef SINGLE_THREADED
wc_UnLockMutex(&ctx->ticketKeyCtx.mutex);
#endif
}

return ret;
Expand All @@ -2327,8 +2336,8 @@ long wolfSSL_CTX_get_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
* @param [in] keys_vp Session ticket keys.
* @param [in] keylen Length of data.
* @return WOLFSSL_SUCCESS on success.
* @return WOLFSSL_FAILURE when ctx is NULL, keys is NULL or keylen is not the
* correct length.
* @return WOLFSSL_FAILURE when ctx is NULL, keys is NULL, keylen is not the
* correct length or the key context mutex cannot be locked.
*/
long wolfSSL_CTX_set_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
const void *keys_vp, int keylen)
Expand All @@ -2340,6 +2349,12 @@ long wolfSSL_CTX_set_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
(keylen != WOLFSSL_TICKET_KEYS_SZ)) {
ret = WOLFSSL_FAILURE;
}
#ifndef SINGLE_THREADED
else if (wc_LockMutex(&ctx->ticketKeyCtx.mutex) != 0) {
WOLFSSL_MSG("Couldn't lock key context mutex");
ret = WOLFSSL_FAILURE;
}
#endif
else {
XMEMCPY(ctx->ticketKeyCtx.name, keys, WOLFSSL_TICKET_NAME_SZ);
Comment thread
yosuke-wolfssl marked this conversation as resolved.
keys += WOLFSSL_TICKET_NAME_SZ;
Expand All @@ -2350,6 +2365,9 @@ long wolfSSL_CTX_set_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
ato32(keys, &ctx->ticketKeyCtx.expirary[0]);
keys += OPAQUE32_LEN;
ato32(keys, &ctx->ticketKeyCtx.expirary[1]);
#ifndef SINGLE_THREADED
wc_UnLockMutex(&ctx->ticketKeyCtx.mutex);
#endif
}

return ret;
Expand Down
173 changes: 173 additions & 0 deletions tests/api/test_session.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,179 @@ int test_wolfSSL_ticket_keys(void)
return EXPECT_RESULT();
}

#if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && \
!defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) && !defined(SINGLE_THREADED)

#define TEST_TICKET_KEYS_ROUNDS 20000
#define TEST_TICKET_KEYS_WRITER_MAX 1000000
#define TEST_TICKET_KEYS_BURST 256
#define TEST_TICKET_KEYS_WAIT_TRIES 200
#define TEST_TICKET_KEYS_WAIT_MS 1

static WOLFSSL_CTX* ticket_keys_ctx = NULL;
static byte ticket_keys_a[WOLFSSL_TICKET_KEYS_SZ];
static byte ticket_keys_b[WOLFSSL_TICKET_KEYS_SZ];
static int ticket_keys_set_err = 0;

/* Guards the fields below, which both threads touch while running. */
static wolfSSL_Mutex ticket_keys_lock;
static int ticket_keys_writer_started = 0;
static int ticket_keys_reader_done = 0;

static THREAD_RETURN WOLFSSL_THREAD test_ticket_keys_writer(void* args)
{
int done = 0;
int i;
long rounds = 0;

(void)args;

if (wc_LockMutex(&ticket_keys_lock) == 0) {
ticket_keys_writer_started = 1;
wc_UnLockMutex(&ticket_keys_lock);
}

/* Keep rotating until the reader is done so the loops overlap. The
* rotations run in bursts to keep the bookkeeping out of the loop. */
while ((!done) && (rounds < TEST_TICKET_KEYS_WRITER_MAX)) {
for (i = 0; i < TEST_TICKET_KEYS_BURST; i++) {
if (wolfSSL_CTX_set_tlsext_ticket_keys(ticket_keys_ctx,
Comment thread
yosuke-wolfssl marked this conversation as resolved.
ticket_keys_a, WOLFSSL_TICKET_KEYS_SZ)
!= WOLFSSL_SUCCESS) {
ticket_keys_set_err = 1;
}
if (wolfSSL_CTX_set_tlsext_ticket_keys(ticket_keys_ctx,
ticket_keys_b, WOLFSSL_TICKET_KEYS_SZ)
!= WOLFSSL_SUCCESS) {
ticket_keys_set_err = 1;
}
}
rounds += TEST_TICKET_KEYS_BURST;

if (wc_LockMutex(&ticket_keys_lock) == 0) {
done = ticket_keys_reader_done;
wc_UnLockMutex(&ticket_keys_lock);
}
}

WOLFSSL_RETURN_FROM_THREAD(0);
}

/* Read the keys once and record which key set came back. */
static int test_ticket_keys_read(int* sawA, int* sawB, int* mixed)
{
byte keys[WOLFSSL_TICKET_KEYS_SZ];
int ret = 0;

if (wolfSSL_CTX_get_tlsext_ticket_keys(ticket_keys_ctx, keys,
WOLFSSL_TICKET_KEYS_SZ) != WOLFSSL_SUCCESS) {
ret = -1;
}
else if (XMEMCMP(keys, ticket_keys_a, sizeof(keys)) == 0) {
*sawA = 1;
}
else if (XMEMCMP(keys, ticket_keys_b, sizeof(keys)) == 0) {
*sawB = 1;
}
/* Every read returns one key set whole, never part of each. */
else {
*mixed = 1;
}

return ret;
}
#endif

int test_wolfSSL_ticket_keys_threaded(void)
{
EXPECT_DECLS;
#if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && \
!defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) && !defined(SINGLE_THREADED)
THREAD_TYPE thread;
func_args args;
int lockInit = 0;
int started = 0;
int sawA = 0;
int sawB = 0;
int getErr = 0;
int mixed = 0;
int i;

XMEMSET(&args, 0, sizeof(args));
XMEMSET(ticket_keys_a, 0xa5, sizeof(ticket_keys_a));
XMEMSET(ticket_keys_b, 0x5c, sizeof(ticket_keys_b));
ticket_keys_writer_started = 0;
ticket_keys_reader_done = 0;
ticket_keys_set_err = 0;

if (wc_InitMutex(&ticket_keys_lock) == 0) {
lockInit = 1;
}
ExpectIntEQ(lockInit, 1);
ExpectNotNull(ticket_keys_ctx =
wolfSSL_CTX_new(wolfSSLv23_server_method()));
ExpectIntEQ(wolfSSL_CTX_set_tlsext_ticket_keys(ticket_keys_ctx,
Comment thread
yosuke-wolfssl marked this conversation as resolved.
Comment thread
yosuke-wolfssl marked this conversation as resolved.
ticket_keys_a, WOLFSSL_TICKET_KEYS_SZ), WOLFSSL_SUCCESS);

if ((lockInit == 1) && (ticket_keys_ctx != NULL)) {
start_thread(test_ticket_keys_writer, &args, &thread);
Comment thread
yosuke-wolfssl marked this conversation as resolved.

/* Start the reads only once the writer is rotating keys. Sleep
* rather than spin so the writer gets scheduled on one core. */
for (i = 0; (i < TEST_TICKET_KEYS_WAIT_TRIES) && (!started); i++) {
if (wc_LockMutex(&ticket_keys_lock) == 0) {
started = ticket_keys_writer_started;
wc_UnLockMutex(&ticket_keys_lock);
}
if (!started) {
XSLEEP_MS(TEST_TICKET_KEYS_WAIT_MS);
}
}

for (i = 0; i < TEST_TICKET_KEYS_ROUNDS; i++) {
if (test_ticket_keys_read(&sawA, &sawB, &mixed) != 0) {
getErr = 1;
break;
}
if (mixed) {
break;
}
}

/* Top up until both key sets have been seen, so a clean run
* above cannot be one where the loops never overlapped. */
for (i = 0; (i < TEST_TICKET_KEYS_WAIT_TRIES) && (!getErr) &&
(!mixed) && ((!sawA) || (!sawB)); i++) {
XSLEEP_MS(TEST_TICKET_KEYS_WAIT_MS);
if (test_ticket_keys_read(&sawA, &sawB, &mixed) != 0) {
getErr = 1;
}
}

if (wc_LockMutex(&ticket_keys_lock) == 0) {
ticket_keys_reader_done = 1;
wc_UnLockMutex(&ticket_keys_lock);
}
join_thread(thread);
}

ExpectIntEQ(getErr, 0);
ExpectIntEQ(mixed, 0);
ExpectIntEQ(ticket_keys_set_err, 0);

ExpectIntEQ(started, 1);
ExpectIntEQ(sawA, 1);
ExpectIntEQ(sawB, 1);

wolfSSL_CTX_free(ticket_keys_ctx);
ticket_keys_ctx = NULL;
if (lockInit == 1) {
wc_FreeMutex(&ticket_keys_lock);
}
#endif
return EXPECT_RESULT();
}

/*----------------------------------------------------------------------------*/
/* SESSION ex_data new index */
/*----------------------------------------------------------------------------*/
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_session.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ int test_wolfSSL_SESSION(void);
int test_wolfSSL_SESSION_expire_downgrade(void);
int test_wolfSSL_CTX_sess_set_remove_cb(void);
int test_wolfSSL_ticket_keys(void);
int test_wolfSSL_ticket_keys_threaded(void);
int test_wolfSSL_SESSION_get_ex_new_index(void);
int test_wolfSSL_GetSessionAtIndex(void);

Expand All @@ -50,6 +51,7 @@ int test_wolfSSL_GetSessionAtIndex(void);
TEST_DECL_GROUP("session", test_wolfSSL_SESSION_expire_downgrade), \
TEST_DECL_GROUP("session", test_wolfSSL_CTX_sess_set_remove_cb), \
TEST_DECL_GROUP("session", test_wolfSSL_ticket_keys), \
TEST_DECL_GROUP("session", test_wolfSSL_ticket_keys_threaded), \
TEST_DECL_GROUP("session", test_wolfSSL_SESSION_get_ex_new_index), \
TEST_DECL_GROUP("session", test_wolfSSL_GetSessionAtIndex)

Expand Down
2 changes: 1 addition & 1 deletion wolfssl/ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6163,7 +6163,7 @@ WOLFSSL_API int PEM_write_bio_WOLFSSL_X509(WOLFSSL_BIO *bio,
OPENSSL_EXTRA || HAVE_LIGHTY */

#if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && \
!defined(NO_WOLFSSL_SERVER)
!defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
WOLFSSL_API long wolfSSL_CTX_get_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
unsigned char *keys, int keylen);
WOLFSSL_API long wolfSSL_CTX_set_tlsext_ticket_keys(WOLFSSL_CTX *ctx,
Expand Down
Loading