Merge branch 'tor-github/pr/655'

changes/ticket28837

@@ -0,0 +1,4 @@
+  o Minor features (performance):
+    - Use OpenSSL's implementations of SHA3 when available (in OpenSSL 1.1.1
+      and later), since they tend to be faster than tiny-keccak. Closes
+      ticket 28837.

configure.ac

@@ -949,21 +949,24 @@ AC_CHECK_MEMBERS([struct ssl_method_st.get_cipher_by_char], , ,
 [#include <openssl/ssl.h>
 ])
 
+dnl OpenSSL functions which we might not have.  In theory, we could just
+dnl check the openssl version number, but in practice that gets pretty
+dnl confusing with LibreSSL, OpenSSL, and various distributions' patches
+dnl to them.
 AC_CHECK_FUNCS([ \
 		ERR_load_KDF_strings \
+		EVP_PBE_scrypt \
+		EVP_sha3_256 \
+		SSL_CIPHER_find \
+		SSL_CTX_set1_groups_list \
+		SSL_CTX_set_security_level \
 		SSL_SESSION_get_master_key \
+		SSL_get_client_ciphers \
+		SSL_get_client_random \
 		SSL_get_server_random \
-                SSL_get_client_ciphers \
-                SSL_get_client_random \
-                SSL_CTX_set1_groups_list \
-		SSL_CIPHER_find \
-                SSL_CTX_set_security_level \
-		TLS_method
+		TLS_method \
 	       ])
 
-dnl Check if OpenSSL has scrypt implementation.
-AC_CHECK_FUNCS([ EVP_PBE_scrypt ])
-
 dnl Check if OpenSSL structures are opaque
 AC_CHECK_MEMBERS([SSL.state], , ,
 [#include <openssl/ssl.h>
@@ -975,6 +978,15 @@ AC_CHECK_SIZEOF(SHA_CTX, , [AC_INCLUDES_DEFAULT()
 
 fi # enable_nss
 
+dnl We will someday make KECCAK_TINY optional, but for now we still need
+dnl it for SHAKE, since OpenSSL's SHAKE can't be squeezed more than
+dnl once.  See comment in the definition of crypto_xof_t.
+
+dnl AM_CONDITIONAL(BUILD_KECCAK_TINY,
+dnl   test "x$ac_cv_func_EVP_sha3_256" != "xyes")
+
+AM_CONDITIONAL(BUILD_KECCAK_TINY, true)
+
 dnl ======================================================================
 dnl Can we use KIST?
 

src/core/crypto/hs_ntor.c

@@ -176,7 +176,6 @@ get_introduce1_key_material(const uint8_t *secret_input,
   uint8_t keystream[CIPHER256_KEY_LEN + DIGEST256_LEN];
   uint8_t info_blob[INFO_BLOB_LEN];
   uint8_t kdf_input[KDF_INPUT_LEN];
-  crypto_xof_t *xof;
   uint8_t *ptr;
 
   /* Let's build info */
@@ -193,10 +192,8 @@ get_introduce1_key_material(const uint8_t *secret_input,
   tor_assert(ptr == kdf_input + sizeof(kdf_input));
 
   /* Now we need to run kdf_input over SHAKE-256 */
-  xof = crypto_xof_new();
-  crypto_xof_add_bytes(xof, kdf_input, sizeof(kdf_input));
-  crypto_xof_squeeze_bytes(xof, keystream, sizeof(keystream)) ;
-  crypto_xof_free(xof);
+  crypto_xof(keystream, sizeof(keystream),
+             kdf_input, sizeof(kdf_input));
 
   { /* Get the keys */
     memcpy(&hs_ntor_intro_cell_keys_out->enc_key, keystream,CIPHER256_KEY_LEN);
@@ -594,7 +591,6 @@ hs_ntor_circuit_key_expansion(const uint8_t *ntor_key_seed, size_t seed_len,
 {
   uint8_t *ptr;
   uint8_t kdf_input[NTOR_KEY_EXPANSION_KDF_INPUT_LEN];
-  crypto_xof_t *xof;
 
   /* Sanity checks on lengths to make sure we are good */
   if (BUG(seed_len != DIGEST256_LEN)) {
@@ -611,10 +607,8 @@ hs_ntor_circuit_key_expansion(const uint8_t *ntor_key_seed, size_t seed_len,
   tor_assert(ptr == kdf_input + sizeof(kdf_input));
 
   /* Generate the keys */
-  xof = crypto_xof_new();
-  crypto_xof_add_bytes(xof, kdf_input, sizeof(kdf_input));
-  crypto_xof_squeeze_bytes(xof, keys_out, HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN);
-  crypto_xof_free(xof);
+  crypto_xof(keys_out, HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN,
+             kdf_input, sizeof(kdf_input));
 
   return 0;
 }

src/ext/include.am

@@ -143,6 +143,7 @@ noinst_HEADERS += $(ED25519_DONNA_HDRS)
 LIBED25519_DONNA=src/ext/ed25519/donna/libed25519_donna.a
 noinst_LIBRARIES += $(LIBED25519_DONNA)
 
+if BUILD_KECCAK_TINY
 src_ext_keccak_tiny_libkeccak_tiny_a_CFLAGS=\
   @CFLAGS_CONSTTIME@
 
@@ -156,6 +157,7 @@ noinst_HEADERS += $(LIBKECCAK_TINY_HDRS)
 
 LIBKECCAK_TINY=src/ext/keccak-tiny/libkeccak-tiny.a
 noinst_LIBRARIES += $(LIBKECCAK_TINY)
+endif
 
 EXTRA_DIST += \
 	src/ext/timeouts/bench/bench-add.lua 		\

src/lib/crypt_ops/crypto_digest.c

@@ -37,6 +37,12 @@ DISABLE_GCC_WARNING(redundant-decls)
 #include <openssl/sha.h>
 
 ENABLE_GCC_WARNING(redundant-decls)
+
+#ifdef HAVE_EVP_SHA3_256
+#define OPENSSL_HAS_SHA3
+#include <openssl/evp.h>
+#endif
+
 #endif
 
 #ifdef ENABLE_NSS
@@ -150,8 +156,13 @@ crypto_digest256(char *digest, const char *m, size_t len,
     ret = (SHA256((const uint8_t*)m,len,(uint8_t*)digest) != NULL);
 #endif
   } else {
+#ifdef OPENSSL_HAS_SHA3
+    unsigned int dlen = DIGEST256_LEN;
+    ret = EVP_Digest(m, len, (uint8_t*)digest, &dlen, EVP_sha3_256(), NULL);
+#else
     ret = (sha3_256((uint8_t *)digest, DIGEST256_LEN,(const uint8_t *)m, len)
            > -1);
+#endif
   }
 
   if (!ret)
@@ -179,8 +190,13 @@ crypto_digest512(char *digest, const char *m, size_t len,
            != NULL);
 #endif
   } else {
+#ifdef OPENSSL_HAS_SHA3
+    unsigned int dlen = DIGEST512_LEN;
+    ret = EVP_Digest(m, len, (uint8_t*)digest, &dlen, EVP_sha3_512(), NULL);
+#else
     ret = (sha3_512((uint8_t*)digest, DIGEST512_LEN, (const uint8_t*)m, len)
            > -1);
+#endif
   }
 
   if (!ret)
@@ -282,7 +298,11 @@ struct crypto_digest_t {
     SHA256_CTX sha2; /**< state for SHA256 */
     SHA512_CTX sha512; /**< state for SHA512 */
 #endif
+#ifdef OPENSSL_HAS_SHA3
+    EVP_MD_CTX *md;
+#else
     keccak_state sha3; /**< state for SHA3-[256,512] */
+#endif
   } d;
 };
 
@@ -325,9 +345,15 @@ crypto_digest_alloc_bytes(digest_algorithm_t alg)
     case DIGEST_SHA512:
       return END_OF_FIELD(d.sha512);
 #endif
-    case DIGEST_SHA3_256:
+#ifdef OPENSSL_HAS_SHA3
+    case DIGEST_SHA3_256: /* Fall through */
+    case DIGEST_SHA3_512:
+      return END_OF_FIELD(d.md);
+#else
+    case DIGEST_SHA3_256: /* Fall through */
     case DIGEST_SHA3_512:
       return END_OF_FIELD(d.sha3);
+#endif
     default:
       tor_assert(0); // LCOV_EXCL_LINE
       return 0;      // LCOV_EXCL_LINE
@@ -373,12 +399,29 @@ crypto_digest_new_internal(digest_algorithm_t algorithm)
       SHA512_Init(&r->d.sha512);
       break;
 #endif
+#ifdef OPENSSL_HAS_SHA3
+    case DIGEST_SHA3_256:
+      r->d.md = EVP_MD_CTX_new();
+      if (!EVP_DigestInit(r->d.md, EVP_sha3_256())) {
+        crypto_digest_free(r);
+        return NULL;
+      }
+      break;
+    case DIGEST_SHA3_512:
+      r->d.md = EVP_MD_CTX_new();
+      if (!EVP_DigestInit(r->d.md, EVP_sha3_512())) {
+        crypto_digest_free(r);
+        return NULL;
+      }
+      break;
+#else
     case DIGEST_SHA3_256:
       keccak_digest_init(&r->d.sha3, 256);
       break;
     case DIGEST_SHA3_512:
       keccak_digest_init(&r->d.sha3, 512);
       break;
+#endif
     default:
       tor_assert_unreached();
     }
@@ -427,6 +470,14 @@ crypto_digest_free_(crypto_digest_t *digest)
   if (library_supports_digest(digest->algorithm)) {
     PK11_DestroyContext(digest->d.ctx, PR_TRUE);
   }
+#endif
+#ifdef OPENSSL_HAS_SHA3
+  if (digest->algorithm == DIGEST_SHA3_256 ||
+      digest->algorithm == DIGEST_SHA3_512) {
+    if (digest->d.md) {
+      EVP_MD_CTX_free(digest->d.md);
+    }
+  }
 #endif
   size_t bytes = crypto_digest_alloc_bytes(digest->algorithm);
   memwipe(digest, 0, bytes);
@@ -471,10 +522,19 @@ crypto_digest_add_bytes(crypto_digest_t *digest, const char *data,
       SHA512_Update(&digest->d.sha512, (void*)data, len);
       break;
 #endif
+#ifdef OPENSSL_HAS_SHA3
+    case DIGEST_SHA3_256: /* FALLSTHROUGH */
+    case DIGEST_SHA3_512: {
+      int r = EVP_DigestUpdate(digest->d.md, data, len);
+      tor_assert(r);
+  }
+      break;
+#else
     case DIGEST_SHA3_256: /* FALLSTHROUGH */
     case DIGEST_SHA3_512:
       keccak_digest_update(&digest->d.sha3, (const uint8_t *)data, len);
       break;
+#endif
     default:
       /* LCOV_EXCL_START */
       tor_fragile_assert();
@@ -499,12 +559,24 @@ crypto_digest_get_digest(crypto_digest_t *digest,
   tor_assert(out);
   tor_assert(out_len <= crypto_digest_algorithm_get_length(digest->algorithm));
 
-  /* The SHA-3 code handles copying into a temporary ctx, and also can handle
-   * short output buffers by truncating appropriately. */
   if (digest->algorithm == DIGEST_SHA3_256 ||
       digest->algorithm == DIGEST_SHA3_512) {
+#ifdef OPENSSL_HAS_SHA3
+    unsigned dlen = (unsigned)
+      crypto_digest_algorithm_get_length(digest->algorithm);
+    EVP_MD_CTX *tmp = EVP_MD_CTX_new();
+    EVP_MD_CTX_copy(tmp, digest->d.md);
+    memset(r, 0xff, sizeof(r));
+    int res = EVP_DigestFinal(tmp, r, &dlen);
+    EVP_MD_CTX_free(tmp);
+    tor_assert(res == 1);
+    goto done;
+#else
+    /* Tiny-Keccak handles copying into a temporary ctx, and also can handle
+     * short output buffers by truncating appropriately. */
     keccak_digest_sum(&digest->d.sha3, (uint8_t *)out, out_len);
     return;
+#endif
   }
 
 #ifdef ENABLE_NSS
@@ -550,6 +622,10 @@ crypto_digest_get_digest(crypto_digest_t *digest,
 //LCOV_EXCL_STOP
   }
 #endif
+
+#ifdef OPENSSL_HAS_SHA3
+ done:
+#endif
   memcpy(out, r, out_len);
   memwipe(r, 0, sizeof(r));
 }
@@ -570,6 +646,13 @@ crypto_digest_dup(const crypto_digest_t *digest)
   if (library_supports_digest(digest->algorithm)) {
     result->d.ctx = PK11_CloneContext(digest->d.ctx);
   }
+#endif
+#ifdef OPENSSL_HAS_SHA3
+  if (digest->algorithm == DIGEST_SHA3_256 ||
+      digest->algorithm == DIGEST_SHA3_512) {
+    result->d.md = EVP_MD_CTX_new();
+    EVP_MD_CTX_copy(result->d.md, digest->d.md);
+  }
 #endif
   return result;
 }
@@ -637,6 +720,15 @@ crypto_digest_assign(crypto_digest_t *into,
     return;
   }
 #endif
+
+#ifdef OPENSSL_HAS_SHA3
+  if (from->algorithm == DIGEST_SHA3_256 ||
+      from->algorithm == DIGEST_SHA3_512) {
+    EVP_MD_CTX_copy(into->d.md, from->d.md);
+    return;
+  }
+#endif
+
   memcpy(into,from,alloc_bytes);
 }
 
@@ -779,7 +871,23 @@ crypto_mac_sha3_256(uint8_t *mac_out, size_t len_out,
 
 /** Internal state for a eXtendable-Output Function (XOF). */
 struct crypto_xof_t {
+#ifdef OPENSSL_HAS_SHAKE3_EVP
+  /* XXXX We can't enable this yet, because OpenSSL's
+   * DigestFinalXOF function can't be called repeatedly on the same
+   * XOF.
+   *
+   * We could in theory use the undocumented SHA3_absorb and SHA3_squeeze
+   * functions, but let's not mess with undocumented OpenSSL internals any
+   * more than we have to.
+   *
+   * We could also revise our XOF code so that it only allows a single
+   * squeeze operation; we don't require streaming squeeze operations
+   * outside the tests yet.
+   */
+  EVP_MD_CTX *ctx;
+#else
   keccak_state s;
+#endif
 };
 
 /** Allocate a new XOF object backed by SHAKE-256.  The security level
@@ -792,7 +900,14 @@ crypto_xof_new(void)
 {
   crypto_xof_t *xof;
   xof = tor_malloc(sizeof(crypto_xof_t));
+#ifdef OPENSSL_HAS_SHAKE256
+  xof->ctx = EVP_MD_CTX_new();
+  tor_assert(xof->ctx);
+  int r = EVP_DigestInit(xof->ctx, EVP_shake256());
+  tor_assert(r == 1);
+#else
   keccak_xof_init(&xof->s, 256);
+#endif
   return xof;
 }
 
@@ -803,8 +918,13 @@ crypto_xof_new(void)
 void
 crypto_xof_add_bytes(crypto_xof_t *xof, const uint8_t *data, size_t len)
 {
+#ifdef OPENSSL_HAS_SHAKE256
+  int r = EVP_DigestUpdate(xof->ctx, data, len);
+  tor_assert(r == 1);
+#else
   int i = keccak_xof_absorb(&xof->s, data, len);
   tor_assert(i == 0);
+#endif
 }
 
 /** Squeeze bytes out of a XOF object.  Calling this routine will render
@@ -813,8 +933,13 @@ crypto_xof_add_bytes(crypto_xof_t *xof, const uint8_t *data, size_t len)
 void
 crypto_xof_squeeze_bytes(crypto_xof_t *xof, uint8_t *out, size_t len)
 {
+#ifdef OPENSSL_HAS_SHAKE256
+  int r = EVP_DigestFinalXOF(xof->ctx, out, len);
+  tor_assert(r == 1);
+#else
   int i = keccak_xof_squeeze(&xof->s, out, len);
   tor_assert(i == 0);
+#endif
 }
 
 /** Cleanse and deallocate a XOF object. */
@@ -823,6 +948,34 @@ crypto_xof_free_(crypto_xof_t *xof)
 {
   if (!xof)
     return;
+#ifdef OPENSSL_HAS_SHAKE256
+  if (xof->ctx)
+    EVP_MD_CTX_free(xof->ctx);
+#endif
   memwipe(xof, 0, sizeof(crypto_xof_t));
   tor_free(xof);
 }
+
+/** Compute the XOF (SHAKE256) of a <b>input_len</b> bytes at <b>input</b>,
+ * putting <b>output_len</b> bytes at <b>output</b>. */
+void
+crypto_xof(uint8_t *output, size_t output_len,
+           const uint8_t *input, size_t input_len)
+{
+#ifdef OPENSSL_HAS_SHA3
+  EVP_MD_CTX *ctx = EVP_MD_CTX_new();
+  tor_assert(ctx);
+  int r = EVP_DigestInit(ctx, EVP_shake256());
+  tor_assert(r == 1);
+  r = EVP_DigestUpdate(ctx, input, input_len);
+  tor_assert(r == 1);
+  r = EVP_DigestFinalXOF(ctx, output, output_len);
+  tor_assert(r == 1);
+  EVP_MD_CTX_free(ctx);
+#else
+  crypto_xof_t *xof = crypto_xof_new();
+  crypto_xof_add_bytes(xof, input, input_len);
+  crypto_xof_squeeze_bytes(xof, output, output_len);
+  crypto_xof_free(xof);
+#endif
+}

src/lib/crypt_ops/crypto_digest.h

@@ -124,6 +124,8 @@ void crypto_xof_squeeze_bytes(crypto_xof_t *xof, uint8_t *out, size_t len);
 void crypto_xof_free_(crypto_xof_t *xof);
 #define crypto_xof_free(xof) \
   FREE_AND_NULL(crypto_xof_t, crypto_xof_free_, (xof))
+void crypto_xof(uint8_t *output, size_t output_len,
+                const uint8_t *input, size_t input_len);
 
 #ifdef TOR_UNIT_TESTS
 digest_algorithm_t crypto_digest_get_algorithm(crypto_digest_t *digest);

src/test/test_crypto.c

@@ -1011,13 +1011,19 @@ test_crypto_sha3_xof(void *arg)
   crypto_xof_free(xof);
   memset(out, 0, sizeof(out));
 
+  /* Test one-function absorb/squeeze. */
+  crypto_xof(out, sizeof(out), msg, sizeof(msg));
+  test_memeq_hex(out, squeezed_hex);
+  memset(out, 0, sizeof(out));
+
   /* Test incremental absorb/squeeze. */
   xof = crypto_xof_new();
   tt_assert(xof);
   for (size_t i = 0; i < sizeof(msg); i++)
     crypto_xof_add_bytes(xof, msg + i, 1);
-  for (size_t i = 0; i < sizeof(out); i++)
+  for (size_t i = 0; i < sizeof(out); i++) {
     crypto_xof_squeeze_bytes(xof, out + i, 1);
+  }
   test_memeq_hex(out, squeezed_hex);
 
  done: