[KS-250] Use report context to validate signatures (#13366)

1. Parse report context in write target.
2. Fix 'v' value when validating signatures.
3. Add a stub of KeystoneFeedsConsumer contract.
4. Fix owner length in Encoder.

.changeset/twenty-pets-look.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#internal keystone report context

contracts/.changeset/moody-days-share.md

@@ -0,0 +1,5 @@
+---
+'@chainlink/contracts': patch
+---
+
+#internal stub of keystone feed consumer contract

contracts/src/v0.8/keystone/KeystoneFeedsConsumer.sol

@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+import {IReceiver} from "./interfaces/IReceiver.sol";
+
+contract KeystoneFeedsConsumer is IReceiver {
+  event MessageReceived(bytes32 indexed workflowId, address indexed workflowOwner, uint256 nReports);
+  event FeedReceived(bytes32 indexed feedId, uint256 price, uint64 timestamp);
+
+  constructor() {}
+
+  struct FeedReport {
+    bytes32 FeedID;
+    uint256 Price;
+    uint64 Timestamp;
+  }
+
+  function onReport(bytes32 workflowId, address workflowOwner, bytes calldata rawReport) external {
+    // TODO: validate sender and workflowOwner
+
+    FeedReport[] memory feeds = abi.decode(rawReport, (FeedReport[]));
+    for (uint32 i = 0; i < feeds.length; i++) {
+      emit FeedReceived(feeds[i].FeedID, feeds[i].Price, feeds[i].Timestamp);
+    }
+
+    emit MessageReceived(workflowId, workflowOwner, feeds.length);
+  }
+}

contracts/src/v0.8/keystone/KeystoneForwarder.sol

@@ -130,6 +130,7 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
   function report(
     address receiverAddress,
     bytes calldata rawReport,
+    bytes calldata reportContext,
     bytes[] calldata signatures
   ) external nonReentrant {
     if (rawReport.length < REPORT_METADATA_LENGTH) {
@@ -149,14 +150,14 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
 
     // validate signatures
     {
-      bytes32 hash = keccak256(rawReport);
+      bytes32 completeHash = keccak256(abi.encodePacked(keccak256(rawReport), reportContext));
 
       address[MAX_ORACLES] memory signed;
       uint8 index;
       for (uint256 i; i < signatures.length; ++i) {
         // TODO: is libocr-style multiple bytes32 arrays more optimal, gas-wise?
         (bytes32 r, bytes32 s, uint8 v) = _splitSignature(signatures[i]);
-        address signer = ecrecover(hash, v, r, s);
+        address signer = ecrecover(completeHash, v + 27, r, s);
 
         // validate signer is trusted and signature is unique
         index = uint8(s_configs[donId]._positions[signer]);
@@ -167,9 +168,8 @@ contract KeystoneForwarder is IForwarder, ConfirmedOwner, TypeAndVersionInterfac
       }
     }
 
-    IReceiver receiver = IReceiver(receiverAddress);
     bool success;
-    try receiver.onReport(workflowId, workflowOwner, rawReport[REPORT_METADATA_LENGTH:]) {
+    try IReceiver(receiverAddress).onReport(workflowId, workflowOwner, rawReport[REPORT_METADATA_LENGTH:]) {
       success = true;
     } catch {
       // Do nothing, success is already false

contracts/src/v0.8/keystone/test/KeystoneForwarder_ReportTest.t.sol

@@ -19,6 +19,7 @@ contract KeystoneForwarder_ReportTest is BaseTest {
   bytes[] internal mercuryReports = new bytes[](2);
   bytes internal rawReports;
   bytes internal report;
+  bytes internal reportContext = new bytes(96);
   uint256 internal requiredSignaturesNum = F + 1;
   bytes[] internal signatures = new bytes[](2);
 
@@ -34,8 +35,11 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     report = abi.encodePacked(workflowId, DON_ID, executionId, workflowOwner, rawReports);
 
     for (uint256 i = 0; i < requiredSignaturesNum; i++) {
-      (uint8 v, bytes32 r, bytes32 s) = vm.sign(s_signers[i].mockPrivateKey, keccak256(report));
-      signatures[i] = bytes.concat(r, s, bytes1(v));
+      (uint8 v, bytes32 r, bytes32 s) = vm.sign(
+        s_signers[i].mockPrivateKey,
+        keccak256(abi.encodePacked(keccak256(report), reportContext))
+      );
+      signatures[i] = bytes.concat(r, s, bytes1(v - 27));
     }
 
     vm.startPrank(TRANSMITTER);
@@ -52,14 +56,14 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     );
 
     vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.InvalidDonId.selector, invalidDONId));
-    s_forwarder.report(address(s_receiver), reportWithInvalidDONId, signatures);
+    s_forwarder.report(address(s_receiver), reportWithInvalidDONId, reportContext, signatures);
   }
 
   function test_RevertWhen_ReportIsMalformed() public {
     bytes memory shortenedReport = abi.encode(bytes32(report));
 
     vm.expectRevert(KeystoneForwarder.InvalidReport.selector);
-    s_forwarder.report(address(s_receiver), shortenedReport, signatures);
+    s_forwarder.report(address(s_receiver), shortenedReport, reportContext, signatures);
   }
 
   function test_RevertWhen_TooFewSignatures() public {
@@ -68,7 +72,7 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     vm.expectRevert(
       abi.encodeWithSelector(KeystoneForwarder.InvalidSignatureCount.selector, F + 1, fewerSignatures.length)
     );
-    s_forwarder.report(address(s_receiver), report, fewerSignatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, fewerSignatures);
   }
 
   function test_RevertWhen_TooManySignatures() public {
@@ -77,40 +81,43 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     vm.expectRevert(
       abi.encodeWithSelector(KeystoneForwarder.InvalidSignatureCount.selector, F + 1, moreSignatures.length)
     );
-    s_forwarder.report(address(s_receiver), report, moreSignatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, moreSignatures);
   }
 
   function test_RevertWhen_AnySignatureIsInvalid() public {
     signatures[1] = abi.encode(1234); // invalid signature
 
     vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.InvalidSignature.selector, signatures[1]));
-    s_forwarder.report(address(s_receiver), report, signatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, signatures);
   }
 
   function test_RevertWhen_AnySignerIsInvalid() public {
     uint256 mockPK = 999;
 
     Signer memory maliciousSigner = Signer({mockPrivateKey: mockPK, signerAddress: vm.addr(mockPK)});
-    (uint8 v, bytes32 r, bytes32 s) = vm.sign(maliciousSigner.mockPrivateKey, keccak256(report));
-    signatures[1] = bytes.concat(r, s, bytes1(v));
+    (uint8 v, bytes32 r, bytes32 s) = vm.sign(
+      maliciousSigner.mockPrivateKey,
+      keccak256(abi.encodePacked(keccak256(report), reportContext))
+    );
+    signatures[1] = bytes.concat(r, s, bytes1(v - 27));
 
     vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.InvalidSigner.selector, maliciousSigner.signerAddress));
-    s_forwarder.report(address(s_receiver), report, signatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, signatures);
   }
 
   function test_RevertWhen_ReportHasDuplicateSignatures() public {
     signatures[1] = signatures[0]; // repeat a signature
 
     vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.DuplicateSigner.selector, s_signers[0].signerAddress));
-    s_forwarder.report(address(s_receiver), report, signatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, signatures);
   }
 
   function test_RevertWhen_ReportAlreadyProcessed() public {
-    s_forwarder.report(address(s_receiver), report, signatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, signatures);
     bytes32 reportId = keccak256(bytes.concat(bytes20(uint160(address(s_receiver))), executionId));
 
     vm.expectRevert(abi.encodeWithSelector(KeystoneForwarder.ReportAlreadyProcessed.selector, reportId));
-    s_forwarder.report(address(s_receiver), report, signatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, signatures);
   }
 
   function test_Report_SuccessfulDelivery() public {
@@ -123,7 +130,7 @@ contract KeystoneForwarder_ReportTest is BaseTest {
     vm.expectEmit(address(s_forwarder));
     emit ReportProcessed(address(s_receiver), workflowOwner, executionId, true);
 
-    s_forwarder.report(address(s_receiver), report, signatures);
+    s_forwarder.report(address(s_receiver), report, reportContext, signatures);
 
     // validate transmitter was recorded
     address transmitter = s_forwarder.getTransmitter(address(s_receiver), executionId);

core/capabilities/targets/write_target.go

@@ -107,6 +107,7 @@ func (cap *EvmWrite) Execute(ctx context.Context, request capabilities.Capabilit
 
 	var inputs struct {
 		Report     []byte
+		Context    []byte
 		Signatures [][]byte
 	}
 	if err = signedReport.UnwrapTo(&inputs); err != nil {
@@ -127,12 +128,12 @@ func (cap *EvmWrite) Execute(ctx context.Context, request capabilities.Capabilit
 		}()
 		return callback, nil
 	}
-	cap.lggr.Debugw("WriteTarget non-empty report - attempting to push to txmgr", "request", request, "report", inputs.Report, "signatures", inputs.Signatures)
+	cap.lggr.Debugw("WriteTarget non-empty report - attempting to push to txmgr", "request", request, "reportLen", len(inputs.Report), "reportContextLen", len(inputs.Context), "nSignatures", len(inputs.Signatures))
 
 	// TODO: validate encoded report is prefixed with workflowID and executionID that match the request meta
 
 	// construct forwarder payload
-	calldata, err := forwardABI.Pack("report", common.HexToAddress(reqConfig.Address), inputs.Report, inputs.Signatures)
+	calldata, err := forwardABI.Pack("report", common.HexToAddress(reqConfig.Address), inputs.Report, inputs.Context, inputs.Signatures)
 	if err != nil {
 		return nil, err
 	}