From ea30c765268998bf548990fa2cff6cc05187a9db Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Fri, 21 Jun 2024 00:22:07 +0900 Subject: [PATCH 1/2] fix registerEnclaveKey to not allow empty signature when an AVR is dedicated to specific operator Signed-off-by: Jun Kimura --- contracts/LCPClientBase.sol | 6 +- test/LCPClientOperator.t.sol | 74 ++++++++++++++++--- ...0xc1eae5EF781f4EE5867eC6725630E7dC17fa3436 | 1 + 3 files changed, 69 insertions(+), 12 deletions(-) create mode 100644 test/data/reports/valid/operator_0xc1eae5EF781f4EE5867eC6725630E7dC17fa3436 diff --git a/contracts/LCPClientBase.sol b/contracts/LCPClientBase.sol index 7351c0d..18a967b 100644 --- a/contracts/LCPClientBase.sol +++ b/contracts/LCPClientBase.sol @@ -497,9 +497,9 @@ abstract contract LCPClientBase is ILightClient, ILCPClientErrors { operator = verifyECDSASignature( keccak256(LCPOperator.computeEIP712RegisterEnclaveKey(message.report)), message.operator_signature ); - if (reElems.operator != address(0) && reElems.operator != operator) { - revert LCPClientAVRUnexpectedOperator(operator, reElems.operator); - } + } + if (reElems.operator != address(0) && reElems.operator != operator) { + revert LCPClientAVRUnexpectedOperator(operator, reElems.operator); } uint64 expiredAt = reElems.attestationTime + clientStorage.clientState.key_expiration; if (expiredAt <= block.timestamp) { diff --git a/test/LCPClientOperator.t.sol b/test/LCPClientOperator.t.sol index 7b21e3a..9dddfd8 100644 --- a/test/LCPClientOperator.t.sol +++ b/test/LCPClientOperator.t.sol @@ -18,7 +18,6 @@ import {LCPOperator} from "../contracts/LCPOperator.sol"; contract LCPClientOperatorTest is BasicTest { using IBCHeight for Height.Data; - string internal constant commandAvrFile = "test/data/client/03/001-avr"; string internal constant commandResultSuffix = "_result"; LCPClient lc; @@ -28,9 +27,7 @@ contract LCPClientOperatorTest is BasicTest { lc = new LCPClient(address(this), true, vm.readFileBinary("./test/data/certs/simulation_rootca.der")); } - function avr(string memory filename) internal pure returns (string memory) { - return string(abi.encodePacked("test/data/client/03/", filename)); - } + // ---------------------------- Test Cases ---------------------------- function testPreComputationValues() public pure { assertEq(LCPOperator.domainSeparatorUniversal(), LCPOperator.DOMAIN_SEPARATOR_REGISTER_ENCLAVE_KEY); @@ -97,7 +94,7 @@ contract LCPClientOperatorTest is BasicTest { } string memory clientId = generateClientId(1); { - ClientState.Data memory clientState = createInitialState(commandAvrFile, operators, 2, 3); + ClientState.Data memory clientState = createInitialState(avr("001-avr"), operators, 2, 3); ConsensusState.Data memory consensusState; Height.Data memory height = lc.initializeClient( clientId, LCPProtoMarshaler.marshal(clientState), LCPProtoMarshaler.marshal(consensusState) @@ -167,7 +164,7 @@ contract LCPClientOperatorTest is BasicTest { } string memory clientId = generateClientId(1); { - ClientState.Data memory clientState = createInitialState(commandAvrFile, operators, 2, 3); + ClientState.Data memory clientState = createInitialState(avr("001-avr"), operators, 2, 3); ConsensusState.Data memory consensusState; Height.Data memory height = lc.initializeClient( clientId, LCPProtoMarshaler.marshal(clientState), LCPProtoMarshaler.marshal(consensusState) @@ -290,6 +287,63 @@ contract LCPClientOperatorTest is BasicTest { } } + function testRegisterEnclaveKeyOperatorDedicatedAVR() public { + Vm.Wallet[] memory wallets = createWallets(2); + address[] memory operators = new address[](1); + operators[0] = wallets[0].addr; + string memory avrFile = "test/data/reports/valid/operator_0xc1eae5EF781f4EE5867eC6725630E7dC17fa3436"; + + { + string memory clientId = generateClientId(1); + ClientState.Data memory clientState = createInitialState(avrFile, operators, 1, 1); + ConsensusState.Data memory consensusState; + lc.initializeClient( + clientId, LCPProtoMarshaler.marshal(clientState), LCPProtoMarshaler.marshal(consensusState) + ); + // operator matches the operator address in the AVR + lc.registerEnclaveKey(clientId, createRegisterEnclaveKeyMessage(avrFile, wallets[0].privateKey)); + } + { + string memory clientId = generateClientId(2); + ClientState.Data memory clientState = createInitialState(avrFile, operators, 1, 1); + ConsensusState.Data memory consensusState; + lc.initializeClient( + clientId, LCPProtoMarshaler.marshal(clientState), LCPProtoMarshaler.marshal(consensusState) + ); + + // operator does not match the operator address in the AVR + RegisterEnclaveKeyMessage.Data memory msg_ = createRegisterEnclaveKeyMessage(avrFile, wallets[1].privateKey); + vm.expectRevert( + abi.encodeWithSelector( + ILCPClientErrors.LCPClientAVRUnexpectedOperator.selector, wallets[1].addr, wallets[0].addr + ) + ); + lc.registerEnclaveKey(clientId, msg_); + } + { + string memory clientId = generateClientId(3); + ClientState.Data memory clientState = createInitialState(avrFile, operators, 1, 1); + ConsensusState.Data memory consensusState; + lc.initializeClient( + clientId, LCPProtoMarshaler.marshal(clientState), LCPProtoMarshaler.marshal(consensusState) + ); + // an operator dedicated AVR does not allow an empty signature + RegisterEnclaveKeyMessage.Data memory msg_ = createRegisterEnclaveKeyMessage(avrFile, 0); + vm.expectRevert( + abi.encodeWithSelector( + ILCPClientErrors.LCPClientAVRUnexpectedOperator.selector, address(0), wallets[0].addr + ) + ); + lc.registerEnclaveKey(clientId, msg_); + } + } + + // ---------------------------- Helper Functions ---------------------------- + + function avr(string memory filename) internal pure returns (string memory) { + return string(abi.encodePacked("test/data/client/03/", filename)); + } + function generateSignature(Vm.Wallet memory wallet, bytes32 commitment, bool valid) internal pure @@ -363,9 +417,11 @@ contract LCPClientOperatorTest is BasicTest { message.report = readJSON(avrFile, ".avr"); message.signature = readDecodedBytes(avrFile, ".signature"); message.signing_cert = readDecodedBytes(avrFile, ".signing_cert"); - (uint8 v, bytes32 r, bytes32 s) = - vm.sign(privateKey, keccak256(LCPOperatorTestHelper.computeEIP712RegisterEnclaveKey(message.report))); - message.operator_signature = abi.encodePacked(r, s, v); + if (privateKey != 0) { + (uint8 v, bytes32 r, bytes32 s) = + vm.sign(privateKey, keccak256(LCPOperatorTestHelper.computeEIP712RegisterEnclaveKey(message.report))); + message.operator_signature = abi.encodePacked(r, s, v); + } } function createUpdateClientMessage(string memory updateClientFilePrefix) diff --git a/test/data/reports/valid/operator_0xc1eae5EF781f4EE5867eC6725630E7dC17fa3436 b/test/data/reports/valid/operator_0xc1eae5EF781f4EE5867eC6725630E7dC17fa3436 new file mode 100644 index 0000000..65d7f77 --- /dev/null +++ b/test/data/reports/valid/operator_0xc1eae5EF781f4EE5867eC6725630E7dC17fa3436 @@ -0,0 +1 @@ +{"avr":"{\"id\":\"23856791181030202675484781740313693463\",\"timestamp\":\"2024-06-20T14:49:49.761000\",\"version\":4,\"advisoryURL\":\"https://security-center.intel.com\",\"advisoryIDs\":[],\"isvEnclaveQuoteStatus\":\"OK\",\"platformInfoBlob\":null,\"isvEnclaveQuoteBody\":\"AgAAAAsAAADvAO8A7+/v7wAAAAAAAAAAAAAAAAAAAACuQDx+HFlQ7r4EdJ0H5pZhSCDzN2rmsvIDTTt6S0ineAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABwAAAAAAAAAHAAAAAAAAADKAwI4ZtU0VypJeCmwT/tr/Kv9sCX+ThBq9jZ4aMHK4AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACD1xnnferKFHD2uvYqTXdDA8iZ22kCD5xw7h38CMfOngAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABhNeWsFyr39J51cViGUemN7CfskLB6uXveB9O5YZ+xnJWMOfcF/o0NgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"}","signature":"bN61lnClfXx7sQTuvwtaUsUAbJ8GFyI9v2HJBme+8/5XOiQiJbi/HisMtEzgHYWf3u/k3aVPJ4jOXi/sto3JRm8JEAtrUUkNbFQh+ti49O311iQ5PIREecac5WuYNu4UJbzP+OHiG14b9QHiVbnNyGtfZqvVTMOdn+wFocK/ghXvrFBTrmh2i9jr+8XgLEU3e06ap+2YxLh7zR8txc8AtXHD+52HYtAVIz4cyCg9AwabK437s8ZQVeWdcV8Oc9Mpq+gufySgh4ZwHGAT2KbXlfc50yOOwsw6GVbipWCJwRqbbBgpPxynHkRCY1PuZj6VWYJH/G2BF2IAf2ShI7BBfg==","signing_cert":"MIIEADCCAmgCFBubGPb+EfC05/S/6Aw4dX09a1nEMA0GCSqGSIb3DQEBCwUAMH0xCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTEUMBIGA1UEBwwLU2FudGEgQ2xhcmExEDAOBgNVBAoMB0V4YW1wbGUxOTA3BgNVBAMMMFRlc3QgZm9yIEludGVsIFNHWCBBdHRlc3RhdGlvbiBSZXBvcnQgU2lnbmluZyBDQTAgFw0yMzA2MDcwOTQyMjdaGA8yMDUwMTAyMzA5NDIyN1owejELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMRQwEgYDVQQHDAtTYW50YSBDbGFyYTEQMA4GA1UECgwHRXhhbXBsZTE2MDQGA1UEAwwtVGVzdCBmb3IgSW50ZWwgU0dYIEF0dGVzdGF0aW9uIFJlcG9ydCBTaWduaW5nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAkNMKASzEqPmh//fz0QPZcz8805DpSBqZ6ZW0dCiBTFzp3NgU03wsC23wgqVR9LAWfDVbaPiOOLhw99NBQizHcXwuKgA02ISpUyu7bQwFhHKWM7YRv14uKcXtdralZOL6uPo5RHZXCTkslxSy29r/Cyg63zxqvTZjuPpd40W1St2/wH8C1/OXW/ugG3y4bZME01qkHj1nJQLjYav8oHhH93Cu4vniS0ZAifVa+l9BHRj+jy7X8lOTFbUUSjXgL6rN2GQD5Dtso5fTwj2ukVmYYumfjctvFj1KJ1c/3Vx9291d8dmBQNxFZ+eleeGOPZKpgrKEj79WGTE1oHqApxBPbwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBgQC39YvP2YVKBbODxkdMG4FQRSAcywdml2GY+ah67sbLF/34i64JzvahLaWo+gl091ivkkUa//fgvGj/BBQq7oLmEJ1K8jsHWR8F3NYQHHSIIf8+vgpqGyox8miwZMrbVkMqIlxiqPnDcLWhiRfeXcwqc3qlfPgtHfGPx3YHlUXZTQLWNffBXXWYxPz5DX5OeMJjlV1VxFd+3xrNltR4klSrGrQLRqyZe7r5AndZ3b9g86xN7eospyhoAGtSOLw0Ml7j3F4zVXkhoPYTVayhi3q71r4AqfaLLdbYGaXMhezqo51l06iRaMkFL49enpDXnCRbd1yJtZZXwe17cItDRNB7k6GrpibuLFfbd2goYR/Rv+lCY/sR5IFwrKuWoxwTvd7pIzYbl9oRurbvpU3srCvNOkQO1oPxP6PpwuScpx7KCha3QtKrJfMGOQ/siGUXMiseT8hQAj5P4DaPV6xFTbYqHEUx6uW5vFzv5QPhCd6Xh3MeUz6Xw2UMbpR2JahkrSM=","mrenclave":"MoDAjhm1TRXKkl4KbBP+2v8q/2wJf5OEGr2Nnhowcrg=","enclave_key":"hNeWsFyr39J51cViGUemN7CfskI="} \ No newline at end of file From bd6662e073f7c61d1c96856f6c671139be34e653 Mon Sep 17 00:00:00 2001 From: Jun Kimura Date: Fri, 21 Jun 2024 00:35:05 +0900 Subject: [PATCH 2/2] update README Signed-off-by: Jun Kimura --- README.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ab13c72..687fff4 100644 --- a/README.md +++ b/README.md @@ -32,5 +32,13 @@ $ ./target/release/lcp-cgen \ --signing_key=${LCP_PATH}/tests/certs/signing.key \ --enclave=${LCP_PATH}/bin/enclave.signed.so \ --out=/path/to/testdatadir \ - --commands wait_blocks:1 update_client verify_channel wait_blocks:1 update_client verify_packet + --eknum=1 \ + --commands \ + wait_blocks:1 \ + update_client:0 \ + verify_channel:0 \ + wait_blocks:1 \ + update_client:0 \ + verify_packet:0 \ + verify_packet_receipt_absence:0 ```