diff options
| author | tb <> | 2023-09-28 11:29:10 +0000 |
|---|---|---|
| committer | tb <> | 2023-09-28 11:29:10 +0000 |
| commit | 51b553d9a746433693f625d3f21184da15e95972 (patch) | |
| tree | 3522ab66513de919b4cb23f4f9a24a55b2df8066 /src | |
| parent | a285e96748b5df2a7df11ff2aba3404a633929e9 (diff) | |
| download | openbsd-51b553d9a746433693f625d3f21184da15e95972.tar.gz openbsd-51b553d9a746433693f625d3f21184da15e95972.tar.bz2 openbsd-51b553d9a746433693f625d3f21184da15e95972.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(). |
