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
