[PATCH] Use EVP_MAC interface for Poly1305 if supported.
Chris Rapier
rapier at psc.edu
Wed Oct 26 05:37:02 AEDT 2022
Ignore this patch. I forgot to update cipher_chachapoly.c to deal with
the new definition of poly1305_auth. Sorry about that.
On 10/25/22 2:20 PM, Chris Rapier wrote:
> 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 at psc.edu> 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
More information about the openssh-unix-dev
mailing list