diff options
author | tb <> | 2023-09-28 11:29:10 +0000 |
---|---|---|
committer | tb <> | 2023-09-28 11:29:10 +0000 |
commit | 92da04b3fa82724d71f293483fb9284e6da2fa37 (patch) | |
tree | 3522ab66513de919b4cb23f4f9a24a55b2df8066 /src | |
parent | a95a07cea06fc8d354ce9de8b80ef4f91fbfe23c (diff) | |
download | openbsd-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.c | 15 | ||||
-rw-r--r-- | src/lib/libcrypto/evp/e_chacha20poly1305.c | 14 | ||||
-rw-r--r-- | src/lib/libcrypto/evp/evp_lib.c | 17 | ||||
-rw-r--r-- | src/lib/libcrypto/evp/evp_local.h | 8 |
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) | |||
266 | int | 266 | int |
267 | EVP_CIPHER_CTX_iv_length(const EVP_CIPHER_CTX *ctx) | 267 | EVP_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 | ||
272 | unsigned char * | 285 | unsigned 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(). |