From cf506f17d0fe51a43abcc37aecb63601b70218ef Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 25 May 2022 12:10:15 -0400 Subject: [PATCH] Make EVP_CIPHER opaque. If we're to have any hope of fixing EVP_CIPHER_CTX's calling convention, we need to be able to change the shape of its method table. Looking back, it looks like we exported this in https://boringssl-review.googlesource.com/4330, for OpenSSH. I don't remember exactly what OpenSSH was doing, but I see in this commit, they removed a bunch of custom EVP_CIPHERs which would definitely have required an exported EVP_CIPHER struct: https://github.com/openssh/openssh-portable/commit/cdccebdf85204bf7542b7fcc1aa2ea3f36661833 That's been gone for a while now, so hopefully we can hide it again. (If a project needs a cipher not implemented by OpenSSL, it's not strictly necessarily to make a custom EVP_CIPHER. It might be convenient to reuse the abstraction, but you can always just call your own APIs directly.) Update-Note: EVP_CIPHER is now opaque. Use accessors instead. Bug: 494 Change-Id: I9344690c3cfe7d19d6ca12fb66484ced57dbe869 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/52725 Reviewed-by: Bob Beck Commit-Queue: Bob Beck --- crypto/cipher_extra/derive_key.c | 8 +++--- crypto/cipher_extra/e_des.c | 1 + crypto/cipher_extra/e_null.c | 1 + crypto/cipher_extra/e_rc2.c | 1 + crypto/cipher_extra/e_rc4.c | 2 ++ crypto/fipsmodule/cipher/internal.h | 39 +++++++++++++++++++++++++++++ decrepit/blowfish/blowfish.c | 1 + decrepit/cast/cast.c | 1 + decrepit/cfb/cfb.c | 1 + decrepit/xts/xts.c | 3 ++- include/openssl/cipher.h | 39 ----------------------------- 11 files changed, 53 insertions(+), 44 deletions(-) diff --git a/crypto/cipher_extra/derive_key.c b/crypto/cipher_extra/derive_key.c index 45b496374..4b84c4ebc 100644 --- a/crypto/cipher_extra/derive_key.c +++ b/crypto/cipher_extra/derive_key.c @@ -69,12 +69,12 @@ int EVP_BytesToKey(const EVP_CIPHER *type, const EVP_MD *md, unsigned count, uint8_t *key, uint8_t *iv) { EVP_MD_CTX c; uint8_t md_buf[EVP_MAX_MD_SIZE]; - unsigned niv, nkey, addmd = 0; + unsigned addmd = 0; unsigned mds = 0, i; int rv = 0; - nkey = type->key_len; - niv = type->iv_len; + unsigned nkey = EVP_CIPHER_key_length(type); + unsigned niv = EVP_CIPHER_iv_length(type); assert(nkey <= EVP_MAX_KEY_LENGTH); assert(niv <= EVP_MAX_IV_LENGTH); @@ -143,7 +143,7 @@ int EVP_BytesToKey(const EVP_CIPHER *type, const EVP_MD *md, break; } } - rv = type->key_len; + rv = EVP_CIPHER_key_length(type); err: EVP_MD_CTX_cleanup(&c); diff --git a/crypto/cipher_extra/e_des.c b/crypto/cipher_extra/e_des.c index 087029bd2..300ec00e6 100644 --- a/crypto/cipher_extra/e_des.c +++ b/crypto/cipher_extra/e_des.c @@ -58,6 +58,7 @@ #include #include +#include "../fipsmodule/cipher/internal.h" #include "internal.h" diff --git a/crypto/cipher_extra/e_null.c b/crypto/cipher_extra/e_null.c index f5fe8fb7d..e9ee8adc7 100644 --- a/crypto/cipher_extra/e_null.c +++ b/crypto/cipher_extra/e_null.c @@ -60,6 +60,7 @@ #include +#include "../fipsmodule/cipher/internal.h" #include "../internal.h" diff --git a/crypto/cipher_extra/e_rc2.c b/crypto/cipher_extra/e_rc2.c index 221a9c923..ffc5e6b1d 100644 --- a/crypto/cipher_extra/e_rc2.c +++ b/crypto/cipher_extra/e_rc2.c @@ -57,6 +57,7 @@ #include #include +#include "../fipsmodule/cipher/internal.h" #include "../internal.h" diff --git a/crypto/cipher_extra/e_rc4.c b/crypto/cipher_extra/e_rc4.c index e7c2ccaff..2f4f9bbac 100644 --- a/crypto/cipher_extra/e_rc4.c +++ b/crypto/cipher_extra/e_rc4.c @@ -61,6 +61,8 @@ #include #include +#include "../fipsmodule/cipher/internal.h" + static int rc4_init_key(EVP_CIPHER_CTX *ctx, const uint8_t *key, const uint8_t *iv, int enc) { diff --git a/crypto/fipsmodule/cipher/internal.h b/crypto/fipsmodule/cipher/internal.h index 68efe33d4..6ec9a3b3c 100644 --- a/crypto/fipsmodule/cipher/internal.h +++ b/crypto/fipsmodule/cipher/internal.h @@ -112,6 +112,45 @@ struct evp_aead_st { size_t extra_in_len); }; +struct evp_cipher_st { + // type contains a NID identifying the cipher. (e.g. NID_aes_128_gcm.) + int nid; + + // block_size contains the block size, in bytes, of the cipher, or 1 for a + // stream cipher. + unsigned block_size; + + // key_len contains the key size, in bytes, for the cipher. If the cipher + // takes a variable key size then this contains the default size. + unsigned key_len; + + // iv_len contains the IV size, in bytes, or zero if inapplicable. + unsigned iv_len; + + // ctx_size contains the size, in bytes, of the per-key context for this + // cipher. + unsigned ctx_size; + + // flags contains the OR of a number of flags. See |EVP_CIPH_*|. + uint32_t flags; + + // app_data is a pointer to opaque, user data. + void *app_data; + + int (*init)(EVP_CIPHER_CTX *ctx, const uint8_t *key, const uint8_t *iv, + int enc); + + int (*cipher)(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in, + size_t inl); + + // cleanup, if non-NULL, releases memory associated with the context. It is + // called if |EVP_CTRL_INIT| succeeds. Note that |init| may not have been + // called at this point. + void (*cleanup)(EVP_CIPHER_CTX *); + + int (*ctrl)(EVP_CIPHER_CTX *, int type, int arg, void *ptr); +}; + // aes_ctr_set_key initialises |*aes_key| using |key_bytes| bytes from |key|, // where |key_bytes| must either be 16, 24 or 32. If not NULL, |*out_block| is // set to a function that encrypts single blocks. If not NULL, |*gcm_key| is diff --git a/decrepit/blowfish/blowfish.c b/decrepit/blowfish/blowfish.c index aa872bc60..7c209da5d 100644 --- a/decrepit/blowfish/blowfish.c +++ b/decrepit/blowfish/blowfish.c @@ -61,6 +61,7 @@ #include #include +#include "../../crypto/fipsmodule/cipher/internal.h" #include "../../crypto/internal.h" #include "../macros.h" diff --git a/decrepit/cast/cast.c b/decrepit/cast/cast.c index dffee5c1f..314e3da21 100644 --- a/decrepit/cast/cast.c +++ b/decrepit/cast/cast.c @@ -64,6 +64,7 @@ OPENSSL_MSVC_PRAGMA(warning(push, 3)) OPENSSL_MSVC_PRAGMA(warning(pop)) #endif +#include "../../crypto/fipsmodule/cipher/internal.h" #include "../../crypto/internal.h" #include "internal.h" #include "../macros.h" diff --git a/decrepit/cfb/cfb.c b/decrepit/cfb/cfb.c index fa1cfd4fc..d23115357 100644 --- a/decrepit/cfb/cfb.c +++ b/decrepit/cfb/cfb.c @@ -19,6 +19,7 @@ #include #include +#include "../../crypto/fipsmodule/cipher/internal.h" #include "../../crypto/internal.h" typedef struct { diff --git a/decrepit/xts/xts.c b/decrepit/xts/xts.c index a433c3beb..8a66f0f22 100644 --- a/decrepit/xts/xts.c +++ b/decrepit/xts/xts.c @@ -53,7 +53,8 @@ #include #include -#include "../crypto/fipsmodule/modes/internal.h" +#include "../../crypto/fipsmodule/cipher/internal.h" +#include "../../crypto/fipsmodule/modes/internal.h" typedef struct xts128_context { diff --git a/include/openssl/cipher.h b/include/openssl/cipher.h index 2458847e5..c19b78de3 100644 --- a/include/openssl/cipher.h +++ b/include/openssl/cipher.h @@ -582,45 +582,6 @@ typedef struct evp_cipher_info_st { unsigned char iv[EVP_MAX_IV_LENGTH]; } EVP_CIPHER_INFO; -struct evp_cipher_st { - // type contains a NID identifing the cipher. (e.g. NID_aes_128_gcm.) - int nid; - - // block_size contains the block size, in bytes, of the cipher, or 1 for a - // stream cipher. - unsigned block_size; - - // key_len contains the key size, in bytes, for the cipher. If the cipher - // takes a variable key size then this contains the default size. - unsigned key_len; - - // iv_len contains the IV size, in bytes, or zero if inapplicable. - unsigned iv_len; - - // ctx_size contains the size, in bytes, of the per-key context for this - // cipher. - unsigned ctx_size; - - // flags contains the OR of a number of flags. See |EVP_CIPH_*|. - uint32_t flags; - - // app_data is a pointer to opaque, user data. - void *app_data; - - int (*init)(EVP_CIPHER_CTX *ctx, const uint8_t *key, const uint8_t *iv, - int enc); - - int (*cipher)(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in, - size_t inl); - - // cleanup, if non-NULL, releases memory associated with the context. It is - // called if |EVP_CTRL_INIT| succeeds. Note that |init| may not have been - // called at this point. - void (*cleanup)(EVP_CIPHER_CTX *); - - int (*ctrl)(EVP_CIPHER_CTX *, int type, int arg, void *ptr); -}; - #if defined(__cplusplus) } // extern C