Re: [PATCH] Use EVP_MAC interface for Poly1305 if supported.

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

 



Newest version of the patch. This includes creating a OPENSSL_HAVE_EVP_MAC test in configure and moving the EVP_MAC_functions to poly1305.c. There are still 3 ifdefs in cipher-chachapoly_libcrypto.c but none in chachapoly_crypt(). I did have to extend the parameters on poly1305_auth to include the poly evp context.

diff --git a/cipher-chachapoly-libcrypto.c b/cipher-chachapoly-libcrypto.c
index 719f9c843..52decd427 100644
--- a/cipher-chachapoly-libcrypto.c
+++ b/cipher-chachapoly-libcrypto.c
@@ -35,8 +35,18 @@
 #include "ssherr.h"
 #include "cipher-chachapoly.h"

+
+/* using the EVP_MAC interface for poly1305 is significantly
+ * faster than the version bundled with OpenSSH. However,
+ * this interface is only available in OpenSSL 3.0+
+ * -cjr 10/21/2022 */
 struct chachapoly_ctx {
        EVP_CIPHER_CTX *main_evp, *header_evp;
+#ifdef OPENSSL_HAVE_EVP_MAC
+       EVP_MAC_CTX    *poly_ctx;
+#else
+       char           *poly_ctx;
+#endif
 };

 struct chachapoly_ctx *
@@ -57,6 +67,15 @@ chachapoly_new(const u_char *key, u_int keylen)
                goto fail;
        if (EVP_CIPHER_CTX_iv_length(ctx->header_evp) != 16)
                goto fail;
+#ifdef OPENSSL_HAVE_EVP_MAC
+       EVP_MAC *mac = NULL;
+       if ((mac = EVP_MAC_fetch(NULL, "POLY1305", NULL)) == NULL)
+               goto fail;
+       if ((ctx->poly_ctx = EVP_MAC_CTX_new(mac)) == NULL)
+               goto fail;
+#else
+       ctx->poly_ctx = NULL;
+#endif
        return ctx;
  fail:
        chachapoly_free(ctx);
@@ -70,6 +89,9 @@ chachapoly_free(struct chachapoly_ctx *cpctx)
                return;
        EVP_CIPHER_CTX_free(cpctx->main_evp);
        EVP_CIPHER_CTX_free(cpctx->header_evp);
+#ifdef OPENSSL_HAVE_EVP_MAC
+       EVP_MAC_CTX_free(cpctx->poly_ctx);
+#endif
        freezero(cpctx, sizeof(*cpctx));
 }

@@ -107,8 +129,7 @@ chachapoly_crypt(struct chachapoly_ctx *ctx, u_int seqnr, u_char *dest,
        /* If decrypting, check tag before anything else */
        if (!do_encrypt) {
                const u_char *tag = src + aadlen + len;
-
-               poly1305_auth(expected_tag, src, aadlen + len, poly_key);
+ poly1305_auth(ctx->poly_ctx, expected_tag, src, aadlen + len, poly_key); if (timingsafe_bcmp(expected_tag, tag, POLY1305_TAGLEN) != 0) {
                        r = SSH_ERR_MAC_INVALID;
                        goto out;
@@ -134,8 +155,8 @@ chachapoly_crypt(struct chachapoly_ctx *ctx, u_int seqnr, u_char *dest,

        /* If encrypting, calculate and append tag */
        if (do_encrypt) {
-               poly1305_auth(dest + aadlen + len, dest, aadlen + len,
-                   poly_key);
+ poly1305_auth(ctx->poly_ctx, dest + aadlen + len, dest, aadlen + len,
+                       poly_key);
        }
        r = 0;
  out:
diff --git a/configure.ac b/configure.ac
index 8a18f8381..8493f4bc3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2932,6 +2932,11 @@ if test "x$openssl" = "xyes" ; then
                EVP_chacha20 \
        ])

+       # OpenSSL 3.0 API
+       # Does OpenSSL support the EVP_MAC functions?
+       AC_CHECK_FUNCS(EVP_MAC_fetch,
+ [AC_DEFINE([OPENSSL_HAVE_EVP_MAC], [1], [EVP_MAC Functions])])
+
        if test "x$openssl_engine" = "xyes" ; then
                AC_MSG_CHECKING([for OpenSSL ENGINE support])
                AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
diff --git a/poly1305.c b/poly1305.c
index 6fd1fc8cd..edc2002eb 100644
--- a/poly1305.c
+++ b/poly1305.c
@@ -14,6 +14,16 @@

 #include "poly1305.h"

+#ifdef OPENSSL_HAVE_EVP_MAC
+void
+poly1305_auth(EVP_MAC_CTX *poly_ctx, unsigned char out[POLY1305_TAGLEN], const unsigned char *m, size_t inlen, const unsigned char key[POLY1305_KEYLEN]) {
+       size_t poly_out_len;
+       EVP_MAC_init(poly_ctx, (const u_char *)key, POLY1305_KEYLEN, NULL);
+       EVP_MAC_update(poly_ctx, m, inlen);
+ EVP_MAC_final(poly_ctx, out, &poly_out_len, (size_t)POLY1305_TAGLEN);
+}
+#else
+
 #define mul32x32_64(a,b) ((uint64_t)(a) * (b))

 #define U8TO32_LE(p) \
@@ -31,7 +41,7 @@
        } while (0)

 void
-poly1305_auth(unsigned char out[POLY1305_TAGLEN], const unsigned char *m, size_t inlen, const unsigned char key[POLY1305_KEYLEN]) { +poly1305_auth(char *unused, unsigned char out[POLY1305_TAGLEN], const unsigned char *m, size_t inlen, const unsigned char key[POLY1305_KEYLEN]) {
        uint32_t t0,t1,t2,t3;
        uint32_t h0,h1,h2,h3,h4;
        uint32_t r0,r1,r2,r3,r4;
@@ -158,3 +168,4 @@ poly1305_donna_finish:
        U32TO8_LE(&out[ 8], f2); f3 += (f2 >> 32);
        U32TO8_LE(&out[12], f3);
 }
+#endif /* OPENSSL_HAVE_EVP_MAC */
diff --git a/poly1305.h b/poly1305.h
index f7db5f8d7..b4146f92d 100644
--- a/poly1305.h
+++ b/poly1305.h
@@ -13,8 +13,15 @@
 #define POLY1305_KEYLEN                32
 #define POLY1305_TAGLEN                16

-void poly1305_auth(u_char out[POLY1305_TAGLEN], const u_char *m, size_t inlen,
+#ifdef OPENSSL_HAVE_EVP_MAC
+#include <openssl/evp.h>
+
+void poly1305_auth(EVP_MAC_CTX *poly_key, u_char out[POLY1305_TAGLEN], const u_char *m, size_t inlen,
+    const u_char key[POLY1305_KEYLEN])
+#else
+void poly1305_auth(char *unused, u_char out[POLY1305_TAGLEN], const u_char *m, size_t inlen,
     const u_char key[POLY1305_KEYLEN])
+#endif
     __attribute__((__bounded__(__minbytes__, 1, POLY1305_TAGLEN)))
     __attribute__((__bounded__(__buffer__, 2, 3)))
     __attribute__((__bounded__(__minbytes__, 4, POLY1305_KEYLEN)));

On 10/24/22 5:08 PM, Chris Rapier wrote:


On 10/24/22 4:23 PM, Darren Tucker wrote:
On Tue, 25 Oct 2022 at 06:23, Chris Rapier <rapier@xxxxxxx> wrote:

+#if OPENSSL_VERSION_NUMBER >= 0x30000000UL

As mentioned by Dmitry Belyavskiy upthread, since this depends on
EVP_MAC_fetch() this should probably be checked by configure instead
and put inside an ifdef HAVE_EVP_MAC_FETCH.  I'm also wondering if the
additional OpenSSL specific code belongs in the poly1305_auth function
in cipher-chachapoly-libcrypto.c.

Okay, I think I'm understanding. We'd still have the #ifdefs in the code but we'd be moving away from actually checking a specific version string to seeing if the function itself exists. I'll work on that tomorrow.

As for putting it in the poly_auth function itself. I'm concerned that making the parameters work would be difficult and possible confusing if we maintained the current ones for poly1305_auth(). As far as I can figure we'd need 5 parameters and to set ctx->poly_ctx explictly to null in is HAVE_EVP_MAC_FETCH is false. Thoughts?

+       size_t poly_out_len;
+#endif

Since poly_out_len is only ever used inside the  "if (!do_encrypt)"
block below, you could move this declaration inside the existing ifdef
inside that block and reduce this diff by one hunk.

Good point. I've made that change. I'm going to think about a few more things and work out the configure before I submit a new patch.

Chris
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev




[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux