2023-05-01 14:34:18

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] SUNRPC: Avoid relying on crypto API to derive CBC-CTS output IV



> On May 1, 2023, at 10:04 AM, Ard Biesheuvel <[email protected]> wrote:
>
> Scott reports SUNRPC self-test failures regarding the output IV on arm64
> when using the SIMD accelerated implementation of AES in CBC mode with
> ciphertext stealing ("cts(cbc(aes))" in crypto API speak).
>
> These failures are the result of the fact that, while RFC 3962 does
> specify what the output IV should be and includes test vectors for it,
> the general concept of an output IV is poorly defined, and generally,
> not specified by the various algorithms implemented by the crypto API.
> Only algorithms that support transparent chaining (e.g., CBC mode on a
> block boundary) have requirements on the output IV, but ciphertext
> stealing (CTS) is fundamentally about how to encapsulate CBC in a way
> where the length of the entire message may not be an integral multiple
> of the cipher block size, and the concept of an output IV does not exist
> here because it has no defined purpose past the end of the message.
>
> The generic CTS template takes advantage of this chaining capability of
> the CBC implementations, and as a result, happens to return an output
> IV, simply because it passes its IV buffer directly to the encapsulated
> CBC implementation, which operates on full blocks only, and always
> returns an IV. This output IV happens to match how RFC 3962 defines it,
> even though the CTS template itself does not contain any output IV logic
> whatsoever, and, for this reason, lacks any test vectors that exercise
> this accidental output IV generation.
>
> The arm64 SIMD implementation of cts(cbc(aes)) does not use the generic
> CTS template at all, but instead, implements the CBC mode and ciphertext
> stealing directly, and therefore does not encapsule a CBC implementation
> that returns an output IV in the same way. The arm64 SIMD implementation
> complies with the specification and passes all internal tests, but when
> invoked by the SUNRPC code, fails to produce the expected output IV and
> causes its selftests to fail.
>
> Given that the output IV is defined as the penultimate block (where the
> final block may smaller than the block size), we can quite easily derive
> it in the caller by copying the appropriate slice of ciphertext after
> encryption.
>
> Cc: Trond Myklebust <[email protected]>
> Cc: Anna Schumaker <[email protected]>
> Cc: Chuck Lever <[email protected]>
> Cc: Jeff Layton <[email protected]>
> Reported-by: Scott Mayhew <[email protected]>
> Tested-by: Scott Mayhew <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>

Thanks Ard. I always like a patch description with lots
of context. I agree that looking at the output IV is a
GSS Kerberos idiosyncrasy.

Since I'm responsible for the GSS Kerberos Kunit tests,
I can take this one for v6.4-rc1.


> ---
> net/sunrpc/auth_gss/gss_krb5_crypto.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> index 212c5d57465a1bf5..22dca4647ee66b3e 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> @@ -639,6 +639,13 @@ gss_krb5_cts_crypt(struct crypto_sync_skcipher *cipher, struct xdr_buf *buf,
>
> ret = write_bytes_to_xdr_buf(buf, offset, data, len);
>
> + /*
> + * CBC-CTS does not define an output IV but RFC 3962 defines it as the
> + * penultimate block of ciphertext, so copy that into the IV buffer
> + * before returning.
> + */
> + if (encrypt)
> + memcpy(iv, data, crypto_sync_skcipher_ivsize(cipher));
> out:
> kfree(data);
> return ret;
> --
> 2.39.2
>

--
Chuck Lever