Keep EVP_CIPHER/EVP_MD lookup and do_all functions in sync
Node seems uncommonly sensitive to this, so let's write these functions
in a way that stays in sync and test this. See also
https://boringssl-review.googlesource.com/c/boringssl/+/49585
This does incur a cost across all BoringSSL consumers that use these
functions: as a result of Node indiscriminately exposing every cipher,
we end up pulling more and more ciphers into these getters. But that
ship sailed long ago, so, instead, document that EVP_get_cipherby*
should not be used by size-conscious callers.
EVP_get_digestby* probably should have the same warning, but I've left
it alone for now because we don't quite have the same proliferation of
digests as ciphers. (Though there are things in there, like MD4, that
ought to be better disconnected.)
Change-Id: I61ca406c146279bd05a52bed6c57200d1619c5da
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49625
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/crypto/cipher_extra/cipher_extra.c b/crypto/cipher_extra/cipher_extra.c
index 786a5d5..62850ab 100644
--- a/crypto/cipher_extra/cipher_extra.c
+++ b/crypto/cipher_extra/cipher_extra.c
@@ -67,25 +67,42 @@
#include "../internal.h"
+static const struct {
+ int nid;
+ const char *name;
+ const EVP_CIPHER *(*func)(void);
+} kCiphers[] = {
+ {NID_aes_128_cbc, "aes-128-cbc", EVP_aes_128_cbc},
+ {NID_aes_128_ctr, "aes-128-ctr", EVP_aes_128_ctr},
+ {NID_aes_128_ecb, "aes-128-ecb", EVP_aes_128_ecb},
+ {NID_aes_128_gcm, "aes-128-gcm", EVP_aes_128_gcm},
+ {NID_aes_128_ofb128, "aes-128-ofb", EVP_aes_128_ofb},
+ {NID_aes_192_cbc, "aes-192-cbc", EVP_aes_192_cbc},
+ {NID_aes_192_ctr, "aes-192-ctr", EVP_aes_192_ctr},
+ {NID_aes_192_ecb, "aes-192-ecb", EVP_aes_192_ecb},
+ {NID_aes_192_gcm, "aes-192-gcm", EVP_aes_192_gcm},
+ {NID_aes_192_ofb128, "aes-192-ofb", EVP_aes_192_ofb},
+ {NID_aes_256_cbc, "aes-256-cbc", EVP_aes_256_cbc},
+ {NID_aes_256_ctr, "aes-256-ctr", EVP_aes_256_ctr},
+ {NID_aes_256_ecb, "aes-256-ecb", EVP_aes_256_ecb},
+ {NID_aes_256_gcm, "aes-256-gcm", EVP_aes_256_gcm},
+ {NID_aes_256_ofb128, "aes-256-ofb", EVP_aes_256_ofb},
+ {NID_des_cbc, "des-cbc", EVP_des_cbc},
+ {NID_des_ecb, "des-ecb", EVP_des_ecb},
+ {NID_des_ede_cbc, "des-ede-cbc", EVP_des_ede_cbc},
+ {NID_des_ede_ecb, "des-ede", EVP_des_ede},
+ {NID_des_ede3_cbc, "des-ede3-cbc", EVP_des_ede3_cbc},
+ {NID_rc2_cbc, "rc2-cbc", EVP_rc2_cbc},
+ {NID_rc4, "rc4", EVP_rc4},
+};
+
const EVP_CIPHER *EVP_get_cipherbynid(int nid) {
- switch (nid) {
- case NID_rc2_cbc:
- return EVP_rc2_cbc();
- case NID_rc2_40_cbc:
- return EVP_rc2_40_cbc();
- case NID_des_ede3_cbc:
- return EVP_des_ede3_cbc();
- case NID_des_ede_cbc:
- return EVP_des_cbc();
- case NID_aes_128_cbc:
- return EVP_aes_128_cbc();
- case NID_aes_192_cbc:
- return EVP_aes_192_cbc();
- case NID_aes_256_cbc:
- return EVP_aes_256_cbc();
- default:
- return NULL;
+ for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(kCiphers); i++) {
+ if (kCiphers[i].nid == nid) {
+ return kCiphers[i].func();
+ }
}
+ return NULL;
}
const EVP_CIPHER *EVP_get_cipherbyname(const char *name) {
@@ -93,54 +110,17 @@
return NULL;
}
- if (OPENSSL_strcasecmp(name, "rc4") == 0) {
- return EVP_rc4();
- } else if (OPENSSL_strcasecmp(name, "des-cbc") == 0) {
- return EVP_des_cbc();
- } else if (OPENSSL_strcasecmp(name, "des-ede3-cbc") == 0 ||
- // This is not a name used by OpenSSL, but tcpdump registers it
- // with |EVP_add_cipher_alias|. Our |EVP_add_cipher_alias| is a
- // no-op, so we support the name here.
- OPENSSL_strcasecmp(name, "3des") == 0) {
- return EVP_des_ede3_cbc();
- } else if (OPENSSL_strcasecmp(name, "aes-128-cbc") == 0) {
- return EVP_aes_128_cbc();
- } else if (OPENSSL_strcasecmp(name, "aes-192-cbc") == 0) {
- return EVP_aes_192_cbc();
- } else if (OPENSSL_strcasecmp(name, "aes-256-cbc") == 0) {
- return EVP_aes_256_cbc();
- } else if (OPENSSL_strcasecmp(name, "aes-128-ctr") == 0) {
- return EVP_aes_128_ctr();
- } else if (OPENSSL_strcasecmp(name, "aes-192-ctr") == 0) {
- return EVP_aes_192_ctr();
- } else if (OPENSSL_strcasecmp(name, "aes-256-ctr") == 0) {
- return EVP_aes_256_ctr();
- } else if (OPENSSL_strcasecmp(name, "aes-128-ecb") == 0) {
- return EVP_aes_128_ecb();
- } else if (OPENSSL_strcasecmp(name, "aes-192-ecb") == 0) {
- return EVP_aes_192_ecb();
- } else if (OPENSSL_strcasecmp(name, "aes-256-ecb") == 0) {
- return EVP_aes_256_ecb();
- } else if (OPENSSL_strcasecmp(name, "aes-128-gcm") == 0) {
- return EVP_aes_128_gcm();
- } else if (OPENSSL_strcasecmp(name, "aes-192-gcm") == 0) {
- return EVP_aes_192_gcm();
- } else if (OPENSSL_strcasecmp(name, "aes-256-gcm") == 0) {
- return EVP_aes_256_gcm();
- } else if (OPENSSL_strcasecmp(name, "aes-128-ofb") == 0) {
- return EVP_aes_128_ofb();
- } else if (OPENSSL_strcasecmp(name, "aes-192-ofb") == 0) {
- return EVP_aes_192_ofb();
- } else if (OPENSSL_strcasecmp(name, "aes-256-ofb") == 0) {
- return EVP_aes_256_ofb();
- } else if (OPENSSL_strcasecmp(name, "des-ecb") == 0) {
- return EVP_des_ecb();
- } else if (OPENSSL_strcasecmp(name, "des-ede") == 0) {
- return EVP_des_ede();
- } else if (OPENSSL_strcasecmp(name, "des-ede-cbc") == 0) {
- return EVP_des_ede_cbc();
- } else if (OPENSSL_strcasecmp(name, "rc2-cbc") == 0) {
- return EVP_rc2_cbc();
+ // This is not a name used by OpenSSL, but tcpdump registers it with
+ // |EVP_add_cipher_alias|. Our |EVP_add_cipher_alias| is a no-op, so we
+ // support the name here.
+ if (OPENSSL_strcasecmp(name, "3des") == 0) {
+ name = "des-ede3-cbc";
+ }
+
+ for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(kCiphers); i++) {
+ if (OPENSSL_strcasecmp(kCiphers[i].name, name) == 0) {
+ return kCiphers[i].func();
+ }
}
return NULL;
diff --git a/crypto/cipher_extra/cipher_test.cc b/crypto/cipher_extra/cipher_test.cc
index 57fdc8a..ad939e2 100644
--- a/crypto/cipher_extra/cipher_test.cc
+++ b/crypto/cipher_extra/cipher_test.cc
@@ -524,3 +524,22 @@
}
}
}
+
+TEST(CipherTest, GetCipher) {
+ const EVP_CIPHER *cipher = EVP_get_cipherbynid(NID_aes_128_gcm);
+ ASSERT_TRUE(cipher);
+ EXPECT_EQ(NID_aes_128_gcm, EVP_CIPHER_nid(cipher));
+
+ cipher = EVP_get_cipherbyname("aes-128-gcm");
+ ASSERT_TRUE(cipher);
+ EXPECT_EQ(NID_aes_128_gcm, EVP_CIPHER_nid(cipher));
+
+ cipher = EVP_get_cipherbyname("AES-128-GCM");
+ ASSERT_TRUE(cipher);
+ EXPECT_EQ(NID_aes_128_gcm, EVP_CIPHER_nid(cipher));
+
+ // We support a tcpdump-specific alias for 3DES.
+ cipher = EVP_get_cipherbyname("3des");
+ ASSERT_TRUE(cipher);
+ EXPECT_EQ(NID_des_ede3_cbc, EVP_CIPHER_nid(cipher));
+}
diff --git a/decrepit/CMakeLists.txt b/decrepit/CMakeLists.txt
index ef95a6b..16985da 100644
--- a/decrepit/CMakeLists.txt
+++ b/decrepit/CMakeLists.txt
@@ -32,6 +32,7 @@
blowfish/blowfish_test.cc
cast/cast_test.cc
cfb/cfb_test.cc
+ evp/evp_test.cc
ripemd/ripemd_test.cc
xts/xts_test.cc
diff --git a/decrepit/evp/evp_test.cc b/decrepit/evp/evp_test.cc
new file mode 100644
index 0000000..1f05ea5
--- /dev/null
+++ b/decrepit/evp/evp_test.cc
@@ -0,0 +1,45 @@
+/* Copyright (c) 2021, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+#include <gtest/gtest.h>
+
+#include <openssl/cipher.h>
+#include <openssl/digest.h>
+#include <openssl/evp.h>
+
+
+// Node.js assumes every cipher in |EVP_CIPHER_do_all_sorted| is accessible via
+// |EVP_get_cipherby*|.
+TEST(EVPTest, CipherDoAll) {
+ EVP_CIPHER_do_all_sorted(
+ [](const EVP_CIPHER *cipher, const char *name, const char *unused,
+ void *arg) {
+ SCOPED_TRACE(name);
+ EXPECT_EQ(cipher, EVP_get_cipherbyname(name));
+ EXPECT_EQ(cipher, EVP_get_cipherbynid(EVP_CIPHER_nid(cipher)));
+ },
+ nullptr);
+}
+
+// Node.js assumes every digest in |EVP_MD_do_all_sorted| is accessible via
+// |EVP_get_digestby*|.
+TEST(EVPTest, MDDoAll) {
+ EVP_MD_do_all_sorted(
+ [](const EVP_MD *md, const char *name, const char *unused, void *arg) {
+ SCOPED_TRACE(name);
+ EXPECT_EQ(md, EVP_get_digestbyname(name));
+ EXPECT_EQ(md, EVP_get_digestbynid(EVP_MD_nid(md)));
+ },
+ nullptr);
+}
diff --git a/include/openssl/cipher.h b/include/openssl/cipher.h
index badd496..09d72ec 100644
--- a/include/openssl/cipher.h
+++ b/include/openssl/cipher.h
@@ -106,7 +106,10 @@
const EVP_CIPHER *EVP_rc2_40_cbc(void);
// EVP_get_cipherbynid returns the cipher corresponding to the given NID, or
-// NULL if no such cipher is known.
+// NULL if no such cipher is known. Note using this function links almost every
+// cipher implemented by BoringSSL into the binary, whether the caller uses them
+// or not. Size-conscious callers, such as client software, should not use this
+// function.
OPENSSL_EXPORT const EVP_CIPHER *EVP_get_cipherbynid(int nid);
@@ -409,7 +412,10 @@
OPENSSL_EXPORT int EVP_add_cipher_alias(const char *a, const char *b);
// EVP_get_cipherbyname returns an |EVP_CIPHER| given a human readable name in
-// |name|, or NULL if the name is unknown.
+// |name|, or NULL if the name is unknown. Note using this function links almost
+// every cipher implemented by BoringSSL into the binary, not just the ones the
+// caller requests. Size-conscious callers, such as client software, should not
+// use this function.
OPENSSL_EXPORT const EVP_CIPHER *EVP_get_cipherbyname(const char *name);
// These AEADs are deprecated AES-GCM implementations that set
