From 6e734f95a8c11dd054f94200dc3e6141462c451c Mon Sep 17 00:00:00 2001 From: Matilda Clerke Date: Fri, 20 Sep 2024 11:32:30 +1000 Subject: [PATCH 1/2] 7311: Remove PeerTaskFeatureToggle in favor of including isPeerTaskSystemEnabled in SynchronizerConfiguration Signed-off-by: Matilda Clerke --- .../org/hyperledger/besu/cli/BesuCommand.java | 3 - .../options/unstable/SynchronizerOptions.java | 2 +- .../peertask/PeerTaskFeatureToggle.java | 59 ------------------- .../eth/sync/SynchronizerConfiguration.java | 18 +++++- 4 files changed, 17 insertions(+), 65 deletions(-) delete mode 100644 ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskFeatureToggle.java diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 8d2bba300b..79879538bd 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -119,7 +119,6 @@ import org.hyperledger.besu.ethereum.core.MiningParametersMetrics; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.core.VersionMetadata; import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; -import org.hyperledger.besu.ethereum.eth.manager.peertask.PeerTaskFeatureToggle; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; @@ -1925,8 +1924,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable { instantiateSignatureAlgorithmFactory(); - PeerTaskFeatureToggle.initialize(unstableSynchronizerOptions.isPeerTaskSystemEnabled()); - logger.info(generateConfigurationOverview()); logger.info("Security Module: {}", securityModuleName); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java index 4121f32d21..816d9df00a 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/unstable/SynchronizerOptions.java @@ -436,7 +436,7 @@ public class SynchronizerOptions implements CLIOptions Date: Fri, 20 Sep 2024 03:56:25 +0200 Subject: [PATCH 2/2] Fix 7702 signature bound checks (#7641) * create separate signature class for code delegations as they have different bound checks Signed-off-by: Daniel Lehrner * test if increasing xmx let's failing acceptance test pass Signed-off-by: Daniel Lehrner * javadoc Signed-off-by: Sally MacFarlane --------- Signed-off-by: Daniel Lehrner Signed-off-by: Sally MacFarlane Co-authored-by: Sally MacFarlane --- .github/workflows/acceptance-tests.yml | 2 +- .../besu/crypto/AbstractSECP256.java | 6 ++ .../besu/crypto/CodeDelegationSignature.java | 59 ++++++++++++ .../besu/crypto/SignatureAlgorithm.java | 11 +++ .../crypto/CodeDelegationSignatureTest.java | 93 +++++++++++++++++++ .../CodeDelegationTransactionDecoder.java | 5 +- .../mainnet/MainnetTransactionValidator.java | 8 ++ .../encoding/CodeDelegationDecoderTest.java | 12 +++ 8 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/CodeDelegationSignature.java create mode 100644 crypto/algorithms/src/test/java/org/hyperledger/besu/crypto/CodeDelegationSignatureTest.java diff --git a/.github/workflows/acceptance-tests.yml b/.github/workflows/acceptance-tests.yml index 287f402bce..a8f6981d89 100644 --- a/.github/workflows/acceptance-tests.yml +++ b/.github/workflows/acceptance-tests.yml @@ -11,7 +11,7 @@ concurrency: cancel-in-progress: true env: - GRADLE_OPTS: "-Xmx6g" + GRADLE_OPTS: "-Xmx7g" total-runners: 12 jobs: diff --git a/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/AbstractSECP256.java b/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/AbstractSECP256.java index b10e654626..ce37651266 100644 --- a/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/AbstractSECP256.java +++ b/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/AbstractSECP256.java @@ -212,6 +212,12 @@ public abstract class AbstractSECP256 implements SignatureAlgorithm { return SECPSignature.create(r, s, recId, curveOrder); } + @Override + public CodeDelegationSignature createCodeDelegationSignature( + final BigInteger r, final BigInteger s, final long yParity) { + return CodeDelegationSignature.create(r, s, yParity); + } + @Override public SECPSignature decodeSignature(final Bytes bytes) { return SECPSignature.decode(bytes, curveOrder); diff --git a/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/CodeDelegationSignature.java b/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/CodeDelegationSignature.java new file mode 100644 index 0000000000..4bb2e4653e --- /dev/null +++ b/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/CodeDelegationSignature.java @@ -0,0 +1,59 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * 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.crypto; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.math.BigInteger; + +/** Secp signature with code delegation. */ +public class CodeDelegationSignature extends SECPSignature { + private static final BigInteger TWO_POW_256 = BigInteger.TWO.pow(256); + + /** + * Instantiates a new SECPSignature. + * + * @param r the r part of the signature + * @param s the s part of the signature + * @param yParity the parity of the y coordinate of the public key + */ + public CodeDelegationSignature(final BigInteger r, final BigInteger s, final byte yParity) { + super(r, s, yParity); + } + + /** + * Create a new CodeDelegationSignature. + * + * @param r the r part of the signature + * @param s the s part of the signature + * @param yParity the parity of the y coordinate of the public key + * @return the new CodeDelegationSignature + */ + public static CodeDelegationSignature create( + final BigInteger r, final BigInteger s, final long yParity) { + checkNotNull(r); + checkNotNull(s); + + if (r.compareTo(TWO_POW_256) >= 0) { + throw new IllegalArgumentException("Invalid 'r' value, should be < 2^256 but got " + r); + } + + if (s.compareTo(TWO_POW_256) >= 0) { + throw new IllegalArgumentException("Invalid 's' value, should be < 2^256 but got " + s); + } + + return new CodeDelegationSignature(r, s, (byte) yParity); + } +} diff --git a/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithm.java b/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithm.java index db9565d18d..8e19b60854 100644 --- a/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithm.java +++ b/crypto/algorithms/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithm.java @@ -215,6 +215,17 @@ public interface SignatureAlgorithm { */ SECPSignature createSignature(final BigInteger r, final BigInteger s, final byte recId); + /** + * Create code delegation signature which have different bounds than transaction signatures. + * + * @param r the r part of the signature + * @param s the s part of the signature + * @param yParity the parity of the y coordinate of the public key + * @return the code delegation signature + */ + CodeDelegationSignature createCodeDelegationSignature( + final BigInteger r, final BigInteger s, final long yParity); + /** * Decode secp signature. * diff --git a/crypto/algorithms/src/test/java/org/hyperledger/besu/crypto/CodeDelegationSignatureTest.java b/crypto/algorithms/src/test/java/org/hyperledger/besu/crypto/CodeDelegationSignatureTest.java new file mode 100644 index 0000000000..1cc66966a7 --- /dev/null +++ b/crypto/algorithms/src/test/java/org/hyperledger/besu/crypto/CodeDelegationSignatureTest.java @@ -0,0 +1,93 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * 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.crypto; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +import java.math.BigInteger; + +import org.junit.jupiter.api.Test; + +class CodeDelegationSignatureTest { + + private static final BigInteger TWO_POW_256 = BigInteger.valueOf(2).pow(256); + + @Test + void testValidInputs() { + BigInteger r = BigInteger.ONE; + BigInteger s = BigInteger.TEN; + long yParity = 1L; + + CodeDelegationSignature result = CodeDelegationSignature.create(r, s, yParity); + + assertThat(r).isEqualTo(result.getR()); + assertThat(s).isEqualTo(result.getS()); + assertThat((byte) yParity).isEqualTo(result.getRecId()); + } + + @Test + void testNullRValue() { + BigInteger s = BigInteger.TEN; + long yParity = 0L; + + assertThatExceptionOfType(NullPointerException.class) + .isThrownBy(() -> CodeDelegationSignature.create(null, s, yParity)); + } + + @Test + void testNullSValue() { + BigInteger r = BigInteger.ONE; + long yParity = 0L; + + assertThatExceptionOfType(NullPointerException.class) + .isThrownBy(() -> CodeDelegationSignature.create(r, null, yParity)); + } + + @Test + void testRValueExceedsTwoPow256() { + BigInteger r = TWO_POW_256; + BigInteger s = BigInteger.TEN; + long yParity = 0L; + + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> CodeDelegationSignature.create(r, s, yParity)) + .withMessageContainingAll("Invalid 'r' value, should be < 2^256"); + } + + @Test + void testSValueExceedsTwoPow256() { + BigInteger r = BigInteger.ONE; + BigInteger s = TWO_POW_256; + long yParity = 0L; + + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> CodeDelegationSignature.create(r, s, yParity)) + .withMessageContainingAll("Invalid 's' value, should be < 2^256"); + } + + @Test + void testValidYParityZero() { + BigInteger r = BigInteger.ONE; + BigInteger s = BigInteger.TEN; + long yParity = 0L; + + CodeDelegationSignature result = CodeDelegationSignature.create(r, s, yParity); + + assertThat(r).isEqualTo(result.getR()); + assertThat(s).isEqualTo(result.getS()); + assertThat((byte) yParity).isEqualTo(result.getRecId()); + } +} diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationTransactionDecoder.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationTransactionDecoder.java index 8961431c9c..d3ef60bfc4 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationTransactionDecoder.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationTransactionDecoder.java @@ -81,13 +81,14 @@ public class CodeDelegationTransactionDecoder { final Address address = Address.wrap(input.readBytes()); final long nonce = input.readLongScalar(); - final byte yParity = (byte) input.readUnsignedByteScalar(); + final long yParity = input.readUnsignedIntScalar(); final BigInteger r = input.readUInt256Scalar().toUnsignedBigInteger(); final BigInteger s = input.readUInt256Scalar().toUnsignedBigInteger(); input.leaveList(); - final SECPSignature signature = SIGNATURE_ALGORITHM.get().createSignature(r, s, yParity); + final SECPSignature signature = + SIGNATURE_ALGORITHM.get().createCodeDelegationSignature(r, s, yParity); return new org.hyperledger.besu.ethereum.core.CodeDelegation( chainId, address, nonce, signature); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java index e67ccbb4c6..0e86b6b878 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionValidator.java @@ -52,6 +52,8 @@ import org.bouncycastle.crypto.digests.SHA256Digest; */ public class MainnetTransactionValidator implements TransactionValidator { + public static final BigInteger TWO_POW_256 = BigInteger.TWO.pow(256); + private final GasCalculator gasCalculator; private final GasLimitCalculator gasLimitCalculator; private final FeeMarket feeMarket; @@ -163,6 +165,12 @@ public class MainnetTransactionValidator implements TransactionValidator { .map( codeDelegations -> { for (CodeDelegation codeDelegation : codeDelegations) { + if (codeDelegation.chainId().compareTo(TWO_POW_256) >= 0) { + throw new IllegalArgumentException( + "Invalid 'chainId' value, should be < 2^256 but got " + + codeDelegation.chainId()); + } + if (codeDelegation.signature().getS().compareTo(halfCurveOrder) > 0) { return ValidationResult.invalid( TransactionInvalidReason.INVALID_SIGNATURE, diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationDecoderTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationDecoderTest.java index d6ff585b4f..a0c7689bc7 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationDecoderTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/CodeDelegationDecoderTest.java @@ -99,4 +99,16 @@ class CodeDelegationDecoderTest { assertThat(signature.getS().toString(16)) .isEqualTo("3c8a25b2becd6e666f69803d1ae3322f2e137b7745c2c7f19da80f993ffde4df"); } + + @Test + void shouldDecodeInnerPayloadWhenSignatureIsZero() { + final BytesValueRLPInput input = + new BytesValueRLPInput( + Bytes.fromHexString( + "0xdf8501a1f0ff5a947a40026a3b9a41754a95eec8c92c6b99886f440c5b808080"), + true); + final CodeDelegation authorization = CodeDelegationTransactionDecoder.decodeInnerPayload(input); + + assertThat(authorization.chainId()).isEqualTo(new BigInteger("01a1f0ff5a", 16)); + } }