summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authortb <>2023-09-28 11:29:10 +0000
committertb <>2023-09-28 11:29:10 +0000
commit92da04b3fa82724d71f293483fb9284e6da2fa37 (patch)
tree3522ab66513de919b4cb23f4f9a24a55b2df8066 /src
parenta95a07cea06fc8d354ce9de8b80ef4f91fbfe23c (diff)
downloadopenbsd-92da04b3fa82724d71f293483fb9284e6da2fa37.tar.gz
openbsd-92da04b3fa82724d71f293483fb9284e6da2fa37.tar.bz2
openbsd-92da04b3fa82724d71f293483fb9284e6da2fa37.zip
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.
Diffstat (limited to 'src')
-rw-r--r--src/lib/libcrypto/evp/e_aes.c15
-rw-r--r--src/lib/libcrypto/evp/e_chacha20poly1305.c14
-rw-r--r--src/lib/libcrypto/evp/evp_lib.c17
-rw-r--r--src/lib/libcrypto/evp/evp_local.h8
4 files changed, 45 insertions, 9 deletions
diff --git a/src/lib/libcrypto/evp/e_aes.c b/src/lib/libcrypto/evp/e_aes.c
index 3d3b1a9d6c..3d357f0119 100644
--- a/src/lib/libcrypto/evp/e_aes.c
+++ b/src/lib/libcrypto/evp/e_aes.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: e_aes.c,v 1.53 2023/07/07 19:37:53 beck Exp $ */ 1/* $OpenBSD: e_aes.c,v 1.54 2023/09/28 11:29:10 tb Exp $ */
2/* ==================================================================== 2/* ====================================================================
3 * Copyright (c) 2001-2011 The OpenSSL Project. All rights reserved. 3 * Copyright (c) 2001-2011 The OpenSSL Project. All rights reserved.
4 * 4 *
@@ -1305,7 +1305,11 @@ aes_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
1305 gctx->tls_aad_len = -1; 1305 gctx->tls_aad_len = -1;
1306 return 1; 1306 return 1;
1307 1307
1308 case EVP_CTRL_GCM_SET_IVLEN: 1308 case EVP_CTRL_AEAD_GET_IVLEN:
1309 *(int *)ptr = gctx->ivlen;
1310 return 1;
1311
1312 case EVP_CTRL_AEAD_SET_IVLEN:
1309 if (arg <= 0) 1313 if (arg <= 0)
1310 return 0; 1314 return 0;
1311 /* Allocate memory for IV if needed */ 1315 /* Allocate memory for IV if needed */
@@ -1631,6 +1635,7 @@ aes_gcm_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out,
1631 1635
1632#define CUSTOM_FLAGS \ 1636#define CUSTOM_FLAGS \
1633 ( EVP_CIPH_FLAG_DEFAULT_ASN1 | EVP_CIPH_CUSTOM_IV | \ 1637 ( EVP_CIPH_FLAG_DEFAULT_ASN1 | EVP_CIPH_CUSTOM_IV | \
1638 EVP_CIPH_FLAG_CUSTOM_IV_LENGTH | \
1634 EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_ALWAYS_CALL_INIT | \ 1639 EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_ALWAYS_CALL_INIT | \
1635 EVP_CIPH_CTRL_INIT | EVP_CIPH_CUSTOM_COPY ) 1640 EVP_CIPH_CTRL_INIT | EVP_CIPH_CUSTOM_COPY )
1636 1641
@@ -1968,7 +1973,11 @@ aes_ccm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
1968 cctx->len_set = 0; 1973 cctx->len_set = 0;
1969 return 1; 1974 return 1;
1970 1975
1971 case EVP_CTRL_CCM_SET_IVLEN: 1976 case EVP_CTRL_AEAD_GET_IVLEN:
1977 *(int *)ptr = 15 - cctx->L;
1978 return 1;
1979
1980 case EVP_CTRL_AEAD_SET_IVLEN:
1972 arg = 15 - arg; 1981 arg = 15 - arg;
1973 1982
1974 case EVP_CTRL_CCM_SET_L: 1983 case EVP_CTRL_CCM_SET_L:
diff --git a/src/lib/libcrypto/evp/e_chacha20poly1305.c b/src/lib/libcrypto/evp/e_chacha20poly1305.c
index 33d09315e0..4a393c2458 100644
--- a/src/lib/libcrypto/evp/e_chacha20poly1305.c
+++ b/src/lib/libcrypto/evp/e_chacha20poly1305.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: e_chacha20poly1305.c,v 1.31 2023/08/24 04:33:08 tb Exp $ */ 1/* $OpenBSD: e_chacha20poly1305.c,v 1.32 2023/09/28 11:29:10 tb Exp $ */
2 2
3/* 3/*
4 * Copyright (c) 2022 Joel Sing <jsing@openbsd.org> 4 * Copyright (c) 2022 Joel Sing <jsing@openbsd.org>
@@ -18,6 +18,7 @@
18 * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. 18 * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
19 */ 19 */
20 20
21#include <limits.h>
21#include <stdint.h> 22#include <stdint.h>
22#include <string.h> 23#include <string.h>
23 24
@@ -551,6 +552,12 @@ chacha20_poly1305_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *ptr)
551 cpx->nonce_len = sizeof(cpx->nonce); 552 cpx->nonce_len = sizeof(cpx->nonce);
552 return 1; 553 return 1;
553 554
555 case EVP_CTRL_AEAD_GET_IVLEN:
556 if (cpx->nonce_len > INT_MAX)
557 return 0;
558 *(int *)ptr = (int)cpx->nonce_len;
559 return 1;
560
554 case EVP_CTRL_AEAD_SET_IVLEN: 561 case EVP_CTRL_AEAD_SET_IVLEN:
555 if (arg <= 0 || arg > sizeof(cpx->nonce)) 562 if (arg <= 0 || arg > sizeof(cpx->nonce))
556 return 0; 563 return 0;
@@ -592,8 +599,9 @@ static const EVP_CIPHER cipher_chacha20_poly1305 = {
592 .key_len = 32, 599 .key_len = 32,
593 .iv_len = 12, 600 .iv_len = 12,
594 .flags = EVP_CIPH_ALWAYS_CALL_INIT | EVP_CIPH_CTRL_INIT | 601 .flags = EVP_CIPH_ALWAYS_CALL_INIT | EVP_CIPH_CTRL_INIT |
595 EVP_CIPH_CUSTOM_IV | EVP_CIPH_FLAG_AEAD_CIPHER | 602 EVP_CIPH_CUSTOM_IV | EVP_CIPH_FLAG_CUSTOM_IV_LENGTH |
596 EVP_CIPH_FLAG_CUSTOM_CIPHER | EVP_CIPH_FLAG_DEFAULT_ASN1, 603 EVP_CIPH_FLAG_AEAD_CIPHER | EVP_CIPH_FLAG_CUSTOM_CIPHER |
604 EVP_CIPH_FLAG_DEFAULT_ASN1,
597 .init = chacha20_poly1305_init, 605 .init = chacha20_poly1305_init,
598 .do_cipher = chacha20_poly1305_cipher, 606 .do_cipher = chacha20_poly1305_cipher,
599 .cleanup = chacha20_poly1305_cleanup, 607 .cleanup = chacha20_poly1305_cleanup,
diff --git a/src/lib/libcrypto/evp/evp_lib.c b/src/lib/libcrypto/evp/evp_lib.c
index 24ce1963df..f4e46aea41 100644
--- a/src/lib/libcrypto/evp/evp_lib.c
+++ b/src/lib/libcrypto/evp/evp_lib.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: evp_lib.c,v 1.27 2023/07/07 19:37:53 beck Exp $ */ 1/* $OpenBSD: evp_lib.c,v 1.28 2023/09/28 11:29:10 tb Exp $ */
2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) 2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
3 * All rights reserved. 3 * All rights reserved.
4 * 4 *
@@ -266,7 +266,20 @@ EVP_CIPHER_iv_length(const EVP_CIPHER *cipher)
266int 266int
267EVP_CIPHER_CTX_iv_length(const EVP_CIPHER_CTX *ctx) 267EVP_CIPHER_CTX_iv_length(const EVP_CIPHER_CTX *ctx)
268{ 268{
269 return ctx->cipher->iv_len; 269 int iv_length = 0;
270
271 if ((ctx->cipher->flags & EVP_CIPH_FLAG_CUSTOM_IV_LENGTH) == 0)
272 return ctx->cipher->iv_len;
273
274 /*
275 * XXX - sanity would suggest to pass the size of the pointer along,
276 * but unfortunately we have to match the other crowd.
277 */
278 if (EVP_CIPHER_CTX_ctrl((EVP_CIPHER_CTX *)ctx, EVP_CTRL_GET_IVLEN, 0,
279 &iv_length) != 1)
280 return -1;
281
282 return iv_length;
270} 283}
271 284
272unsigned char * 285unsigned char *
diff --git a/src/lib/libcrypto/evp/evp_local.h b/src/lib/libcrypto/evp/evp_local.h
index e0a8afd6b8..015fbb50a9 100644
--- a/src/lib/libcrypto/evp/evp_local.h
+++ b/src/lib/libcrypto/evp/evp_local.h
@@ -1,4 +1,4 @@
1/* $OpenBSD: evp_local.h,v 1.4 2023/08/11 05:10:35 tb Exp $ */ 1/* $OpenBSD: evp_local.h,v 1.5 2023/09/28 11:29:10 tb Exp $ */
2/* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL 2/* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
3 * project 2000. 3 * project 2000.
4 */ 4 */
@@ -61,6 +61,12 @@
61 61
62__BEGIN_HIDDEN_DECLS 62__BEGIN_HIDDEN_DECLS
63 63
64/* XXX - move these to evp.h after unlock. */
65#define EVP_CTRL_GET_IVLEN 0x25
66#define EVP_CIPH_FLAG_CUSTOM_IV_LENGTH 0x400000
67
68#define EVP_CTRL_AEAD_GET_IVLEN EVP_CTRL_GET_IVLEN
69
64/* 70/*
65 * Don't free md_ctx->pctx in EVP_MD_CTX_cleanup(). Needed for ownership 71 * Don't free md_ctx->pctx in EVP_MD_CTX_cleanup(). Needed for ownership
66 * handling in EVP_MD_CTX_set_pkey_ctx(). 72 * handling in EVP_MD_CTX_set_pkey_ctx().