lib: Make crypt contexts thread-safe

Moves the per-packet EVP context to a thread-local state. Now refuses
a security config with -ENOTSUP without supported crypto backend
(OpenSSL).

Signed-off-by: Dimitri Staessens <dimitri@ouroboros.rocks>
Signed-off-by: Sander Vrijders <sander@ouroboros.rocks>

4 files changed, 292 insertions, 81 deletions

diff --git a/src/lib/crypt.c b/src/lib/crypt.c
index cbbb9bc2..bc3dcd16 100644
--- a/src/lib/crypt.c
+++ b/src/lib/crypt.c
@@ -252,6 +252,23 @@ int parse_sec_config(struct sec_config * cfg,
         return 0;
 }
 
+/*
+ * Not in header, but non-static for unit testing. Without a crypto
+ * backend a present security config is refused.
+ */
+int crypt_load_sec_config(struct sec_config * cfg,
+                          FILE *              fp)
+{
+        assert(cfg != NULL);
+        assert(fp != NULL);
+
+#ifndef HAVE_OPENSSL
+        return -ENOTSUP;
+#endif
+
+        return parse_sec_config(cfg, fp);
+}
+
 /* Parse key exchange config from file */
 int load_sec_config_file(struct sec_config * cfg,
                          const char *        path)
@@ -273,7 +290,9 @@ int load_sec_config_file(struct sec_config * cfg,
         }
 
         pthread_cleanup_push(__cleanup_fclose, fp);
-        ret = parse_sec_config(cfg, fp);
+
+        ret = crypt_load_sec_config(cfg, fp);
+
         pthread_cleanup_pop(0);
 
         fclose(fp);
@@ -830,6 +849,11 @@ int crypt_decrypt(struct crypt_ctx * ctx,
 
 struct crypt_ctx * crypt_create_ctx(struct crypt_sk * sk)
 {
+#ifndef HAVE_OPENSSL
+        (void) sk;
+
+        return NULL; /* nothing to seal with */
+#else
         struct crypt_ctx * crypt;
 
         if (crypt_validate_nid(sk->nid) != 0)
@@ -845,21 +869,19 @@ struct crypt_ctx * crypt_create_ctx(struct crypt_sk * sk)
         if (crypt->kr == NULL)
                 goto fail_kr;
 
-#ifdef HAVE_OPENSSL
         crypt->cipher = openssl_crypt_create_ctx(sk);
         if (crypt->cipher == NULL)
                 goto fail_cipher;
-#endif
+
         return crypt;
 
-#ifdef HAVE_OPENSSL
  fail_cipher:
         keyrot_destroy(crypt->kr);
-#endif
  fail_kr:
         free(crypt);
  fail_crypt:
         return NULL;
+#endif
 }
 
 void crypt_destroy_ctx(struct crypt_ctx * crypt)
@@ -947,7 +969,7 @@ int crypt_load_privkey_file(const char * path,
 #else
         (void) path;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
@@ -961,7 +983,7 @@ int crypt_load_privkey_str(const char * str,
 #else
         (void) str;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
@@ -975,7 +997,7 @@ int crypt_load_pubkey_str(const char * str,
 #else
         (void) str;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
@@ -989,7 +1011,7 @@ int crypt_load_pubkey_file(const char * path,
 #else
         (void) path;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
@@ -1194,7 +1216,7 @@ int crypt_check_crt_name(void *       crt,
         (void) crt;
         (void) name;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
@@ -1292,7 +1314,7 @@ int auth_verify_crt(struct auth_ctx * ctx,
         (void) ctx;
         (void) crt;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
@@ -1307,7 +1329,7 @@ int auth_verify_crt_pin(struct auth_ctx * ctx,
         (void) crt;
         (void) pin;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
@@ -1354,7 +1376,7 @@ int auth_verify_sig(void *   pk,
         (void) msg;
         (void) sig;
 
-        return 0;
+        return -ENOTSUP;
 #endif
 }
 
diff --git a/src/lib/crypt/openssl.c b/src/lib/crypt/openssl.c
index d5d9adf5..7716f27f 100644
--- a/src/lib/crypt/openssl.c
+++ b/src/lib/crypt/openssl.c
@@ -37,6 +37,7 @@
 
 #include <openssl/evp.h>
 #include <openssl/bio.h>
+#include <openssl/crypto.h>
 #include <openssl/ec.h>
 #include <openssl/err.h>
 #include <openssl/kdf.h>
@@ -58,12 +59,39 @@
 #define AEAD_NONCE_LEN     12 /* 96-bit deterministic IV (SP 800-38D) */
 #define AEAD_TAG_LEN       16 /* 128-bit AEAD authentication tag */
 
+/*
+ * Holds only the cipher identity, which is fixed at creation. A flow's
+ * context is sealed and opened by several threads at once, so nothing
+ * here may carry per-packet state.
+ */
 struct ossl_crypt_ctx {
-        EVP_CIPHER_CTX *   evp_ctx;
         const EVP_CIPHER * cipher;
         int                tagsz;
 };
 
+/*
+ * Per-thread AEAD context. A key covers 2^KEY_LEAF_BITS packets, so
+ * the key schedule is worth keeping between them; only the nonce
+ * changes. Thread-local, so concurrent sealers share nothing, and a
+ * miss costs no more than a full install.
+ */
+struct ossl_aead {
+        EVP_CIPHER_CTX *   evp;
+        const EVP_CIPHER * cipher; /* NULL when the state is unusable */
+        uint8_t            key[SYMMKEYSZ];
+        size_t             keylen;
+};
+
+struct ossl_aead_tls {
+        struct ossl_aead seal;
+        struct ossl_aead open;
+};
+
+static struct {
+        pthread_key_t  key;
+        pthread_once_t once;
+} aead_g = { 0, PTHREAD_ONCE_INIT };
+
 struct kdf_info {
         buffer_t secret;
         int      nid;
@@ -274,6 +302,117 @@ int openssl_hkdf_expand(buffer_t key,
         return ret;
 }
 
+static void aead_tls_free(void * p)
+{
+        struct ossl_aead_tls * t = p;
+        if (t == NULL)
+                return;
+
+        EVP_CIPHER_CTX_free(t->seal.evp);
+        EVP_CIPHER_CTX_free(t->open.evp);
+
+        crypt_secure_clear(t->seal.key, SYMMKEYSZ);
+        crypt_secure_clear(t->open.key, SYMMKEYSZ);
+
+        free(t);
+}
+
+static void aead_tls_init(void)
+{
+        pthread_key_create(&aead_g.key, aead_tls_free);
+}
+
+static struct ossl_aead_tls * aead_tls_get(void)
+{
+        struct ossl_aead_tls * t;
+
+        pthread_once(&aead_g.once, aead_tls_init);
+
+        t = pthread_getspecific(aead_g.key);
+        if (t != NULL)
+                return t;
+
+        t = malloc(sizeof(*t));
+        if (t == NULL)
+                return NULL;
+
+        memset(t, 0, sizeof(*t));
+
+        if (pthread_setspecific(aead_g.key, t) != 0) {
+                free(t);
+                return NULL;
+        }
+
+        return t;
+}
+
+/* Install cipher and key; the nonce is set per packet by the caller. */
+static int aead_install(EVP_CIPHER_CTX *   evp,
+                        const EVP_CIPHER * cipher,
+                        const uint8_t *    key,
+                        bool               enc)
+{
+        EVP_CIPHER_CTX_reset(evp);
+
+        if (enc) {
+                if (EVP_EncryptInit_ex(evp, cipher, NULL, NULL, NULL) != 1)
+                        return -1;
+        } else {
+                if (EVP_DecryptInit_ex(evp, cipher, NULL, NULL, NULL) != 1)
+                        return -1;
+        }
+
+        /* Pin the AEAD nonce to 96 bits (SP 800-38D deterministic IV). */
+        if (EVP_CIPHER_CTX_ctrl(evp, EVP_CTRL_AEAD_SET_IVLEN,
+                                AEAD_NONCE_LEN, NULL) != 1)
+                return -1;
+
+        if (enc) {
+                if (EVP_EncryptInit_ex(evp, NULL, NULL, key, NULL) != 1)
+                        return -1;
+        } else {
+                if (EVP_DecryptInit_ex(evp, NULL, NULL, key, NULL) != 1)
+                        return -1;
+        }
+
+        return 0;
+}
+
+/* This thread's context for cipher/key, ready to take a nonce. */
+static EVP_CIPHER_CTX * aead_ctx(struct ossl_aead * a,
+                                 const EVP_CIPHER * cipher,
+                                 const uint8_t *    key,
+                                 bool               enc)
+{
+        int keylen;
+
+        keylen = EVP_CIPHER_get_key_length(cipher);
+        if (keylen <= 0 || (size_t) keylen > SYMMKEYSZ)
+                return NULL;
+
+        /* Compare the bytes: a cache slot can be reused for a new key. */
+        if (a->cipher == cipher && a->keylen == (size_t) keylen
+            && CRYPTO_memcmp(a->key, key, a->keylen) == 0)
+                return a->evp;
+
+        if (a->evp == NULL) {
+                a->evp = EVP_CIPHER_CTX_new();
+                if (a->evp == NULL)
+                        return NULL;
+        }
+
+        a->cipher = NULL;
+        if (aead_install(a->evp, cipher, key, enc) < 0)
+                return NULL;
+
+        memcpy(a->key, key, (size_t) keylen);
+
+        a->keylen = (size_t) keylen;
+        a->cipher = cipher;
+
+        return a->evp;
+}
+
 /* AEAD seal: encrypt in with key/nonce, bind aad, append tag */
 int openssl_seal(struct ossl_crypt_ctx * ctx,
                  const uint8_t *         key,
@@ -283,45 +422,45 @@ int openssl_seal(struct ossl_crypt_ctx * ctx,
                  uint8_t *               out,
                  uint8_t *               tag)
 {
-        int out_sz;
-        int tmp_sz;
+        struct ossl_aead_tls * tls;
+        EVP_CIPHER_CTX *       evp;
+        int                    out_sz;
+        int                    tmp_sz;
 
         assert(ctx != NULL);
         assert(ctx->tagsz > 0); /* AEAD mandated at ctx creation */
 
-        EVP_CIPHER_CTX_reset(ctx->evp_ctx);
+        tls = aead_tls_get();
+        if (tls == NULL)
+                goto fail;
 
-        if (EVP_EncryptInit_ex(ctx->evp_ctx, ctx->cipher, NULL,
-                               NULL, NULL) != 1)
-                return -1;
-
-        /* Pin the AEAD nonce to 96 bits (SP 800-38D deterministic IV). */
-        if (EVP_CIPHER_CTX_ctrl(ctx->evp_ctx, EVP_CTRL_AEAD_SET_IVLEN,
-                                AEAD_NONCE_LEN, NULL) != 1)
-                return -1;
+        evp = aead_ctx(&tls->seal, ctx->cipher, key, true);
+        if (evp == NULL)
+                goto fail;
 
-        if (EVP_EncryptInit_ex(ctx->evp_ctx, NULL, NULL,
-                               key, nonce) != 1)
-                return -1;
+        if (EVP_EncryptInit_ex(evp, NULL, NULL, NULL, nonce) != 1)
+                goto fail_evp;
 
-        if (EVP_EncryptUpdate(ctx->evp_ctx, NULL, &tmp_sz,
-                              aad.data, (int) aad.len) != 1)
-                return -1;
+        if (EVP_EncryptUpdate(evp, NULL, &tmp_sz, aad.data, (int) aad.len) != 1)
+                goto fail_evp;
 
-        if (EVP_EncryptUpdate(ctx->evp_ctx, out, &out_sz,
-                              in.data, (int) in.len) != 1)
-                return -1;
+        if (EVP_EncryptUpdate(evp, out, &out_sz, in.data, (int) in.len) != 1)
+                goto fail_evp;
 
-        if (EVP_EncryptFinal_ex(ctx->evp_ctx, out + out_sz, &tmp_sz) != 1)
-                return -1;
+        if (EVP_EncryptFinal_ex(evp, out + out_sz, &tmp_sz) != 1)
+                goto fail_evp;
 
         out_sz += tmp_sz;
 
-        if (EVP_CIPHER_CTX_ctrl(ctx->evp_ctx, EVP_CTRL_AEAD_GET_TAG,
+        if (EVP_CIPHER_CTX_ctrl(evp, EVP_CTRL_AEAD_GET_TAG,
                                 ctx->tagsz, tag) != 1)
-                return -1;
+                goto fail_evp;
 
         return out_sz;
+ fail_evp:
+        tls->seal.cipher = NULL; /* state unknown; install afresh */
+ fail:
+        return -1;
 }
 
 /* AEAD open: decrypt in with key/nonce, verify aad and tag */
@@ -333,47 +472,50 @@ int openssl_open(struct ossl_crypt_ctx * ctx,
                  const uint8_t *         tag,
                  buffer_t *              out)
 {
-        int out_sz;
-        int tmp_sz;
+        struct ossl_aead_tls * tls;
+        EVP_CIPHER_CTX *       evp;
+        int                    out_sz;
+        int                    tmp_sz;
 
         assert(ctx != NULL);
         assert(ctx->tagsz > 0); /* AEAD mandated at ctx creation */
 
-        EVP_CIPHER_CTX_reset(ctx->evp_ctx);
+        tls = aead_tls_get();
+        if (tls == NULL)
+                goto fail;
 
-        if (EVP_DecryptInit_ex(ctx->evp_ctx, ctx->cipher, NULL,
-                               NULL, NULL) != 1)
-                return -1;
-
-        /* Pin the AEAD nonce to 96 bits (SP 800-38D deterministic IV). */
-        if (EVP_CIPHER_CTX_ctrl(ctx->evp_ctx, EVP_CTRL_AEAD_SET_IVLEN,
-                                AEAD_NONCE_LEN, NULL) != 1)
-                return -1;
+        evp = aead_ctx(&tls->open, ctx->cipher, key, false);
+        if (evp == NULL)
+                goto fail;
 
-        if (EVP_DecryptInit_ex(ctx->evp_ctx, NULL, NULL, key, nonce) != 1)
-                return -1;
+        if (EVP_DecryptInit_ex(evp, NULL, NULL, NULL, nonce) != 1)
+                goto fail_evp;
 
-        if (EVP_CIPHER_CTX_ctrl(ctx->evp_ctx, EVP_CTRL_AEAD_SET_TAG,
+        if (EVP_CIPHER_CTX_ctrl(evp, EVP_CTRL_AEAD_SET_TAG,
                                 ctx->tagsz, (void *) tag) != 1)
-                return -1;
+                goto fail_evp;
 
-        if (EVP_DecryptUpdate(ctx->evp_ctx, NULL, &tmp_sz,
-                              aad.data, (int) aad.len) != 1)
-                return -1;
+        if (EVP_DecryptUpdate(evp, NULL, &tmp_sz, aad.data, (int) aad.len) != 1)
+                goto fail_evp;
 
-        if (EVP_DecryptUpdate(ctx->evp_ctx, out->data, &out_sz,
+        if (EVP_DecryptUpdate(evp, out->data, &out_sz,
                               in.data, (int) in.len) != 1)
-                return -1;
+                goto fail_evp;
 
-        if (EVP_DecryptFinal_ex(ctx->evp_ctx, out->data + out_sz,
-                                &tmp_sz) != 1)
-                return -1;
+        /* A failed verify leaves defined state; keep the key cached. */
+        if (EVP_DecryptFinal_ex(evp, out->data + out_sz, &tmp_sz) != 1)
+                goto fail_verify;
 
         out_sz += tmp_sz;
 
         out->len = (size_t) out_sz;
 
         return out_sz;
+ fail_evp:
+        tls->open.cipher = NULL; /* state unknown; install afresh */
+ fail_verify:
+ fail:
+        return -1;
 }
 
 /*
@@ -994,10 +1136,6 @@ static int ossl_cipher_ctx_init(struct ossl_crypt_ctx * ctx,
 
         ctx->tagsz = AEAD_TAG_LEN;
 
-        ctx->evp_ctx = EVP_CIPHER_CTX_new();
-        if (ctx->evp_ctx == NULL)
-                return -1;
-
         return 0;
 }
 
@@ -1023,7 +1161,7 @@ int openssl_oneshot_seal(int             nid,
 
         out->data = malloc(in.len + EVP_MAX_BLOCK_LENGTH + ctx.tagsz);
         if (out->data == NULL)
-                goto fail_ctx;
+                goto fail_cipher;
 
         out_sz = openssl_seal(&ctx, key, nonce, aad, in,
                               out->data, out->data + in.len);
@@ -1032,14 +1170,10 @@ int openssl_oneshot_seal(int             nid,
 
         out->len = (size_t) out_sz + ctx.tagsz;
 
-        EVP_CIPHER_CTX_free(ctx.evp_ctx);
-
         return 0;
 
  fail_seal:
         free(out->data);
- fail_ctx:
-        EVP_CIPHER_CTX_free(ctx.evp_ctx);
  fail_cipher:
         clrbuf(*out);
         return -ECRYPT;
@@ -1068,13 +1202,13 @@ int openssl_oneshot_open(int             nid,
                 goto fail_cipher;
 
         if (in.len < (size_t) ctx.tagsz)
-                goto fail_ctx;
+                goto fail_cipher;
 
         in_sz = (int) in.len - ctx.tagsz;
 
         out->data = malloc((size_t) in_sz + EVP_MAX_BLOCK_LENGTH);
         if (out->data == NULL)
-                goto fail_ctx;
+                goto fail_cipher;
 
         ct.data = in.data;
         ct.len  = (size_t) in_sz;
@@ -1083,14 +1217,10 @@ int openssl_oneshot_open(int             nid,
         if (openssl_open(&ctx, key, nonce, aad, ct, tag, out) < 0)
                 goto fail_open;
 
-        EVP_CIPHER_CTX_free(ctx.evp_ctx);
-
         return 0;
 
  fail_open:
         free(out->data);
- fail_ctx:
-        EVP_CIPHER_CTX_free(ctx.evp_ctx);
  fail_cipher:
         clrbuf(*out);
         return -ECRYPT;
@@ -1125,7 +1255,6 @@ void openssl_crypt_destroy_ctx(struct ossl_crypt_ctx * ctx)
         if (ctx == NULL)
                 return;
 
-        EVP_CIPHER_CTX_free(ctx->evp_ctx);
         free(ctx);
 }
 
diff --git a/src/lib/tests/crypt_test.c b/src/lib/tests/crypt_test.c
index 50b7268a..88c9634a 100644
--- a/src/lib/tests/crypt_test.c
+++ b/src/lib/tests/crypt_test.c
@@ -364,6 +364,22 @@ static int test_crypt_aad_tamper(int nid)
                 goto fail_rx;
         }
 
+        enc.data[5] ^= 0x01;
+
+        if (crypt_decrypt(rx, enc, &dec) < 0) {
+                printf("Decryption failed after a rejected packet.\n");
+                freebuf(enc);
+                goto fail_rx;
+        }
+
+        if (dec.len != in.len || memcmp(dec.data, in.data, in.len) != 0) {
+                printf("Decrypted data mismatch after rejection.\n");
+                freebuf(dec);
+                freebuf(enc);
+                goto fail_rx;
+        }
+
+        freebuf(dec);
         freebuf(enc);
 
         crypt_destroy_ctx(rx);
diff --git a/src/lib/tests/kex_test.c b/src/lib/tests/kex_test.c
index 5b2ccfc2..38835265 100644
--- a/src/lib/tests/kex_test.c
+++ b/src/lib/tests/kex_test.c
@@ -105,6 +105,9 @@ extern const uint16_t kex_supported_nids[];
 int parse_sec_config(struct sec_config * cfg,
                      FILE *              fp);
 
+int crypt_load_sec_config(struct sec_config * cfg,
+                          FILE *              fp);
+
 static int test_kex_create_destroy(void)
 {
         struct sec_config cfg;
@@ -1191,6 +1194,38 @@ static int test_kex_parse_config_unknown_key(void)
         return TEST_RC_FAIL;
 }
 
+#ifndef HAVE_OPENSSL
+/* A present security config must be refused without a backend. */
+static int test_kex_load_config_enotsup(void)
+{
+        struct sec_config kex;
+        FILE *            fp;
+
+        TEST_START();
+
+        fp = FMEMOPEN_STR(KEX_CONFIG_CUSTOM);
+        if (fp == NULL) {
+                printf("Failed to open config stream.\n");
+                goto fail;
+        }
+
+        if (crypt_load_sec_config(&kex, fp) != -ENOTSUP) {
+                printf("Loaded a config without a crypto backend.\n");
+                fclose(fp);
+                goto fail;
+        }
+
+        fclose(fp);
+
+        TEST_SUCCESS();
+
+        return TEST_RC_SUCCESS;
+ fail:
+        TEST_FAIL();
+        return TEST_RC_FAIL;
+}
+#endif
+
 int kex_test(int     argc,
              char ** argv)
 {
@@ -1200,6 +1235,7 @@ int kex_test(int     argc,
         (void) argv;
 
         ret |= test_kex_create_destroy();
+#ifdef HAVE_OPENSSL
         ret |= test_kex_parse_config_empty();
         ret |= test_kex_parse_config_none_rejected();
         ret |= test_kex_parse_config_no_enc();
@@ -1211,7 +1247,6 @@ int kex_test(int     argc,
         ret |= test_kex_parse_config_auth_no_enc(KEX_CONFIG_NO_ENC_THEN_AUTH);
         ret |= test_kex_parse_config_cacert();
         ret |= test_kex_parse_config_unknown_key();
-#ifdef HAVE_OPENSSL
         ret |= test_kex_parse_config_custom();
         ret |= test_kex_parse_config_whitespace();
         ret |= test_kex_parse_config_cipher();
@@ -1227,6 +1262,18 @@ int kex_test(int     argc,
         ret |= test_kex_load_dhe_privkey();
         ret |= test_kex_load_dhe_pubkey();
 #else
+        ret |= test_kex_load_config_enotsup();
+
+        (void) test_kex_parse_config_empty;
+        (void) test_kex_parse_config_none_rejected;
+        (void) test_kex_parse_config_no_enc;
+        (void) test_kex_parse_config_auth;
+        (void) test_kex_parse_config_auth_invalid;
+        (void) test_kex_parse_config_auth_seed;
+        (void) test_kex_parse_config_auth_optional;
+        (void) test_kex_parse_config_auth_no_enc;
+        (void) test_kex_parse_config_cacert;
+        (void) test_kex_parse_config_unknown_key;
         (void) test_kex_parse_config_custom;
         (void) test_kex_parse_config_whitespace;
         (void) test_kex_parse_config_cipher;
@@ -1240,9 +1287,6 @@ int kex_test(int     argc,
         (void) test_kex_dhe_corrupted_pubkey_all;
         (void) test_kex_load_dhe_privkey;
         (void) test_kex_load_dhe_pubkey;
-
-        if (ret == 0)
-                ret = TEST_RC_SKIP;
 #endif
         return ret;
 }