diff --git a/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java b/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java index 2c70a21ef3b..7c4925c40fc 100644 --- a/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java +++ b/actuator/src/main/java/org/tron/core/utils/ProposalUtil.java @@ -956,6 +956,21 @@ public static void validator(DynamicPropertiesStore dynamicPropertiesStore, } break; } + case ALLOW_STRICT_ECDSA_VALIDATION: { + if (!forkController.pass(ForkBlockVersionEnum.VERSION_4_8_3)) { + throw new ContractValidateException( + "Bad chain parameter id [ALLOW_STRICT_ECDSA_VALIDATION]"); + } + if (dynamicPropertiesStore.allowStrictEcdsaValidation()) { + throw new ContractValidateException( + "[ALLOW_STRICT_ECDSA_VALIDATION] has been valid, no need to propose again"); + } + if (value != 1) { + throw new ContractValidateException( + "This value[ALLOW_STRICT_ECDSA_VALIDATION] is only allowed to be 1"); + } + break; + } default: break; } @@ -1045,7 +1060,8 @@ public enum ProposalType { // current value, value range ALLOW_TVM_OSAKA(96), // 0, 1 ALLOW_HARDEN_RESOURCE_CALCULATION(97), // 0, 1 ALLOW_HARDEN_EXCHANGE_CALCULATION(98), // 0, 1 - ALLOW_OPTIMIZE_TVM_STORAGE(99); // 0, 1 + ALLOW_OPTIMIZE_TVM_STORAGE(99), // 0, 1 + ALLOW_STRICT_ECDSA_VALIDATION(100); // 0, 1 private long code; ProposalType(long code) { diff --git a/actuator/src/main/java/org/tron/core/utils/TransactionUtil.java b/actuator/src/main/java/org/tron/core/utils/TransactionUtil.java index efb492d14c9..891671ff36d 100644 --- a/actuator/src/main/java/org/tron/core/utils/TransactionUtil.java +++ b/actuator/src/main/java/org/tron/core/utils/TransactionUtil.java @@ -18,7 +18,6 @@ import static org.tron.common.crypto.Hash.sha3omit12; import static org.tron.common.math.Maths.max; import static org.tron.core.config.Parameter.ChainConstant.DELEGATE_COST_BASE_SIZE; -import static org.tron.core.Constant.PER_SIGN_LENGTH; import static org.tron.core.config.Parameter.ChainConstant.TRX_PRECISION; import com.google.common.base.CaseFormat; @@ -38,6 +37,7 @@ import org.tron.api.GrpcAPI.TransactionExtention; import org.tron.api.GrpcAPI.TransactionSignWeight; import org.tron.api.GrpcAPI.TransactionSignWeight.Result; +import org.tron.common.crypto.SignUtils; import org.tron.common.utils.Sha256Hash; import org.tron.core.ChainBaseManager; import org.tron.core.capsule.AccountCapsule; @@ -182,16 +182,15 @@ public static String makeUpperCamelMethod(String originName) { .replace("_", ""); } - public static Transaction truncateSignatures(Transaction trx) { - Transaction.Builder builder = trx.toBuilder().clearSignature(); + /** + * Checks every supplied query signature before recovery, without rewriting the transaction. + */ + public static void validateSignatureLengths(Transaction trx) throws SignatureFormatException { for (ByteString sig : trx.getSignatureList()) { - if (sig.size() > PER_SIGN_LENGTH) { - builder.addSignature(ByteString.copyFrom(sig.substring(0, PER_SIGN_LENGTH).toByteArray())); - } else { - builder.addSignature(sig); + if (!SignUtils.isValidLength(sig.size())) { + throw new SignatureFormatException("Signature size is " + sig.size()); } } - return builder.build(); } public TransactionSignWeight getTransactionSignWeight(Transaction trx) { @@ -205,7 +204,13 @@ public TransactionSignWeight getTransactionSignWeight(Transaction trx) { return tswBuilder.build(); } - trx = truncateSignatures(trx); + try { + validateSignatureLengths(trx); + } catch (SignatureFormatException e) { + return tswBuilder.setResult(resultBuilder.setCode(Result.response_code.SIGNATURE_FORMAT_ERROR) + .setMessage(e.getMessage())).build(); + } + TransactionExtention.Builder trxExBuilder = TransactionExtention.newBuilder(); trxExBuilder.setTransaction(trx); trxExBuilder.setTxid(ByteString.copyFrom(Sha256Hash.hash(trx.getRawData().toByteArray()))); diff --git a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java index 36bbf49a935..2c4304398cb 100644 --- a/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java +++ b/actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java @@ -377,7 +377,8 @@ private static byte[] recoverAddrBySign(byte[] sign, byte[] hash) { Rsv rsv = Rsv.fromSignature(sign); ECDSASignature signature = SignUtils.fromComponents(rsv.getR(), rsv.getS(), rsv.getV()); if (signature.validateComponents()) { - out = SignUtils.signatureToAddress(hash, signature); + out = SignUtils.signatureToAddress(hash, signature, + VMConfig.allowStrictEcdsaValidation()); } } catch (Throwable any) { logger.info("ECRecover error", any.getMessage()); @@ -609,7 +610,8 @@ public Pair execute(byte[] data) { ECDSASignature signature = SignUtils.fromComponents(r, s, v[31]); if (validateV(v) && signature.validateComponents()) { - out = new DataWord(SignUtils.signatureToAddress(h, signature)); + out = new DataWord(SignUtils.signatureToAddress(h, signature, + VMConfig.allowStrictEcdsaValidation())); } } catch (Throwable any) { } @@ -1093,6 +1095,9 @@ public Pair execute(byte[] rawData) { List executedSignList = new ArrayList<>(); for (byte[] sign : signatures) { byte[] recoveredAddr = recoverAddrBySign(sign, hash); + if (recoveredAddr == null) { + return Pair.of(true, DATA_FALSE); + } sign = merge(recoveredAddr, sign); if (ByteArray.matrixContains(executedSignList, recoveredAddr)) { diff --git a/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java b/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java index 9d927f7039b..7e003446f85 100644 --- a/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java +++ b/actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java @@ -51,6 +51,7 @@ public static void load(StoreFactory storeFactory, boolean isolate) { snapshot.allowTvmOsaka = ds.getAllowTvmOsaka() == 1; snapshot.allowHardenResourceCalculation = ds.getAllowHardenResourceCalculation() == 1; snapshot.allowOptimizeTvmStorage = ds.getAllowOptimizeTvmStorage() == 1; + snapshot.allowStrictEcdsaValidation = ds.allowStrictEcdsaValidation(); if (isolate) { VMConfig.setLocalSnapshot(snapshot); } else { diff --git a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java index 42a1299ca47..4303845b782 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java @@ -15,6 +15,7 @@ package org.tron.core.capsule; +import static org.tron.core.Constant.PER_SIGN_LENGTH; import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED; import com.google.common.primitives.Longs; @@ -183,9 +184,14 @@ private Sha256Hash getRawHash() { public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore, AccountStore accountStore) throws ValidateSignatureException { try { + ByteString witnessSignature = block.getBlockHeader().getWitnessSignature(); + boolean strictEcdsaValidation = dynamicPropertiesStore.allowStrictEcdsaValidation(); + if (strictEcdsaValidation + && !SignUtils.isValidLength(witnessSignature.size())) { + throw new ValidateSignatureException("Invalid ECDSA signature format"); + } byte[] sigAddress = SignUtils.signatureToAddress(getRawHash().getBytes(), - TransactionCapsule.getBase64FromByteString( - block.getBlockHeader().getWitnessSignature())); + TransactionCapsule.getBase64FromByteString(witnessSignature), strictEcdsaValidation); byte[] witnessAccountAddress = block.getBlockHeader().getRawData().getWitnessAddress() .toByteArray(); @@ -328,18 +334,27 @@ public boolean hasWitnessSignature() { public boolean sanitize() { boolean blockHasUnknown = !this.block.getUnknownFields().asMap().isEmpty(); boolean headerHasUnknown = !this.block.getBlockHeader().getUnknownFields().asMap().isEmpty(); - if (!blockHasUnknown && !headerHasUnknown) { + ByteString witnessSignature = this.block.getBlockHeader().getWitnessSignature(); + boolean hasOverlongWitnessSignature = witnessSignature.size() > PER_SIGN_LENGTH; + if (!blockHasUnknown && !headerHasUnknown && !hasOverlongWitnessSignature) { return false; } UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance(); Block.Builder builder = this.block.toBuilder(); + BlockHeader.Builder headerBuilder = this.block.getBlockHeader().toBuilder(); if (blockHasUnknown) { builder.setUnknownFields(empty); } if (headerHasUnknown) { - builder.setBlockHeader(this.block.getBlockHeader().toBuilder() - .setUnknownFields(empty) - .build()); + headerBuilder.setUnknownFields(empty); + } + if (hasOverlongWitnessSignature) { + // Copy the canonical prefix so it does not retain the oversized signature's backing array. + headerBuilder.setWitnessSignature(ByteString.copyFrom( + witnessSignature.substring(0, PER_SIGN_LENGTH).toByteArray())); + } + if (headerHasUnknown || hasOverlongWitnessSignature) { + builder.setBlockHeader(headerBuilder.build()); } this.block = builder.build(); return true; diff --git a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java index f85158119a5..f1376c3c33c 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java @@ -100,8 +100,8 @@ public class TransactionCapsule implements ProtoCapsule { private static final long SLOW_SIG_VERIFY_MS = 50; private Transaction transaction; - @Setter - private boolean isVerified = false; + @Getter + private volatile boolean isVerified = false; @Setter @Getter private long blockNum = -1; @@ -233,6 +233,12 @@ public static long getWeight(Permission permission, byte[] address) { public static long checkWeight(Permission permission, List sigs, byte[] hash, List approveList) throws SignatureException, PermissionException, SignatureFormatException { + return checkWeight(permission, sigs, hash, approveList, true); + } + + public static long checkWeight(Permission permission, List sigs, byte[] hash, + List approveList, boolean strictEcdsaValidation) + throws SignatureException, PermissionException, SignatureFormatException { long currentWeight = 0; if (sigs.size() > permission.getKeysCount()) { throw new PermissionException( @@ -241,12 +247,13 @@ public static long checkWeight(Permission permission, List sigs, byt } HashMap addMap = new HashMap(); for (ByteString sig : sigs) { - if (sig.size() < 65) { + if (sig.size() < 65 + || (strictEcdsaValidation && !SignUtils.isValidLength(sig.size()))) { throw new SignatureFormatException( "Signature size is " + sig.size()); } String base64 = TransactionCapsule.getBase64FromByteString(sig); - byte[] address = SignUtils.signatureToAddress(hash, base64); + byte[] address = SignUtils.signatureToAddress(hash, base64, strictEcdsaValidation); long weight = getWeight(permission, address); if (weight == 0) { throw new PermissionException( @@ -486,7 +493,9 @@ public static boolean validateSignature(Transaction transaction, throw new PermissionException("permission isn't exit"); } checkPermission(permissionId, permission, contract); - long weight = checkWeight(permission, transaction.getSignatureList(), hash, null); + boolean strictEcdsaValidation = dynamicPropertiesStore.allowStrictEcdsaValidation(); + long weight = checkWeight(permission, transaction.getSignatureList(), hash, null, + strictEcdsaValidation); if (weight >= permission.getThreshold()) { return true; } @@ -689,7 +698,7 @@ void logSlowSigVerify(long startNs) { /** * validate signature */ - public boolean validateSignature(AccountStore accountStore, + public synchronized boolean validateSignature(AccountStore accountStore, DynamicPropertiesStore dynamicPropertiesStore) throws ValidateSignatureException { if (!isVerified) { //Do not support multi contracts in one transaction @@ -712,6 +721,10 @@ public boolean validateSignature(AccountStore accountStore, return true; } + public synchronized void setVerified(boolean verified) { + isVerified = verified; + } + public Sha256Hash getTransactionId() { if (this.id == null) { this.id = getRawHash(); diff --git a/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java b/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java index cc7cdbd9fa3..8b0f81bbf21 100644 --- a/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java +++ b/chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java @@ -255,6 +255,9 @@ public class DynamicPropertiesStore extends TronStoreWithRevoking private static final byte[] ALLOW_HARDEN_EXCHANGE_CALCULATION = "ALLOW_HARDEN_EXCHANGE_CALCULATION".getBytes(); + private static final byte[] ALLOW_STRICT_ECDSA_VALIDATION = + "ALLOW_STRICT_ECDSA_VALIDATION".getBytes(); + private static final byte[] TURKISH_KEY_MIGRATION_DONE = "TURKISH_KEY_MIGRATION_DONE".getBytes(); @@ -3073,6 +3076,21 @@ public boolean allowHardenExchangeCalculation() { return getAllowHardenExchangeCalculation() == 1L; } + public long getAllowStrictEcdsaValidation() { + return Optional.ofNullable(getUnchecked(ALLOW_STRICT_ECDSA_VALIDATION)) + .map(BytesCapsule::getData) + .map(ByteArray::toLong) + .orElse(0L); + } + + public void saveAllowStrictEcdsaValidation(long value) { + this.put(ALLOW_STRICT_ECDSA_VALIDATION, new BytesCapsule(ByteArray.fromLong(value))); + } + + public boolean allowStrictEcdsaValidation() { + return getAllowStrictEcdsaValidation() == 1L; + } + public void saveTurkishKeyMigrationDone(long num) { this.put(TURKISH_KEY_MIGRATION_DONE, new BytesCapsule(ByteArray.fromLong(num))); diff --git a/common/src/main/java/org/tron/common/prometheus/MetricKeys.java b/common/src/main/java/org/tron/common/prometheus/MetricKeys.java index 95a38c4b479..027829ffd80 100644 --- a/common/src/main/java/org/tron/common/prometheus/MetricKeys.java +++ b/common/src/main/java/org/tron/common/prometheus/MetricKeys.java @@ -20,6 +20,7 @@ public static class Counter { public static final String SR_SET_CHANGE = "tron:sr_set_change"; public static final String P2P_ERROR = "tron:p2p_error"; public static final String P2P_DISCONNECT = "tron:p2p_disconnect"; + public static final String TX_REJECT = "tron:tx_reject"; public static final String INTERNAL_SERVICE_FAIL = "tron:internal_service_fail"; private Counter() { diff --git a/common/src/main/java/org/tron/common/prometheus/MetricLabels.java b/common/src/main/java/org/tron/common/prometheus/MetricLabels.java index 1f0da214085..ac5e6d9fe26 100644 --- a/common/src/main/java/org/tron/common/prometheus/MetricLabels.java +++ b/common/src/main/java/org/tron/common/prometheus/MetricLabels.java @@ -31,6 +31,9 @@ public static class Counter { public static final String TXS_FAIL_SIG = "sig"; public static final String TXS_FAIL_TAPOS = "tapos"; public static final String TXS_FAIL_DUP = "dup"; + public static final String TX_REJECT_P2P = "p2p"; + public static final String TX_REJECT_API = "api"; + public static final String TX_REJECT_INVALID_SIGNATURE_LENGTH = "invalid_signature_length"; public static final String SR_ADD = "add"; public static final String SR_REMOVE = "remove"; diff --git a/common/src/main/java/org/tron/common/prometheus/MetricsCounter.java b/common/src/main/java/org/tron/common/prometheus/MetricsCounter.java index 7231baaba8f..a3fdc13efb1 100644 --- a/common/src/main/java/org/tron/common/prometheus/MetricsCounter.java +++ b/common/src/main/java/org/tron/common/prometheus/MetricsCounter.java @@ -17,6 +17,8 @@ class MetricsCounter { init(MetricKeys.Counter.SR_SET_CHANGE, "tron sr set change .", "action", "witness"); init(MetricKeys.Counter.P2P_ERROR, "tron p2p error info .", "type"); init(MetricKeys.Counter.P2P_DISCONNECT, "tron p2p disconnect .", "type"); + init(MetricKeys.Counter.TX_REJECT, "Transactions rejected at admission entry points.", + "source", "reason"); init(MetricKeys.Counter.INTERNAL_SERVICE_FAIL, "internal Service fail.", "class", "method"); } diff --git a/common/src/main/java/org/tron/core/Constant.java b/common/src/main/java/org/tron/core/Constant.java index 2710787b890..b8ecdba5d29 100644 --- a/common/src/main/java/org/tron/core/Constant.java +++ b/common/src/main/java/org/tron/core/Constant.java @@ -20,7 +20,6 @@ public class Constant { public static final long TRANSACTION_DEFAULT_EXPIRATION_TIME = 60 * 1_000L; //60 seconds public static final long TRANSACTION_FEE_POOL_PERIOD = 1; //1 blocks public static final int PER_SIGN_LENGTH = 65; - public static final int MAX_PER_SIGN_LENGTH = 68; public static final long MAX_CONTRACT_RESULT_SIZE = 2L; // Smart contract / Energy diff --git a/common/src/main/java/org/tron/core/config/Parameter.java b/common/src/main/java/org/tron/core/config/Parameter.java index 57954f2add4..3b3d4e2ffd7 100644 --- a/common/src/main/java/org/tron/core/config/Parameter.java +++ b/common/src/main/java/org/tron/core/config/Parameter.java @@ -32,7 +32,8 @@ public enum ForkBlockVersionEnum { VERSION_4_8_1_1(35, 1596780000000L, 70), VERSION_4_8_2(36, 1596780000000L, 80), VERSION_4_8_2_2(37, 1596780000000L, 70), - VERSION_4_8_2_3(38, 1596780000000L, 70); + VERSION_4_8_2_3(38, 1596780000000L, 70), + VERSION_4_8_3(39, 1596780000000L, 80); // if add a version, modify BLOCK_VERSION simultaneously @Getter @@ -81,7 +82,7 @@ public class ChainConstant { public static final int SINGLE_REPEAT = 1; public static final int BLOCK_FILLED_SLOTS_NUMBER = 128; public static final int MAX_FROZEN_NUMBER = 1; - public static final int BLOCK_VERSION = 38; + public static final int BLOCK_VERSION = 39; public static final long FROZEN_PERIOD = 86_400_000L; public static final long DELEGATE_PERIOD = 3 * 86_400_000L; public static final long TRX_PRECISION = 1000_000L; diff --git a/common/src/main/java/org/tron/core/vm/config/VMConfig.java b/common/src/main/java/org/tron/core/vm/config/VMConfig.java index 07b0bbe149e..11024ed5b73 100644 --- a/common/src/main/java/org/tron/core/vm/config/VMConfig.java +++ b/common/src/main/java/org/tron/core/vm/config/VMConfig.java @@ -45,6 +45,7 @@ public static class Snapshot { public boolean allowTvmOsaka; public boolean allowHardenResourceCalculation; public boolean allowOptimizeTvmStorage; + public boolean allowStrictEcdsaValidation; } // HEAD / block-processing config, written by the consensus path; read by everyone with no @@ -203,6 +204,10 @@ public static void initAllowOptimizeTvmStorage(long allow) { globalSnapshot.allowOptimizeTvmStorage = allow == 1; } + public static void initAllowStrictEcdsaValidation(long allow) { + globalSnapshot.allowStrictEcdsaValidation = allow == 1; + } + public static boolean getEnergyLimitHardFork() { return CommonParameter.ENERGY_LIMIT_HARD_FORK; } @@ -314,4 +319,8 @@ public static boolean allowHardenResourceCalculation() { public static boolean allowOptimizeTvmStorage() { return current().allowOptimizeTvmStorage; } + + public static boolean allowStrictEcdsaValidation() { + return current().allowStrictEcdsaValidation; + } } diff --git a/crypto/src/main/java/org/tron/common/crypto/ECKey.java b/crypto/src/main/java/org/tron/common/crypto/ECKey.java index f2a59854bc3..65ea263769d 100644 --- a/crypto/src/main/java/org/tron/common/crypto/ECKey.java +++ b/crypto/src/main/java/org/tron/common/crypto/ECKey.java @@ -45,6 +45,7 @@ import org.bouncycastle.math.ec.ECAlgorithms; import org.bouncycastle.math.ec.ECCurve; import org.bouncycastle.math.ec.ECPoint; +import org.bouncycastle.util.BigIntegers; import org.bouncycastle.util.encoders.Base64; import org.bouncycastle.util.encoders.Hex; import org.tron.common.crypto.jce.ECKeyFactory; @@ -373,6 +374,13 @@ public static ECKey fromNodeId(byte[] nodeId) { public static byte[] signatureToKeyBytes(byte[] messageHash, String signatureBase64) throws SignatureException { + return signatureToKeyBytes(messageHash, signatureBase64, false); + } + + public static byte[] signatureToKeyBytes(byte[] messageHash, String + signatureBase64, boolean strictValidation) throws SignatureException { + check(messageHash != null, "messageHash must not be null"); + check(signatureBase64 != null, "signature must not be null"); byte[] signatureEncoded; try { signatureEncoded = Base64.decode(signatureBase64); @@ -382,9 +390,10 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, String throw new SignatureException("Could not decode base64", e); } // Parse the signature bytes into r/s and the selector value. - if (signatureEncoded.length < 65) { - throw new SignatureException("Signature truncated, expected 65 " + - "bytes and got " + signatureEncoded.length); + if (signatureEncoded.length < 65 + || (strictValidation && signatureEncoded.length != 65)) { + throw new SignatureException("Signature has invalid length " + signatureEncoded.length + + ", expected " + (strictValidation ? "exactly " : "at least ") + "65 bytes"); } return signatureToKeyBytes( @@ -392,11 +401,15 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, String ECDSASignature.fromComponents( Arrays.copyOfRange(signatureEncoded, 1, 33), Arrays.copyOfRange(signatureEncoded, 33, 65), - (byte) (signatureEncoded[0] & 0xFF))); + (byte) (signatureEncoded[0] & 0xFF)), + strictValidation); } public static byte[] signatureToKeyBytes(byte[] messageHash, - ECDSASignature sig) throws SignatureException { + ECDSASignature sig, boolean strictValidation) throws SignatureException { + check(messageHash != null, "messageHash must not be null"); + check(sig != null && sig.r != null && sig.s != null, + "signature and its components must not be null"); check(messageHash.length == 32, "messageHash argument has length " + messageHash.length); int header = sig.v; @@ -412,7 +425,7 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, } int recId = header - 27; byte[] key = ECKey.recoverPubBytesFromSignature(recId, sig, - messageHash); + messageHash, strictValidation); if (key == null) { throw new SignatureException("Could not recover public key from " + "signature"); @@ -429,8 +442,13 @@ public static byte[] signatureToKeyBytes(byte[] messageHash, */ public static byte[] signatureToAddress(byte[] messageHash, String signatureBase64) throws SignatureException { + return signatureToAddress(messageHash, signatureBase64, false); + } + + public static byte[] signatureToAddress(byte[] messageHash, String + signatureBase64, boolean strictValidation) throws SignatureException { return Hash.computeAddress(signatureToKeyBytes(messageHash, - signatureBase64)); + signatureBase64, strictValidation)); } /** @@ -441,9 +459,8 @@ public static byte[] signatureToAddress(byte[] messageHash, String * @return 20-byte address */ public static byte[] signatureToAddress(byte[] messageHash, - ECDSASignature sig) throws - SignatureException { - return Hash.computeAddress(signatureToKeyBytes(messageHash, sig)); + ECDSASignature sig, boolean strictValidation) throws SignatureException { + return Hash.computeAddress(signatureToKeyBytes(messageHash, sig, strictValidation)); } /** @@ -454,9 +471,9 @@ public static byte[] signatureToAddress(byte[] messageHash, * @return ECKey */ public static ECKey signatureToKey(byte[] messageHash, String - signatureBase64) throws SignatureException { + signatureBase64, boolean strictValidation) throws SignatureException { final byte[] keyBytes = signatureToKeyBytes(messageHash, - signatureBase64); + signatureBase64, strictValidation); return ECKey.fromPublicOnly(keyBytes); } @@ -507,6 +524,14 @@ public static boolean isPubKeyCanonical(byte[] pubkey) { @Nullable public static byte[] recoverPubBytesFromSignature(int recId, ECDSASignature sig, byte[] messageHash) { + return recoverPubBytesFromSignature(recId, sig, messageHash, false); + } + + @Nullable + public static byte[] recoverPubBytesFromSignature(int recId, + ECDSASignature sig, byte[] messageHash, boolean strictValidation) { + check(sig != null && sig.r != null && sig.s != null, + "signature and its components must not be null"); check(recId >= 0, "recId must be positive"); check(sig.r.signum() >= 0, "r must be positive"); check(sig.s.signum() >= 0, "s must be positive"); @@ -515,6 +540,14 @@ public static byte[] recoverPubBytesFromSignature(int recId, // this function) // 1.1 Let x = r + jn BigInteger n = CURVE.getN(); // Curve order. + if (strictValidation) { + check(recId <= 3, "recId must be in the range [0, 3]"); + check(sig.r.signum() > 0 && sig.r.compareTo(n) < 0, + "r must be in the range [1, n)"); + check(sig.s.signum() > 0 && sig.s.compareTo(n) < 0, + "s must be in the range [1, n)"); + check(messageHash.length == 32, "messageHash must be 32 bytes"); + } BigInteger i = BigInteger.valueOf((long) recId / 2); BigInteger x = sig.r.add(i.multiply(n)); // 1.2. Convert the integer x to an octet string X of length mlen @@ -567,11 +600,16 @@ public static byte[] recoverPubBytesFromSignature(int recId, // inverse of 3 modulo 11 is 8 because 3 + 8 mod 11 = 0, and -3 mod // 11 = 8. BigInteger eInv = BigInteger.ZERO.subtract(e).mod(n); - BigInteger rInv = sig.r.modInverse(n); + BigInteger rInv = strictValidation + ? BigIntegers.modOddInverse(n, sig.r) + : sig.r.modInverse(n); BigInteger srInv = rInv.multiply(sig.s).mod(n); BigInteger eInvrInv = rInv.multiply(eInv).mod(n); ECPoint.Fp q = (ECPoint.Fp) ECAlgorithms.sumOfTwoMultiplies(CURVE .getG(), eInvrInv, R, srInv); + if (strictValidation && q.isInfinity()) { + return null; + } return q.getEncoded(/* compressed */ false); } @@ -584,8 +622,14 @@ public static byte[] recoverPubBytesFromSignature(int recId, @Nullable public static byte[] recoverAddressFromSignature(int recId, ECDSASignature sig, byte[] messageHash) { + return recoverAddressFromSignature(recId, sig, messageHash, false); + } + + @Nullable + public static byte[] recoverAddressFromSignature(int recId, + ECDSASignature sig, byte[] messageHash, boolean strictValidation) { final byte[] pubBytes = recoverPubBytesFromSignature(recId, sig, - messageHash); + messageHash, strictValidation); if (pubBytes == null) { return null; } else { @@ -602,8 +646,14 @@ public static byte[] recoverAddressFromSignature(int recId, @Nullable public static ECKey recoverFromSignature(int recId, ECDSASignature sig, byte[] messageHash) { + return recoverFromSignature(recId, sig, messageHash, false); + } + + @Nullable + public static ECKey recoverFromSignature(int recId, ECDSASignature sig, + byte[] messageHash, boolean strictValidation) { final byte[] pubBytes = recoverPubBytesFromSignature(recId, sig, - messageHash); + messageHash, strictValidation); if (pubBytes == null) { return null; } else { @@ -782,7 +832,7 @@ public ECDSASignature sign(byte[] messageHash) { int recId = -1; byte[] thisKey = this.pub.getEncoded(/* compressed */ false); for (int i = 0; i < 4; i++) { - byte[] k = ECKey.recoverPubBytesFromSignature(i, sig, messageHash); + byte[] k = ECKey.recoverPubBytesFromSignature(i, sig, messageHash, true); if (k != null && Arrays.equals(k, thisKey)) { recId = i; break; @@ -913,7 +963,6 @@ public static boolean validateComponents(BigInteger r, BigInteger s, return BIUtil.isLessThan(s, SECP256K1N); } - public boolean validateComponents() { return validateComponents(r, s, v); } diff --git a/crypto/src/main/java/org/tron/common/crypto/SignUtils.java b/crypto/src/main/java/org/tron/common/crypto/SignUtils.java index 0dae8ec82ba..fa3958e253a 100644 --- a/crypto/src/main/java/org/tron/common/crypto/SignUtils.java +++ b/crypto/src/main/java/org/tron/common/crypto/SignUtils.java @@ -1,6 +1,5 @@ package org.tron.common.crypto; -import static org.tron.core.Constant.MAX_PER_SIGN_LENGTH; import static org.tron.core.Constant.PER_SIGN_LENGTH; import java.security.SecureRandom; @@ -11,17 +10,15 @@ public class SignUtils { /** * Strict signature-length check for admission entry-points (RPC broadcast, - * P2P transaction ingress, peer hello handshake). Accepts only sizes in - * [{@link org.tron.core.Constant#PER_SIGN_LENGTH PER_SIGN_LENGTH}, - * {@link org.tron.core.Constant#MAX_PER_SIGN_LENGTH MAX_PER_SIGN_LENGTH}]. + * P2P transaction ingress, peer hello handshake). Accepts exactly + * {@link org.tron.core.Constant#PER_SIGN_LENGTH PER_SIGN_LENGTH} bytes. * - *

Consensus paths (e.g. {@code TransactionCapsule.checkWeight}) intentionally - * keep the looser {@code size < 65} check to remain compatible with historical - * on-chain signatures that carry trailing padding bytes; do not call this - * helper from those paths. + *

Consensus paths use the governance-controlled strict-validation flag. Their + * legacy behavior remains compatible with historical on-chain signatures that + * carry trailing padding bytes. */ public static boolean isValidLength(int size) { - return size >= PER_SIGN_LENGTH && size <= MAX_PER_SIGN_LENGTH; + return size == PER_SIGN_LENGTH; } public static SignInterface getGeneratedRandomSign(SecureRandom secureRandom) { @@ -34,8 +31,14 @@ public static SignInterface fromPrivate(byte[] privKeyBytes) { public static byte[] signatureToAddress(byte[] messageHash, String signatureBase64) throws SignatureException { + return signatureToAddress(messageHash, signatureBase64, false); + } + + public static byte[] signatureToAddress( + byte[] messageHash, String signatureBase64, boolean strictEcdsaValidation) + throws SignatureException { try { - return ECKey.signatureToAddress(messageHash, signatureBase64); + return ECKey.signatureToAddress(messageHash, signatureBase64, strictEcdsaValidation); } catch (Exception e) { throw new SignatureException(e); } @@ -47,6 +50,16 @@ public static ECDSASignature fromComponents(byte[] r, byte[] s, byte v) { public static byte[] signatureToAddress(byte[] messageHash, ECDSASignature signature) throws SignatureException { - return ECKey.signatureToAddress(messageHash, signature); + return signatureToAddress(messageHash, signature, false); + } + + public static byte[] signatureToAddress( + byte[] messageHash, ECDSASignature signature, boolean strictEcdsaValidation) + throws SignatureException { + try { + return ECKey.signatureToAddress(messageHash, signature, strictEcdsaValidation); + } catch (RuntimeException e) { + throw new SignatureException(e); + } } } diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 84fb41bf9f0..61be49a2d43 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -112,6 +112,9 @@ import org.tron.common.crypto.SignInterface; import org.tron.common.crypto.SignUtils; import org.tron.common.parameter.CommonParameter; +import org.tron.common.prometheus.MetricKeys; +import org.tron.common.prometheus.MetricLabels; +import org.tron.common.prometheus.Metrics; import org.tron.common.runtime.ProgramResult; import org.tron.common.runtime.vm.LogInfo; import org.tron.common.utils.ByteArray; @@ -511,6 +514,9 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) { try { for (ByteString sig : signedTransaction.getSignatureList()) { if (!SignUtils.isValidLength(sig.size())) { + Metrics.counterInc(MetricKeys.Counter.TX_REJECT, 1, + MetricLabels.Counter.TX_REJECT_API, + MetricLabels.Counter.TX_REJECT_INVALID_SIGNATURE_LENGTH); String info = "Signature size is " + sig.size(); logger.warn("Broadcast transaction {} has failed, {}.", txID, info); return builder.setResult(false).setCode(response_code.SIGERROR) @@ -640,7 +646,14 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) { return tswBuilder.build(); } - trx = TransactionUtil.truncateSignatures(trx); + try { + TransactionUtil.validateSignatureLengths(trx); + } catch (SignatureFormatException e) { + return tswBuilder.setResult(resultBuilder + .setCode(TransactionApprovedList.Result.response_code.SIGNATURE_FORMAT_ERROR) + .setMessage(e.getMessage())).build(); + } + TransactionExtention.Builder trxExBuilder = TransactionExtention.newBuilder(); trxExBuilder.setTransaction(trx); trxExBuilder.setTxid(ByteString.copyFrom(Sha256Hash.hash(trx.getRawData().toByteArray()))); @@ -1526,6 +1539,11 @@ public Protocol.ChainParameters getChainParameters() { .setValue(dbManager.getDynamicPropertiesStore().getAllowOptimizeTvmStorage()) .build()); + builder.addChainParameter(Protocol.ChainParameters.ChainParameter.newBuilder() + .setKey("getAllowStrictEcdsaValidation") + .setValue(dbManager.getDynamicPropertiesStore().getAllowStrictEcdsaValidation()) + .build()); + return builder.build(); } diff --git a/framework/src/main/java/org/tron/core/consensus/ProposalService.java b/framework/src/main/java/org/tron/core/consensus/ProposalService.java index 0442078fdec..ff0783fe61f 100644 --- a/framework/src/main/java/org/tron/core/consensus/ProposalService.java +++ b/framework/src/main/java/org/tron/core/consensus/ProposalService.java @@ -416,6 +416,11 @@ public static boolean process(Manager manager, ProposalCapsule proposalCapsule) manager.getDynamicPropertiesStore().saveAllowOptimizeTvmStorage(entry.getValue()); break; } + case ALLOW_STRICT_ECDSA_VALIDATION: { + manager.getDynamicPropertiesStore() + .saveAllowStrictEcdsaValidation(entry.getValue()); + break; + } default: find = false; break; diff --git a/framework/src/main/java/org/tron/core/db/Manager.java b/framework/src/main/java/org/tron/core/db/Manager.java index b9c94258fb3..d5ebaf8c8d7 100644 --- a/framework/src/main/java/org/tron/core/db/Manager.java +++ b/framework/src/main/java/org/tron/core/db/Manager.java @@ -52,6 +52,7 @@ import org.tron.common.args.GenesisBlock; import org.tron.common.bloom.Bloom; import org.tron.common.cron.CronExpression; +import org.tron.common.crypto.SignUtils; import org.tron.common.es.ExecutorServiceManager; import org.tron.common.exit.ExitManager; import org.tron.common.logsfilter.EventPluginLoader; @@ -1218,7 +1219,9 @@ public List getVerifyTxs(BlockCapsule block) { block.getTransactions().forEach(capsule -> { String address = Hex.toHexString(capsule.getOwnerAddress()); String txId = Hex.toHexString(capsule.getTransactionId().getBytes()); - if (multiAddresses.contains(address) || !isSameSig(capsule, txMap.get(txId))) { + TransactionCapsule pendingTx = txMap.get(txId); + if (multiAddresses.contains(address) || pendingTx == null || !pendingTx.isVerified() + || !isSameSig(capsule, pendingTx)) { txs.add(capsule); } else { capsule.setVerified(true); @@ -1877,7 +1880,14 @@ private void processBlock(BlockCapsule block, List txs) boolean flag = chainBaseManager.getDynamicPropertiesStore().getNextMaintenanceTime() <= block.getTimeStamp(); if (flag) { + boolean strictEcdsaValidation = getDynamicPropertiesStore() + .allowStrictEcdsaValidation(); proposalController.processProposals(); + if (!strictEcdsaValidation && getDynamicPropertiesStore() + .allowStrictEcdsaValidation()) { + // Legacy verification results must not survive the consensus-rule activation boundary. + invalidateTransactionVerificationCache(); + } } if (!consensus.applyBlock(block)) { @@ -1901,6 +1911,13 @@ private void processBlock(BlockCapsule block, List txs) block.setBloom(blockBloom); } + private void invalidateTransactionVerificationCache() { + pendingTransactions.forEach(tx -> tx.setVerified(false)); + rePushTransactions.forEach(tx -> tx.setVerified(false)); + poppedTransactions.forEach(tx -> tx.setVerified(false)); + pushTransactionQueue.forEach(tx -> tx.setVerified(false)); + } + private void payReward(BlockCapsule block) { WitnessCapsule witnessCapsule = chainBaseManager.getWitnessStore().getUnchecked(block.getInstance().getBlockHeader() @@ -2067,6 +2084,7 @@ private void preValidateTransactionSign(List txs) Histogram.Timer requestTimer = Metrics.histogramStartTimer( MetricKeys.Histogram.VERIFY_SIGN_LATENCY, MetricLabels.TRX); try { + validateTransactionSignatureLengths(txs); CountDownLatch countDownLatch = new CountDownLatch(transSize); List> futures = new ArrayList<>(transSize); @@ -2089,6 +2107,20 @@ private void preValidateTransactionSign(List txs) } } + private void validateTransactionSignatureLengths(List txs) + throws ValidateSignatureException { + boolean strictEcdsaValidation = chainBaseManager.getDynamicPropertiesStore() + .allowStrictEcdsaValidation(); + for (TransactionCapsule transaction : txs) { + for (ByteString signature : transaction.getInstance().getSignatureList()) { + if (signature.size() < Constant.PER_SIGN_LENGTH + || (strictEcdsaValidation && !SignUtils.isValidLength(signature.size()))) { + throw new ValidateSignatureException("Signature size is " + signature.size()); + } + } + } + } + public void rePush(TransactionCapsule tx) { if (containsTransaction(tx)) { return; diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java index 52137c5881c..a01c343c7d9 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java @@ -16,6 +16,9 @@ import org.springframework.stereotype.Component; import org.tron.common.crypto.SignUtils; import org.tron.common.es.ExecutorServiceManager; +import org.tron.common.prometheus.MetricKeys; +import org.tron.common.prometheus.MetricLabels; +import org.tron.common.prometheus.Metrics; import org.tron.common.utils.Sha256Hash; import org.tron.core.ChainBaseManager; import org.tron.core.config.args.Args; @@ -96,11 +99,16 @@ public void processMessage(PeerConnection peer, TronMessage msg) throws P2pExcep int smartContractQueueSize = 0; int trxHandlePoolQueueSize = 0; int dropSmartContractCount = 0; + int invalidSignatureCount = 0; for (Transaction trx : transactionsMessage.getTransactions().getTransactionsList()) { if (isClosed) { logger.info("TransactionsMsgHandler is closed during processing, stop submit"); break; } + if (!hasValidSignatureLengths(trx)) { + invalidSignatureCount++; + continue; + } int type = trx.getRawData().getContract(0).getType().getNumber(); if (type == ContractType.TriggerSmartContract_VALUE || type == ContractType.CreateSmartContract_VALUE) { @@ -120,6 +128,13 @@ public void processMessage(PeerConnection peer, TronMessage msg) throws P2pExcep } } + if (invalidSignatureCount > 0) { + Metrics.counterInc(MetricKeys.Counter.TX_REJECT, invalidSignatureCount, + MetricLabels.Counter.TX_REJECT_P2P, + MetricLabels.Counter.TX_REJECT_INVALID_SIGNATURE_LENGTH); + logger.debug("Drop {} transactions with invalid signature lengths from {}", + invalidSignatureCount, peer.getInetSocketAddress()); + } if (dropSmartContractCount > 0) { logger.warn("Add smart contract failed, drop count: {}, queueSize {}:{}", dropSmartContractCount, smartContractQueueSize, trxHandlePoolQueueSize); @@ -144,13 +159,16 @@ private void check(PeerConnection peer, TransactionsMessage msg) throws P2pExcep throw new P2pException(TypeEnum.BAD_TRX, "tx " + item.getHash() + " contract size should be greater than 0"); } - for (ByteString sig : trx.getSignatureList()) { - if (!SignUtils.isValidLength(sig.size())) { - throw new P2pException(TypeEnum.BAD_TRX, - "tx " + item.getHash() + " signature size is " + sig.size()); - } + } + } + + private boolean hasValidSignatureLengths(Transaction trx) { + for (ByteString sig : trx.getSignatureList()) { + if (!SignUtils.isValidLength(sig.size())) { + return false; } } + return true; } private void handleSmartContract() { @@ -216,4 +234,4 @@ public TrxEvent(PeerConnection peer, TransactionMessage msg) { this.time = System.currentTimeMillis(); } } -} \ No newline at end of file +} diff --git a/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java b/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java index c6810ef4167..4797457d982 100644 --- a/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java +++ b/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java @@ -158,7 +158,7 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) { Sha256Hash hash = Sha256Hash.of(ByteArray.fromLong(msg.getTimestamp())); String sig = TransactionCapsule.getBase64FromByteString(msg.getSignature()); - byte[] sigAddress = SignUtils.signatureToAddress(hash.getBytes(), sig); + byte[] sigAddress = SignUtils.signatureToAddress(hash.getBytes(), sig, true); if (manager.getDynamicPropertiesStore().getAllowMultiSign() != 1) { flag = Arrays.equals(sigAddress, msg.getAddress().toByteArray()); } else { diff --git a/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java b/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java index f847ef30bd8..21934cbc823 100644 --- a/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java +++ b/framework/src/test/java/org/tron/common/crypto/ECKeyTest.java @@ -5,16 +5,20 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.tron.common.utils.client.utils.AbiUtil.generateOccupationConstantPrivateKey; import java.math.BigInteger; +import java.nio.charset.StandardCharsets; import java.security.SignatureException; import java.util.Arrays; import lombok.extern.slf4j.Slf4j; import org.bouncycastle.asn1.sec.SECNamedCurves; +import org.bouncycastle.util.BigIntegers; +import org.bouncycastle.util.encoders.Base64; import org.bouncycastle.util.encoders.Hex; import org.junit.Test; import org.tron.common.crypto.ECKey.ECDSASignature; @@ -182,17 +186,148 @@ public void testSignIncorrectInputSize() { @Test(expected = SignatureException.class) public void testBadBase64Sig() throws SignatureException { byte[] messageHash = new byte[32]; - ECKey.signatureToKey(messageHash, "This is not valid Base64!"); + ECKey.signatureToKeyBytes(messageHash, "This is not valid Base64!", false); fail("Expecting a SignatureException for invalid Base64"); } @Test(expected = SignatureException.class) public void testInvalidSignatureLength() throws SignatureException { byte[] messageHash = new byte[32]; - ECKey.signatureToKey(messageHash, "abcdefg"); + ECKey.signatureToKeyBytes(messageHash, "abcdefg", false); fail("Expecting a SignatureException for invalid signature length"); } + @Test + public void testStrictSignatureLength() throws SignatureException { + byte[] messageHash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.ONE); + byte[] signature = Base64.decode(key.signHash(messageHash)); + byte[] padded = Arrays.copyOf(signature, 66); + String paddedBase64 = new String(Base64.encode(padded), StandardCharsets.UTF_8); + + assertArrayEquals(key.getPubKey(), + ECKey.signatureToKeyBytes(messageHash, paddedBase64, false)); + assertThrows(SignatureException.class, + () -> ECKey.signatureToKeyBytes(messageHash, paddedBase64, true)); + } + + @Test + public void testStrictRecoveryBoundsAndHighS() throws SignatureException { + byte[] messageHash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + ECDSASignature lowS = key.sign(messageHash); + int recId = lowS.v - 27; + BigInteger curveOrder = ECKey.CURVE.getN(); + + assertArrayEquals(key.getPubKey(), + ECKey.recoverPubBytesFromSignature(recId, lowS, messageHash, false)); + assertArrayEquals(key.getPubKey(), + ECKey.recoverPubBytesFromSignature(recId, lowS, messageHash, true)); + assertArrayEquals(key.getAddress(), + ECKey.recoverAddressFromSignature(recId, lowS, messageHash, true)); + assertArrayEquals(key.getPubKey(), + ECKey.recoverFromSignature(recId, lowS, messageHash, true).getPubKey()); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(-1, lowS, messageHash, true)); + assertNull(ECKey.recoverPubBytesFromSignature(4, lowS, messageHash, false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(4, lowS, messageHash, true)); + + for (BigInteger invalidScalar : Arrays.asList( + BigInteger.ZERO, curveOrder, curveOrder.add(BigInteger.ONE))) { + ECDSASignature invalidR = new ECDSASignature(invalidScalar, lowS.s); + invalidR.v = lowS.v; + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(recId, invalidR, messageHash, true)); + + ECDSASignature invalidS = new ECDSASignature(lowS.r, invalidScalar); + invalidS.v = lowS.v; + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(recId, invalidS, messageHash, true)); + } + + for (byte invalidHeader : new byte[]{26, 35}) { + ECDSASignature invalidSignature = new ECDSASignature(lowS.r, lowS.s); + invalidSignature.v = invalidHeader; + assertThrows(SignatureException.class, + () -> ECKey.signatureToKeyBytes(messageHash, invalidSignature, true)); + } + + ECDSASignature highS = new ECDSASignature( + lowS.r, curveOrder.subtract(lowS.s)); + highS.v = (byte) (27 + (recId ^ 1)); + assertArrayEquals(key.getPubKey(), ECKey.signatureToKeyBytes(messageHash, highS, true)); + } + + @Test + public void testRecoveryRejectsNullSignatureComponents() { + byte[] messageHash = new byte[32]; + ECDSASignature signature = new ECDSASignature(BigInteger.ONE, BigInteger.ONE); + signature.v = 27; + + assertThrows(IllegalArgumentException.class, + () -> ECKey.signatureToKeyBytes(messageHash, (ECDSASignature) null, true)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.signatureToKeyBytes(messageHash, (ECDSASignature) null, false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.signatureToKeyBytes(null, signature, true)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.signatureToKeyBytes(messageHash, (String) null, true)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.signatureToKeyBytes(messageHash, + new ECDSASignature(null, BigInteger.ONE), true)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.signatureToKeyBytes(messageHash, + new ECDSASignature(BigInteger.ONE, null), false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(0, null, messageHash, false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(0, + new ECDSASignature(null, BigInteger.ONE), messageHash, false)); + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverPubBytesFromSignature(0, + new ECDSASignature(BigInteger.ONE, null), messageHash, false)); + } + + @Test + public void testStrictRecoveryRejectsPointAtInfinity() { + byte[] messageHash = new byte[32]; + messageHash[messageHash.length - 1] = 1; + ECDSASignature signature = new ECDSASignature( + ECKey.CURVE.getG().getAffineXCoord().toBigInteger(), BigInteger.ONE); + signature.v = 27; + + assertArrayEquals(new byte[]{0}, + ECKey.recoverPubBytesFromSignature(0, signature, messageHash, false)); + assertNotNull(ECKey.recoverAddressFromSignature(0, signature, messageHash)); + // Key construction rejects infinity independently of the consensus recovery mode. + assertThrows(IllegalArgumentException.class, + () -> ECKey.recoverFromSignature(0, signature, messageHash)); + assertNull(ECKey.recoverPubBytesFromSignature(0, signature, messageHash, true)); + assertNull(ECKey.recoverAddressFromSignature(0, signature, messageHash, true)); + assertNull(ECKey.recoverFromSignature(0, signature, messageHash, true)); + assertThrows(SignatureException.class, + () -> ECKey.signatureToKeyBytes(messageHash, signature, true)); + } + + @Test + public void testModOddInverseConformance() { + BigInteger n = ECKey.CURVE.getN(); + BigInteger[] values = { + BigInteger.ONE, + BigInteger.valueOf(2), + BigInteger.TEN, + new BigInteger("123456789abcdef", 16), + n.subtract(BigInteger.ONE) + }; + + for (BigInteger value : values) { + BigInteger inverse = BigIntegers.modOddInverse(n, value); + assertEquals(value.modInverse(n), inverse); + assertEquals(BigInteger.ONE, value.multiply(inverse).mod(n)); + } + } + @Test public void testPublicKeyFromPrivate() { byte[] pubFromPriv = ECKey.publicKeyFromPrivate(privateKey, false); diff --git a/framework/src/test/java/org/tron/common/crypto/SignUtilsTest.java b/framework/src/test/java/org/tron/common/crypto/SignUtilsTest.java new file mode 100644 index 00000000000..8f70181faad --- /dev/null +++ b/framework/src/test/java/org/tron/common/crypto/SignUtilsTest.java @@ -0,0 +1,91 @@ +package org.tron.common.crypto; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.math.BigInteger; +import java.security.SignatureException; +import java.util.Arrays; +import org.junit.Test; +import org.tron.common.crypto.ECKey.ECDSASignature; + +public class SignUtilsTest { + + @Test + public void testValidEcdsaComponents() throws SignatureException { + byte[] hash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + ECDSASignature signature = key.sign(hash); + + assertArrayEquals(key.getAddress(), SignUtils.signatureToAddress(hash, signature)); + for (boolean strictValidation : new boolean[]{false, true}) { + assertArrayEquals(key.getAddress(), + SignUtils.signatureToAddress(hash, signature, strictValidation)); + } + } + + @Test + public void testValidEcdsaBase64() throws SignatureException { + byte[] hash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + String signature = key.sign(hash).toBase64(); + + assertArrayEquals(key.getAddress(), SignUtils.signatureToAddress(hash, signature)); + for (boolean strictValidation : new boolean[]{false, true}) { + assertArrayEquals(key.getAddress(), + SignUtils.signatureToAddress(hash, signature, strictValidation)); + } + } + + @Test + public void testStrictScalarBounds() { + byte[] hash = new byte[32]; + ECDSASignature valid = ECKey.fromPrivate(BigInteger.TEN).sign(hash); + BigInteger order = ECKey.CURVE.getN(); + + for (BigInteger scalar : Arrays.asList(BigInteger.ZERO, order, order.add(BigInteger.ONE))) { + ECDSASignature invalidR = new ECDSASignature(scalar, valid.s); + invalidR.v = valid.v; + ECDSASignature invalidS = new ECDSASignature(valid.r, scalar); + invalidS.v = valid.v; + for (ECDSASignature signature : Arrays.asList(invalidR, invalidS)) { + SignatureException componentError = assertThrows(SignatureException.class, + () -> SignUtils.signatureToAddress(hash, signature, true)); + SignatureException base64Error = assertThrows(SignatureException.class, + () -> SignUtils.signatureToAddress(hash, signature.toBase64(), true)); + assertTrue(componentError.getCause() instanceof IllegalArgumentException); + assertTrue(base64Error.getCause() instanceof IllegalArgumentException); + assertEquals(base64Error.getCause().getMessage(), componentError.getCause().getMessage()); + } + } + } + + @Test + public void testNullComponents() { + byte[] hash = new byte[32]; + for (ECDSASignature signature : Arrays.asList(null, + new ECDSASignature(null, BigInteger.ONE), new ECDSASignature(BigInteger.ONE, null))) { + SignatureException legacyError = assertThrows(SignatureException.class, + () -> SignUtils.signatureToAddress(hash, signature)); + SignatureException strictError = assertThrows(SignatureException.class, + () -> SignUtils.signatureToAddress(hash, signature, true)); + assertTrue(legacyError.getCause() instanceof IllegalArgumentException); + assertTrue(strictError.getCause() instanceof IllegalArgumentException); + } + } + + @Test + public void testInvalidHeader() { + byte[] hash = new byte[32]; + ECDSASignature signature = new ECDSASignature(BigInteger.ONE, BigInteger.ONE); + signature.v = 26; + + SignatureException error = assertThrows(SignatureException.class, + () -> SignUtils.signatureToAddress(hash, signature, true)); + assertEquals("Header byte out of range: 26", error.getMessage()); + assertNull(error.getCause()); + } +} diff --git a/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java b/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java index 8849e114c94..bb735ed93d8 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/BatchValidateSignContractTest.java @@ -1,5 +1,6 @@ package org.tron.common.runtime.vm; +import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -35,6 +36,54 @@ public class BatchValidateSignContractTest { PrecompiledContracts.BatchValidateSign contract = new BatchValidateSign(); + @Test + public void testStrictValidationMixedBatchConstantCall() { + assertStrictValidationMixedBatch(true); + } + + @Test + public void testStrictValidationMixedBatchWorkerCall() { + assertStrictValidationMixedBatch(false); + } + + private void assertStrictValidationMixedBatch(boolean constantCall) { + boolean previousStrict = VMConfig.allowStrictEcdsaValidation(); + try { + contract.setConstantCall(constantCall); + byte[] hash = DataWord.ONE().getData(); + ECKey first = ECKey.fromPrivate(BigInteger.ONE); + ECKey last = ECKey.fromPrivate(BigInteger.TEN); + // For hash = 1, R = G and s = 1 recover the point at infinity. + ECKey.ECDSASignature infinity = new ECKey.ECDSASignature( + ECKey.CURVE.getG().getAffineXCoord().toBigInteger(), BigInteger.ONE); + infinity.v = 27; + List signatures = Arrays.asList( + Hex.toHexString(first.sign(hash).toByteArray()), + Hex.toHexString(infinity.toByteArray()), + Hex.toHexString(last.sign(hash).toByteArray())); + List addresses = Arrays.asList(StringUtil.encode58Check(first.getAddress()), + StringUtil.encode58Check(Hash.sha3omit12(new byte[0])), + StringUtil.encode58Check(last.getAddress())); + List parameters = Arrays.asList("0x" + Hex.toHexString(hash), signatures, addresses); + byte[] input = Hex.decode(AbiUtil.parseParameters(METHOD_SIGN, parameters)); + + for (int strict : new int[] {0, 1}) { + VMConfig.initAllowStrictEcdsaValidation(strict); + // Allow for cold worker startup under coverage; this test checks results, not timing. + contract.setVmShouldEndInUs(System.nanoTime() / 1000 + 30_000_000); + Pair result = contract.execute(input); + byte[] expected = new byte[32]; + expected[0] = 1; + expected[1] = (byte) (strict == 0 ? 1 : 0); + expected[2] = 1; + Assert.assertTrue(result.getLeft()); + Assert.assertArrayEquals("strict=" + strict, expected, result.getRight()); + } + } finally { + VMConfig.initAllowStrictEcdsaValidation(previousStrict ? 1 : 0); + } + } + @Test public void staticCallTest() { contract.setConstantCall(true); diff --git a/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java b/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java index d5a50ea4f9d..5cde2cf97f7 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsTest.java @@ -11,12 +11,14 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.tuple.Pair; import org.bouncycastle.util.Arrays; +import org.bouncycastle.util.BigIntegers; import org.bouncycastle.util.encoders.Hex; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.crypto.ECKey; import org.tron.common.runtime.ProgramResult; import org.tron.common.utils.ByteArray; import org.tron.common.utils.ByteUtil; @@ -167,6 +169,29 @@ private PrecompiledContract createPrecompiledContract(DataWord addr, String owne return contract; } + @Test + public void testECRecoverPointAtInfinityStrictValidation() { + byte[] input = new byte[128]; + input[31] = 1; + input[63] = 27; + byte[] r = BigIntegers.asUnsignedByteArray( + 32, ECKey.CURVE.getG().getAffineXCoord().toBigInteger()); + System.arraycopy(r, 0, input, 64, r.length); + input[127] = 1; + PrecompiledContract contract = new PrecompiledContracts.ECRecover(); + boolean previousStrictValidation = VMConfig.allowStrictEcdsaValidation(); + + try { + VMConfig.initAllowStrictEcdsaValidation(0); + Assert.assertEquals(32, contract.execute(input).getRight().length); + + VMConfig.initAllowStrictEcdsaValidation(1); + Assert.assertArrayEquals(new byte[0], contract.execute(input).getRight()); + } finally { + VMConfig.initAllowStrictEcdsaValidation(previousStrictValidation ? 1 : 0); + } + } + //@Test public void voteWitnessNativeTest() throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, diff --git a/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java b/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java index 6dc561034db..1dfc8b709a0 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/VMConfigIsolationTest.java @@ -73,6 +73,20 @@ public void testSetGlobalConfigDropsLocalView() { assertFalse("setGlobalSnapshot must drop the thread-local view", VMConfig.allowTvmOsaka()); } + @Test + public void testSnapshotGlobalPreservesGovernanceFlags() { + VMConfig.initAllowOptimizeTvmStorage(1); + VMConfig.initAllowStrictEcdsaValidation(1); + VMConfig.Snapshot snapshot = snapshotGlobal(); + + VMConfig.initAllowOptimizeTvmStorage(0); + VMConfig.initAllowStrictEcdsaValidation(0); + VMConfig.setGlobalSnapshot(snapshot); + + assertTrue(VMConfig.allowOptimizeTvmStorage()); + assertTrue(VMConfig.allowStrictEcdsaValidation()); + } + // Deep-copy the current global config through the public getters (no thread-local set here, so // the getters read the global) so @After can restore the exact prior state. private static VMConfig.Snapshot snapshotGlobal() { @@ -104,6 +118,7 @@ private static VMConfig.Snapshot snapshotGlobal() { snapshot.allowTvmOsaka = VMConfig.allowTvmOsaka(); snapshot.allowHardenResourceCalculation = VMConfig.allowHardenResourceCalculation(); snapshot.allowOptimizeTvmStorage = VMConfig.allowOptimizeTvmStorage(); + snapshot.allowStrictEcdsaValidation = VMConfig.allowStrictEcdsaValidation(); return snapshot; } } diff --git a/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java b/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java index 15f5203a779..fe6dd02c70a 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/ValidateMultiSignContractTest.java @@ -1,6 +1,7 @@ package org.tron.common.runtime.vm; import com.google.protobuf.ByteString; +import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -64,6 +65,51 @@ public void testAddressNonExist() { .getValue(), DataWord.ZERO().getData()); } + @Test + public void testStrictValidationRejectsInfinityPermissionSigner() { + boolean previousStrict = VMConfig.allowStrictEcdsaValidation(); + try { + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + byte[] address = key.getAddress(); + byte[] data = DataWord.ONE().getData(); + byte[] hash = Sha256Hash.hash(ByteUtil.merge(address, ByteArray.fromInt(0), data)); + // R = G and s = hash mod n make sR - hash*G the point at infinity. + ECKey.ECDSASignature infinity = new ECKey.ECDSASignature( + ECKey.CURVE.getG().getAffineXCoord().toBigInteger(), + new BigInteger(1, hash).mod(ECKey.CURVE.getN())); + infinity.v = 27; + byte[] infinityAddress = Hash.sha3omit12(new byte[0]); + Assert.assertTrue(infinity.validateComponents()); + Assert.assertArrayEquals(infinityAddress, + ECKey.recoverAddressFromSignature(0, infinity, hash)); + + Protocol.Permission permission = Protocol.Permission.newBuilder() + .setType(Protocol.Permission.PermissionType.Owner).setId(0).setThreshold(1) + .addKeys(Protocol.Key.newBuilder().setAddress(ByteString.copyFrom(infinityAddress)) + .setWeight(1)) + .addKeys(Protocol.Key.newBuilder().setAddress(ByteString.copyFrom(address)).setWeight(1)) + .build(); + AccountCapsule account = new AccountCapsule(Protocol.Account.newBuilder() + .setAddress(ByteString.copyFrom(address)).setOwnerPermission(permission).build()); + dbManager.getAccountStore().put(address, account); + + for (int strict : new int[] {0, 1}) { + VMConfig.initAllowStrictEcdsaValidation(strict); + Pair result = validateMultiSign(StringUtil.encode58Check(address), 0, + data, Collections.singletonList(Hex.toHexString(infinity.toByteArray()))); + Assert.assertTrue(result.getLeft()); + Assert.assertArrayEquals(new DataWord(strict == 0 ? 1 : 0).getData(), result.getRight()); + + Pair valid = validateMultiSign(StringUtil.encode58Check(address), 0, + data, Collections.singletonList(Hex.toHexString(key.sign(hash).toByteArray()))); + Assert.assertTrue(valid.getLeft()); + Assert.assertArrayEquals(DataWord.ONE().getData(), valid.getRight()); + } + } finally { + VMConfig.initAllowStrictEcdsaValidation(previousStrict ? 1 : 0); + } + } + @Test public void testDifferentCase() { //Create an account with permission diff --git a/framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtils.java b/framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtils.java index e662d657e5f..9923a3486b1 100644 --- a/framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtils.java +++ b/framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtils.java @@ -24,6 +24,7 @@ import org.slf4j.LoggerFactory; import org.tron.common.crypto.ECKey; import org.tron.common.crypto.ECKey.ECDSASignature; +import org.tron.common.crypto.SignUtils; import org.tron.common.utils.ByteArray; import org.tron.common.utils.Sha256Hash; import org.tron.core.capsule.TransactionCapsule; @@ -138,10 +139,14 @@ public static boolean validTransaction(Transaction signedTransaction) { } for (int i = 0; i < count; ++i) { try { + ByteString signature = signedTransaction.getSignature(i); + if (!SignUtils.isValidLength(signature.size())) { + return false; + } Transaction.Contract contract = listContract.get(i); byte[] owner = getOwner(contract); - byte[] address = ECKey - .signatureToAddress(hash, getBase64FromByteString(signedTransaction.getSignature(i))); + byte[] address = SignUtils.signatureToAddress(hash, + getBase64FromByteString(signature), true); if (!Arrays.equals(owner, address)) { return false; } diff --git a/framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtilsTest.java b/framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtilsTest.java new file mode 100644 index 00000000000..2a260beaef3 --- /dev/null +++ b/framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtilsTest.java @@ -0,0 +1,97 @@ +package org.tron.common.utils.client.utils; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.google.protobuf.Any; +import com.google.protobuf.ByteString; +import java.math.BigInteger; +import java.security.SignatureException; +import java.util.Arrays; +import org.junit.Before; +import org.junit.Test; +import org.tron.common.crypto.ECKey; +import org.tron.common.crypto.ECKey.ECDSASignature; +import org.tron.common.crypto.Hash; +import org.tron.common.utils.Sha256Hash; +import org.tron.protos.Protocol.Transaction; +import org.tron.protos.Protocol.Transaction.Contract; +import org.tron.protos.Protocol.Transaction.Contract.ContractType; +import org.tron.protos.contract.BalanceContract.TransferContract; + +public class TransactionUtilsTest { + + private Transaction signed; + + @Before + public void setUp() { + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + signed = TransactionUtils.sign(createTransaction(key.getAddress()), key); + } + + @Test + public void testValidSignature() { + assertTrue(TransactionUtils.validTransaction(signed)); + } + + @Test + public void testShortSignatures() { + for (int length : new int[]{0, 64}) { + Transaction transaction = signed.toBuilder() + .setSignature(0, ByteString.copyFrom(new byte[length])).build(); + assertFalse(TransactionUtils.validTransaction(transaction)); + } + } + + @Test + public void testPaddedSignatures() { + for (int length : new int[]{66, 68, 69}) { + byte[] signature = Arrays.copyOf(signed.getSignature(0).toByteArray(), length); + Transaction transaction = signed.toBuilder() + .setSignature(0, ByteString.copyFrom(signature)).build(); + assertFalse(TransactionUtils.validTransaction(transaction)); + } + } + + @Test + public void testInvalidScalars() { + BigInteger order = ECKey.CURVE.getN(); + for (BigInteger scalar : Arrays.asList(BigInteger.ZERO, order, order.add(BigInteger.ONE))) { + for (ECDSASignature signature : Arrays.asList( + new ECDSASignature(scalar, BigInteger.ONE), + new ECDSASignature(BigInteger.ONE, scalar))) { + signature.v = 27; + Transaction transaction = signed.toBuilder() + .setSignature(0, ByteString.copyFrom(signature.toByteArray())).build(); + assertFalse(TransactionUtils.validTransaction(transaction)); + } + } + } + + @Test + public void testPointAtInfinity() throws SignatureException { + byte[] owner = Hash.computeAddress(new byte[]{0}); + Transaction transaction = createTransaction(owner); + byte[] hash = Sha256Hash.hash(transaction.getRawData().toByteArray()); + ECDSASignature signature = new ECDSASignature( + ECKey.CURVE.getG().getAffineXCoord().toBigInteger(), + new BigInteger(1, hash).mod(ECKey.CURVE.getN())); + signature.v = 27; + assertArrayEquals(owner, ECKey.signatureToAddress(hash, signature, false)); + + Transaction signedTransaction = transaction.toBuilder() + .addSignature(ByteString.copyFrom(signature.toByteArray())).build(); + assertFalse(TransactionUtils.validTransaction(signedTransaction)); + } + + private Transaction createTransaction(byte[] owner) { + TransferContract transfer = TransferContract.newBuilder() + .setOwnerAddress(ByteString.copyFrom(owner)) + .setToAddress(ByteString.copyFrom(owner)).setAmount(1).build(); + Contract contract = Contract.newBuilder().setType(ContractType.TransferContract) + .setParameter(Any.pack(transfer)).build(); + return Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder().addContract(contract)).build(); + } +} diff --git a/framework/src/test/java/org/tron/core/SignatureQueryTest.java b/framework/src/test/java/org/tron/core/SignatureQueryTest.java new file mode 100644 index 00000000000..edeb531fc28 --- /dev/null +++ b/framework/src/test/java/org/tron/core/SignatureQueryTest.java @@ -0,0 +1,228 @@ +package org.tron.core; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.google.protobuf.Any; +import com.google.protobuf.ByteString; +import java.util.Arrays; +import java.util.Collection; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.mockito.MockedStatic; +import org.springframework.test.util.ReflectionTestUtils; +import org.tron.api.GrpcAPI.TransactionApprovedList; +import org.tron.api.GrpcAPI.TransactionSignWeight; +import org.tron.common.crypto.SignInterface; +import org.tron.common.crypto.SignUtils; +import org.tron.common.utils.ForkController; +import org.tron.common.utils.Sha256Hash; +import org.tron.core.capsule.AccountCapsule; +import org.tron.core.capsule.TransactionCapsule; +import org.tron.core.services.http.Util; +import org.tron.core.store.AccountStore; +import org.tron.core.store.DynamicPropertiesStore; +import org.tron.core.utils.TransactionUtil; +import org.tron.protos.Protocol.Account; +import org.tron.protos.Protocol.Key; +import org.tron.protos.Protocol.Permission; +import org.tron.protos.Protocol.Transaction; +import org.tron.protos.Protocol.Transaction.Contract; +import org.tron.protos.Protocol.Transaction.Contract.ContractType; +import org.tron.protos.contract.BalanceContract.TransferContract; + +@RunWith(Parameterized.class) +public class SignatureQueryTest { + + @Parameterized.Parameters(name = "strict={0}") + public static Collection parameters() { + return Arrays.asList(new Object[][] { + {false}, {true} + }); + } + + @Parameterized.Parameter + public boolean strict; + + private Wallet wallet; + private TransactionUtil transactionUtil; + private AccountStore accountStore; + private Transaction unsigned; + private Transaction signed; + private Permission permission; + + @Before + public void setUp() { + DynamicPropertiesStore properties = mock(DynamicPropertiesStore.class); + when(properties.getTotalSignNum()).thenReturn(5); + when(properties.allowStrictEcdsaValidation()).thenReturn(strict); + when(properties.getAllowStrictEcdsaValidation()).thenReturn(strict ? 1L : 0L); + accountStore = mock(AccountStore.class); + ChainBaseManager manager = mock(ChainBaseManager.class); + when(manager.getDynamicPropertiesStore()).thenReturn(properties); + when(manager.getAccountStore()).thenReturn(accountStore); + wallet = new Wallet(); + transactionUtil = new TransactionUtil(); + ReflectionTestUtils.setField(wallet, "chainBaseManager", manager); + ReflectionTestUtils.setField(transactionUtil, "chainBaseManager", manager); + + byte[] privateKey = new byte[32]; + privateKey[31] = 10; + SignInterface signer = SignUtils.fromPrivate(privateKey); + ByteString address = ByteString.copyFrom(signer.getAddress()); + permission = Permission.newBuilder().setThreshold(1) + .addKeys(Key.newBuilder().setAddress(address).setWeight(1)).build(); + setPermission(permission); + unsigned = Transaction.newBuilder().setRawData(Transaction.raw.newBuilder() + .addContract(Contract.newBuilder().setType(ContractType.TransferContract) + .setParameter(Any.pack(TransferContract.newBuilder() + .setOwnerAddress(address).setToAddress(address).setAmount(1).build())))).build(); + TransactionCapsule capsule = new TransactionCapsule(unsigned); + capsule.sign(privateKey); + signed = capsule.getInstance(); + assertEquals(65, signed.getSignature(0).size()); + } + + @Test + public void rejectsEveryInvalidLengthBeforeConversionOrRecovery() { + try (MockedStatic capsules = + mockStatic(TransactionCapsule.class, CALLS_REAL_METHODS)) { + for (int length : new int[] {0, 64, 66, 68}) { + ByteString invalid = ByteString.copyFrom(new byte[length]); + assertFormatError(unsigned.toBuilder().addSignature(invalid).build()); + assertFormatError(signed.toBuilder().addSignature(invalid).build()); + assertFormatError(unsigned.toBuilder().addSignature(invalid) + .addSignature(signed.getSignature(0)).build()); + } + capsules.verify(() -> TransactionCapsule.getBase64FromByteString(any()), never()); + } + verifyNoInteractions(accountStore); + } + + @Test + public void rejectsPaddingWithoutRewritingTransactionOrChangingId() { + Transaction padded = signed.toBuilder().setSignature(0, + signed.getSignature(0).concat(ByteString.copyFrom(new byte[] {1}))).build(); + try (MockedStatic forks = mockStatic(ForkController.class)) { + forks.when(ForkController::instance).thenReturn(mock(ForkController.class)); + assertQueryResults(signed, TransactionSignWeight.Result.response_code.ENOUGH_PERMISSION, + TransactionApprovedList.Result.response_code.SUCCESS, 1); + assertFormatError(padded); + assertEquals(new TransactionCapsule(signed).getTransactionId(), + new TransactionCapsule(padded).getTransactionId()); + } + } + + @Test + public void preservesUnsignedQueryBehavior() { + assertQueryResults(unsigned, TransactionSignWeight.Result.response_code.NOT_ENOUGH_PERMISSION, + TransactionApprovedList.Result.response_code.SUCCESS, 0); + assertFormatError(unsigned.toBuilder().addSignature(ByteString.EMPTY).build()); + } + + @Test + public void validLengthStillRequiresPermission() { + Permission differentSigner = permission.toBuilder().setKeys(0, + permission.getKeys(0).toBuilder().setAddress(ByteString.copyFrom(new byte[21]))).build(); + setPermission(differentSigner); + assertQueryResults(signed, TransactionSignWeight.Result.response_code.PERMISSION_ERROR, + TransactionApprovedList.Result.response_code.OTHER_ERROR, 0); + } + + @Test + public void validLengthDoesNotGuaranteeEnoughWeight() { + setPermission(permission.toBuilder().setThreshold(2).build()); + try (MockedStatic forks = mockStatic(ForkController.class)) { + forks.when(ForkController::instance).thenReturn(mock(ForkController.class)); + assertQueryResults(signed, TransactionSignWeight.Result.response_code.NOT_ENOUGH_PERMISSION, + TransactionApprovedList.Result.response_code.SUCCESS, 1); + } + } + + @Test + public void checksSignatureCountBeforeLengthsOrHashing() { + Transaction.Builder tooMany = unsigned.toBuilder(); + for (int i = 0; i < 6; i++) { + tooMany.addSignature(signed.getSignature(0)); + } + for (Transaction input : Arrays.asList(tooMany.build(), + tooMany.addSignature(ByteString.EMPTY).build())) { + Transaction transaction = spy(input); + TransactionSignWeight weight = transactionUtil.getTransactionSignWeight(transaction); + TransactionApprovedList approved = wallet.getTransactionApprovedList(transaction); + assertEquals(TransactionSignWeight.Result.response_code.OTHER_ERROR, + weight.getResult().getCode()); + assertEquals(TransactionApprovedList.Result.response_code.OTHER_ERROR, + approved.getResult().getCode()); + assertEquals("too many signatures", weight.getResult().getMessage()); + assertEquals("too many signatures", approved.getResult().getMessage()); + assertFalse(weight.hasTransaction()); + assertFalse(approved.hasTransaction()); + verify(transaction, never()).getSignatureList(); + verify(transaction, never()).getRawData(); + } + verifyNoInteractions(accountStore); + } + + @Test + public void oversizedSignatureProducesSmallErrorResponse() { + Transaction input = unsigned.toBuilder() + .addSignature(ByteString.copyFrom(new byte[1024 * 1024])).build(); + assertFormatError(input); + TransactionSignWeight weight = transactionUtil.getTransactionSignWeight(input); + TransactionApprovedList approved = wallet.getTransactionApprovedList(input); + assertTrue(Util.printTransactionSignWeight(weight, false).length() < 256); + assertTrue(Util.printTransactionApprovedList(approved, false).length() < 256); + verifyNoInteractions(accountStore); + } + + private void setPermission(Permission value) { + when(accountStore.get(any(byte[].class))).thenReturn(new AccountCapsule(Account.newBuilder() + .setOwnerPermission(value).build())); + } + + private void assertFormatError(Transaction input) { + Transaction transaction = spy(input); + TransactionSignWeight weight = transactionUtil.getTransactionSignWeight(transaction); + TransactionApprovedList approved = wallet.getTransactionApprovedList(transaction); + assertEquals(TransactionSignWeight.Result.response_code.SIGNATURE_FORMAT_ERROR, + weight.getResult().getCode()); + assertEquals(TransactionApprovedList.Result.response_code.SIGNATURE_FORMAT_ERROR, + approved.getResult().getCode()); + assertFalse(weight.getResult().getMessage().isEmpty()); + assertFalse(approved.getResult().getMessage().isEmpty()); + assertEquals(0, weight.getApprovedListCount()); + assertEquals(0, approved.getApprovedListCount()); + assertFalse(weight.hasTransaction()); + assertFalse(approved.hasTransaction()); + verify(transaction, never()).getRawData(); + } + + private void assertQueryResults(Transaction input, + TransactionSignWeight.Result.response_code weightCode, + TransactionApprovedList.Result.response_code approvedCode, int approvedCount) { + TransactionSignWeight weight = transactionUtil.getTransactionSignWeight(input); + TransactionApprovedList approved = wallet.getTransactionApprovedList(input); + assertEquals(weightCode, weight.getResult().getCode()); + assertEquals(approvedCode, approved.getResult().getCode()); + assertEquals(approvedCount, weight.getApprovedListCount()); + assertEquals(approvedCount, approved.getApprovedListCount()); + assertEquals(input, weight.getTransaction().getTransaction()); + assertEquals(input, approved.getTransaction().getTransaction()); + ByteString txId = ByteString.copyFrom(Sha256Hash.hash(input.getRawData().toByteArray())); + assertEquals(txId, weight.getTransaction().getTxid()); + assertEquals(txId, approved.getTransaction().getTxid()); + } +} diff --git a/framework/src/test/java/org/tron/core/WalletMockTest.java b/framework/src/test/java/org/tron/core/WalletMockTest.java index 2f4c08d8f9f..4d8c87aeea6 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -1,6 +1,7 @@ package org.tron.core; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; @@ -19,6 +20,7 @@ import com.google.common.cache.CacheBuilder; import com.google.protobuf.Any; import com.google.protobuf.ByteString; +import io.prometheus.client.CollectorRegistry; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -38,6 +40,8 @@ import org.tron.api.GrpcAPI; import org.tron.common.crypto.Hash; import org.tron.common.parameter.CommonParameter; +import org.tron.common.prometheus.MetricKeys; +import org.tron.common.prometheus.MetricLabels; import org.tron.common.utils.ByteArray; import org.tron.common.utils.ByteUtil; import org.tron.common.utils.Sha256Hash; @@ -184,7 +188,7 @@ public void testBroadcastTxInvalidSigLength() throws Exception { GrpcAPI.Return ret = wallet.broadcastTransaction(shortSig); assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); - // signature longer than 68 bytes → SIGERROR + // signature longer than 65 bytes → SIGERROR Protocol.Transaction longSig = Protocol.Transaction.newBuilder() .addSignature(ByteString.copyFrom(new byte[69])) .build(); @@ -209,12 +213,93 @@ public void testBroadcastTxInvalidSigLength() throws Exception { ret = wallet.broadcastTransaction(validSig); assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode()); - // 68-byte signature (upper bound) also passes the length check + // padded signatures are rejected even when the first 65 bytes are present Protocol.Transaction paddedSig = Protocol.Transaction.newBuilder() .addSignature(ByteString.copyFrom(new byte[68])) .build(); ret = wallet.broadcastTransaction(paddedSig); - assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode()); + assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); + } + + @Test + public void testBroadcastSignatureRejectionMetric() { + boolean metricsEnabled = Args.getInstance().isMetricsPrometheusEnable(); + Args.getInstance().setMetricsPrometheusEnable(true); + try { + Wallet wallet = new Wallet(); + double before = rejectedTransactions(MetricLabels.Counter.TX_REJECT_API); + double p2pBefore = rejectedTransactions(MetricLabels.Counter.TX_REJECT_P2P); + int rejected = 0; + for (int length : new int[]{0, 64, 66, 68, 69}) { + Protocol.Transaction transaction = Protocol.Transaction.newBuilder() + .addSignature(ByteString.copyFrom(new byte[65])) + .addSignature(ByteString.copyFrom(new byte[length])) + .addSignature(ByteString.copyFrom(new byte[67])) + .build(); + GrpcAPI.Return ret = wallet.broadcastTransaction(transaction); + assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); + assertFalse(ret.getResult()); + assertEquals("Validate signature error: Signature size is " + length, + ret.getMessage().toStringUtf8()); + rejected++; + assertEquals(before + rejected, + rejectedTransactions(MetricLabels.Counter.TX_REJECT_API), 0); + } + assertEquals(p2pBefore, rejectedTransactions(MetricLabels.Counter.TX_REJECT_P2P), 0); + } finally { + Args.getInstance().setMetricsPrometheusEnable(metricsEnabled); + } + } + + @Test + public void testBroadcastValidLengthsDoNotIncrementRejectionMetric() throws Exception { + boolean metricsEnabled = Args.getInstance().isMetricsPrometheusEnable(); + Args.getInstance().setMetricsPrometheusEnable(true); + try { + Wallet wallet = new Wallet(); + TronNetDelegate delegate = mock(TronNetDelegate.class); + when(delegate.isBlockUnsolidified()).thenReturn(true); + Field field = Wallet.class.getDeclaredField("tronNetDelegate"); + field.setAccessible(true); + field.set(wallet, delegate); + double before = rejectedTransactions(MetricLabels.Counter.TX_REJECT_API); + for (int signatureCount : new int[]{0, 1, 2}) { + Protocol.Transaction.Builder builder = Protocol.Transaction.newBuilder(); + for (int i = 0; i < signatureCount; i++) { + builder.addSignature(ByteString.copyFrom(new byte[65])); + } + GrpcAPI.Return ret = wallet.broadcastTransaction(builder.build()); + assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode()); + assertEquals(before, rejectedTransactions(MetricLabels.Counter.TX_REJECT_API), 0); + } + } finally { + Args.getInstance().setMetricsPrometheusEnable(metricsEnabled); + } + } + + @Test + public void testBroadcastDisabledMetricPreservesSignatureRejection() { + boolean metricsEnabled = Args.getInstance().isMetricsPrometheusEnable(); + Args.getInstance().setMetricsPrometheusEnable(false); + try { + double before = rejectedTransactions(MetricLabels.Counter.TX_REJECT_API); + Protocol.Transaction transaction = Protocol.Transaction.newBuilder() + .addSignature(ByteString.copyFrom(new byte[68])) + .build(); + GrpcAPI.Return ret = new Wallet().broadcastTransaction(transaction); + assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); + assertFalse(ret.getResult()); + assertEquals(before, rejectedTransactions(MetricLabels.Counter.TX_REJECT_API), 0); + } finally { + Args.getInstance().setMetricsPrometheusEnable(metricsEnabled); + } + } + + private double rejectedTransactions(String source) { + Double value = CollectorRegistry.defaultRegistry.getSampleValue( + MetricKeys.Counter.TX_REJECT + "_total", new String[]{"source", "reason"}, + new String[]{source, MetricLabels.Counter.TX_REJECT_INVALID_SIGNATURE_LENGTH}); + return value == null ? 0 : value; } @Test diff --git a/framework/src/test/java/org/tron/core/WalletTest.java b/framework/src/test/java/org/tron/core/WalletTest.java index 7215a287912..14724c6704e 100644 --- a/framework/src/test/java/org/tron/core/WalletTest.java +++ b/framework/src/test/java/org/tron/core/WalletTest.java @@ -1498,7 +1498,7 @@ public void testApprovedListSigBound() { } @Test - public void testApprovedListSigTruncate() { + public void testApprovedListRejectsPaddedSignature() { ECKey ecKey = new ECKey(Utils.getRandom()); AccountCapsule owner = new AccountCapsule( ByteString.copyFromUtf8("approved-owner-trunc"), @@ -1529,15 +1529,10 @@ public void testApprovedListSigTruncate() { GrpcAPI.TransactionApprovedList reply = wallet.getTransactionApprovedList( unsigned.toBuilder().addSignature(oversized).build()); - // Recovery still succeeds and resolves the owner. - assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.SUCCESS, + assertEquals(GrpcAPI.TransactionApprovedList.Result.response_code.SIGNATURE_FORMAT_ERROR, reply.getResult().getCode()); - assertEquals(1, reply.getApprovedListCount()); - // The echoed-back transaction has the signature truncated to 65 bytes. - Transaction echoed = reply.getTransaction().getTransaction(); - assertEquals(1, echoed.getSignatureCount()); - assertEquals(65, echoed.getSignature(0).size()); - assertEquals(validSig, echoed.getSignature(0)); + assertEquals(0, reply.getApprovedListCount()); + Assert.assertFalse(reply.hasTransaction()); } @Test @@ -1577,4 +1572,3 @@ public void testApprovedListTooManySigs() { assertEquals(0, rejected.getApprovedListCount()); } } - diff --git a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java index 16a3cb3a5bb..ec16e1bffb3 100644 --- a/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java +++ b/framework/src/test/java/org/tron/core/actuator/utils/ProposalUtilTest.java @@ -69,6 +69,8 @@ public void validProposalTypeCheck() throws ContractValidateException { long code = 32; Assert.assertEquals(ProposalType.ALLOW_TVM_SOLIDITY_059, ProposalType.getEnum(code)); + Assert.assertEquals(ProposalType.ALLOW_OPTIMIZE_TVM_STORAGE, ProposalType.getEnum(99)); + Assert.assertEquals(ProposalType.ALLOW_STRICT_ECDSA_VALIDATION, ProposalType.getEnum(100)); } @@ -349,6 +351,8 @@ public void validateCheck() { testAllowHardenExchangeCalculationProposal(); + testAllowStrictEcdsaValidationProposal(); + forkUtils.getManager().getDynamicPropertiesStore() .statsByVersion(ForkBlockVersionEnum.ENERGY_LIMIT.getValue(), stats); forkUtils.reset(); @@ -744,6 +748,41 @@ private void testAllowHardenExchangeCalculationProposal() { } } + private void testAllowStrictEcdsaValidationProposal() { + long code = ProposalType.ALLOW_STRICT_ECDSA_VALIDATION.getCode(); + ThrowingRunnable proposeZero = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, + code, 0); + ThrowingRunnable proposeOne = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, + code, 1); + + ContractValidateException thrown = assertThrows(ContractValidateException.class, proposeOne); + assertEquals("Bad chain parameter id [ALLOW_STRICT_ECDSA_VALIDATION]", + thrown.getMessage()); + + activateFork(ForkBlockVersionEnum.VERSION_4_8_2_3); + thrown = assertThrows(ContractValidateException.class, proposeOne); + assertEquals("Bad chain parameter id [ALLOW_STRICT_ECDSA_VALIDATION]", + thrown.getMessage()); + + activateFork(ForkBlockVersionEnum.VERSION_4_8_3); + + thrown = assertThrows(ContractValidateException.class, proposeZero); + assertEquals("This value[ALLOW_STRICT_ECDSA_VALIDATION] is only allowed to be 1", + thrown.getMessage()); + + try { + proposeOne.run(); + } catch (Throwable e) { + Assert.fail("Should allow one-way activation: " + e.getMessage()); + } + + dynamicPropertiesStore.saveAllowStrictEcdsaValidation(1); + thrown = assertThrows(ContractValidateException.class, proposeOne); + assertEquals( + "[ALLOW_STRICT_ECDSA_VALIDATION] has been valid, no need to propose again", + thrown.getMessage()); + } + private void testAllowMarketTransaction() { ThrowingRunnable off = () -> ProposalUtil.validator(dynamicPropertiesStore, forkUtils, ProposalType.ALLOW_MARKET_TRANSACTION.getCode(), 0); diff --git a/framework/src/test/java/org/tron/core/actuator/utils/TransactionUtilTest.java b/framework/src/test/java/org/tron/core/actuator/utils/TransactionUtilTest.java index 54e611e0aac..126133ab041 100644 --- a/framework/src/test/java/org/tron/core/actuator/utils/TransactionUtilTest.java +++ b/framework/src/test/java/org/tron/core/actuator/utils/TransactionUtilTest.java @@ -464,7 +464,7 @@ public void testConcurrentToString() throws InterruptedException { } @Test - public void testSignWeightSigTruncate() { + public void testSignWeightRejectsPaddedSignature() { ECKey ecKey = new ECKey(Utils.getRandom()); AccountCapsule owner = new AccountCapsule( ByteString.copyFromUtf8("sign-weight-owner"), @@ -495,15 +495,10 @@ public void testSignWeightSigTruncate() { TransactionSignWeight reply = transactionUtil.getTransactionSignWeight( unsigned.toBuilder().addSignature(oversized).build()); - // Recovery still resolves the owner (weight reached the default threshold). - assertEquals(TransactionSignWeight.Result.response_code.ENOUGH_PERMISSION, + assertEquals(TransactionSignWeight.Result.response_code.SIGNATURE_FORMAT_ERROR, reply.getResult().getCode()); - assertEquals(1, reply.getApprovedListCount()); - // The echoed-back transaction has the signature truncated to 65 bytes. - Transaction echoed = reply.getTransaction().getTransaction(); - assertEquals(1, echoed.getSignatureCount()); - assertEquals(65, echoed.getSignature(0).size()); - assertEquals(validSig, echoed.getSignature(0)); + assertEquals(0, reply.getApprovedListCount()); + assertFalse(reply.hasTransaction()); } @Test diff --git a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java index 2f21064a842..7810fffb707 100644 --- a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java +++ b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java @@ -239,6 +239,37 @@ public void testValidateSignatureReturnsTrueWhenSignerMatches() throws Exception Assert.assertTrue(block.validateSignature(dps, accountStore)); } + @Test + public void shouldGateStrictWitnessSignatureLength() throws Exception { + String key = PublicMethod.getRandomPrivateKey(); + byte[] witnessAddress = PublicMethod.getAddressByteByPrivateKey(key); + BlockCapsule block = new BlockCapsule(4, + Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString( + "9938a342238077182498b464ac0292229938a342238077182498b464ac029222"))), + 6789, + ByteString.copyFrom(witnessAddress)); + block.sign(ByteArray.fromHexString(key)); + + ByteString signature = block.getInstance().getBlockHeader().getWitnessSignature(); + BlockHeader paddedHeader = block.getInstance().getBlockHeader().toBuilder() + .setWitnessSignature(signature.concat(ByteString.copyFrom(new byte[3]))) + .build(); + BlockCapsule paddedBlock = new BlockCapsule(block.getInstance().toBuilder() + .setBlockHeader(paddedHeader) + .build()); + + DynamicPropertiesStore dps = mock(DynamicPropertiesStore.class); + when(dps.getAllowMultiSign()).thenReturn(0L); + AccountStore accountStore = mock(AccountStore.class); + + Assert.assertTrue(paddedBlock.validateSignature(dps, accountStore)); + + when(dps.allowStrictEcdsaValidation()).thenReturn(true); + Assert.assertTrue(block.validateSignature(dps, accountStore)); + Assert.assertThrows(ValidateSignatureException.class, + () -> paddedBlock.validateSignature(dps, accountStore)); + } + /** * The other failure mode switchFork must handle: signature bytes are * malformed (cannot recover a public key). validateSignature wraps the diff --git a/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java b/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java index 9c2e004931e..d7907b76a72 100644 --- a/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java +++ b/framework/src/test/java/org/tron/core/capsule/TransactionCapsuleTest.java @@ -9,8 +9,13 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.read.ListAppender; import com.google.protobuf.ByteString; +import java.math.BigInteger; +import java.security.SignatureException; +import java.util.Arrays; import java.util.List; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.junit.Assert; @@ -20,10 +25,18 @@ import org.slf4j.LoggerFactory; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.crypto.ECKey; +import org.tron.common.utils.ByteUtil; import org.tron.common.utils.StringUtil; import org.tron.core.Wallet; import org.tron.core.config.args.Args; +import org.tron.core.exception.SignatureFormatException; +import org.tron.core.exception.ValidateSignatureException; +import org.tron.core.store.AccountStore; +import org.tron.core.store.DynamicPropertiesStore; import org.tron.protos.Protocol.AccountType; +import org.tron.protos.Protocol.Key; +import org.tron.protos.Protocol.Permission; import org.tron.protos.Protocol.Transaction; import org.tron.protos.Protocol.Transaction.Contract.ContractType; import org.tron.protos.Protocol.Transaction.Result; @@ -64,6 +77,93 @@ public void trxCapsuleClearTest() { .getRet(0).getContractRet(), Result.contractResult.OUT_OF_TIME); } + @Test + public void shouldGateStrictSignatureLength() throws Exception { + byte[] hash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + Permission permission = permissionFor(key); + ByteString signature = ByteString.copyFrom(key.Base64toBytes(key.signHash(hash))); + ByteString padded = signature.concat(ByteString.copyFrom(new byte[3])); + + Assert.assertEquals(1L, TransactionCapsule.checkWeight( + permission, Arrays.asList(signature), hash, null, true)); + Assert.assertEquals(1L, TransactionCapsule.checkWeight( + permission, Arrays.asList(signature), hash, null)); + Assert.assertEquals(1L, TransactionCapsule.checkWeight( + permission, Arrays.asList(padded), hash, null, false)); + Assert.assertThrows(SignatureFormatException.class, + () -> TransactionCapsule.checkWeight( + permission, Arrays.asList(padded), hash, null, true)); + } + + @Test + public void shouldRejectInvalidComponentsInStrictMode() { + byte[] hash = new byte[32]; + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + Permission permission = permissionFor(key); + byte[] signature = key.Base64toBytes(key.signHash(hash)); + BigInteger curveOrder = ECKey.CURVE.getN(); + + for (BigInteger invalidScalar : Arrays.asList( + BigInteger.ZERO, curveOrder, curveOrder.add(BigInteger.ONE))) { + assertStrictComponentRejected(permission, hash, + replaceScalar(signature, 0, invalidScalar)); + assertStrictComponentRejected(permission, hash, + replaceScalar(signature, 32, invalidScalar)); + } + + for (byte invalidV : new byte[]{8, 26, 35}) { + byte[] invalidSignature = Arrays.copyOf(signature, signature.length); + invalidSignature[64] = invalidV; + assertStrictComponentRejected(permission, hash, invalidSignature); + } + } + + @Test + public void testCheckWeightRejectsInfinityByDefault() throws Exception { + byte[] hash = new byte[32]; + hash[31] = 1; + byte[] signature = new byte[65]; + System.arraycopy(ByteUtil.bigIntegerToBytes( + ECKey.CURVE.getG().getAffineXCoord().toBigInteger(), 32), 0, signature, 0, 32); + signature[63] = 1; + Permission permission = Permission.newBuilder() + .setThreshold(1) + .addKeys(Key.newBuilder() + .setAddress(StringUtil.hexString2ByteString( + "41dcc703c0e500b653ca82273b7bfad8045d85a470")) + .setWeight(1)) + .build(); + List signatures = Arrays.asList(ByteString.copyFrom(signature)); + + Assert.assertEquals(1L, TransactionCapsule.checkWeight( + permission, signatures, hash, null, false)); + Assert.assertThrows(SignatureException.class, + () -> TransactionCapsule.checkWeight(permission, signatures, hash, null)); + } + + private byte[] replaceScalar(byte[] signature, int offset, BigInteger scalar) { + byte[] result = Arrays.copyOf(signature, signature.length); + System.arraycopy(ByteUtil.bigIntegerToBytes(scalar, 32), 0, result, offset, 32); + return result; + } + + private void assertStrictComponentRejected(Permission permission, byte[] hash, + byte[] signature) { + Assert.assertThrows(SignatureException.class, + () -> TransactionCapsule.checkWeight(permission, + Arrays.asList(ByteString.copyFrom(signature)), hash, null, true)); + } + + private Permission permissionFor(ECKey key) { + return Permission.newBuilder() + .setThreshold(1) + .addKeys(Key.newBuilder() + .setAddress(ByteString.copyFrom(key.getAddress())) + .setWeight(1)) + .build(); + } + @Test public void testRemoveRedundantRet() { Transaction.Builder transaction = Transaction.newBuilder().setRawData(raw.newBuilder() @@ -134,4 +234,65 @@ public void fastVerify() { capsuleLogger.setLevel(originalLevel); } } + + @Test + public void shouldInvalidateVerificationCacheAfterInFlightValidation() throws Exception { + CountDownLatch validationStarted = new CountDownLatch(1); + CountDownLatch continueValidation = new CountDownLatch(1); + CountDownLatch invalidationCompleted = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + Transaction transaction = Transaction.newBuilder() + .setRawData(raw.newBuilder() + .addContract(Transaction.Contract.newBuilder() + .setType(ContractType.TransferContract))) + .build(); + TransactionCapsule capsule = new TransactionCapsule(transaction) { + @Override + public boolean validatePubSignature(AccountStore accountStore, + DynamicPropertiesStore dynamicPropertiesStore) throws ValidateSignatureException { + validationStarted.countDown(); + try { + if (!continueValidation.await(5, TimeUnit.SECONDS)) { + throw new ValidateSignatureException("timed out waiting to continue validation"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new ValidateSignatureException("validation interrupted"); + } + return true; + } + }; + + Thread validationThread = new Thread(() -> { + try { + capsule.validateSignature(null, null); + } catch (Throwable t) { + failure.set(t); + } + }); + Thread invalidationThread = new Thread(() -> { + capsule.setVerified(false); + invalidationCompleted.countDown(); + }); + + try { + validationThread.start(); + Assert.assertTrue(validationStarted.await(5, TimeUnit.SECONDS)); + invalidationThread.start(); + Assert.assertFalse(invalidationCompleted.await(100, TimeUnit.MILLISECONDS)); + + continueValidation.countDown(); + validationThread.join(TimeUnit.SECONDS.toMillis(5)); + invalidationThread.join(TimeUnit.SECONDS.toMillis(5)); + + Assert.assertFalse(validationThread.isAlive()); + Assert.assertFalse(invalidationThread.isAlive()); + Assert.assertNull(failure.get()); + Assert.assertFalse(capsule.isVerified()); + } finally { + continueValidation.countDown(); + validationThread.interrupt(); + invalidationThread.interrupt(); + } + } } diff --git a/framework/src/test/java/org/tron/core/db/ManagerMockTest.java b/framework/src/test/java/org/tron/core/db/ManagerMockTest.java index 946bef022d2..4640cc3954a 100644 --- a/framework/src/test/java/org/tron/core/db/ManagerMockTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerMockTest.java @@ -26,6 +26,8 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; @@ -702,6 +704,37 @@ public void testSwitchForkPassesValidSignatureBlockToApply() { verify(goodBlock, atLeastOnce()).setSwitch(true); } + @Test + public void shouldInvalidateTransactionVerificationCacheAcrossAllQueues() + throws Exception { + Manager manager = new Manager(); + TransactionCapsule pendingTx = mock(TransactionCapsule.class); + TransactionCapsule rePushTx = mock(TransactionCapsule.class); + TransactionCapsule poppedTx = mock(TransactionCapsule.class); + TransactionCapsule pushingTx = mock(TransactionCapsule.class); + BlockingQueue pending = new LinkedBlockingQueue<>(); + BlockingQueue rePush = new LinkedBlockingQueue<>(); + List popped = new ArrayList<>(); + BlockingQueue pushing = new LinkedBlockingQueue<>(); + pending.add(pendingTx); + rePush.add(rePushTx); + popped.add(poppedTx); + pushing.add(pushingTx); + setField(manager, "pendingTransactions", pending); + setField(manager, "rePushTransactions", rePush); + setField(manager, "poppedTransactions", popped); + setField(manager, "pushTransactionQueue", pushing); + + Method method = Manager.class.getDeclaredMethod("invalidateTransactionVerificationCache"); + method.setAccessible(true); + method.invoke(manager); + + verify(pendingTx).setVerified(false); + verify(rePushTx).setVerified(false); + verify(poppedTx).setVerified(false); + verify(pushingTx).setVerified(false); + } + private static void setField(Object target, String name, Object value) throws Exception { Field f = target.getClass().getSuperclass() != null ? findField(target.getClass(), name) @@ -722,4 +755,4 @@ private static Field findField(Class cls, String name) throws NoSuchFieldExce throw new NoSuchFieldException(name); } -} \ No newline at end of file +} diff --git a/framework/src/test/java/org/tron/core/db/ManagerSignaturePreValidationTest.java b/framework/src/test/java/org/tron/core/db/ManagerSignaturePreValidationTest.java new file mode 100644 index 00000000000..091b7ebd4e5 --- /dev/null +++ b/framework/src/test/java/org/tron/core/db/ManagerSignaturePreValidationTest.java @@ -0,0 +1,144 @@ +package org.tron.core.db; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.protobuf.ByteString; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.FutureTask; +import org.junit.Test; +import org.tron.core.ChainBaseManager; +import org.tron.core.capsule.TransactionCapsule; +import org.tron.core.exception.ValidateSignatureException; +import org.tron.core.store.AccountStore; +import org.tron.core.store.DynamicPropertiesStore; +import org.tron.protos.Protocol.Transaction; + +public class ManagerSignaturePreValidationTest { + + @Test + public void testRejectStrictInvalidLengthBeforeSubmission() throws Exception { + ExecutorService validateSignService = mock(ExecutorService.class); + DynamicPropertiesStore dynamicPropertiesStore = mock(DynamicPropertiesStore.class); + Manager manager = createManager(validateSignService, dynamicPropertiesStore); + TransactionCapsule valid = transactionWithSignatures(65); + TransactionCapsule invalidMultiSign = transactionWithSignatures(65, 66); + + when(dynamicPropertiesStore.allowStrictEcdsaValidation()).thenReturn(true); + runSubmittedTasks(validateSignService); + + InvocationTargetException exception = assertThrows(InvocationTargetException.class, + () -> invokePreValidate(manager, Arrays.asList(valid, invalidMultiSign))); + + assertTrue(exception.getCause() instanceof ValidateSignatureException); + assertEquals("Signature size is 66", exception.getCause().getMessage()); + verify(validateSignService, never()).submit(any(Callable.class)); + } + + @Test + public void testRejectTruncatedLengthBeforeSubmission() throws Exception { + ExecutorService validateSignService = mock(ExecutorService.class); + DynamicPropertiesStore dynamicPropertiesStore = mock(DynamicPropertiesStore.class); + Manager manager = createManager(validateSignService, dynamicPropertiesStore); + TransactionCapsule transaction = transactionWithSignatures(64); + + when(dynamicPropertiesStore.allowStrictEcdsaValidation()).thenReturn(false); + runSubmittedTasks(validateSignService); + + InvocationTargetException exception = assertThrows(InvocationTargetException.class, + () -> invokePreValidate(manager, Collections.singletonList(transaction))); + + assertTrue(exception.getCause() instanceof ValidateSignatureException); + assertEquals("Signature size is 64", exception.getCause().getMessage()); + verify(validateSignService, never()).submit(any(Callable.class)); + } + + @Test + public void testPreserveLegacyPaddedSignature() throws Exception { + ExecutorService validateSignService = mock(ExecutorService.class); + DynamicPropertiesStore dynamicPropertiesStore = mock(DynamicPropertiesStore.class); + Manager manager = createManager(validateSignService, dynamicPropertiesStore); + TransactionCapsule transaction = transactionWithSignatures(66); + + when(dynamicPropertiesStore.allowStrictEcdsaValidation()).thenReturn(false); + runSubmittedTasks(validateSignService); + + invokePreValidate(manager, Collections.singletonList(transaction)); + + verify(validateSignService).submit(any(Callable.class)); + } + + @Test + public void testSubmitCanonicalStrictSignature() throws Exception { + ExecutorService validateSignService = mock(ExecutorService.class); + DynamicPropertiesStore dynamicPropertiesStore = mock(DynamicPropertiesStore.class); + Manager manager = createManager(validateSignService, dynamicPropertiesStore); + TransactionCapsule transaction = transactionWithSignatures(65); + + when(dynamicPropertiesStore.allowStrictEcdsaValidation()).thenReturn(true); + runSubmittedTasks(validateSignService); + + invokePreValidate(manager, Collections.singletonList(transaction)); + + verify(validateSignService).submit(any(Callable.class)); + } + + private Manager createManager(ExecutorService validateSignService, + DynamicPropertiesStore dynamicPropertiesStore) throws Exception { + Manager manager = new Manager(); + ChainBaseManager chainBaseManager = mock(ChainBaseManager.class); + when(chainBaseManager.getAccountStore()).thenReturn(mock(AccountStore.class)); + when(chainBaseManager.getDynamicPropertiesStore()).thenReturn(dynamicPropertiesStore); + setField(manager, "chainBaseManager", chainBaseManager); + setField(manager, "validateSignService", validateSignService); + return manager; + } + + private TransactionCapsule transactionWithSignatures(int... signatureLengths) + throws ValidateSignatureException { + Transaction.Builder transactionBuilder = Transaction.newBuilder(); + for (int signatureLength : signatureLengths) { + transactionBuilder.addSignature(ByteString.copyFrom(new byte[signatureLength])); + } + TransactionCapsule transaction = mock(TransactionCapsule.class); + when(transaction.getInstance()).thenReturn(transactionBuilder.build()); + when(transaction.validateSignature(any(AccountStore.class), + any(DynamicPropertiesStore.class))).thenReturn(true); + return transaction; + } + + private void runSubmittedTasks(ExecutorService validateSignService) { + when(validateSignService.submit(any(Callable.class))).thenAnswer(invocation -> { + Callable task = invocation.getArgument(0); + FutureTask future = new FutureTask<>(task); + future.run(); + return future; + }); + } + + private void invokePreValidate(Manager manager, List transactions) + throws Exception { + Method method = Manager.class.getDeclaredMethod("preValidateTransactionSign", List.class); + method.setAccessible(true); + method.invoke(manager, transactions); + } + + private void setField(Manager manager, String fieldName, Object value) throws Exception { + Field field = Manager.class.getDeclaredField(fieldName); + field.setAccessible(true); + field.set(manager, value); + } +} diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java index ecb315d8724..36035a0065d 100755 --- a/framework/src/test/java/org/tron/core/db/ManagerTest.java +++ b/framework/src/test/java/org/tron/core/db/ManagerTest.java @@ -844,10 +844,16 @@ public void getVerifyTxsTest() { List txs = dbManager.getVerifyTxs(capsule); Assert.assertEquals(txs.size(), 1); + t1.setVerified(true); dbManager.getPendingTransactions().add(t1); txs = dbManager.getVerifyTxs(capsule); Assert.assertEquals(txs.size(), 0); + t1.setVerified(false); + txs = dbManager.getVerifyTxs(capsule); + Assert.assertEquals(txs.size(), 1); + t1.setVerified(true); + list.add(t2.getInstance()); capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); txs = dbManager.getVerifyTxs(capsule); @@ -859,6 +865,7 @@ public void getVerifyTxsTest() { dbManager.getPendingTransactions().clear(); capsule = new BlockCapsule(0, ByteString.EMPTY, 0, list); + t2.setVerified(true); dbManager.getPendingTransactions().add(t1); dbManager.getPendingTransactions().add(t2); txs = dbManager.getVerifyTxs(capsule); diff --git a/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java b/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java index 7d883b7207d..e4bba0305e9 100644 --- a/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java +++ b/framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java @@ -6,9 +6,12 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.tron.core.Constant.PER_SIGN_LENGTH; import com.google.protobuf.ByteString; import com.google.protobuf.UnknownFieldSet; +import com.google.protobuf.UnsafeByteOperations; +import java.util.Arrays; import org.junit.BeforeClass; import org.junit.Test; import org.mockito.Mockito; @@ -22,9 +25,8 @@ /** * Verifies the {@code sanitize()} helpers on {@link BlockCapsule}, - * {@link TransactionCapsule} and {@link BlockMessage}: they strip outer - * unknown protobuf fields while leaving every consensus-hashed / signed - * region byte-identical. + * {@link TransactionCapsule} and {@link BlockMessage}: they canonicalize + * representations outside consensus-hashed / signed regions. */ public class SanitizeUnknownFieldsTest { @@ -107,6 +109,56 @@ public void blockCapsuleSanitizePreservesBlockHeaderRawData() { capsule.getInstance().getBlockHeader().getRawData()); } + @Test + public void testBlockCapsuleSanitizeTruncatesOverlongWitnessSignature() { + byte[] oversizedSignature = new byte[PER_SIGN_LENGTH + 1024]; + for (int i = 0; i < PER_SIGN_LENGTH; i++) { + oversizedSignature[i] = (byte) i; + } + byte[] canonicalSignature = Arrays.copyOf(oversizedSignature, PER_SIGN_LENGTH); + ByteString paddedSignature = UnsafeByteOperations.unsafeWrap(oversizedSignature); + Block padded = sampleBlock().toBuilder() + .setBlockHeader(sampleBlock().getBlockHeader().toBuilder() + .setWitnessSignature(paddedSignature) + .build()) + .build(); + BlockCapsule capsule = new BlockCapsule(padded); + BlockCapsule.BlockId originalBlockId = capsule.getBlockId(); + long originalSize = capsule.getSerializedSize(); + + assertTrue("sanitize() should report it truncated the witness signature", + capsule.sanitize()); + + ByteString sanitizedSignature = capsule.getInstance().getBlockHeader() + .getWitnessSignature(); + Arrays.fill(oversizedSignature, 0, PER_SIGN_LENGTH, (byte) 0x7f); + assertEquals(PER_SIGN_LENGTH, sanitizedSignature.size()); + assertArrayEquals("Sanitized signature must not retain the oversized backing array", + canonicalSignature, sanitizedSignature.toByteArray()); + assertEquals("Witness signature padding must not affect the block id", + originalBlockId, new BlockCapsule(capsule.getInstance()).getBlockId()); + assertTrue("Sanitized capsule bytes should shrink", + capsule.getSerializedSize() < originalSize); + assertFalse("Canonicalization should be idempotent", capsule.sanitize()); + } + + @Test + public void testBlockCapsuleSanitizePreservesCanonicalWitnessSignature() { + ByteString signature = ByteString.copyFrom(new byte[PER_SIGN_LENGTH]); + Block clean = sampleBlock().toBuilder() + .setBlockHeader(sampleBlock().getBlockHeader().toBuilder() + .setWitnessSignature(signature) + .build()) + .build(); + BlockCapsule capsule = new BlockCapsule(clean); + Block beforeInstance = capsule.getInstance(); + + assertFalse("A canonical witness signature should not be rewritten", capsule.sanitize()); + assertSame(beforeInstance, capsule.getInstance()); + assertEquals(signature, + capsule.getInstance().getBlockHeader().getWitnessSignature()); + } + @Test public void blockCapsuleSanitizeIsNoOpOnCleanBlock() { Block clean = sampleBlock(); @@ -186,6 +238,27 @@ public void blockMessageSanitizeUpdatesBothCapsuleAndWireBytes() throws Exceptio paddedBytes.length, msg.getData().length); } + @Test + public void testBlockMessageSanitizeRewritesOverlongWitnessSignature() throws Exception { + ByteString paddedSignature = ByteString.copyFrom(new byte[PER_SIGN_LENGTH + 1024]); + Block padded = sampleBlock().toBuilder() + .setBlockHeader(sampleBlock().getBlockHeader().toBuilder() + .setWitnessSignature(paddedSignature) + .build()) + .build(); + BlockMessage msg = new BlockMessage(padded.toByteArray()); + int originalSize = msg.getData().length; + + msg.sanitize(); + + assertEquals(PER_SIGN_LENGTH, msg.getBlockCapsule().getInstance() + .getBlockHeader().getWitnessSignature().size()); + assertTrue(msg.getData().length < originalSize); + assertArrayEquals(msg.getBlockCapsule().getData(), msg.getData()); + assertEquals(PER_SIGN_LENGTH, Block.parseFrom(msg.getData()) + .getBlockHeader().getWitnessSignature().size()); + } + @Test public void blockMessageSanitizeSkipsDataRewriteOnCleanBlock() throws Exception { byte[] cleanBytes = sampleBlock().toByteArray(); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java index 82ea2b6cb57..48d7e93bc1f 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/BlockMsgHandlerTest.java @@ -8,8 +8,11 @@ import com.google.protobuf.ByteString; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.math.BigInteger; import java.net.InetSocketAddress; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import javax.annotation.Resource; import lombok.extern.slf4j.Slf4j; @@ -17,21 +20,33 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.crypto.ECKey; import org.tron.common.utils.ByteArray; +import org.tron.common.utils.ReflectUtils; import org.tron.common.utils.Sha256Hash; import org.tron.core.Constant; import org.tron.core.capsule.BlockCapsule; import org.tron.core.capsule.BlockCapsule.BlockId; +import org.tron.core.capsule.TransactionCapsule; import org.tron.core.config.Parameter; import org.tron.core.config.args.Args; +import org.tron.core.db.Manager; import org.tron.core.exception.P2pException; import org.tron.core.net.TronNetDelegate; import org.tron.core.net.message.adv.BlockMessage; import org.tron.core.net.peer.Item; import org.tron.core.net.peer.PeerConnection; +import org.tron.core.net.service.adv.AdvService; +import org.tron.core.net.service.fetchblock.FetchBlockService; +import org.tron.core.net.service.sync.SyncService; +import org.tron.core.services.WitnessProductBlockService; +import org.tron.core.store.AccountStore; +import org.tron.core.store.DynamicPropertiesStore; +import org.tron.core.store.WitnessScheduleStore; import org.tron.p2p.connection.Channel; import org.tron.protos.Protocol.Inventory.InventoryType; import org.tron.protos.Protocol.Transaction; @@ -168,4 +183,140 @@ public void testProcessBlock() { Assert.fail(); } } + + @Test + public void testPaddedWitnessIngress() throws Exception { + BlockCapsule canonical = signedWitnessBlock(); + ByteString signature = canonical.getInstance().getBlockHeader().getWitnessSignature(); + for (boolean strict : new boolean[]{false, true}) { + TronNetDelegate delegate = witnessDelegate(strict, canonical.getWitnessAddress()); + AdvService adv = Mockito.mock(AdvService.class); + BlockMsgHandler ingress = witnessHandler(delegate, adv, Mockito.mock(SyncService.class)); + BlockMessage message = withWitnessSignature(canonical, + signature.concat(ByteString.copyFrom(new byte[3]))); + PeerConnection sender = witnessPeer(); + sender.getAdvInvRequest().put(new Item(message.getBlockId(), InventoryType.BLOCK), 0L); + + ingress.processMessage(sender, message); + + ArgumentCaptor processed = ArgumentCaptor.forClass(BlockCapsule.class); + Mockito.verify(delegate).processBlock(processed.capture(), Mockito.eq(false)); + Assert.assertEquals(canonical.getInstance(), processed.getValue().getInstance()); + ArgumentCaptor forwarded = ArgumentCaptor.forClass(BlockMessage.class); + Mockito.verify(adv).broadcast(forwarded.capture()); + Assert.assertArrayEquals(canonical.getData(), forwarded.getValue().getData()); + Assert.assertArrayEquals(canonical.getData(), message.getData()); + Assert.assertEquals(canonical.getBlockId(), message.getBlockId()); + Assert.assertTrue(sender.getAdvInvRequest().isEmpty()); + } + } + + @Test + public void testInvalidWitnessIngress() throws Exception { + BlockCapsule canonical = signedWitnessBlock(); + ByteString signature = canonical.getInstance().getBlockHeader().getWitnessSignature(); + for (boolean strict : new boolean[]{false, true}) { + for (ByteString invalid : Arrays.asList(signature.substring(0, 64), + ByteString.copyFrom(new byte[68]))) { + TronNetDelegate delegate = witnessDelegate(strict, canonical.getWitnessAddress()); + AdvService adv = Mockito.mock(AdvService.class); + BlockMsgHandler ingress = witnessHandler(delegate, adv, Mockito.mock(SyncService.class)); + BlockMessage message = withWitnessSignature(canonical, invalid); + PeerConnection sender = witnessPeer(); + sender.getAdvInvRequest().put(new Item(message.getBlockId(), InventoryType.BLOCK), 0L); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> ingress.processMessage(sender, message)); + + Assert.assertEquals(P2pException.TypeEnum.BLOCK_SIGN_INVALID, error.getType()); + Mockito.verify(delegate, Mockito.never()).processBlock(any(BlockCapsule.class), + anyBoolean()); + Mockito.verify(adv, Mockito.never()).broadcast(any()); + } + } + } + + @Test + public void testPaddedWitnessSyncHandoff() throws Exception { + BlockCapsule canonical = signedWitnessBlock(); + ByteString signature = canonical.getInstance().getBlockHeader().getWitnessSignature(); + BlockMessage message = withWitnessSignature(canonical, + signature.concat(ByteString.copyFrom(new byte[3]))); + SyncService sync = Mockito.mock(SyncService.class); + BlockMsgHandler ingress = witnessHandler(Mockito.mock(TronNetDelegate.class), + Mockito.mock(AdvService.class), sync); + PeerConnection sender = witnessPeer(); + sender.getSyncBlockRequested().put(message.getBlockId(), 0L); + + ingress.processMessage(sender, message); + + ArgumentCaptor queued = ArgumentCaptor.forClass(BlockMessage.class); + Mockito.verify(sync).processBlock(Mockito.eq(sender), queued.capture()); + Assert.assertArrayEquals(canonical.getData(), queued.getValue().getData()); + Assert.assertEquals(canonical.getInstance(), queued.getValue().getBlockCapsule().getInstance()); + Assert.assertTrue(sender.getSyncBlockRequested().isEmpty()); + Assert.assertTrue(sender.getSyncBlockInProcess().contains(canonical.getBlockId())); + } + + private BlockCapsule signedWitnessBlock() { + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + BlockCapsule block = new BlockCapsule(1, Sha256Hash.ZERO_HASH, 1234, + ByteString.copyFrom(key.getAddress())); + // A padded transaction signature must survive witness normalization, including sync handoff. + Transaction transaction = Transaction.newBuilder() + .setRawData(Transaction.raw.newBuilder().setTimestamp(1234)) + .addSignature(ByteString.copyFrom(new byte[68])).build(); + block.addTransaction(new TransactionCapsule(transaction)); + block.setMerkleRoot(); + block.sign(key.getPrivKeyBytes()); + return block; + } + + private BlockMessage withWitnessSignature(BlockCapsule block, ByteString signature) + throws Exception { + return new BlockMessage(block.getInstance().toBuilder() + .setBlockHeader(block.getInstance().getBlockHeader().toBuilder() + .setWitnessSignature(signature)).build().toByteArray()); + } + + private TronNetDelegate witnessDelegate(boolean strict, ByteString witness) throws Exception { + DynamicPropertiesStore properties = Mockito.mock(DynamicPropertiesStore.class); + Mockito.when(properties.allowStrictEcdsaValidation()).thenReturn(strict); + Manager manager = Mockito.mock(Manager.class); + Mockito.when(manager.getDynamicPropertiesStore()).thenReturn(properties); + Mockito.when(manager.getAccountStore()).thenReturn(Mockito.mock(AccountStore.class)); + WitnessScheduleStore schedule = Mockito.mock(WitnessScheduleStore.class); + Mockito.when(schedule.getActiveWitnesses()).thenReturn(Collections.singletonList(witness)); + TronNetDelegate delegate = Mockito.spy(new TronNetDelegate()); + ReflectUtils.setFieldValue(delegate, "dbManager", manager); + ReflectUtils.setFieldValue(delegate, "witnessScheduleStore", schedule); + Mockito.doReturn(true).when(delegate).containBlock(any(BlockId.class)); + Mockito.doReturn(new BlockId(Sha256Hash.ZERO_HASH, 0)).when(delegate).getHeadBlockId(); + Mockito.doReturn(Collections.emptyList()).when(delegate).getActivePeer(); + Mockito.doNothing().when(delegate).processBlock(any(BlockCapsule.class), anyBoolean()); + return delegate; + } + + private BlockMsgHandler witnessHandler(TronNetDelegate delegate, AdvService adv, SyncService sync) + throws Exception { + BlockMsgHandler ingress = new BlockMsgHandler(); + ReflectUtils.setFieldValue(ingress, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(ingress, "advService", adv); + ReflectUtils.setFieldValue(ingress, "syncService", sync); + ReflectUtils.setFieldValue(ingress, "fetchBlockService", Mockito.mock(FetchBlockService.class)); + ReflectUtils.setFieldValue(ingress, "witnessProductBlockService", + Mockito.mock(WitnessProductBlockService.class)); + ReflectUtils.setFieldValue(ingress, "fastForward", false); + return ingress; + } + + private PeerConnection witnessPeer() { + InetSocketAddress address = new InetSocketAddress("127.0.0.254", 10001); + Channel channel = Mockito.mock(Channel.class); + Mockito.when(channel.getInetAddress()).thenReturn(address.getAddress()); + Mockito.when(channel.getInetSocketAddress()).thenReturn(address); + PeerConnection sender = new PeerConnection(); + sender.setChannel(channel); + return sender; + } } diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java index 282c80f9f6d..f589b770ee5 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java @@ -2,10 +2,13 @@ import com.google.protobuf.Any; import com.google.protobuf.ByteString; +import io.prometheus.client.CollectorRegistry; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.time.ZonedDateTime; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.BlockingQueue; @@ -20,12 +23,16 @@ import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.prometheus.MetricKeys; +import org.tron.common.prometheus.MetricLabels; import org.tron.common.runtime.TvmTestUtils; import org.tron.common.utils.ByteArray; import org.tron.core.ChainBaseManager; +import org.tron.core.capsule.TransactionCapsule; import org.tron.core.config.args.Args; import org.tron.core.exception.P2pException; import org.tron.core.exception.P2pException.TypeEnum; @@ -396,102 +403,245 @@ public void testDuplicateTransactionRejected() throws Exception { @Test public void testInvalidSigLength() throws Exception { - TransactionsMsgHandler handler = new TransactionsMsgHandler(); - handler.init(); - ExecutorService originalPool = null; + Protocol.Transaction base = buildTransferMessage(1).getTransactions().getTransactions(0); + for (int length : new int[]{0, 64, 65, 66, 67, 68, 96}) { + Protocol.Transaction transaction = withSignatures(base, length); + List expected = length == 65 + ? Collections.singletonList(transaction) : Collections.emptyList(); + assertSignatureBatch(Collections.singletonList(transaction), expected, + Collections.emptyList()); + } + assertSignatureBatch(Collections.singletonList(base), Collections.singletonList(base), + Collections.emptyList()); + } + + @Test + public void testMixedSignatureLengths() throws Exception { + List base = buildTransferMessage(3).getTransactions() + .getTransactionsList(); + for (int invalidIndex = 0; invalidIndex < base.size(); invalidIndex++) { + List transactions = new ArrayList<>(); + List expected = new ArrayList<>(); + for (int i = 0; i < base.size(); i++) { + Protocol.Transaction transaction = i == invalidIndex + ? withSignatures(base.get(i), 65, 66 + invalidIndex) + : withSignatures(base.get(i), 65); + transactions.add(transaction); + if (i != invalidIndex) { + expected.add(transaction); + } + } + assertSignatureBatch(transactions, expected, Collections.emptyList()); + } + } + + @Test + public void testDroppedTransactionMetricCountsTransactions() throws Exception { + boolean metricsEnabled = Args.getInstance().isMetricsPrometheusEnable(); + Args.getInstance().setMetricsPrometheusEnable(true); try { - // Mock pool never executes submitted tasks: the async worker would invoke isBadPeer() - // on the stubbed peer concurrently with main-thread re-stubbing of getAdvInvRequest(), - // and Mockito's per-mock invocationForStubbing state is not thread-safe - // (intermittent WrongTypeOfReturnValue: ConcurrentHashMap cannot be returned by - // isBadPeer()). This test only asserts the synchronous check() length validation, - // so not running the worker is intentional. - ExecutorService mockPool = Mockito.mock(ExecutorService.class); - Future submittedTask = Mockito.mock(Future.class); - Mockito.when(mockPool.submit(Mockito.any(Runnable.class))) - .thenAnswer(invocation -> submittedTask); - originalPool = replaceTrxHandlePool(handler, mockPool); + List base = buildTransferMessage(3).getTransactions() + .getTransactionsList(); + Protocol.Transaction valid = withSignatures(base.get(0), 65); + Protocol.Transaction shortSignature = withSignatures(base.get(1), 64); + Protocol.Transaction invalidMultiSign = withSignatures(base.get(2), 65, 66, 68); + double before = droppedTransactions(); + + for (int batch = 1; batch <= 2; batch++) { + assertSignatureBatch(Arrays.asList(shortSignature, valid, invalidMultiSign), + Collections.singletonList(valid), Collections.emptyList()); + Assert.assertEquals(before + 2 * batch, droppedTransactions(), 0); + } + } finally { + Args.getInstance().setMetricsPrometheusEnable(metricsEnabled); + } + } - PeerConnection peer = Mockito.mock(PeerConnection.class); + @Test + public void testAcceptedTransactionsDoNotIncrementDropMetric() throws Exception { + boolean metricsEnabled = Args.getInstance().isMetricsPrometheusEnable(); + Args.getInstance().setMetricsPrometheusEnable(true); + try { + List base = buildTransferMessage(2).getTransactions() + .getTransactionsList(); + List transactions = Arrays.asList( + withSignatures(base.get(0), 65, 65), base.get(1)); + double before = droppedTransactions(); - BalanceContract.TransferContract transferContract = BalanceContract.TransferContract - .newBuilder() - .setAmount(10) - .setOwnerAddress(ByteString.copyFrom(ByteArray.fromHexString("121212a9cf"))) - .setToAddress(ByteString.copyFrom(ByteArray.fromHexString("232323a9cf"))) - .build(); + assertSignatureBatch(transactions, transactions, Collections.emptyList()); - // signature shorter than 65 bytes → BAD_TRX - Protocol.Transaction shortSigTrx = Protocol.Transaction.newBuilder() - .setRawData(Protocol.Transaction.raw.newBuilder() - .addContract(Protocol.Transaction.Contract.newBuilder() - .setType(Protocol.Transaction.Contract.ContractType.TransferContract) - .setParameter(Any.pack(transferContract)).build()) - .build()) - .addSignature(ByteString.copyFrom(new byte[64])) - .build(); + Assert.assertEquals(before, droppedTransactions(), 0); + } finally { + Args.getInstance().setMetricsPrometheusEnable(metricsEnabled); + } + } - List shortList = new ArrayList<>(); - shortList.add(shortSigTrx); - stubAdvInvRequest(peer, new TransactionsMessage(shortList)); - P2pException shortEx = Assert.assertThrows(P2pException.class, - () -> handler.processMessage(peer, new TransactionsMessage(shortList))); - Assert.assertEquals(TypeEnum.BAD_TRX, shortEx.getType()); + @Test + public void testDisabledDropMetricPreservesTransactionHandling() throws Exception { + boolean metricsEnabled = Args.getInstance().isMetricsPrometheusEnable(); + Args.getInstance().setMetricsPrometheusEnable(false); + try { + List base = buildTransferMessage(2).getTransactions() + .getTransactionsList(); + Protocol.Transaction valid = withSignatures(base.get(0), 65); + Protocol.Transaction invalid = withSignatures(base.get(1), 68); + double before = droppedTransactions(); - // signature longer than 68 bytes → BAD_TRX - Protocol.Transaction longSigTrx = Protocol.Transaction.newBuilder() - .setRawData(Protocol.Transaction.raw.newBuilder() - .setRefBlockNum(1) - .addContract(Protocol.Transaction.Contract.newBuilder() - .setType(Protocol.Transaction.Contract.ContractType.TransferContract) - .setParameter(Any.pack(transferContract)).build()) - .build()) - .addSignature(ByteString.copyFrom(new byte[69])) - .build(); + assertSignatureBatch(Arrays.asList(invalid, valid), + Collections.singletonList(valid), Collections.emptyList()); + + Assert.assertEquals(before, droppedTransactions(), 0); + } finally { + Args.getInstance().setMetricsPrometheusEnable(metricsEnabled); + } + } - List longList = new ArrayList<>(); - longList.add(longSigTrx); - stubAdvInvRequest(peer, new TransactionsMessage(longList)); - P2pException longEx = Assert.assertThrows(P2pException.class, - () -> handler.processMessage(peer, new TransactionsMessage(longList))); - Assert.assertEquals(TypeEnum.BAD_TRX, longEx.getType()); + private double droppedTransactions() { + Double value = CollectorRegistry.defaultRegistry.getSampleValue( + MetricKeys.Counter.TX_REJECT + "_total", new String[]{"source", "reason"}, + new String[]{MetricLabels.Counter.TX_REJECT_P2P, + MetricLabels.Counter.TX_REJECT_INVALID_SIGNATURE_LENGTH}); + return value == null ? 0 : value; + } - // exactly 65 bytes → passes the length check (no P2pException from check) - Protocol.Transaction validSigTrx = Protocol.Transaction.newBuilder() - .setRawData(Protocol.Transaction.raw.newBuilder() - .setRefBlockNum(2) - .addContract(Protocol.Transaction.Contract.newBuilder() - .setType(Protocol.Transaction.Contract.ContractType.TransferContract) - .setParameter(Any.pack(transferContract)).build()) - .build()) - .addSignature(ByteString.copyFrom(new byte[65])) - .build(); + @Test + public void testSmartContractSignatureLengths() throws Exception { + byte[] address = ByteArray.fromHexString("121212a9cf"); + Protocol.Transaction valid = withSignatures( + TvmTestUtils.generateTriggerSmartContractAndGetTransaction(address, address, + ByteArray.fromHexString("123456"), 100, 100000000, 0, 0), 65); + Protocol.Transaction invalid = withSignatures( + TvmTestUtils.generateTriggerSmartContractAndGetTransaction(address, address, + ByteArray.fromHexString("123457"), 100, 100000000, 0, 0), 65, 68); + Protocol.Transaction transfer = withSignatures( + buildTransferMessage(1).getTransactions().getTransactions(0), 65); + assertSignatureBatch(Arrays.asList(invalid, valid, transfer), + Collections.singletonList(transfer), Collections.singletonList(valid)); + } - List validList = new ArrayList<>(); - validList.add(validSigTrx); - stubAdvInvRequest(peer, new TransactionsMessage(validList)); - handler.processMessage(peer, new TransactionsMessage(validList)); + @Test + public void testInvalidSignaturePreservesProtocolChecks() throws Exception { + boolean metricsEnabled = Args.getInstance().isMetricsPrometheusEnable(); + Args.getInstance().setMetricsPrometheusEnable(true); + try { + double before = droppedTransactions(); + List base = buildTransferMessage(2).getTransactions() + .getTransactionsList(); + Protocol.Transaction padded = withSignatures(base.get(0), 68); + Protocol.Transaction valid = withSignatures(base.get(1), 65); + assertProtocolRejection(Arrays.asList(padded, valid), 1, TypeEnum.BAD_MESSAGE); + assertProtocolRejection(Arrays.asList(padded, withSignatures(base.get(0), 65)), 2, + TypeEnum.BAD_MESSAGE); + Protocol.Transaction noContract = valid.toBuilder() + .setRawData(valid.getRawData().toBuilder().clearContract()).build(); + assertProtocolRejection(Arrays.asList(padded, noContract), 2, TypeEnum.BAD_TRX); + Assert.assertEquals(before, droppedTransactions(), 0); + } finally { + Args.getInstance().setMetricsPrometheusEnable(metricsEnabled); + } + } - // 68 bytes (upper bound) also passes the length check - Protocol.Transaction paddedSigTrx = Protocol.Transaction.newBuilder() - .setRawData(Protocol.Transaction.raw.newBuilder() - .setRefBlockNum(3) - .addContract(Protocol.Transaction.Contract.newBuilder() - .setType(Protocol.Transaction.Contract.ContractType.TransferContract) - .setParameter(Any.pack(transferContract)).build()) - .build()) - .addSignature(ByteString.copyFrom(new byte[68])) - .build(); + private Protocol.Transaction withSignatures(Protocol.Transaction transaction, int... lengths) { + Protocol.Transaction.Builder builder = transaction.toBuilder().clearSignature(); + for (int length : lengths) { + builder.addSignature(ByteString.copyFrom(new byte[length])); + } + return builder.build(); + } - List paddedList = new ArrayList<>(); - paddedList.add(paddedSigTrx); - stubAdvInvRequest(peer, new TransactionsMessage(paddedList)); - handler.processMessage(peer, new TransactionsMessage(paddedList)); + private void assertSignatureBatch(List transactions, + List expectedTransfers, List expectedContracts) + throws Exception { + TransactionsMsgHandler handler = new TransactionsMsgHandler(); + ExecutorService originalPool = null; + try { + TronNetDelegate delegate = Mockito.mock(TronNetDelegate.class); + AdvService advService = Mockito.mock(AdvService.class); + ChainBaseManager chainBaseManager = Mockito.mock(ChainBaseManager.class); + setField(handler, "tronNetDelegate", delegate); + setField(handler, "advService", advService); + setField(handler, "chainBaseManager", chainBaseManager); + + ExecutorService pool = Mockito.mock(ExecutorService.class); + Future submittedTask = Mockito.mock(Future.class); + Mockito.when(pool.submit(Mockito.any(Runnable.class))).thenAnswer(invocation -> { + ((Runnable) invocation.getArgument(0)).run(); + return submittedTask; + }); + originalPool = replaceTrxHandlePool(handler, pool); + + PeerConnection peer = Mockito.mock(PeerConnection.class); + TransactionsMessage message = new TransactionsMessage(transactions); + stubAdvInvRequest(peer, message); + Map requests = peer.getAdvInvRequest(); + Protocol.Transaction unrelated = buildTransferMessage(4).getTransactions().getTransactions(3); + Item unrelatedItem = new Item(new TransactionMessage(unrelated).getMessageId(), + Protocol.Inventory.InventoryType.TRX); + requests.put(unrelatedItem, 0L); + + handler.processMessage(peer, message); + + Assert.assertEquals(Collections.singletonMap(unrelatedItem, 0L), requests); + Mockito.verify(peer, Mockito.never()).disconnect(Mockito.any()); + Mockito.verify(peer, Mockito.never()).setBadPeer(Mockito.anyBoolean()); + ArgumentCaptor pushed = ArgumentCaptor.forClass(TransactionCapsule.class); + Mockito.verify(delegate, Mockito.times(expectedTransfers.size())) + .pushTransaction(pushed.capture()); + List actualTransfers = new ArrayList<>(); + pushed.getAllValues().forEach(transaction -> actualTransfers.add(transaction.getInstance())); + Assert.assertEquals(expectedTransfers, actualTransfers); + ArgumentCaptor broadcast = + ArgumentCaptor.forClass(TransactionMessage.class); + Mockito.verify(advService, Mockito.times(expectedTransfers.size())) + .broadcast(broadcast.capture()); + List actualBroadcasts = new ArrayList<>(); + broadcast.getAllValues().forEach(transaction -> + actualBroadcasts.add(transaction.getTransactionCapsule().getInstance())); + Assert.assertEquals(expectedTransfers, actualBroadcasts); + + Field queueField = TransactionsMsgHandler.class.getDeclaredField("smartContractQueue"); + queueField.setAccessible(true); + BlockingQueue contracts = (BlockingQueue) queueField.get(handler); + List actualContracts = new ArrayList<>(); + for (Object entry : contracts) { + TransactionsMsgHandler.TrxEvent event = (TransactionsMsgHandler.TrxEvent) entry; + actualContracts.add(event.getMsg().getTransactionCapsule().getInstance()); + } + Assert.assertEquals(expectedContracts, actualContracts); } finally { closeHandlerAndOriginalPool(handler, originalPool); } } + private void assertProtocolRejection(List transactions, int requestedCount, + TypeEnum expected) throws Exception { + TransactionsMsgHandler handler = new TransactionsMsgHandler(); + ExecutorService originalPool = null; + try { + ExecutorService pool = Mockito.mock(ExecutorService.class); + originalPool = replaceTrxHandlePool(handler, pool); + PeerConnection peer = Mockito.mock(PeerConnection.class); + stubAdvInvRequest(peer, new TransactionsMessage(transactions.subList(0, requestedCount))); + int requestCount = peer.getAdvInvRequest().size(); + + P2pException error = Assert.assertThrows(P2pException.class, + () -> handler.processMessage(peer, new TransactionsMessage(transactions))); + + Assert.assertEquals(expected, error.getType()); + Assert.assertEquals(requestCount, peer.getAdvInvRequest().size()); + Mockito.verify(pool, Mockito.never()).submit(Mockito.any(Runnable.class)); + } finally { + closeHandlerAndOriginalPool(handler, originalPool); + } + } + + private void setField(TransactionsMsgHandler handler, String name, Object value) + throws Exception { + Field field = TransactionsMsgHandler.class.getDeclaredField(name); + field.setAccessible(true); + field.set(handler, value); + } + @Test public void testIsBusyWithCachedTransactions() throws Exception { TransactionsMsgHandler handler = new TransactionsMsgHandler(); diff --git a/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java b/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java index 6477640579e..0449a8cf992 100644 --- a/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java +++ b/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java @@ -8,8 +8,11 @@ import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.math.BigInteger; import java.net.InetSocketAddress; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Set; @@ -24,8 +27,11 @@ import org.springframework.context.ApplicationContext; import org.tron.common.BaseTest; import org.tron.common.TestConstants; +import org.tron.common.crypto.ECKey; +import org.tron.common.crypto.ECKey.ECDSASignature; import org.tron.common.crypto.SignInterface; import org.tron.common.crypto.SignUtils; +import org.tron.common.parameter.CommonParameter; import org.tron.common.utils.ByteArray; import org.tron.common.utils.ReflectUtils; import org.tron.common.utils.Sha256Hash; @@ -33,6 +39,7 @@ import org.tron.core.capsule.BlockCapsule; import org.tron.core.capsule.WitnessCapsule; import org.tron.core.config.args.Args; +import org.tron.core.db.Manager; import org.tron.core.net.P2pEventHandlerImpl; import org.tron.core.net.TronNetDelegate; import org.tron.core.net.TronNetService; @@ -42,6 +49,8 @@ import org.tron.core.net.peer.PeerConnection; import org.tron.core.net.peer.PeerManager; import org.tron.core.net.service.relay.RelayService; +import org.tron.core.store.DynamicPropertiesStore; +import org.tron.core.store.WitnessScheduleStore; import org.tron.p2p.P2pConfig; import org.tron.p2p.connection.Channel; import org.tron.p2p.discover.Node; @@ -246,6 +255,78 @@ private void testCheckHelloMessage() { } } + @Test + public void testStrictHelloAuthentication() throws Exception { + long timestamp = 1720000000000L; + byte[] hash = Sha256Hash.of(ByteArray.fromLong(timestamp)).getBytes(); + ECKey key = ECKey.fromPrivate(BigInteger.TEN); + ByteString address = ByteString.copyFrom(key.getAddress()); + ByteString signature = ByteString.copyFrom(key.sign(hash).toByteArray()); + ByteString paddedSignature = signature.concat(ByteString.copyFrom(new byte[3])); + + // R = G and s = e mod n make sR - eG the point at infinity for this HELLO digest. + BigInteger s = new BigInteger(1, hash).mod(ECKey.CURVE.getN()); + Assert.assertTrue(s.signum() > 0); + ECDSASignature infinity = new ECDSASignature( + ECKey.CURVE.getG().getAffineXCoord().toBigInteger(), s); + infinity.v = 27; + ByteString infinitySignature = ByteString.copyFrom(infinity.toByteArray()); + ByteString infinityAddress = getFromHexString("41dcc703c0e500b653ca82273b7bfad8045d85a470"); + Assert.assertArrayEquals(infinityAddress.toByteArray(), SignUtils.signatureToAddress(hash, + infinity.toBase64(), false)); + + RelayService relay = new RelayService(); + P2pConfig previousConfig = TronNetService.getP2pConfig(); + try { + CommonParameter parameter = mock(CommonParameter.class); + Mockito.when(parameter.isFastForward()).thenReturn(true); + WitnessScheduleStore schedule = mock(WitnessScheduleStore.class); + Mockito.when(schedule.getActiveWitnesses()) + .thenReturn(Arrays.asList(address, infinityAddress)); + DynamicPropertiesStore properties = mock(DynamicPropertiesStore.class); + Mockito.when(properties.getAllowMultiSign()).thenReturn(0L); + Manager manager = mock(Manager.class); + Mockito.when(manager.getDynamicPropertiesStore()).thenReturn(properties); + TronNetDelegate delegate = mock(TronNetDelegate.class); + Mockito.when(delegate.getActivePeer()).thenReturn(Collections.emptyList()); + ReflectUtils.setFieldValue(relay, "parameter", parameter); + ReflectUtils.setFieldValue(relay, "witnessScheduleStore", schedule); + ReflectUtils.setFieldValue(relay, "manager", manager); + ReflectUtils.setFieldValue(relay, "tronNetDelegate", delegate); + ReflectUtils.setFieldValue(tronNetService, "p2pConfig", new P2pConfig()); + + for (long flag : new long[]{0, 1}) { + Mockito.when(properties.getAllowStrictEcdsaValidation()).thenReturn(flag); + Mockito.when(properties.allowStrictEcdsaValidation()).thenReturn(flag == 1); + assertHelloAuthentication(relay, timestamp, address, signature, + "127.0." + flag + ".1", true); + assertHelloAuthentication(relay, timestamp, address, paddedSignature, + "127.0." + flag + ".2", false); + assertHelloAuthentication(relay, timestamp, infinityAddress, infinitySignature, + "127.0." + flag + ".3", false); + } + } finally { + ReflectUtils.setFieldValue(tronNetService, "p2pConfig", previousConfig); + relay.close(); + } + } + + private void assertHelloAuthentication(RelayService relay, long timestamp, ByteString address, + ByteString signature, String host, boolean expected) throws Exception { + InetSocketAddress socketAddress = new InetSocketAddress(host, 10001); + Channel channel = mock(Channel.class); + Mockito.when(channel.getInetSocketAddress()).thenReturn(socketAddress); + Mockito.when(channel.getInetAddress()).thenReturn(socketAddress.getAddress()); + HelloMessage message = new HelloMessage(Protocol.HelloMessage.newBuilder() + .setTimestamp(timestamp).setAddress(address).setSignature(signature).build().toByteArray()); + + Assert.assertFalse(TronNetService.getP2pConfig().getTrustNodes() + .contains(socketAddress.getAddress())); + Assert.assertEquals(expected, relay.checkHelloMessage(message, channel)); + Assert.assertEquals(expected, TronNetService.getP2pConfig().getTrustNodes() + .contains(socketAddress.getAddress())); + } + @Test public void testNullWitnessAddress() { try { diff --git a/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java b/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java index 5732e6f1cde..19719c3f1b6 100644 --- a/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java +++ b/framework/src/test/java/org/tron/core/services/ProposalServiceTest.java @@ -134,6 +134,31 @@ public void testUpdateConsensusLogicOptimization() { Assert.assertTrue(dbManager.getDynamicPropertiesStore().disableJavaLangMath()); } + @Test + public void testIndependentGovernanceActivation() { + long savedStorage = dbManager.getDynamicPropertiesStore().getAllowOptimizeTvmStorage(); + long savedEcdsa = dbManager.getDynamicPropertiesStore().getAllowStrictEcdsaValidation(); + try { + dbManager.getDynamicPropertiesStore().saveAllowOptimizeTvmStorage(0); + dbManager.getDynamicPropertiesStore().saveAllowStrictEcdsaValidation(0); + Proposal storageProposal = Proposal.newBuilder().putParameters(99, 1).build(); + + Assert.assertTrue(ProposalService.process(dbManager, new ProposalCapsule(storageProposal))); + Assert.assertTrue(dbManager.getDynamicPropertiesStore().allowOptimizeTvmStorage()); + Assert.assertFalse(dbManager.getDynamicPropertiesStore().allowStrictEcdsaValidation()); + + dbManager.getDynamicPropertiesStore().saveAllowOptimizeTvmStorage(0); + Proposal ecdsaProposal = Proposal.newBuilder().putParameters(100, 1).build(); + + Assert.assertTrue(ProposalService.process(dbManager, new ProposalCapsule(ecdsaProposal))); + Assert.assertTrue(dbManager.getDynamicPropertiesStore().allowStrictEcdsaValidation()); + Assert.assertFalse(dbManager.getDynamicPropertiesStore().allowOptimizeTvmStorage()); + } finally { + dbManager.getDynamicPropertiesStore().saveAllowOptimizeTvmStorage(savedStorage); + dbManager.getDynamicPropertiesStore().saveAllowStrictEcdsaValidation(savedEcdsa); + } + } + @Test public void testProposalExpireTime() { long defaultWindow = dbManager.getDynamicPropertiesStore().getProposalExpireTime(); @@ -151,4 +176,4 @@ public void testProposalExpireTime() { Assert.assertEquals(MAX_PROPOSAL_EXPIRE_TIME - 3000, window); } -} \ No newline at end of file +}