fix(crypto): resolve the invalid witness (#6368)

* feat(crypto): upgrade the bouncycastle denpendency

* fix(crypto): optimize the null localwitness

* fix(metric): process the null witness address

* feat(crypto): optimize the code

* feat(doc): add the comment

* feat(test): add the nullWitnessAddress test

* add witness config test

* strengthen the private key validation

* increase the test coverage

* optimiz the witnessAddress initialization

* simplify the localWitness

* refactor the code

* add the test unit

Author: Federico2014

build.gradle

@@ -84,7 +84,7 @@ subprojects {
         implementation group: 'org.apache.commons', name: 'commons-math', version: '2.2'
         implementation "org.apache.commons:commons-collections4:4.1"
         implementation group: 'joda-time', name: 'joda-time', version: '2.3'
-        implementation group: 'org.bouncycastle', name: 'bcprov-jdk15on', version: '1.69'
+        implementation group: 'org.bouncycastle', name: 'bcprov-jdk18on', version: '1.79'
 
         compileOnly 'org.projectlombok:lombok:1.18.34'
         annotationProcessor 'org.projectlombok:lombok:1.18.34'

chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java

@@ -32,6 +32,7 @@ public class LocalWitnesses {
   @Getter
   private List<String> privateKeys = Lists.newArrayList();
 
+  @Getter
   private byte[] witnessAccountAddress;
 
   public LocalWitnesses() {
@@ -45,21 +46,11 @@ public LocalWitnesses(List<String> privateKeys) {
     setPrivateKeys(privateKeys);
   }
 
-  public byte[] getWitnessAccountAddress(boolean isECKeyCryptoEngine) {
-    if (witnessAccountAddress == null) {
-      byte[] privateKey = ByteArray.fromHexString(getPrivateKey());
-      final SignInterface cryptoEngine = SignUtils.fromPrivate(privateKey, isECKeyCryptoEngine);
-      this.witnessAccountAddress = cryptoEngine.getAddress();
-    }
-    return witnessAccountAddress;
-  }
-
-  public void setWitnessAccountAddress(final byte[] localWitnessAccountAddress) {
-    this.witnessAccountAddress = localWitnessAccountAddress;
-  }
-
-  public void initWitnessAccountAddress(boolean isECKeyCryptoEngine) {
-    if (witnessAccountAddress == null) {
+  public void initWitnessAccountAddress(final byte[] witnessAddress,
+      boolean isECKeyCryptoEngine) {
+    if (witnessAddress != null) {
+      this.witnessAccountAddress = witnessAddress;
+    } else if (!CollectionUtils.isEmpty(privateKeys)) {
       byte[] privateKey = ByteArray.fromHexString(getPrivateKey());
       final SignInterface ecKey = SignUtils.fromPrivate(privateKey,
           isECKeyCryptoEngine);
@@ -85,11 +76,15 @@ private void validate(String privateKey) {
       privateKey = privateKey.substring(2);
     }
 
-    if (StringUtils.isNotBlank(privateKey)
-        && privateKey.length() != ChainConstant.PRIVATE_KEY_LENGTH) {
+    if (StringUtils.isBlank(privateKey)
+        || privateKey.length() != ChainConstant.PRIVATE_KEY_LENGTH) {
       throw new IllegalArgumentException(
-          String.format("private key must be %d-bits hex string, actual: %d",
-              ChainConstant.PRIVATE_KEY_LENGTH, privateKey.length()));
+          String.format("private key must be %d hex string, actual: %d",
+              ChainConstant.PRIVATE_KEY_LENGTH,
+              StringUtils.isBlank(privateKey) ? 0 : privateKey.length()));
+    }
+    if (!StringUtil.isHexadecimal(privateKey)) {
+      throw new IllegalArgumentException("private key must be hex string");
     }
   }
 

common/src/main/java/org/tron/common/utils/StringUtil.java

@@ -44,4 +44,21 @@ public static String createReadableString(ByteString string) {
   public static ByteString hexString2ByteString(String hexString) {
     return ByteString.copyFrom(ByteArray.fromHexString(hexString));
   }
+
+  public static boolean isHexadecimal(String str) {
+    if (str == null || str.isEmpty()) {
+      return false;
+    }
+    if (str.length() % 2 != 0) {
+      return false;
+    }
+
+    for (int i = 0; i < str.length(); i++) {
+      char c = str.charAt(i);
+      if (Character.digit(c, 16) == -1) {
+        return false;
+      }
+    }
+    return true;
+  }
 }

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

@@ -55,6 +55,7 @@
 import org.tron.common.crypto.jce.ECKeyPairGenerator;
 import org.tron.common.crypto.jce.TronCastleProvider;
 import org.tron.common.utils.BIUtil;
+import org.tron.common.utils.ByteArray;
 import org.tron.common.utils.ByteUtil;
 
 @Slf4j(topic = "crypto")
@@ -285,7 +286,7 @@ public static ECKey fromPrivate(BigInteger privKey) {
    * @return -
    */
   public static ECKey fromPrivate(byte[] privKeyBytes) {
-    if (Objects.isNull(privKeyBytes)) {
+    if (ByteArray.isEmpty(privKeyBytes)) {
       return null;
     }
     return fromPrivate(new BigInteger(1, privKeyBytes));

crypto/src/main/java/org/tron/common/crypto/sm2/SM2.java

@@ -41,6 +41,7 @@
 import org.tron.common.crypto.SignatureInterface;
 import org.tron.common.crypto.jce.ECKeyFactory;
 import org.tron.common.crypto.jce.TronCastleProvider;
+import org.tron.common.utils.ByteArray;
 import org.tron.common.utils.ByteUtil;
 
 /**
@@ -247,7 +248,7 @@ public static SM2 fromPrivate(BigInteger privKey) {
    * @return -
    */
   public static SM2 fromPrivate(byte[] privKeyBytes) {
-    if (Objects.isNull(privKeyBytes)) {
+    if (ByteArray.isEmpty(privKeyBytes)) {
       return null;
     }
     return fromPrivate(new BigInteger(1, privKeyBytes));

framework/src/main/java/org/tron/core/config/args/Args.java

@@ -53,7 +53,6 @@
 import org.tron.common.args.Witness;
 import org.tron.common.config.DbBackupConfig;
 import org.tron.common.cron.CronExpression;
-import org.tron.common.crypto.SignInterface;
 import org.tron.common.logsfilter.EventPluginConfig;
 import org.tron.common.logsfilter.FilterQuery;
 import org.tron.common.logsfilter.TriggerConfig;
@@ -62,19 +61,15 @@
 import org.tron.common.parameter.CommonParameter;
 import org.tron.common.parameter.RateLimiterInitialization;
 import org.tron.common.setting.RocksDbSettings;
-import org.tron.common.utils.ByteArray;
 import org.tron.common.utils.Commons;
 import org.tron.common.utils.LocalWitnesses;
 import org.tron.core.Constant;
 import org.tron.core.Wallet;
 import org.tron.core.config.Configuration;
 import org.tron.core.config.Parameter.NetConstants;
 import org.tron.core.config.Parameter.NodeConstant;
-import org.tron.core.exception.CipherException;
 import org.tron.core.exception.TronError;
 import org.tron.core.store.AccountStore;
-import org.tron.keystore.Credentials;
-import org.tron.keystore.WalletUtils;
 import org.tron.p2p.P2pConfig;
 import org.tron.p2p.dns.update.DnsType;
 import org.tron.p2p.dns.update.PublishConfig;
@@ -412,61 +407,7 @@ public static void setParam(final Config config) {
     PARAMETER.cryptoEngine = config.hasPath(Constant.CRYPTO_ENGINE) ? config
         .getString(Constant.CRYPTO_ENGINE) : Constant.ECKey_ENGINE;
 
-    if (StringUtils.isNoneBlank(PARAMETER.privateKey)) {
-      localWitnesses = (new LocalWitnesses(PARAMETER.privateKey));
-      if (StringUtils.isNoneBlank(PARAMETER.witnessAddress)) {
-        byte[] bytes = Commons.decodeFromBase58Check(PARAMETER.witnessAddress);
-        if (bytes != null) {
-          localWitnesses.setWitnessAccountAddress(bytes);
-          logger.debug("Got localWitnessAccountAddress from cmd");
-        } else {
-          PARAMETER.witnessAddress = "";
-          logger.warn(IGNORE_WRONG_WITNESS_ADDRESS_FORMAT);
-        }
-      }
-      localWitnesses.initWitnessAccountAddress(PARAMETER.isECKeyCryptoEngine());
-      logger.debug("Got privateKey from cmd");
-    } else if (config.hasPath(Constant.LOCAL_WITNESS)) {
-      localWitnesses = new LocalWitnesses();
-      List<String> localwitness = config.getStringList(Constant.LOCAL_WITNESS);
-      localWitnesses.setPrivateKeys(localwitness);
-      witnessAddressCheck(config);
-      localWitnesses.initWitnessAccountAddress(PARAMETER.isECKeyCryptoEngine());
-      logger.debug("Got privateKey from config.conf");
-    } else if (config.hasPath(Constant.LOCAL_WITNESS_KEYSTORE)) {
-      localWitnesses = new LocalWitnesses();
-      List<String> privateKeys = new ArrayList<String>();
-      if (PARAMETER.isWitness()) {
-        List<String> localwitness = config.getStringList(Constant.LOCAL_WITNESS_KEYSTORE);
-        if (localwitness.size() > 0) {
-          String fileName = System.getProperty("user.dir") + "/" + localwitness.get(0);
-          String password;
-          if (StringUtils.isEmpty(PARAMETER.password)) {
-            System.out.println("Please input your password.");
-            password = WalletUtils.inputPassword();
-          } else {
-            password = PARAMETER.password;
-            PARAMETER.password = null;
-          }
-
-          try {
-            Credentials credentials = WalletUtils
-                .loadCredentials(password, new File(fileName));
-            SignInterface sign = credentials.getSignInterface();
-            String prikey = ByteArray.toHexString(sign.getPrivateKey());
-            privateKeys.add(prikey);
-          } catch (IOException | CipherException e) {
-            logger.error("Witness node start failed!");
-            throw new TronError(e, TronError.ErrCode.WITNESS_KEYSTORE_LOAD);
-          }
-        }
-      }
-      localWitnesses.setPrivateKeys(privateKeys);
-      witnessAddressCheck(config);
-      localWitnesses.initWitnessAccountAddress(PARAMETER.isECKeyCryptoEngine());
-      logger.debug("Got privateKey from keystore");
-    }
-
+    localWitnesses = new WitnessInitializer(config).initLocalWitnesses();
     if (PARAMETER.isWitness()
         && CollectionUtils.isEmpty(localWitnesses.getPrivateKeys())) {
       throw new TronError("This is a witness node, but localWitnesses is null",
@@ -1817,19 +1758,6 @@ public static void logConfig() {
     logger.info("\n");
   }
 
-  private static void witnessAddressCheck(Config config) {
-    if (config.hasPath(Constant.LOCAL_WITNESS_ACCOUNT_ADDRESS)) {
-      byte[] bytes = Commons
-          .decodeFromBase58Check(config.getString(Constant.LOCAL_WITNESS_ACCOUNT_ADDRESS));
-      if (bytes != null) {
-        localWitnesses.setWitnessAccountAddress(bytes);
-        logger.debug("Got localWitnessAccountAddress from config.conf");
-      } else {
-        logger.warn(IGNORE_WRONG_WITNESS_ADDRESS_FORMAT);
-      }
-    }
-  }
-
   /**
    * get output directory.
    */

framework/src/main/java/org/tron/core/config/args/WitnessInitializer.java

@@ -0,0 +1,149 @@
+package org.tron.core.config.args;
+
+import com.typesafe.config.Config;
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
+import org.tron.common.crypto.SignInterface;
+import org.tron.common.utils.ByteArray;
+import org.tron.common.utils.Commons;
+import org.tron.common.utils.LocalWitnesses;
+import org.tron.core.Constant;
+import org.tron.core.exception.CipherException;
+import org.tron.core.exception.TronError;
+import org.tron.keystore.Credentials;
+import org.tron.keystore.WalletUtils;
+
+@Slf4j
+public class WitnessInitializer {
+
+  private final Config config;
+
+  private LocalWitnesses localWitnesses;
+
+  public WitnessInitializer(Config config) {
+    this.config = config;
+    this.localWitnesses = new LocalWitnesses();
+  }
+
+  public LocalWitnesses initLocalWitnesses() {
+    if (!Args.PARAMETER.isWitness()) {
+      return localWitnesses;
+    }
+
+    if (tryInitFromCommandLine()) {
+      return localWitnesses;
+    }
+
+    if (tryInitFromConfig()) {
+      return localWitnesses;
+    }
+
+    tryInitFromKeystore();
+
+    return localWitnesses;
+  }
+
+  private boolean tryInitFromCommandLine() {
+    if (StringUtils.isBlank(Args.PARAMETER.privateKey)) {
+      return false;
+    }
+
+    byte[] witnessAddress = null;
+    this.localWitnesses = new LocalWitnesses(Args.PARAMETER.privateKey);
+    if (StringUtils.isNotEmpty(Args.PARAMETER.witnessAddress)) {
+      witnessAddress = Commons.decodeFromBase58Check(Args.PARAMETER.witnessAddress);
+      if (witnessAddress == null) {
+        throw new TronError("LocalWitnessAccountAddress format from cmd is incorrect",
+            TronError.ErrCode.WITNESS_INIT);
+      }
+      logger.debug("Got localWitnessAccountAddress from cmd");
+    }
+
+    this.localWitnesses.initWitnessAccountAddress(witnessAddress,
+        Args.PARAMETER.isECKeyCryptoEngine());
+    logger.debug("Got privateKey from cmd");
+    return true;
+  }
+
+  private boolean tryInitFromConfig() {
+    if (!config.hasPath(Constant.LOCAL_WITNESS) || config.getStringList(Constant.LOCAL_WITNESS)
+        .isEmpty()) {
+      return false;
+    }
+
+    List<String> localWitness = config.getStringList(Constant.LOCAL_WITNESS);
+    this.localWitnesses.setPrivateKeys(localWitness);
+    logger.debug("Got privateKey from config.conf");
+    byte[] witnessAddress = getWitnessAddress();
+    this.localWitnesses.initWitnessAccountAddress(witnessAddress,
+        Args.PARAMETER.isECKeyCryptoEngine());
+    return true;
+  }
+
+  private void tryInitFromKeystore() {
+    if (!config.hasPath(Constant.LOCAL_WITNESS_KEYSTORE)
+        || config.getStringList(Constant.LOCAL_WITNESS_KEYSTORE).isEmpty()) {
+      return;
+    }
+
+    List<String> localWitness = config.getStringList(Constant.LOCAL_WITNESS_KEYSTORE);
+    if (localWitness.size() > 1) {
+      logger.warn(
+          "Multiple keystores detected. Only the first keystore will be used as witness, all "
+              + "others will be ignored.");
+    }
+
+    List<String> privateKeys = new ArrayList<>();
+    String fileName = System.getProperty("user.dir") + "/" + localWitness.get(0);
+    String password;
+    if (StringUtils.isEmpty(Args.PARAMETER.password)) {
+      System.out.println("Please input your password.");
+      password = WalletUtils.inputPassword();
+    } else {
+      password = Args.PARAMETER.password;
+      Args.PARAMETER.password = null;
+    }
+
+    try {
+      Credentials credentials = WalletUtils
+          .loadCredentials(password, new File(fileName));
+      SignInterface sign = credentials.getSignInterface();
+      String prikey = ByteArray.toHexString(sign.getPrivateKey());
+      privateKeys.add(prikey);
+    } catch (IOException | CipherException e) {
+      logger.error("Witness node start failed!");
+      throw new TronError(e, TronError.ErrCode.WITNESS_KEYSTORE_LOAD);
+    }
+
+    this.localWitnesses.setPrivateKeys(privateKeys);
+    byte[] witnessAddress = getWitnessAddress();
+    this.localWitnesses.initWitnessAccountAddress(witnessAddress,
+        Args.PARAMETER.isECKeyCryptoEngine());
+    logger.debug("Got privateKey from keystore");
+  }
+
+  private byte[] getWitnessAddress() {
+    if (!config.hasPath(Constant.LOCAL_WITNESS_ACCOUNT_ADDRESS)) {
+      return null;
+    }
+
+    if (localWitnesses.getPrivateKeys().size() != 1) {
+      throw new TronError(
+          "LocalWitnessAccountAddress can only be set when there is only one private key",
+          TronError.ErrCode.WITNESS_INIT);
+    }
+    byte[] witnessAddress = Commons
+        .decodeFromBase58Check(config.getString(Constant.LOCAL_WITNESS_ACCOUNT_ADDRESS));
+    if (witnessAddress != null) {
+      logger.debug("Got localWitnessAccountAddress from config.conf");
+    } else {
+      throw new TronError("LocalWitnessAccountAddress format from config is incorrect",
+          TronError.ErrCode.WITNESS_INIT);
+    }
+    return witnessAddress;
+  }
+}