summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTheo Buehler <tb@cvs.openbsd.org>2023-09-28 11:29:11 +0000
committerTheo Buehler <tb@cvs.openbsd.org>2023-09-28 11:29:11 +0000
commit51d78e1714852b5ccb1dc0d7a2d5c958fc8b9401 (patch)
tree248959498a969310e4854938286fa43951df475e
parentcdf2fe7575aea1a991fd76d43c780062ebb7bfad (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.c15
-rw-r--r--lib/libcrypto/evp/e_chacha20poly1305.c14
-rw-r--r--lib/libcrypto/evp/evp_lib.c17
-rw-r--r--lib/libcrypto/evp/evp_local.h8
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().