feat(crypto): add proposal-gated ECDSA validation

actuator/src/main/java/org/tron/core/utils/ProposalUtil.java

@@ -941,6 +941,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;
     }
@@ -1029,7 +1044,8 @@ public enum ProposalType {         // current value, value range
     ALLOW_TVM_PRAGUE(95), // 0, 1
     ALLOW_TVM_OSAKA(96), // 0, 1
     ALLOW_HARDEN_RESOURCE_CALCULATION(97), // 0, 1
-    ALLOW_HARDEN_EXCHANGE_CALCULATION(98); // 0, 1
+    ALLOW_HARDEN_EXCHANGE_CALCULATION(98), // 0, 1
+    ALLOW_STRICT_ECDSA_VALIDATION(99); // 0, 1
     private long code;
 
     ProposalType(long code) {

actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java

@@ -379,7 +379,8 @@ private static byte[] recoverAddrBySign(byte[] sign, byte[] hash) {
           CommonParameter.getInstance().isECKeyCryptoEngine());
       if (signature.validateComponents()) {
         out = SignUtils.signatureToAddress(hash, signature,
-            CommonParameter.getInstance().isECKeyCryptoEngine());
+            CommonParameter.getInstance().isECKeyCryptoEngine(),
+            VMConfig.allowStrictEcdsaValidation());
       }
     } catch (Throwable any) {
       logger.info("ECRecover error", any.getMessage());
@@ -616,8 +617,9 @@ public Pair<Boolean, byte[]> execute(byte[] data) {
         SignatureInterface signature = SignUtils.fromComponents(r, s, v[31]
             , CommonParameter.getInstance().isECKeyCryptoEngine());
         if (validateV(v) && signature.validateComponents()) {
-          out = new DataWord(SignUtils.signatureToAddress(h, signature
-              , CommonParameter.getInstance().isECKeyCryptoEngine()));
+          out = new DataWord(SignUtils.signatureToAddress(h, signature,
+              CommonParameter.getInstance().isECKeyCryptoEngine(),
+              VMConfig.allowStrictEcdsaValidation()));
         }
       } catch (Throwable any) {
       }

actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java

@@ -50,6 +50,7 @@ public static void load(StoreFactory storeFactory, boolean isolate) {
         snapshot.allowTvmSelfdestructRestriction = ds.getAllowTvmSelfdestructRestriction() == 1;
         snapshot.allowTvmOsaka = ds.getAllowTvmOsaka() == 1;
         snapshot.allowHardenResourceCalculation = ds.getAllowHardenResourceCalculation() == 1;
+        snapshot.allowStrictEcdsaValidation = ds.allowStrictEcdsaValidation();
         if (isolate) {
           VMConfig.setLocalSnapshot(snapshot);
         } else {

chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java

@@ -186,10 +186,17 @@ private Sha256Hash getRawHash() {
   public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore,
       AccountStore accountStore) throws ValidateSignatureException {
     try {
+      ByteString witnessSignature = block.getBlockHeader().getWitnessSignature();
+      boolean strictEcdsaValidation = CommonParameter.getInstance().isECKeyCryptoEngine()
+          && 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()),
-          CommonParameter.getInstance().isECKeyCryptoEngine());
+              witnessSignature),
+          CommonParameter.getInstance().isECKeyCryptoEngine(), strictEcdsaValidation);
       byte[] witnessAccountAddress = block.getBlockHeader().getRawData().getWitnessAddress()
           .toByteArray();
 

chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java

@@ -100,8 +100,8 @@ public class TransactionCapsule implements ProtoCapsule<Transaction> {
   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<ByteString> sigs, byte[] hash,
       List<ByteString> approveList)
       throws SignatureException, PermissionException, SignatureFormatException {
+    return checkWeight(permission, sigs, hash, approveList, false);
+  }
+
+  public static long checkWeight(Permission permission, List<ByteString> sigs, byte[] hash,
+      List<ByteString> approveList, boolean strictEcdsaValidation)
+      throws SignatureException, PermissionException, SignatureFormatException {
     long currentWeight = 0;
     if (sigs.size() > permission.getKeysCount()) {
       throw new PermissionException(
@@ -241,13 +247,15 @@ public static long checkWeight(Permission permission, List<ByteString> 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, CommonParameter.getInstance().isECKeyCryptoEngine());
+          .signatureToAddress(hash, base64, CommonParameter.getInstance().isECKeyCryptoEngine(),
+              strictEcdsaValidation);
       long weight = getWeight(permission, address);
       if (weight == 0) {
         throw new PermissionException(
@@ -488,7 +496,10 @@ 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 = CommonParameter.getInstance().isECKeyCryptoEngine()
+        && dynamicPropertiesStore.allowStrictEcdsaValidation();
+    long weight = checkWeight(permission, transaction.getSignatureList(), hash, null,
+        strictEcdsaValidation);
     if (weight >= permission.getThreshold()) {
       return true;
     }
@@ -695,7 +706,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
@@ -718,6 +729,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();

chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java

@@ -255,6 +255,9 @@ public class DynamicPropertiesStore extends TronStoreWithRevoking<BytesCapsule>
   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();
 
@@ -3071,6 +3074,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)));

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

common/src/main/java/org/tron/core/config/Parameter.java

@@ -30,7 +30,8 @@ public enum ForkBlockVersionEnum {
     VERSION_4_8_0_1(33, 1596780000000L, 70),
     VERSION_4_8_1(34, 1596780000000L, 80),
     VERSION_4_8_1_1(35, 1596780000000L, 70),
-    VERSION_4_8_2(36, 1596780000000L, 80);
+    VERSION_4_8_2(36, 1596780000000L, 80),
+    VERSION_4_8_3(37, 1596780000000L, 80);
     // if add a version, modify BLOCK_VERSION simultaneously
 
     @Getter
@@ -79,7 +80,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 = 36;
+    public static final int BLOCK_VERSION = 37;
     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;

common/src/main/java/org/tron/core/vm/config/VMConfig.java

@@ -46,6 +46,7 @@ public static class Snapshot {
     public boolean allowTvmSelfdestructRestriction;
     public boolean allowTvmOsaka;
     public boolean allowHardenResourceCalculation;
+    public boolean allowStrictEcdsaValidation;
   }
 
   // HEAD / block-processing config, written by the consensus path; read by everyone with no
@@ -204,6 +205,10 @@ public static void initAllowHardenResourceCalculation(long allow) {
     globalSnapshot.allowHardenResourceCalculation = allow == 1;
   }
 
+  public static void initAllowStrictEcdsaValidation(long allow) {
+    globalSnapshot.allowStrictEcdsaValidation = allow == 1;
+  }
+
   public static boolean getEnergyLimitHardFork() {
     return CommonParameter.ENERGY_LIMIT_HARD_FORK;
   }
@@ -311,4 +316,8 @@ public static boolean allowTvmOsaka() {
   public static boolean allowHardenResourceCalculation() {
     return current().allowHardenResourceCalculation;
   }
+
+  public static boolean allowStrictEcdsaValidation() {
+    return current().allowStrictEcdsaValidation;
+  }
 }

crypto/src/main/java/org/tron/common/crypto/ECKey.java

@@ -49,6 +49,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;
@@ -386,6 +387,11 @@ 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 {
     byte[] signatureEncoded;
     try {
       signatureEncoded = Base64.decode(signatureBase64);
@@ -395,21 +401,28 @@ 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(
         messageHash,
         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 {
+    return signatureToKeyBytes(messageHash, sig, false);
+  }
+
+  public static byte[] signatureToKeyBytes(byte[] messageHash,
+      ECDSASignature sig, boolean strictValidation) throws SignatureException {
     check(messageHash.length == 32, "messageHash argument has length " +
         messageHash.length);
     int header = sig.v;
@@ -425,7 +438,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");
@@ -442,8 +455,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));
   }
 
   /**
@@ -456,7 +474,12 @@ public static byte[] signatureToAddress(byte[] messageHash, String
   public static byte[] signatureToAddress(byte[] messageHash,
       ECDSASignature sig) throws
       SignatureException {
-    return Hash.computeAddress(signatureToKeyBytes(messageHash, sig));
+    return signatureToAddress(messageHash, sig, false);
+  }
+
+  public static byte[] signatureToAddress(byte[] messageHash,
+      ECDSASignature sig, boolean strictValidation) throws SignatureException {
+    return Hash.computeAddress(signatureToKeyBytes(messageHash, sig, strictValidation));
   }
 
   /**
@@ -517,6 +540,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");
@@ -525,6 +556,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
@@ -577,11 +616,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);
   }
 
@@ -940,7 +984,6 @@ public static boolean validateComponents(BigInteger r, BigInteger s,
       return BIUtil.isLessThan(s, SECP256K1N);
     }
 
-
     public boolean validateComponents() {
       return validateComponents(r, s, v);
     }