openssh unix dev - Oct 2022 - [PATCH] Use EVP_MAC interface for Poly1305 if supported.

On Sat, 22 Oct 2022 at 07:53, Chris Rapier <rapier at psc.edu> wrote:
[...]
> I normally wouldn't clutter up the code with library version specific
> ifdefs but it might be worth considering.
Instead of ifdefs, you can check if the MAC init succeeded before
calling the EVP functions, else fall back to the existing code path.
> +       /* fetch the mac and create and initialize the context */
> +       if ((mac = EVP_MAC_fetch(NULL, "POLY1305", NULL)) == NULL
||
> +           (poly_ctx = EVP_MAC_CTX_new(mac)) == NULL ||
You're initializing the MAC context on every call to this function.
If you initialize the context once, cache it (say, as a static) and
reuse it does it go any faster?

[...]
> +#if OPENSSL_VERSION_NUMBER >= 0x30000000UL
> +               /* EVP_MAC_update doesn't put the poly_mac into a
buffer
> +                * we need EVP_MAC_final for that */
> +               EVP_MAC_update(poly_ctx, src, aadlen + len);
> +               EVP_MAC_final(poly_ctx, expected_tag, &poly_out_len,
(size_t)POLY1305_TAGLEN);
> +#else
>                  poly1305_auth(expected_tag, src, aadlen + len, poly_key);
> +#endif
You'd also want to only try to init the context once instead of every
time in the case where libcrypto did not support it, so something
like:

if (ctx_inited && poly_ctx != NULL) {
    EVP_MAC_update(poly_ctx, src, aadlen + len);
    EVP_MAC_final(poly_ctx, expected_tag, &poly_out_len,
(size_t)POLY1305_TAGLEN);
} else {
    poly1305_auth(expected_tag, src, aadlen + len, poly_key);
}

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.

On Sun, Oct 23, 2022 at 12:51 AM Darren Tucker <dtucker at dtucker.net>
wrote:
> On Sat, 22 Oct 2022 at 07:53, Chris Rapier <rapier at psc.edu> wrote:
> [...]
> > I normally wouldn't clutter up the code with library version
specific
> > ifdefs but it might be worth considering.
>
> Instead of ifdefs, you can check if the MAC init succeeded before
> calling the EVP functions, else fall back to the existing code path.
>
 EVP_MAC_fetch is a new OpenSSL 3.0 API, so to follow this way it will be
necessary to detect it via configure.


-- 
Dmitry Belyavskiy

On 10/22/22 6:49 PM, Darren Tucker wrote:
> On Sat, 22 Oct 2022 at 07:53, Chris Rapier <rapier at psc.edu> wrote:
> [...]
>> I normally wouldn't clutter up the code with library version
specific
>> ifdefs but it might be worth considering.
> 
> Instead of ifdefs, you can check if the MAC init succeeded before
> calling the EVP functions, else fall back to the existing code path.
As pointed out, this is only in OSSL3. That said, for hpnssh we've been 
looking at extracting the necessary code/assembly from OSSL3 and 
incorporating it into our code base to provide this functionality. 
Maybe. Depends on the complexity of the task.
>> +       /* fetch the mac and create and initialize the context */
>> +       if ((mac = EVP_MAC_fetch(NULL, "POLY1305", NULL)) ==
NULL ||
>> +           (poly_ctx = EVP_MAC_CTX_new(mac)) == NULL ||
> 
> You're initializing the MAC context on every call to this function.
> If you initialize the context once, cache it (say, as a static) and
> reuse it does it go any faster?
That's a fine question and one I hope to explore today. I also noticed 
that I'm neglecting to free the the EVP_MAC and the EVP_MAC_CTX. Kind of 
jumped the gun on that.

Chris