Fully clear ECC k when calling wc_ecc_sign_hash_ex with SP math. Avoid aliasing the ticket in wolfSSL_GetSessionAtIndex. Fix IAR warnings.

doc/dox_comments/header_files/ssl.h

@@ -2828,24 +2828,30 @@ int wolfSSL_GetSessionIndex(WOLFSSL* ssl);
 
     \brief This function gets the session at specified index of the session
     cache and copies it into memory. The WOLFSSL_SESSION structure holds
-    the session information.
+    the session information. The copy is independent of the cache entry: it
+    does not share the ticket buffer, peer certificate or ex_data with the
+    cache, and it stays valid after the cache entry is overwritten or evicted.
+    The caller owns the copy and releases it with wolfSSL_SESSION_free().
 
     \return SSL_SUCCESS returned if the function executed successfully and
     no errors were thrown.
     \return BAD_MUTEX_E returned if there was an unlock or lock mutex error.
     \return SSL_FAILURE returned if the function did not execute successfully.
 
     \param index an int type representing the session index.
-    \param session a pointer to the WOLFSSL_SESSION structure.
+    \param session a pointer to a WOLFSSL_SESSION structure to copy into,
+    obtained from wolfSSL_SESSION_new().
 
     _Example_
     \code
     int idx; // The index to locate the session.
-    WOLFSSL_SESSION* session;  // Buffer to copy to.
+    WOLFSSL_SESSION* session = wolfSSL_SESSION_new();  // Buffer to copy to.
     ...
     if(wolfSSL_GetSessionAtIndex(idx, session) != SSL_SUCCESS){
     	// Failure case.
     }
+    ...
+    wolfSSL_SESSION_free(session);
     \endcode
 
     \sa UnLockMutex

src/ssl_certman.c

@@ -23,6 +23,10 @@
 
 #include <wolfssl/internal.h>
 
+#if defined(WOLFSSL_RENESAS_TSIP_TLS) || defined(WOLFSSL_RENESAS_FSPSM_TLS)
+#include <wolfssl/wolfcrypt/port/Renesas/renesas_cmn.h>
+#endif
+
 #if !defined(WOLFSSL_SSL_CERTMAN_INCLUDED)
     #ifndef WOLFSSL_IGNORE_FILE_WARN
         #warning ssl_certman.c not to be compiled separately from ssl.c

src/ssl_sess.c

@@ -2271,8 +2271,18 @@ int wolfSSL_GetSessionAtIndex(int idx, WOLFSSL_SESSION* session)
     cacheSession = &sessRow->Sessions[col];
 #endif
     if (cacheSession) {
-        XMEMCPY(session, cacheSession, sizeof(WOLFSSL_SESSION));
-        result = WOLFSSL_SUCCESS;
+#ifdef HAVE_EX_DATA
+        /* The copy keeps its own ex_data. The struct copy below carries over
+         * the cache's pointers, which the cache frees when the entry goes. */
+        WOLFSSL_CRYPTO_EX_DATA exData;
+        XMEMCPY(&exData, &session->ex_data, sizeof(exData));
+#endif
+        /* Must not alias the ticket, peer cert and ex_data the cache owns and
+         * frees on overwrite or eviction. */
+        result = wolfSSL_DupSession(cacheSession, session, 0);
+#ifdef HAVE_EX_DATA
+        XMEMCPY(&session->ex_data, &exData, sizeof(exData));
+#endif
     }
     else {
         result = WOLFSSL_FAILURE;

src/tls13.c

@@ -5393,6 +5393,11 @@ typedef struct Dsh13Args {
 #endif
 } Dsh13Args;
 
+/* sessIdSz below bounds both the copy into arrays->sessionID and the comparison
+ * against arrays->clientRandom, so one check only covers both while these
+ * match. */
+wc_static_assert(ID_LEN == RAN_LEN);
+
 int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
                        word32 helloSz, byte* extMsgType)
 {
@@ -5554,7 +5559,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
 
     /* Session id */
     args->sessIdSz = input[args->idx++];
-    if (args->sessIdSz > ID_LEN || args->sessIdSz > RAN_LEN ||
+    if (args->sessIdSz > ID_LEN ||
         ((args->idx - args->begin) + args->sessIdSz > helloSz))
         return BUFFER_ERROR;
     args->sessId = input + args->idx;

tests/api/test_session.c

@@ -1614,3 +1614,128 @@ int test_wolfSSL_SESSION_get_ex_new_index(void)
     return TEST_SKIPPED;
 }
 #endif
+
+/*----------------------------------------------------------------------------*/
+/* wolfSSL_GetSessionAtIndex                                                  */
+/*----------------------------------------------------------------------------*/
+
+#if defined(SESSION_INDEX) && defined(HAVE_SESSION_TICKET) && \
+    !defined(NO_SESSION_CACHE) && !defined(NO_WOLFSSL_CLIENT) && \
+    !defined(NO_TLS)
+
+/* Cache a client session under id with a ticLen byte ticket of fill bytes.
+ * ticLen over SESSION_TICKET_LEN makes the cache allocate a ticket buffer. */
+static int test_session_at_index_add(WOLFSSL_CTX* ctx, const byte* id,
+    word16 ticLen, byte fill, int* idx)
+{
+    EXPECT_DECLS;
+    WOLFSSL_SESSION* sess = NULL;
+    byte* tic = NULL;
+
+    ExpectNotNull(tic = (byte*)XMALLOC(ticLen, NULL, DYNAMIC_TYPE_TMP_BUFFER));
+    ExpectNotNull(sess = wolfSSL_SESSION_new());
+    if (EXPECT_SUCCESS()) {
+        XMEMSET(tic, fill, ticLen);
+        XMEMCPY(sess->sessionID, id, ID_LEN);
+        sess->sessionIDSz = ID_LEN;
+        sess->side = WOLFSSL_CLIENT_END;
+        sess->isSetup = 1;
+        /* Borrowed buffer - ticketLenAlloc stays 0 so sess does not free it. */
+        sess->ticket = tic;
+        sess->ticketLen = ticLen;
+    }
+    ExpectIntEQ(AddSessionToCache(ctx, sess, id, ID_LEN, idx,
+        WOLFSSL_CLIENT_END, 1, NULL), 0);
+
+    if (sess != NULL) {
+        sess->ticket = sess->staticTicket;
+        sess->ticketLen = 0;
+        wolfSSL_SESSION_free(sess);
+    }
+    XFREE(tic, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+
+    return EXPECT_RESULT();
+}
+
+static int test_session_at_index_ticket_is(const WOLFSSL_SESSION* sess,
+    word16 ticLen, byte fill)
+{
+    word16 i;
+
+    if ((sess == NULL) || (sess->ticket == NULL) || (sess->ticketLen != ticLen))
+        return 0;
+    for (i = 0; i < ticLen; i++) {
+        if (sess->ticket[i] != fill)
+            return 0;
+    }
+    return 1;
+}
+
+int test_wolfSSL_GetSessionAtIndex(void)
+{
+    EXPECT_DECLS;
+    WOLFSSL_CTX* ctx = NULL;
+    WOLFSSL_SESSION* copy = NULL;
+    WOLFSSL_SESSION* copy2 = NULL;
+    byte id[ID_LEN];
+    word16 ticLen = (word16)(SESSION_TICKET_LEN + 128);
+    int idx = -1;
+
+    XMEMSET(id, 0x5A, sizeof(id));
+    ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()));
+    ExpectIntEQ(test_session_at_index_add(ctx, id, ticLen, 0xA1, &idx),
+        TEST_SUCCESS);
+    ExpectIntGE(idx, 0);
+
+    ExpectNotNull(copy = wolfSSL_SESSION_new());
+#ifdef HAVE_EX_DATA
+    if (copy != NULL) {
+        copy->ex_data.ex_data[0] = (void*)copy;
+    }
+#endif
+    ExpectIntEQ(wolfSSL_GetSessionAtIndex(idx, copy), WOLFSSL_SUCCESS);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy, ticLen, 0xA1), 1);
+#ifdef HAVE_EX_DATA
+    /* The copy keeps its own ex_data and the right to free it. */
+    if (copy != NULL) {
+        ExpectPtrEq(copy->ex_data.ex_data[0], (void*)copy);
+        ExpectIntEQ(copy->ownExData, 1);
+    }
+#endif
+
+    /* Each copy owns its ticket rather than pointing into the cache. */
+    ExpectNotNull(copy2 = wolfSSL_SESSION_new());
+    ExpectIntEQ(wolfSSL_GetSessionAtIndex(idx, copy2), WOLFSSL_SUCCESS);
+    if ((copy != NULL) && (copy2 != NULL)) {
+        ExpectPtrNE(copy->ticket, copy2->ticket);
+    }
+
+    /* Same length overwrite reuses the cache buffer - copies keep their
+     * bytes. */
+    ExpectIntEQ(test_session_at_index_add(ctx, id, ticLen, 0xB2, NULL),
+        TEST_SUCCESS);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy, ticLen, 0xA1), 1);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy2, ticLen, 0xA1), 1);
+
+    /* Longer ticket makes the cache free its buffer, and releasing a copy must
+     * not free a buffer the cache still uses. */
+    wolfSSL_SESSION_free(copy2);
+    copy2 = NULL;
+    ExpectIntEQ(test_session_at_index_add(ctx, id, (word16)(ticLen + 512),
+        0xC3, NULL), TEST_SUCCESS);
+    ExpectIntEQ(test_session_at_index_ticket_is(copy, ticLen, 0xA1), 1);
+
+    wolfSSL_SESSION_free(copy);
+    wolfSSL_CTX_free(ctx);
+    return EXPECT_RESULT();
+}
+
+#else
+
+int test_wolfSSL_GetSessionAtIndex(void)
+{
+    return TEST_SKIPPED;
+}
+
+#endif /* SESSION_INDEX && HAVE_SESSION_TICKET && !NO_SESSION_CACHE &&
+        * !NO_WOLFSSL_CLIENT && !NO_TLS */

tests/api/test_session.h

@@ -36,6 +36,7 @@ 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_SESSION_get_ex_new_index(void);
+int test_wolfSSL_GetSessionAtIndex(void);
 
 #define TEST_SESSION_DECLS                                                     \
     TEST_DECL_GROUP("session", test_wolfSSL_CTX_add_session),                  \
@@ -49,6 +50,7 @@ int test_wolfSSL_SESSION_get_ex_new_index(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_SESSION_get_ex_new_index)
+    TEST_DECL_GROUP("session", test_wolfSSL_SESSION_get_ex_new_index),         \
+    TEST_DECL_GROUP("session", test_wolfSSL_GetSessionAtIndex)
 
 #endif /* WOLFCRYPT_TEST_SESSION_H */

wolfcrypt/src/ecc.c

@@ -7473,6 +7473,29 @@ static int ecc_sign_hash_sw(ecc_key* key, ecc_key* pubkey, WC_RNG* rng,
 #endif
 
 #ifdef WOLFSSL_HAVE_SP_ECC
+#if defined(WOLFSSL_ECDSA_SET_K) || defined(WOLFSSL_ECDSA_SET_K_ONE_LOOP) || \
+    defined(WOLFSSL_ECDSA_DETERMINISTIC_K) || \
+    defined(WOLFSSL_ECDSA_DETERMINISTIC_K_VARIANT)
+/* SP only resets the logical length of k, leaving its digits in the backing
+ * store. Clear it the way the software path does. */
+static void ecc_sign_k_forcezero(ecc_key* key)
+{
+#ifndef WOLFSSL_NO_MALLOC
+    if (key->sign_k != NULL) {
+        mp_forcezero(key->sign_k);
+        mp_free(key->sign_k);
+        XFREE(key->sign_k, key->heap, DYNAMIC_TYPE_ECC);
+        key->sign_k = NULL;
+    }
+#else
+    if (key->sign_k_set) {
+        mp_forcezero(key->sign_k);
+        key->sign_k_set = 0;
+    }
+#endif
+}
+#endif
+
 static int ecc_sign_hash_sp(const byte* in, word32 inlen, WC_RNG* rng,
     ecc_key* key, mp_int *r, mp_int *s)
 {
@@ -7702,7 +7725,20 @@ int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng,
 
 #if defined(WOLFSSL_HAVE_SP_ECC)
    err = ecc_sign_hash_sp(in, inlen, rng, key, r, s);
+   /* WC_KEY_SIZE_E only means SP left this curve to the software path below,
+    * which needs k and clears it itself. Every other result consumed k. */
    if (err != WC_NO_ERR_TRACE(WC_KEY_SIZE_E)) {
+   #if defined(WOLFSSL_ECDSA_SET_K) || defined(WOLFSSL_ECDSA_SET_K_ONE_LOOP) || \
+       defined(WOLFSSL_ECDSA_DETERMINISTIC_K) || \
+       defined(WOLFSSL_ECDSA_DETERMINISTIC_K_VARIANT)
+       #ifdef WC_ECC_NONBLOCK
+       /* An incomplete operation still needs k. */
+       if (err != FP_WOULDBLOCK)
+       #endif
+       {
+           ecc_sign_k_forcezero(key);
+       }
+   #endif
        return err;
    }
 #else