From b783957d45f023f1b230d41e2c003dc098042b02 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Thu, 18 Jul 2019 13:28:01 +1200 Subject: [PATCH] PIE-1781: Enforce nonce validation on PrivateTransactionProcessor (#1713) Signed-off-by: Adrian Sutton --- .../privacy/PrivateTransactionHandler.java | 33 ++------ .../privacy/PrivateTransactionProcessor.java | 16 ++-- .../privacy/PrivateTransactionValidator.java | 52 +++++++++++++ .../PrivateTransactionHandlerTest.java | 22 +++++- .../PrivateTransactionValidatorTest.java | 76 +++++++++++++++++++ 5 files changed, 162 insertions(+), 37 deletions(-) create mode 100644 ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidator.java create mode 100644 ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidatorTest.java diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandler.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandler.java index ab06d46b52..11b8f6be63 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandler.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandler.java @@ -12,9 +12,6 @@ */ package tech.pegasys.pantheon.ethereum.privacy; -import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INCORRECT_PRIVATE_NONCE; -import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW; - import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.enclave.Enclave; import tech.pegasys.pantheon.enclave.types.ReceiveRequest; @@ -51,6 +48,7 @@ public class PrivateTransactionHandler { private final String enclavePublicKey; private final PrivateStateStorage privateStateStorage; private final WorldStateArchive privateWorldStateArchive; + private final PrivateTransactionValidator privateTransactionValidator; public PrivateTransactionHandler(final PrivacyParameters privacyParameters) { this( @@ -59,7 +57,8 @@ public class PrivateTransactionHandler { privacyParameters.getSigningKeyPair(), privacyParameters.getEnclavePublicKey(), privacyParameters.getPrivateStateStorage(), - privacyParameters.getPrivateWorldStateArchive()); + privacyParameters.getPrivateWorldStateArchive(), + new PrivateTransactionValidator()); } public PrivateTransactionHandler( @@ -68,7 +67,8 @@ public class PrivateTransactionHandler { final SECP256K1.KeyPair nodeKeyPair, final String enclavePublicKey, final PrivateStateStorage privateStateStorage, - final WorldStateArchive privateWorldStateArchive) { + final WorldStateArchive privateWorldStateArchive, + final PrivateTransactionValidator privateTransactionValidator) { this.enclave = enclave; this.privacyPrecompileAddress = privacyPrecompileAddress; this.nodeKeyPair = nodeKeyPair; @@ -76,6 +76,7 @@ public class PrivateTransactionHandler { this.enclavePublicKey = enclavePublicKey; this.privateStateStorage = privateStateStorage; this.privateWorldStateArchive = privateWorldStateArchive; + this.privateTransactionValidator = privateTransactionValidator; } public String sendToOrion(final PrivateTransaction privateTransaction) throws Exception { @@ -130,26 +131,8 @@ public class PrivateTransactionHandler { public ValidationResult validatePrivateTransaction( final PrivateTransaction privateTransaction, final String privacyGroupId) { - final long actualNonce = privateTransaction.getNonce(); - final long expectedNonce = getSenderNonce(privateTransaction.getSender(), privacyGroupId); - LOG.debug("Validating actual nonce {} with expected nonce {}", actualNonce, expectedNonce); - if (expectedNonce > actualNonce) { - return ValidationResult.invalid( - PRIVATE_NONCE_TOO_LOW, - String.format( - "private transaction nonce %s does not match sender account nonce %s.", - actualNonce, expectedNonce)); - } - - if (expectedNonce != actualNonce) { - return ValidationResult.invalid( - INCORRECT_PRIVATE_NONCE, - String.format( - "private transaction nonce %s does not match sender account nonce %s.", - actualNonce, expectedNonce)); - } - - return ValidationResult.valid(); + return privateTransactionValidator.validate( + privateTransaction, getSenderNonce(privateTransaction.getSender(), privacyGroupId)); } private SendRequest createSendRequest(final PrivateTransaction privateTransaction) { diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionProcessor.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionProcessor.java index 00b7182426..7babbbdb7c 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionProcessor.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionProcessor.java @@ -12,8 +12,6 @@ */ package tech.pegasys.pantheon.ethereum.privacy; -import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.NONCE_TOO_LOW; - import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Account; import tech.pegasys.pantheon.ethereum.core.Address; @@ -53,6 +51,8 @@ public class PrivateTransactionProcessor { @SuppressWarnings("unused") private final TransactionValidator transactionValidator; + private final PrivateTransactionValidator privateTransactionValidator; + private final AbstractMessageProcessor contractCreationProcessor; private final AbstractMessageProcessor messageCallProcessor; @@ -166,6 +166,7 @@ public class PrivateTransactionProcessor { final int createContractAccountVersion) { this.gasCalculator = gasCalculator; this.transactionValidator = transactionValidator; + this.privateTransactionValidator = new PrivateTransactionValidator(); this.contractCreationProcessor = contractCreationProcessor; this.messageCallProcessor = messageCallProcessor; this.clearEmptyAccounts = clearEmptyAccounts; @@ -193,13 +194,10 @@ public class PrivateTransactionProcessor { ? maybePrivateSender : privateWorldState.createAccount(senderAddress, 0, Wei.ZERO); - if (transaction.getNonce() < sender.getNonce()) { - return Result.invalid( - ValidationResult.invalid( - NONCE_TOO_LOW, - String.format( - "transaction nonce %s below sender account nonce %s", - transaction.getNonce(), sender.getNonce()))); + final ValidationResult validationResult = + privateTransactionValidator.validate(transaction, sender.getNonce()); + if (!validationResult.isValid()) { + return Result.invalid(validationResult); } final long previousNonce = sender.incrementNonce(); diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidator.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidator.java new file mode 100644 index 0000000000..c2b371183d --- /dev/null +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidator.java @@ -0,0 +1,52 @@ +/* + * Copyright 2019 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. + */ +package tech.pegasys.pantheon.ethereum.privacy; + +import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INCORRECT_PRIVATE_NONCE; +import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW; + +import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason; +import tech.pegasys.pantheon.ethereum.mainnet.ValidationResult; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +class PrivateTransactionValidator { + + private static final Logger LOG = LogManager.getLogger(); + + public ValidationResult validate( + final PrivateTransaction transaction, final Long accountNonce) { + final long transactionNonce = transaction.getNonce(); + + LOG.debug("Validating actual nonce {} with expected nonce {}", transactionNonce, accountNonce); + + if (accountNonce > transactionNonce) { + return ValidationResult.invalid( + PRIVATE_NONCE_TOO_LOW, + String.format( + "private transaction nonce %s does not match sender account nonce %s.", + transactionNonce, accountNonce)); + } + + if (accountNonce != transactionNonce) { + return ValidationResult.invalid( + INCORRECT_PRIVATE_NONCE, + String.format( + "private transaction nonce %s does not match sender account nonce %s.", + transactionNonce, accountNonce)); + } + + return ValidationResult.valid(); + } +} diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandlerTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandlerTest.java index 7dddee0684..b65bfc79cf 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandlerTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandlerTest.java @@ -61,6 +61,7 @@ public class PrivateTransactionHandlerTest { private PrivateTransactionHandler privateTransactionHandler; private PrivateTransactionHandler brokenPrivateTransactionHandler; + private PrivateTransactionValidator privateTransactionValidator; private static final Transaction PUBLIC_TRANSACTION = Transaction.builder() @@ -89,6 +90,12 @@ public class PrivateTransactionHandlerTest { return mockEnclave; } + PrivateTransactionValidator mockPrivateTransactionValidator() { + PrivateTransactionValidator validator = mock(PrivateTransactionValidator.class); + when(validator.validate(any(), any())).thenReturn(ValidationResult.valid()); + return validator; + } + @Before public void setUp() throws Exception { PrivateStateStorage privateStateStorage = mock(PrivateStateStorage.class); @@ -102,6 +109,8 @@ public class PrivateTransactionHandlerTest { when(worldStateArchive.getMutable(any(Hash.class))).thenReturn(Optional.of(mutableWorldState)); when(mutableWorldState.get(any(Address.class))).thenReturn(account); + privateTransactionValidator = mockPrivateTransactionValidator(); + privateTransactionHandler = new PrivateTransactionHandler( mockEnclave(), @@ -109,7 +118,8 @@ public class PrivateTransactionHandlerTest { KEY_PAIR, OrionKeyUtils.loadKey("orion_key_0.pub"), privateStateStorage, - worldStateArchive); + worldStateArchive, + privateTransactionValidator); brokenPrivateTransactionHandler = new PrivateTransactionHandler( brokenMockEnclave(), @@ -117,7 +127,8 @@ public class PrivateTransactionHandlerTest { KEY_PAIR, OrionKeyUtils.loadKey("orion_key_0.pub"), privateStateStorage, - worldStateArchive); + worldStateArchive, + privateTransactionValidator); } @Test @@ -173,8 +184,10 @@ public class PrivateTransactionHandlerTest { @Test public void nonceTooLowError() throws Exception { - final PrivateTransaction transaction = buildLegacyPrivateTransaction(0); + when(privateTransactionValidator.validate(any(), any())) + .thenReturn(ValidationResult.invalid(PRIVATE_NONCE_TOO_LOW)); + final PrivateTransaction transaction = buildLegacyPrivateTransaction(0); final String enclaveKey = privateTransactionHandler.sendToOrion(transaction); final String privacyGroupId = privateTransactionHandler.getPrivacyGroup(enclaveKey, transaction); @@ -185,6 +198,9 @@ public class PrivateTransactionHandlerTest { @Test public void incorrectNonceError() throws Exception { + when(privateTransactionValidator.validate(any(), any())) + .thenReturn(ValidationResult.invalid(INCORRECT_PRIVATE_NONCE)); + final PrivateTransaction transaction = buildLegacyPrivateTransaction(2); final String enclaveKey = privateTransactionHandler.sendToOrion(transaction); diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidatorTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidatorTest.java new file mode 100644 index 0000000000..0f19c11b35 --- /dev/null +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidatorTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2019 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. + */ +package tech.pegasys.pantheon.ethereum.privacy; + +import static org.assertj.core.api.Assertions.assertThat; +import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INCORRECT_PRIVATE_NONCE; +import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW; + +import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.core.Wei; +import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason; +import tech.pegasys.pantheon.ethereum.mainnet.ValidationResult; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.math.BigInteger; + +import org.junit.Before; +import org.junit.Test; + +public class PrivateTransactionValidatorTest { + + private PrivateTransactionValidator validator; + + @Before + public void before() { + validator = new PrivateTransactionValidator(); + } + + @Test + public void transactionWithNonceLowerThanAccountNonceShouldReturnLowNonceError() { + ValidationResult validationResult = + validator.validate(privateTransactionWithNonce(1L), 2L); + + assertThat(validationResult).isEqualTo(ValidationResult.invalid(PRIVATE_NONCE_TOO_LOW)); + } + + @Test + public void transactionWithNonceGreaterThanAccountNonceShouldReturnIncorrectNonceError() { + ValidationResult validationResult = + validator.validate(privateTransactionWithNonce(3L), 2L); + + assertThat(validationResult).isEqualTo(ValidationResult.invalid(INCORRECT_PRIVATE_NONCE)); + } + + @Test + public void transactionWithNonceMatchingThanAccountNonceShouldReturnValidTransactionResult() { + ValidationResult validationResult = + validator.validate(privateTransactionWithNonce(1L), 1L); + + assertThat(validationResult).isEqualTo(ValidationResult.valid()); + } + + private static PrivateTransaction privateTransactionWithNonce(final long nonce) { + return PrivateTransaction.builder() + .nonce(nonce) + .gasPrice(Wei.of(1000)) + .gasLimit(3000000) + .to(Address.fromHexString("0x627306090abab3a6e1400e9345bc60c78a8bef57")) + .value(Wei.ZERO) + .payload(BytesValue.fromHexString("0x")) + .sender(Address.fromHexString("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73")) + .chainId(BigInteger.valueOf(2018)) + .restriction(Restriction.RESTRICTED) + .build(); + } +}