diff options
author | Theo Buehler <tb@cvs.openbsd.org> | 2023-09-28 11:29:11 +0000 |
---|---|---|
committer | Theo Buehler <tb@cvs.openbsd.org> | 2023-09-28 11:29:11 +0000 |
commit | 51d78e1714852b5ccb1dc0d7a2d5c958fc8b9401 (patch) | |
tree | 248959498a969310e4854938286fa43951df475e | |
parent | cdf2fe7575aea1a991fd76d43c780062ebb7bfad (diff) |
Fix EVP_CIPHER_CTX_iv_length()
In today's episode of "curly nonsense from EVP land" we deal with a quite
harmless oversight and a not too bad suboptimal fix, relatively speaking.
At some point EVP_CIPHER_{CCM,GCM}_SET_IVLEN was added. It modified some
object hanging off of EVP_CIPHER. However, EVP_CIPHER_CTX_iv_length() wasn't
taught about this and kept returning the hardcoded default value on the
EVP_CIPHER. Once it transpired that a doc fix isn't going to cut it, this
was fixed. And of course it's easy to fix: you only have to dive through
about three layers of EVP, test and set a flag and handle a control in a
couple methods.
The upstream fix was done poorly and we begrudgingly have to match the API:
the caller is expected to pass a raw pointer next to a 0 length along with
EVP_CIPHER_GET_IV_LENGTH and the control handler goes *(int *)ptr = length
in full YOLO mode. That's never going to be an issue because of course the
caller will always pass a properly aligned pointer backing a sufficient
amount of memory. Yes, unlikely to be a real issue, but it could have been
done with proper semantics and checks without complicating the code. But
why do I even bother to complain? We're used to this.
Of note here is that there was some pushback painting other corners of a
bikeshed until the reviewer gave up with a resigned
That kind of changes the semantics and is one extra complexity level,
but [shrug] ok...
Anyway, the reason this matters now after so many years is that rust-openssl
has an assert, notably added in a +758 -84 commit with the awesome message
"Docs" that gets triggered by recent tests added to py-cryptography.
Thanks to Alex Gaynor for reporting this. Let me take the opportunity to
point out that pyca contributed to improve rust-openssl, in particular its
libressl support, quite a bit. That's much appreciated and very noticeable.
Regress coverage to follow in subsequent commits.
Based on OpenSSL PR #9499 and issue #8330.
ok beck jsing
PS: A few macros were kept internal for now to avoid impact on the release
cycle that is about to finish. They will be exposed after release.
-rw-r--r-- | lib/libcrypto/evp/e_aes.c | 15 | ||||
-rw-r--r-- | lib/libcrypto/evp/e_chacha20poly1305.c | 14 | ||||
-rw-r--r-- | lib/libcrypto/evp/evp_lib.c | 17 | ||||
-rw-r--r-- | lib/libcrypto/evp/evp_local.h | 8 |
4 files changed, 45 insertions, 9 deletions
diff --git a/lib/libcrypto/evp/e_aes.c b/lib/libcrypto/evp/e_aes.c index 3d3b1a9d6c2..3d357f01197 100644 --- a/lib/libcrypto/evp/e_aes.c +++ b/lib/libcrypto/evp/e_aes.c @@ -1,4 +1,4 @@ -/* $OpenBSD: e_aes.c,v 1.53 2023/07/07 19:37:53 beck Exp $ */ +/* $OpenBSD: e_aes.c,v 1.54 2023/09/28 11:29:10 tb Exp $ */ /* ==================================================================== * Copyright (c) 2001-2011 The OpenSSL Project. All rights reserved. * @@ -1305,7 +1305,11 @@ aes_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr) gctx->tls_aad_len = -1; return 1; - case EVP_CTRL_GCM_SET_IVLEN: + case EVP_CTRL_AEAD_GET_IVLEN: + *(int *)ptr = gctx->ivlen; + return 1; + + case EVP_CTRL_AEAD_SET_IVLEN: if (arg <= 0) return 0; /* Allocate memory for IV if needed */ @@ -1631,6 +1635,7 @@ aes_gcm_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out, #define CUSTOM_FLAGS \ ( EVP_CIPH_FLAG_DEFAULT_ASN1 | EVP_CIPH_CUSTOM_IV | \ + EVP_CIPH_FLAG_CUSTOM_IV_LENGTH | \ EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_ALWAYS_CALL_INIT | \ EVP_CIPH_CTRL_INIT | EVP_CIPH_CUSTOM_COPY ) @@ -1968,7 +1973,11 @@ aes_ccm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr) cctx->len_set = 0; return 1; - case EVP_CTRL_CCM_SET_IVLEN: + case EVP_CTRL_AEAD_GET_IVLEN: + *(int *)ptr = 15 - cctx->L; + return 1; + + case EVP_CTRL_AEAD_SET_IVLEN: arg = 15 - arg; case EVP_CTRL_CCM_SET_L: diff --git a/lib/libcrypto/evp/e_chacha20poly1305.c b/lib/libcrypto/evp/e_chacha20poly1305.c index 33d09315e0d..4a393c2458d 100644 --- a/lib/libcrypto/evp/e_chacha20poly1305.c +++ b/lib/libcrypto/evp/e_chacha20poly1305.c @@ -1,4 +1,4 @@ -/* $OpenBSD: e_chacha20poly1305.c,v 1.31 2023/08/24 04:33:08 tb Exp $ */ +/* $OpenBSD: e_chacha20poly1305.c,v 1.32 2023/09/28 11:29:10 tb Exp $ */ /* * Copyright (c) 2022 Joel Sing <jsing@openbsd.org> @@ -18,6 +18,7 @@ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include <limits.h> #include <stdint.h> #include <string.h> @@ -551,6 +552,12 @@ chacha20_poly1305_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *ptr) cpx->nonce_len = sizeof(cpx->nonce); return 1; + case EVP_CTRL_AEAD_GET_IVLEN: + if (cpx->nonce_len > INT_MAX) + return 0; + *(int *)ptr = (int)cpx->nonce_len; + return 1; + case EVP_CTRL_AEAD_SET_IVLEN: if (arg <= 0 || arg > sizeof(cpx->nonce)) return 0; @@ -592,8 +599,9 @@ static const EVP_CIPHER cipher_chacha20_poly1305 = { .key_len = 32, .iv_len = 12, .flags = EVP_CIPH_ALWAYS_CALL_INIT | EVP_CIPH_CTRL_INIT | - EVP_CIPH_CUSTOM_IV | EVP_CIPH_FLAG_AEAD_CIPHER | - EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_FLAG_DEFAULT_ASN1, + EVP_CIPH_CUSTOM_IV | EVP_CIPH_FLAG_CUSTOM_IV_LENGTH | + EVP_CIPH_FLAG_AEAD_CIPHER | EVP_CIPH_FLAG_CUSTOM_CIPHER | + EVP_CIPH_FLAG_DEFAULT_ASN1, .init = chacha20_poly1305_init, .do_cipher = chacha20_poly1305_cipher, .cleanup = chacha20_poly1305_cleanup, diff --git a/lib/libcrypto/evp/evp_lib.c b/lib/libcrypto/evp/evp_lib.c index 24ce1963dfb..f4e46aea412 100644 --- a/lib/libcrypto/evp/evp_lib.c +++ b/lib/libcrypto/evp/evp_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: evp_lib.c,v 1.27 2023/07/07 19:37:53 beck Exp $ */ +/* $OpenBSD: evp_lib.c,v 1.28 2023/09/28 11:29:10 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -266,7 +266,20 @@ EVP_CIPHER_iv_length(const EVP_CIPHER *cipher) int EVP_CIPHER_CTX_iv_length(const EVP_CIPHER_CTX *ctx) { - return ctx->cipher->iv_len; + int iv_length = 0; + + if ((ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_IV_LENGTH) == 0) + return ctx->cipher->iv_len; + + /* + * XXX - sanity would suggest to pass the size of the pointer along, + * but unfortunately we have to match the other crowd. + */ + if (EVP_CIPHER_CTX_ctrl((EVP_CIPHER_CTX *)ctx, EVP_CTRL_GET_IVLEN, 0, + &iv_length) != 1) + return -1; + + return iv_length; } unsigned char * diff --git a/lib/libcrypto/evp/evp_local.h b/lib/libcrypto/evp/evp_local.h index e0a8afd6b8b..015fbb50a94 100644 --- a/lib/libcrypto/evp/evp_local.h +++ b/lib/libcrypto/evp/evp_local.h @@ -1,4 +1,4 @@ -/* $OpenBSD: evp_local.h,v 1.4 2023/08/11 05:10:35 tb Exp $ */ +/* $OpenBSD: evp_local.h,v 1.5 2023/09/28 11:29:10 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2000. */ @@ -61,6 +61,12 @@ __BEGIN_HIDDEN_DECLS +/* XXX - move these to evp.h after unlock. */ +#define EVP_CTRL_GET_IVLEN 0x25 +#define EVP_CIPH_FLAG_CUSTOM_IV_LENGTH 0x400000 + +#define EVP_CTRL_AEAD_GET_IVLEN EVP_CTRL_GET_IVLEN + /* * Don't free md_ctx->pctx in EVP_MD_CTX_cleanup(). Needed for ownership * handling in EVP_MD_CTX_set_pkey_ctx(). |