From 38468fc0b3ae810f4c34795d72be82d6d496a9bf Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Fri, 2 Jul 2021 09:57:38 -0400 Subject: [PATCH 1/2] add pubkey Signed-off-by: Asra Ali --- .github/workflows/cpp.yml | 4 --- include/proxy-wasm/signature_util.h | 2 +- include/proxy-wasm/wasm.h | 6 ++-- src/signature_util.cc | 15 ++++------ src/wasm.cc | 21 ++++++++------ test/BUILD | 1 + test/signature_util_test.cc | 43 +++++++++++++++++++---------- 7 files changed, 52 insertions(+), 40 deletions(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 9e58e997..b7e1ef5e 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -81,7 +81,3 @@ jobs: - name: Test run: | bazel test --define runtime=${{ matrix.runtime }} //test/... - - - name: Test (signed Wasm module) - run: | - bazel test --define runtime=${{ matrix.runtime }} --per_file_copt=//...@-DPROXY_WASM_VERIFY_WITH_ED25519_PUBKEY=\"$(xxd -p -c 256 test/test_data/signature_key1.pub | cut -b9-)\" //test:signature_util_test diff --git a/include/proxy-wasm/signature_util.h b/include/proxy-wasm/signature_util.h index 68579dcd..e6eb0fd9 100644 --- a/include/proxy-wasm/signature_util.h +++ b/include/proxy-wasm/signature_util.h @@ -27,7 +27,7 @@ class SignatureUtil { * @param message is the reference to store the message (success or error). * @return indicates whether the bytecode has a valid Wasm signature. */ - static bool verifySignature(std::string_view bytecode, std::string &message); + static bool verifySignature(std::string_view bytecode, const std::string pubkey, std::string &message); }; } // namespace proxy_wasm diff --git a/include/proxy-wasm/wasm.h b/include/proxy-wasm/wasm.h index 9accb4c7..6ca0eddd 100644 --- a/include/proxy-wasm/wasm.h +++ b/include/proxy-wasm/wasm.h @@ -58,7 +58,7 @@ class WasmBase : public std::enable_shared_from_this { WasmBase(const std::shared_ptr &other, WasmVmFactory factory); virtual ~WasmBase(); - bool load(const std::string &code, bool allow_precompiled = false); + bool load(const std::string &code, bool allow_precompiled = false, const std::string pubkey = ""); bool initialize(); void startVm(ContextBase *root_context); bool configure(ContextBase *root_context, std::shared_ptr plugin); @@ -311,8 +311,10 @@ using WasmHandleCloneFactory = std::function(std::shared_ptr wasm)>; // Returns nullptr on failure (i.e. initialization of the VM fails). +// TODO: Consider a VerificationOptions struct rather than a single pubkey. std::shared_ptr -createWasm(std::string vm_key, std::string code, std::shared_ptr plugin, + createWasm(std::string vm_key, std::string code, std::string pubkey, + std::shared_ptr plugin, WasmHandleFactory factory, WasmHandleCloneFactory clone_factory, bool allow_precompiled); // Get an existing ThreadLocal VM matching 'vm_id' or nullptr if there isn't one. std::shared_ptr getThreadLocalWasm(std::string_view vm_id); diff --git a/src/signature_util.cc b/src/signature_util.cc index 71b1341c..8c1a8ed4 100644 --- a/src/signature_util.cc +++ b/src/signature_util.cc @@ -24,7 +24,6 @@ namespace { -#ifdef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY static uint8_t hex2dec(const unsigned char c) { if (c >= '0' && c <= '9') { @@ -46,15 +45,13 @@ template constexpr std::array hex2pubkey(const char (&hex return pubkey; } -#endif } // namespace namespace proxy_wasm { -bool SignatureUtil::verifySignature(std::string_view bytecode, std::string &message) { - -#ifdef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY +bool SignatureUtil::verifySignature(std::string_view bytecode, std::string pubkey, std::string &message) { + if (!pubkey.empty()) { /* * Ed25519 signature generated using https://github.com/jedisct1/wasmsign @@ -101,7 +98,9 @@ bool SignatureUtil::verifySignature(std::string_view bytecode, std::string &mess uint8_t hash[SHA512_DIGEST_LENGTH]; SHA512_Final(hash, &ctx); - static const auto ed25519_pubkey = hex2pubkey<32>(PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY); + char pubkey_char[65]; + strcpy(pubkey_char, pubkey.c_str()); + static const auto ed25519_pubkey = hex2pubkey<32>(pubkey_char); if (!ED25519_verify(hash, sizeof(hash), signature, ed25519_pubkey.data())) { message = "Signature mismatch"; @@ -110,9 +109,7 @@ bool SignatureUtil::verifySignature(std::string_view bytecode, std::string &mess message = "Wasm signature OK (Ed25519)"; return true; - -#endif - + } return true; } diff --git a/src/wasm.cc b/src/wasm.cc index 85a7b1e7..2870b618 100644 --- a/src/wasm.cc +++ b/src/wasm.cc @@ -234,7 +234,7 @@ WasmBase::~WasmBase() { pending_delete_.clear(); } -bool WasmBase::load(const std::string &code, bool allow_precompiled) { +bool WasmBase::load(const std::string &code, bool allow_precompiled, const std::string pubkey) { assert(!started_from_.has_value()); if (!wasm_vm_) { @@ -251,13 +251,15 @@ bool WasmBase::load(const std::string &code, bool allow_precompiled) { return true; } - // Verify signature. - std::string message; - if (!SignatureUtil::verifySignature(code, message)) { - fail(FailState::UnableToInitializeCode, message); - return false; - } else { - wasm_vm_->integration()->trace(message); + // Verify signature if a pubkey is present. + if (!pubkey.empty()) { + std::string message; + if (!SignatureUtil::verifySignature(code, pubkey, message)) { + fail(FailState::UnableToInitializeCode, message); + return false; + } else { + wasm_vm_->integration()->trace(message); + } } // Get ABI version from the module. @@ -465,6 +467,7 @@ WasmForeignFunction WasmBase::getForeignFunction(std::string_view function_name) } std::shared_ptr createWasm(std::string vm_key, std::string code, + std::string pubkey, std::shared_ptr plugin, WasmHandleFactory factory, WasmHandleCloneFactory clone_factory, @@ -492,7 +495,7 @@ std::shared_ptr createWasm(std::string vm_key, std::string code, (*base_wasms)[vm_key] = wasm_handle; } - if (!wasm_handle->wasm()->load(code, allow_precompiled)) { + if (!wasm_handle->wasm()->load(code, allow_precompiled, pubkey)) { wasm_handle->wasm()->fail(FailState::UnableToInitializeCode, "Failed to load Wasm code"); return nullptr; } diff --git a/test/BUILD b/test/BUILD index 132404b3..359b0cd2 100644 --- a/test/BUILD +++ b/test/BUILD @@ -35,6 +35,7 @@ cc_test( "//test/test_data:abi_export.signed.with.key1.wasm", "//test/test_data:abi_export.signed.with.key2.wasm", "//test/test_data:abi_export.wasm", + "//test/test_data:signature_key1.pub", ], # Test only when compiled to verify plugins. tags = ["manual"], diff --git a/test/signature_util_test.cc b/test/signature_util_test.cc index f4b2ae24..3ba2aa9d 100644 --- a/test/signature_util_test.cc +++ b/test/signature_util_test.cc @@ -24,36 +24,49 @@ namespace proxy_wasm { -TEST(TestSignatureUtil, GoodSignature) { -#ifndef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY - FAIL() << "Built without a key for verifying signed Wasm modules."; -#endif +std::string BytesToHex(std::vector bytes) { + static const char *const hex = "0123456789ABCDEF"; + std::string result; + result.reserve(bytes.size() * 2); + for (auto byte : bytes) { + result.push_back(hex[byte >> 4]); + result.push_back(hex[byte & 0xf]); + } + return result; +} + +static std::string publickey(std::string filename) { + auto path = "test/test_data/" + filename; + std::ifstream file(path, std::ios::binary); + EXPECT_FALSE(file.fail()) << "failed to open: " << path; + std::stringstream file_string_stream; + file_string_stream << file.rdbuf(); + std::string input = file_string_stream.str(); + std::vector bytes(input.begin() + 4, input.end()); + return BytesToHex(bytes); +} +TEST(TestSignatureUtil, GoodSignature) { + std::string pubkey = publickey("signature_key1.pub"); const auto bytecode = readTestWasmFile("abi_export.signed.with.key1.wasm"); std::string message; - EXPECT_TRUE(SignatureUtil::verifySignature(bytecode, message)); + EXPECT_TRUE(SignatureUtil::verifySignature(bytecode, pubkey, message)); EXPECT_EQ(message, "Wasm signature OK (Ed25519)"); } TEST(TestSignatureUtil, BadSignature) { -#ifndef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY - FAIL() << "Built without a key for verifying signed Wasm modules."; -#endif - + std::string pubkey = publickey("signature_key1.pub"); const auto bytecode = readTestWasmFile("abi_export.signed.with.key2.wasm"); std::string message; - EXPECT_FALSE(SignatureUtil::verifySignature(bytecode, message)); + EXPECT_FALSE(SignatureUtil::verifySignature(bytecode, pubkey, message)); EXPECT_EQ(message, "Signature mismatch"); } TEST(TestSignatureUtil, NoSignature) { -#ifndef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY - FAIL() << "Built without a key for verifying signed Wasm modules."; -#endif - + std::string pubkey = publickey("signature_key1.pub"); const auto bytecode = readTestWasmFile("abi_export.wasm"); std::string message; - EXPECT_FALSE(SignatureUtil::verifySignature(bytecode, message)); + EXPECT_FALSE(SignatureUtil::verifySignature(bytecode, pubkey, message)); EXPECT_EQ(message, "Custom Section \"signature_wasmsign\" not found"); } From 884a366037339339831af922c4b039376c34c4eb Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Thu, 12 Aug 2021 10:37:50 -0400 Subject: [PATCH 2/2] fix comments Signed-off-by: Asra Ali --- .github/workflows/cpp.yml | 5 ++ src/signature_util.cc | 130 +++++++++++++++++++----------------- src/wasm.cc | 14 ++-- test/BUILD | 1 + test/signature_util_test.cc | 23 +++++++ 5 files changed, 105 insertions(+), 68 deletions(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index e64ad806..c3d3f21d 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -84,3 +84,8 @@ jobs: - name: Test run: | bazel test --define runtime=${{ matrix.runtime }} //test/... + + - name: Test (signed Wasm module) + run: | + bazel test --define runtime=${{ matrix.runtime }} --per_file_copt=//...@-DPROXY_WASM_VERIFY_WITH_ED25519_PUBKEY=\"$(xxd -p -c 256 test/test_data/signature_key1.pub | cut -b9-)\" //test:signature_util_test + diff --git a/src/signature_util.cc b/src/signature_util.cc index 8c1a8ed4..61ef6c89 100644 --- a/src/signature_util.cc +++ b/src/signature_util.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -24,7 +25,6 @@ namespace { - static uint8_t hex2dec(const unsigned char c) { if (c >= '0' && c <= '9') { return c - '0'; @@ -45,70 +45,80 @@ template constexpr std::array hex2pubkey(const char (&hex return pubkey; } - -} // namespace - -namespace proxy_wasm { - -bool SignatureUtil::verifySignature(std::string_view bytecode, std::string pubkey, std::string &message) { +std::optional> getEd25519PubKey(std::string pubkey) { +#ifdef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY + static const auto ed25519_pubkey = hex2pubkey<32>(PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY); + return ed25519_pubkey; +#else if (!pubkey.empty()) { - - /* - * Ed25519 signature generated using https://github.com/jedisct1/wasmsign - */ - - std::string_view payload; - if (!BytecodeUtil::getCustomSection(bytecode, "signature_wasmsign", payload)) { - message = "Failed to parse corrupted Wasm module"; - return false; + char pubkey_char[65]; + strcpy(pubkey_char, pubkey.c_str()); + return hex2pubkey<32>(pubkey_char); } +#endif + return std::nullopt; +} - if (payload.empty()) { - message = "Custom Section \"signature_wasmsign\" not found"; - return false; - } - - if (bytecode.data() + bytecode.size() != payload.data() + payload.size()) { - message = "Custom Section \"signature_wasmsign\" not at the end of Wasm module"; - return false; - } - - if (payload.size() != 68) { - message = "Signature has a wrong size (want: 68, is: " + std::to_string(payload.size()) + ")"; - return false; - } - - uint32_t alg_id; - std::memcpy(&alg_id, payload.data(), sizeof(uint32_t)); - - if (alg_id != 2) { - message = "Signature has a wrong alg_id (want: 2, is: " + std::to_string(alg_id) + ")"; - return false; - } +} // namespace - const auto *signature = reinterpret_cast(payload.data()) + sizeof(uint32_t); - - SHA512_CTX ctx; - SHA512_Init(&ctx); - SHA512_Update(&ctx, "WasmSignature", sizeof("WasmSignature") - 1); - const uint32_t ad_len = 0; - SHA512_Update(&ctx, &ad_len, sizeof(uint32_t)); - const size_t section_len = 3 + sizeof("signature_wasmsign") - 1 + 68; - SHA512_Update(&ctx, bytecode.data(), bytecode.size() - section_len); - uint8_t hash[SHA512_DIGEST_LENGTH]; - SHA512_Final(hash, &ctx); - - char pubkey_char[65]; - strcpy(pubkey_char, pubkey.c_str()); - static const auto ed25519_pubkey = hex2pubkey<32>(pubkey_char); - - if (!ED25519_verify(hash, sizeof(hash), signature, ed25519_pubkey.data())) { - message = "Signature mismatch"; - return false; - } +namespace proxy_wasm { - message = "Wasm signature OK (Ed25519)"; - return true; +bool SignatureUtil::verifySignature(std::string_view bytecode, std::string pubkey, + std::string &message) { + const auto ed25519_pubkey = getEd25519PubKey(pubkey); + if (ed25519_pubkey) { + /* + * Ed25519 signature generated using https://github.com/jedisct1/wasmsign + */ + + std::string_view payload; + if (!BytecodeUtil::getCustomSection(bytecode, "signature_wasmsign", payload)) { + message = "Failed to parse corrupted Wasm module"; + return false; + } + + if (payload.empty()) { + message = "Custom Section \"signature_wasmsign\" not found"; + return false; + } + + if (bytecode.data() + bytecode.size() != payload.data() + payload.size()) { + message = "Custom Section \"signature_wasmsign\" not at the end of Wasm module"; + return false; + } + + if (payload.size() != 68) { + message = "Signature has a wrong size (want: 68, is: " + std::to_string(payload.size()) + ")"; + return false; + } + + uint32_t alg_id; + std::memcpy(&alg_id, payload.data(), sizeof(uint32_t)); + + if (alg_id != 2) { + message = "Signature has a wrong alg_id (want: 2, is: " + std::to_string(alg_id) + ")"; + return false; + } + + const auto *signature = reinterpret_cast(payload.data()) + sizeof(uint32_t); + + SHA512_CTX ctx; + SHA512_Init(&ctx); + SHA512_Update(&ctx, "WasmSignature", sizeof("WasmSignature") - 1); + const uint32_t ad_len = 0; + SHA512_Update(&ctx, &ad_len, sizeof(uint32_t)); + const size_t section_len = 3 + sizeof("signature_wasmsign") - 1 + 68; + SHA512_Update(&ctx, bytecode.data(), bytecode.size() - section_len); + uint8_t hash[SHA512_DIGEST_LENGTH]; + SHA512_Final(hash, &ctx); + + if (!ED25519_verify(hash, sizeof(hash), signature, ed25519_pubkey.value().data())) { + message = "Signature mismatch"; + return false; + } + + message = "Wasm signature OK (Ed25519)"; + return true; } return true; } diff --git a/src/wasm.cc b/src/wasm.cc index 08dcd58f..938f2b59 100644 --- a/src/wasm.cc +++ b/src/wasm.cc @@ -250,14 +250,12 @@ bool WasmBase::load(const std::string &code, bool allow_precompiled, const std:: } // Verify signature if a pubkey is present. - if (!pubkey.empty()) { - std::string message; - if (!SignatureUtil::verifySignature(code, pubkey, message)) { - fail(FailState::UnableToInitializeCode, message); - return false; - } else { - wasm_vm_->integration()->trace(message); - } + std::string message; + if (!SignatureUtil::verifySignature(code, pubkey, message)) { + fail(FailState::UnableToInitializeCode, message); + return false; + } else { + wasm_vm_->integration()->trace(message); } // Get ABI version from the module. diff --git a/test/BUILD b/test/BUILD index 0c14b4b8..4a3284c5 100644 --- a/test/BUILD +++ b/test/BUILD @@ -36,6 +36,7 @@ cc_test( "//test/test_data:abi_export.signed.with.key2.wasm", "//test/test_data:abi_export.wasm", "//test/test_data:signature_key1.pub", + "//test/test_data:signature_key2.pub", ], # Test only when compiled to verify plugins. tags = ["manual"], diff --git a/test/signature_util_test.cc b/test/signature_util_test.cc index 3ba2aa9d..b74868b3 100644 --- a/test/signature_util_test.cc +++ b/test/signature_util_test.cc @@ -46,6 +46,15 @@ static std::string publickey(std::string filename) { return BytesToHex(bytes); } +TEST(TestSignatureUtil, NoPublicKey) { +#ifndef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY + const auto bytecode = readTestWasmFile("abi_export.signed.with.key1.wasm"); + std::string message; + EXPECT_TRUE(SignatureUtil::verifySignature(bytecode, "", message)); + EXPECT_EQ(message, ""); +#endif +} + TEST(TestSignatureUtil, GoodSignature) { std::string pubkey = publickey("signature_key1.pub"); const auto bytecode = readTestWasmFile("abi_export.signed.with.key1.wasm"); @@ -62,6 +71,20 @@ TEST(TestSignatureUtil, BadSignature) { EXPECT_EQ(message, "Signature mismatch"); } +TEST(TestSignatureUtil, BuildTimeKeyPrecedence) { + // Uses hard-coded key if defined. + std::string pubkey = publickey("signature_key2.pub"); + const auto bytecode = readTestWasmFile("abi_export.signed.with.key1.wasm"); + std::string message; +#ifdef PROXY_WASM_VERIFY_WITH_ED25519_PUBKEY + EXPECT_TRUE(SignatureUtil::verifySignature(bytecode, pubkey, message)); + EXPECT_EQ(message, "Wasm signature OK (Ed25519)"); +#else + EXPECT_FALSE(SignatureUtil::verifySignature(bytecode, pubkey, message)); + EXPECT_EQ(message, "Signature mismatch"); +#endif +} + TEST(TestSignatureUtil, NoSignature) { std::string pubkey = publickey("signature_key1.pub"); const auto bytecode = readTestWasmFile("abi_export.wasm");