Add strict check for privateFrom in private txs (#976)

* Add strict check for privateFrom in private txs

Signed-off-by: Lucas Saldanha <lucas.saldanha@consensys.net>

CHANGELOG.md

@@ -45,7 +45,10 @@ v1.6. Older versions of Orion will no longer work with Besu v1.5.
 * Performance Improvements: The addition of native libraries ([\#775](https://github.com/hyperledger/besu/pull/775)) and changes to data structures in the EVM ([\#1089](https://github.com/hyperledger/besu/pull/1089)) have improved Besu sync and EVM execution times. 
 * Tracing API Improvements: The [Tracing API](https://besu.hyperledger.org/en/latest/Reference/API-Methods/#trace-methods) is no longer an Early Access feature and now has full support for `trace_replayBlockTransactions`, `trace_Block` and `trace_transaction`.  
 * New Plugin API Block Events: `BlockAdded` and `BlockReorg` are now exposed via the Plugin API [\#637](https://github.com/hyperledger/besu/pull/637). 
-- Add CLI option `--Xnat-kube-pod-name` to specify the name of the loadbalancer used by the Kubernetes nat manager [\#1078](https://github.com/hyperledger/besu/pull/1078)
+* Add CLI option `--Xnat-kube-pod-name` to specify the name of the loadbalancer used by the Kubernetes nat manager [\#1078](https://github.com/hyperledger/besu/pull/1078)
+* Besu now has a strict check on private transactions to ensure the privateFrom in the transaction
+matches the sender Orion key that has distributed the payload. Besu 1.5+ requires Orion 1.6+ to work. 
+[#357](https://github.com/PegaSysEng/orion/issues/357)
 
 ### Bug fixes 
 

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/privacy/multitenancy/MultiTenancyAcceptanceTest.java

@@ -272,7 +272,7 @@ public class MultiTenancyAcceptanceTest extends AcceptanceTestBase {
 
   private void receiveEnclaveStub(final PrivateTransaction privTx) throws JsonProcessingException {
     final BytesValueRLPOutput rlpOutput = getRLPOutputForReceiveResponse(privTx);
-    final String senderKey = "QTFhVnRNeExDVUhtQlZIWG9aenpCZ1BiVy93ajVheERwVzlYOGw5MVNHbz0=";
+    final String senderKey = privTx.getPrivateFrom().toBase64String();
     final String receiveResponse =
         mapper.writeValueAsString(
             new ReceiveResponse(

enclave/src/main/java/org/hyperledger/besu/enclave/EnclaveConfigurationException.java

@@ -0,0 +1,22 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.enclave;
+
+public class EnclaveConfigurationException extends IllegalStateException {
+
+  public EnclaveConfigurationException(final String message) {
+    super(message);
+  }
+}

ethereum/core/src/integration-test/java/org/hyperledger/besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContractIntegrationTest.java

@@ -30,6 +30,7 @@ import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
 import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
+import org.hyperledger.besu.ethereum.core.PrivateTransactionDataFixture;
 import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.ethereum.core.WorldUpdater;
 import org.hyperledger.besu.ethereum.mainnet.SpuriousDragonGasCalculator;
@@ -169,8 +170,10 @@ public class PrivacyPrecompiledContractIntegrationTest {
   public void testSendAndReceive() {
     final List<String> publicKeys = testHarness.getPublicKeys();
 
+    final PrivateTransaction privateTransaction =
+        PrivateTransactionDataFixture.privateContractDeploymentTransactionBesu(publicKeys.get(0));
     final BytesValueRLPOutput bytesValueRLPOutput = new BytesValueRLPOutput();
-    bytesValueRLPOutput.writeRLP(VALID_PRIVATE_TRANSACTION_RLP);
+    privateTransaction.writeTo(bytesValueRLPOutput);
 
     final String s = bytesValueRLPOutput.encoded().toBase64String();
     final SendResponse sr =

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractPrecompiledContract.java

@@ -14,15 +14,20 @@
  */
 package org.hyperledger.besu.ethereum.mainnet;
 
+import org.hyperledger.besu.enclave.EnclaveConfigurationException;
 import org.hyperledger.besu.ethereum.core.Gas;
 import org.hyperledger.besu.ethereum.vm.GasCalculator;
 import org.hyperledger.besu.ethereum.vm.MessageFrame;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
 
 /** Skeleton class for @{link PrecompileContract} implementations. */
 public abstract class AbstractPrecompiledContract implements PrecompiledContract {
 
+  private static final Logger LOG = LogManager.getLogger();
+
   private final GasCalculator gasCalculator;
 
   private final String name;
@@ -46,4 +51,26 @@ public abstract class AbstractPrecompiledContract implements PrecompiledContract
 
   @Override
   public abstract Bytes compute(Bytes input, MessageFrame messageFrame);
+
+  protected boolean privateFromMatchesSenderKey(
+      final Bytes transactionPrivateFrom, final String payloadSenderKey) {
+    if (payloadSenderKey == null) {
+      LOG.warn(
+          "Missing sender key from Orion response. Upgrade Orion to 1.6 to enforce privateFrom check.");
+      throw new EnclaveConfigurationException(
+          "Incompatible Orion version. Orion version must be 1.6.0 or greater.");
+    }
+
+    if (transactionPrivateFrom == null || transactionPrivateFrom.isEmpty()) {
+      LOG.warn("Private transaction is missing privateFrom");
+      return false;
+    }
+
+    if (!payloadSenderKey.equals(transactionPrivateFrom.toBase64String())) {
+      LOG.warn("Private transaction privateFrom doesn't match payload sender key");
+      return false;
+    }
+
+    return true;
+  }
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/precompiles/privacy/OnChainPrivacyPrecompiledContract.java

@@ -95,6 +95,12 @@ public class OnChainPrivacyPrecompiledContract extends PrivacyPrecompiledContrac
         VersionedPrivateTransaction.readFrom(bytesValueRLPInput);
     final PrivateTransaction privateTransaction =
         versionedPrivateTransaction.getPrivateTransaction();
+
+    if (!privateFromMatchesSenderKey(
+        privateTransaction.getPrivateFrom(), receiveResponse.getSenderKey())) {
+      return Bytes.EMPTY;
+    }
+
     final Bytes32 version = versionedPrivateTransaction.getVersion();
 
     final Optional<Bytes> maybeGroupId = privateTransaction.getPrivacyGroupId();

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContract.java

@@ -134,6 +134,11 @@ public class PrivacyPrecompiledContract extends AbstractPrecompiledContract {
     final PrivateTransaction privateTransaction =
         PrivateTransaction.readFrom(bytesValueRLPInput.readAsRlp());
 
+    if (!privateFromMatchesSenderKey(
+        privateTransaction.getPrivateFrom(), receiveResponse.getSenderKey())) {
+      return Bytes.EMPTY;
+    }
+
     final Bytes32 privacyGroupId =
         Bytes32.wrap(Bytes.fromBase64String(receiveResponse.getPrivacyGroupId()));
 

ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/PrivateTransactionDataFixture.java

@@ -123,6 +123,14 @@ public class PrivateTransactionDataFixture {
         .createTransaction(KEY_PAIR);
   }
 
+  public static PrivateTransaction privateContractDeploymentTransactionBesu(
+      final String privateFrom) {
+    return new PrivateTransactionTestFixture()
+        .payload(VALID_CONTRACT_DEPLOYMENT_PAYLOAD)
+        .privacyGroupId(Bytes.fromBase64String(privateFrom))
+        .createTransaction(KEY_PAIR);
+  }
+
   public static ReceiveResponse generateReceiveResponse(
       final PrivateTransaction privateTransaction) {
     final BytesValueRLPOutput rlpOutput = new BytesValueRLPOutput();

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContractTest.java

@@ -16,13 +16,18 @@ package org.hyperledger.besu.ethereum.mainnet.precompiles.privacy;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.hyperledger.besu.ethereum.core.PrivateTransactionDataFixture.privateTransactionBesu;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.enclave.Enclave;
 import org.hyperledger.besu.enclave.EnclaveClientException;
+import org.hyperledger.besu.enclave.EnclaveConfigurationException;
 import org.hyperledger.besu.enclave.types.ReceiveResponse;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.Address;
@@ -31,7 +36,6 @@ import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
 import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.core.Log;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
-import org.hyperledger.besu.ethereum.core.PrivateTransactionDataFixture;
 import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.ethereum.core.WorldUpdater;
 import org.hyperledger.besu.ethereum.mainnet.SpuriousDragonGasCalculator;
@@ -62,7 +66,7 @@ public class PrivacyPrecompiledContractTest {
   @Rule public final TemporaryFolder temp = new TemporaryFolder();
 
   private final String actual = "Test String";
-  private final Bytes key = Bytes.wrap(actual.getBytes(UTF_8));
+  private final Bytes txEnclaveKey = Bytes.wrap(actual.getBytes(UTF_8));
   private MessageFrame messageFrame;
   private Blockchain blockchain;
   private final String DEFAULT_OUTPUT = "0x01";
@@ -133,26 +137,18 @@ public class PrivacyPrecompiledContractTest {
   @Test
   public void testPayloadFoundInEnclave() {
     final Enclave enclave = mock(Enclave.class);
-    final PrivacyPrecompiledContract contract =
-        new PrivacyPrecompiledContract(
-            new SpuriousDragonGasCalculator(),
-            enclave,
-            worldStateArchive,
-            privateStateStorage,
-            privateStateRootResolver);
+    final PrivacyPrecompiledContract contract = buildPrivacyPrecompiledContract(enclave);
     contract.setPrivateTransactionProcessor(mockPrivateTxProcessor());
 
-    BytesValueRLPOutput bytesValueRLPOutput = new BytesValueRLPOutput();
-    PrivateTransactionDataFixture.privateTransactionBesu().writeTo(bytesValueRLPOutput);
+    final PrivateTransaction privateTransaction = privateTransactionBesu();
+    byte[] payload = convertPrivateTransactionToBytes(privateTransaction);
+    final String privateFrom = privateTransaction.getPrivateFrom().toBase64String();
 
     final ReceiveResponse response =
-        new ReceiveResponse(
-            bytesValueRLPOutput.encoded().toBase64String().getBytes(UTF_8),
-            PAYLOAD_TEST_PRIVACY_GROUP_ID,
-            null);
+        new ReceiveResponse(payload, PAYLOAD_TEST_PRIVACY_GROUP_ID, privateFrom);
     when(enclave.receive(any(String.class))).thenReturn(response);
 
-    final Bytes actual = contract.compute(key, messageFrame);
+    final Bytes actual = contract.compute(txEnclaveKey, messageFrame);
 
     assertThat(actual).isEqualTo(Bytes.fromHexString(DEFAULT_OUTPUT));
   }
@@ -160,35 +156,86 @@ public class PrivacyPrecompiledContractTest {
   @Test
   public void testPayloadNotFoundInEnclave() {
     final Enclave enclave = mock(Enclave.class);
-
-    final PrivacyPrecompiledContract contract =
-        new PrivacyPrecompiledContract(
-            new SpuriousDragonGasCalculator(),
-            enclave,
-            worldStateArchive,
-            privateStateStorage,
-            privateStateRootResolver);
+    final PrivacyPrecompiledContract contract = buildPrivacyPrecompiledContract(enclave);
 
     when(enclave.receive(any(String.class))).thenThrow(EnclaveClientException.class);
 
-    final Bytes expected = contract.compute(key, messageFrame);
+    final Bytes expected = contract.compute(txEnclaveKey, messageFrame);
     assertThat(expected).isEqualTo(Bytes.EMPTY);
   }
 
   @Test(expected = RuntimeException.class)
   public void testEnclaveDown() {
     final Enclave enclave = mock(Enclave.class);
+    final PrivacyPrecompiledContract contract = buildPrivacyPrecompiledContract(enclave);
+
+    when(enclave.receive(any(String.class))).thenThrow(new RuntimeException());
+
+    contract.compute(txEnclaveKey, messageFrame);
+  }
+
+  @Test
+  public void testEnclaveBelowRequiredVersion() {
+    final Enclave enclave = mock(Enclave.class);
+    final PrivacyPrecompiledContract contract = buildPrivacyPrecompiledContract(enclave);
+    final PrivateTransaction privateTransaction = privateTransactionBesu();
+    final byte[] payload = convertPrivateTransactionToBytes(privateTransaction);
+
+    final ReceiveResponse responseWithoutSenderKey =
+        new ReceiveResponse(payload, PAYLOAD_TEST_PRIVACY_GROUP_ID, null);
+    when(enclave.receive(eq(txEnclaveKey.toBase64String()))).thenReturn(responseWithoutSenderKey);
+
+    assertThatThrownBy(() -> contract.compute(txEnclaveKey, messageFrame))
+        .isInstanceOf(EnclaveConfigurationException.class)
+        .hasMessage("Incompatible Orion version. Orion version must be 1.6.0 or greater.");
+  }
+
+  @Test
+  public void testPrivateTransactionWithoutPrivateFrom() {
+    final Enclave enclave = mock(Enclave.class);
+    final PrivacyPrecompiledContract contract = buildPrivacyPrecompiledContract(enclave);
+    final PrivateTransaction privateTransaction = spy(privateTransactionBesu());
+    when(privateTransaction.getPrivateFrom()).thenReturn(Bytes.EMPTY);
+    final byte[] payload = convertPrivateTransactionToBytes(privateTransaction);
+
+    final String senderKey = privateTransaction.getPrivateFrom().toBase64String();
+    final ReceiveResponse response =
+        new ReceiveResponse(payload, PAYLOAD_TEST_PRIVACY_GROUP_ID, senderKey);
+    when(enclave.receive(eq(txEnclaveKey.toBase64String()))).thenReturn(response);
+
+    final Bytes expected = contract.compute(txEnclaveKey, messageFrame);
+    assertThat(expected).isEqualTo(Bytes.EMPTY);
+  }
+
+  @Test
+  public void testPayloadNotMatchingPrivateFrom() {
+    final Enclave enclave = mock(Enclave.class);
+    final PrivacyPrecompiledContract contract = buildPrivacyPrecompiledContract(enclave);
+    final PrivateTransaction privateTransaction = privateTransactionBesu();
+    final byte[] payload = convertPrivateTransactionToBytes(privateTransaction);
 
-    final PrivacyPrecompiledContract contract =
-        new PrivacyPrecompiledContract(
+    final String wrongSenderKey = Bytes.random(32).toBase64String();
+    final ReceiveResponse responseWithWrongSenderKey =
+        new ReceiveResponse(payload, PAYLOAD_TEST_PRIVACY_GROUP_ID, wrongSenderKey);
+    when(enclave.receive(eq(txEnclaveKey.toBase64String()))).thenReturn(responseWithWrongSenderKey);
+
+    final Bytes expected = contract.compute(txEnclaveKey, messageFrame);
+    assertThat(expected).isEqualTo(Bytes.EMPTY);
+  }
+
+  private byte[] convertPrivateTransactionToBytes(final PrivateTransaction privateTransaction) {
+    final BytesValueRLPOutput bytesValueRLPOutput = new BytesValueRLPOutput();
+    privateTransaction.writeTo(bytesValueRLPOutput);
+
+    return bytesValueRLPOutput.encoded().toBase64String().getBytes(UTF_8);
+  }
+
+  private PrivacyPrecompiledContract buildPrivacyPrecompiledContract(final Enclave enclave) {
+    return new PrivacyPrecompiledContract(
         new SpuriousDragonGasCalculator(),
         enclave,
         worldStateArchive,
         privateStateStorage,
         privateStateRootResolver);
-
-    when(enclave.receive(any(String.class))).thenThrow(new RuntimeException());
-
-    contract.compute(key, messageFrame);
   }
 }

gradle/versions.gradle

@@ -64,7 +64,7 @@ dependencyManagement {
 
     dependency 'junit:junit:4.13'
 
-    dependency 'net.consensys:orion:1.5.0'
+    dependency 'net.consensys:orion:1.6.0'
 
     dependency 'net.java.dev.jna:jna:5.5.0'
 

testutil/src/main/java/org/hyperledger/orion/testutil/OrionConfiguration.java

@@ -24,17 +24,20 @@ public class OrionConfiguration {
   private final Path privateKey;
   private final Path tempDir;
   private final List<String> otherNodes = new ArrayList<>();
+  private final boolean clearKnownNodes;
 
   public OrionConfiguration(
       final Path publicKey,
       final Path privateKey,
       final Path tempDir,
-      final List<String> otherNodes) {
+      final List<String> otherNodes,
+      final boolean clearKnownNodes) {
 
     this.publicKey = publicKey;
     this.privateKey = privateKey;
     this.tempDir = tempDir;
     this.otherNodes.addAll(otherNodes);
+    this.clearKnownNodes = clearKnownNodes;
   }
 
   public Path getPublicKey() {
@@ -56,4 +59,8 @@ public class OrionConfiguration {
   public void addOtherNode(final String otherNode) {
     otherNodes.add(otherNode);
   }
+
+  public boolean isClearKnownNodes() {
+    return clearKnownNodes;
+  }
 }

testutil/src/main/java/org/hyperledger/orion/testutil/OrionTestHarness.java

@@ -56,7 +56,7 @@ public class OrionTestHarness {
   public void start() {
     if (!isRunning) {
       config = buildConfig();
-      orion.run(System.out, System.err, config);
+      orion.run(config, orionConfiguration.isClearKnownNodes());
       isRunning = true;
       LOG.info("Orion node port: {}", orion.nodePort());
       LOG.info("Orion client port: {}", orion.clientPort());

testutil/src/main/java/org/hyperledger/orion/testutil/OrionTestHarnessFactory.java

@@ -47,6 +47,7 @@ public class OrionTestHarnessFactory {
   public static OrionTestHarness create(
       final Path tempDir, final Path key1pub, final Path key1key, final List<String> othernodes) {
 
-    return new OrionTestHarness(new OrionConfiguration(key1pub, key1key, tempDir, othernodes));
+    return new OrionTestHarness(
+        new OrionConfiguration(key1pub, key1key, tempDir, othernodes, false));
   }
 }