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.

diff --git a/lib/libcrypto/evp/e_aes.c b/lib/libcrypto/evp/e_aes.c
index 3d3b1a9..3d357f0 100644 (file)
--- 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 33d0931..4a393c2 100644 (file)
--- 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 24ce196..f4e46ae 100644 (file)
--- 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 e0a8afd..015fbb5 100644 (file)
--- 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().