feat: Add validation for creating PublicKey from bytes. (#2107)

Signed-off-by: Ivan Ivanov <[email protected]>

sdk/src/main/java/com/hedera/hashgraph/sdk/Key.java

@@ -55,13 +55,13 @@ public abstract class Key {
     static Key fromProtobufKey(com.hedera.hashgraph.sdk.proto.Key key) {
         switch (key.getKeyCase()) {
             case ED25519 -> {
-                return new PublicKeyED25519(key.getEd25519().toByteArray());
+                return PublicKeyED25519.fromBytesInternal(key.getEd25519().toByteArray());
             }
             case ECDSA_SECP256K1 -> {
                 if (key.getECDSASecp256K1().size() == 20) {
                     return new EvmAddress(key.getECDSASecp256K1().toByteArray());
                 } else {
-                    return new PublicKeyECDSA(key.getECDSASecp256K1().toByteArray());
+                    return PublicKeyECDSA.fromBytesInternal(key.getECDSASecp256K1().toByteArray());
                 }
             }
             case KEYLIST -> {

sdk/src/main/java/com/hedera/hashgraph/sdk/PrivateKeyECDSA.java

@@ -218,7 +218,7 @@ public PublicKey getPublicKey() {
 
         var q = ECDSA_SECP256K1_DOMAIN.getG().multiply(keyData);
         var publicParams = new ECPublicKeyParameters(q, ECDSA_SECP256K1_DOMAIN);
-        publicKey = new PublicKeyECDSA(publicParams.getQ().getEncoded(true));
+        publicKey = PublicKeyECDSA.fromBytesInternal(publicParams.getQ().getEncoded(true));
         return publicKey;
     }
 

sdk/src/main/java/com/hedera/hashgraph/sdk/PrivateKeyED25519.java

@@ -20,6 +20,12 @@
 package com.hedera.hashgraph.sdk;
 
 import com.hedera.hashgraph.sdk.utils.Bip32Utils;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import javax.annotation.Nullable;
 import org.bouncycastle.asn1.ASN1OctetString;
 import org.bouncycastle.asn1.DEROctetString;
 import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
@@ -30,13 +36,6 @@
 import org.bouncycastle.crypto.params.KeyParameter;
 import org.bouncycastle.math.ec.rfc8032.Ed25519;
 
-import javax.annotation.Nullable;
-import java.io.IOException;
-import java.nio.ByteBuffer;
-import java.nio.ByteOrder;
-import java.nio.charset.StandardCharsets;
-import java.util.Arrays;
-
 /**
  * Encapsulate the ED25519 private key.
  */
@@ -221,7 +220,7 @@ public PublicKey getPublicKey() {
         byte[] publicKeyData = new byte[Ed25519.PUBLIC_KEY_SIZE];
         Ed25519.generatePublicKey(keyData, 0, publicKeyData, 0);
 
-        publicKey = new PublicKeyED25519(publicKeyData);
+        publicKey = PublicKeyED25519.fromBytesInternal(publicKeyData);
         return publicKey;
     }
 

sdk/src/main/java/com/hedera/hashgraph/sdk/PublicKey.java

@@ -43,13 +43,13 @@ public abstract class PublicKey extends Key {
     public static PublicKey fromBytes(byte[] publicKey) {
         if (publicKey.length == Ed25519.PUBLIC_KEY_SIZE) {
             // If this is a 32 byte string, assume an Ed25519 public key
-            return new PublicKeyED25519(publicKey);
+            return PublicKeyED25519.fromBytesInternal(publicKey);
         } else if (publicKey.length == 33) {
             // compressed 33 byte raw form
-            return new PublicKeyECDSA(publicKey);
+            return PublicKeyECDSA.fromBytesInternal(publicKey);
         } else if (publicKey.length == 65) {
             // compress the 65 byte form
-            return new PublicKeyECDSA(
+            return PublicKeyECDSA.fromBytesInternal(
                 Key.ECDSA_SECP256K1_CURVE.getCurve().decodePoint(publicKey).getEncoded(true)
             );
         }

sdk/src/main/java/com/hedera/hashgraph/sdk/PublicKeyECDSA.java

@@ -19,23 +19,22 @@
  */
 package com.hedera.hashgraph.sdk;
 
+import static com.hedera.hashgraph.sdk.Crypto.calcKeccak256;
+
 import com.google.protobuf.ByteString;
 import com.hedera.hashgraph.sdk.proto.SignaturePair;
+import java.io.IOException;
+import java.math.BigInteger;
+import java.util.Arrays;
 import org.bouncycastle.asn1.x509.AlgorithmIdentifier;
 import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
 import org.bouncycastle.crypto.params.ECPublicKeyParameters;
 import org.bouncycastle.crypto.signers.ECDSASigner;
 
-import java.io.IOException;
-import java.math.BigInteger;
-import java.util.Arrays;
-
-import static com.hedera.hashgraph.sdk.Crypto.calcKeccak256;
-
 /**
  * Encapsulate the ECDSA public key.
  */
-public class PublicKeyECDSA extends PublicKey {
+class PublicKeyECDSA extends PublicKey {
     // Compressed 33 byte form
     private byte[] keyData;
 
@@ -44,7 +43,7 @@ public class PublicKeyECDSA extends PublicKey {
      *
      * @param keyData                   the byte array representing the key
      */
-    PublicKeyECDSA(byte[] keyData) {
+    private PublicKeyECDSA(byte[] keyData) {
         this.keyData = keyData;
     }
 
@@ -55,14 +54,14 @@ public class PublicKeyECDSA extends PublicKey {
      * @return                          the new key
      */
     static PublicKeyECDSA fromBytesInternal(byte[] publicKey) {
-        if (publicKey.length == 33) {
-            // compressed 33 byte raw form
+        // Validate the key if it's not all zero public key, see HIP-540
+        if (Arrays.equals(publicKey, new byte[33])) {
             return new PublicKeyECDSA(publicKey);
-        } else if (publicKey.length == 65) {
-            // compress the 65 byte form
+        }
+        if (publicKey.length == 33 || publicKey.length == 65) {
             return new PublicKeyECDSA(
-                Key.ECDSA_SECP256K1_CURVE.getCurve().decodePoint(publicKey).getEncoded(true)
-            );
+                // compress and validate the key
+                Key.ECDSA_SECP256K1_CURVE.getCurve().decodePoint(publicKey).getEncoded(true));
         }
 
         // Assume a DER-encoded public key descriptor

sdk/src/main/java/com/hedera/hashgraph/sdk/PublicKeyED25519.java

@@ -23,6 +23,7 @@
 import com.hedera.hashgraph.sdk.proto.SignaturePair;
 import org.bouncycastle.asn1.x509.AlgorithmIdentifier;
 import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
+import org.bouncycastle.crypto.params.Ed25519PublicKeyParameters;
 import org.bouncycastle.math.ec.rfc8032.Ed25519;
 
 import javax.annotation.Nullable;
@@ -40,7 +41,7 @@ class PublicKeyED25519 extends PublicKey {
      *
      * @param keyData                   the byte array representing the key
      */
-    PublicKeyED25519(byte[] keyData) {
+    private PublicKeyED25519(byte[] keyData) {
         this.keyData = keyData;
     }
 
@@ -52,6 +53,11 @@ class PublicKeyED25519 extends PublicKey {
      */
     static PublicKeyED25519 fromBytesInternal(byte[] publicKey) {
         if (publicKey.length == Ed25519.PUBLIC_KEY_SIZE) {
+            // Validate the key if it's not all zero public key, see HIP-540
+            if (!Arrays.equals(publicKey, new byte[32])) {
+                // Will throw if the key is invalid
+                new Ed25519PublicKeyParameters(publicKey, 0);
+            }
             // If this is a 32 byte string, assume an Ed25519 public key
             return new PublicKeyED25519(publicKey);
         }

sdk/src/test/java/com/hedera/hashgraph/sdk/ECDSAPublicKeyTest.java

@@ -27,6 +27,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 
 public class ECDSAPublicKeyTest {
     @Test
@@ -66,6 +68,39 @@ void keyByteSerialization2() {
         assertThat(key2Bytes).containsExactly(key1Bytes);
     }
 
+    @Test
+    void keyByteValidation() {
+        byte[] invalidKeyECDSA = new byte[33];
+        assertDoesNotThrow(() -> PublicKey.fromBytes(invalidKeyECDSA));
+        assertDoesNotThrow(() -> PublicKey.fromBytesECDSA(invalidKeyECDSA));
+
+        byte[] invalidCompressedKey = new byte[]{
+            0x00,
+            (byte) 0xca, (byte) 0x35, 0x4b, 0x7c, (byte) 0xf4, (byte) 0x87, (byte) 0xd1, (byte) 0xbc, 0x43,
+            0x5a, 0x25, 0x66, 0x77, 0x09, (byte) 0xc1, (byte) 0xab, (byte) 0x98, 0x0c, 0x11, 0x4d,
+            0x35, (byte) 0x94, (byte) 0xe6, 0x25, (byte) 0x9e, (byte) 0x81, 0x2e, 0x6a, 0x70, 0x3d,
+            0x4f, 0x51
+        };
+        assertThatExceptionOfType(IllegalArgumentException.class)
+            .isThrownBy(() -> PublicKey.fromBytesECDSA(invalidCompressedKey));
+
+        byte[] malformedKey = new byte[]{0x00, 0x01, 0x02};
+        assertThatExceptionOfType(IllegalArgumentException.class)
+            .isThrownBy(() -> PublicKey.fromBytesECDSA(malformedKey));
+
+        byte[] validCompressedKey = new byte[]{
+            0x02, // Prefix for compressed keys
+            (byte) 0xca, (byte) 0x35, 0x4b, 0x7c, (byte) 0xf4, (byte) 0x87, (byte) 0xd1, (byte) 0xbc, 0x43,
+            0x5a, 0x25, 0x66, 0x77, 0x09, (byte) 0xc1, (byte) 0xab, (byte) 0x98, 0x0c, 0x1f, 0x4d,
+            0x35, (byte) 0x94, (byte) 0xe6, 0x25, (byte) 0x9e, (byte) 0x81, 0x2e, 0x6a, 0x75, 0x3d,
+            0x4f, 0x59
+        };
+        assertDoesNotThrow(() -> PublicKey.fromBytesECDSA(validCompressedKey));
+
+        byte[] validDERKey = PrivateKey.generateECDSA().getPublicKey().toBytesDER();
+        assertDoesNotThrow(() -> PublicKey.fromBytesECDSA(validDERKey));
+    }
+
     @Test
     @DisplayName("public key can be recovered from DER bytes")
     void keyByteSerialization3() {

sdk/src/test/java/com/hedera/hashgraph/sdk/Ed25519PublicKeyTest.java

@@ -29,6 +29,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 
 class Ed25519PublicKeyTest {
     private static final String TEST_KEY_STR = "302a300506032b6570032100e0c8ec2758a5879ffac226a13c0c516b799e72e35141a0dd828f94d37988a4b7";
@@ -47,6 +49,33 @@ void verifyTransaction() {
         assertThat(key.getPublicKey().verifyTransaction(transaction)).isTrue();
     }
 
+    @Test
+    void keyByteValidation() {
+        byte[] invalidKeyED25519 = new byte[32];
+        assertDoesNotThrow(() -> PublicKey.fromBytes(invalidKeyED25519));
+        assertDoesNotThrow(() -> PublicKey.fromBytesED25519(invalidKeyED25519));
+
+        byte[] invalidKey = new byte[]{
+            0x00,
+            (byte) 0xca, (byte) 0x35, 0x4b, 0x7c, (byte) 0xf4, (byte) 0x87, (byte) 0xd1, (byte) 0xbc, 0x43,
+            0x5a, 0x25, 0x66, 0x77, 0x09, (byte) 0xc1, (byte) 0xab, (byte) 0x98, 0x0c, 0x11, 0x4d,
+            0x35, (byte) 0x94, (byte) 0xe6, 0x25, (byte) 0x9e, (byte) 0x81, 0x2e, 0x6a, 0x70, 0x3d,
+            0x4f, 0x51
+        };
+        assertThatExceptionOfType(IllegalArgumentException.class)
+            .isThrownBy(() -> PublicKey.fromBytesED25519(invalidKey));
+
+        byte[] malformedKey = new byte[]{0x00, 0x01, 0x02};
+        assertThatExceptionOfType(IllegalArgumentException.class)
+            .isThrownBy(() -> PublicKey.fromBytesED25519(malformedKey));
+
+        byte[] validKey = PrivateKey.generateED25519().getPublicKey().toBytes();
+        assertDoesNotThrow(() -> PublicKey.fromBytesED25519(validKey));
+
+        byte[] validDERKey = PrivateKey.generateED25519().getPublicKey().toBytesDER();
+        assertDoesNotThrow(() -> PublicKey.fromBytesED25519(validDERKey));
+    }
+
     @Test
     @DisplayName("public key can be recovered from bytes")
     void keyByteSerialization() {

sdk/src/testIntegration/java/com/hedera/hashgraph/sdk/test/integration/TokenUpdateIntegrationTest.java

@@ -2438,8 +2438,8 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThat(tokenInfoBeforeUpdate.feeScheduleKey.toString()).isEqualTo(feeScheduleKey.getPublicKey().toString());
             assertThat(tokenInfoBeforeUpdate.metadataKey.toString()).isEqualTo(metadataKey.getPublicKey().toString());
 
-            // This key is truly invalid, as all Ed25519 public keys must be 32 bytes long
-            var structurallyInvalidKey = PublicKey.fromString("000000000000000000000000000000000000000000000000000000000000000000");
+            // invalid ecdsa key
+            var ecdsaKey = PublicKey.fromBytesECDSA(new byte[33]);
 
             // update all of token’s lower-privilege keys
             // to a structurally invalid key (trying to update keys one by one to check all errors),
@@ -2448,7 +2448,7 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThatExceptionOfType(PrecheckStatusException.class).isThrownBy(() -> {
                 new TokenUpdateTransaction()
                     .setTokenId(tokenId)
-                    .setWipeKey(structurallyInvalidKey)
+                    .setWipeKey(ecdsaKey)
                     .setKeyVerificationMode(TokenKeyValidation.NO_VALIDATION)
                     .freezeWith(testEnv.client)
                     .sign(wipeKey)
@@ -2459,7 +2459,7 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThatExceptionOfType(PrecheckStatusException.class).isThrownBy(() -> {
                 new TokenUpdateTransaction()
                     .setTokenId(tokenId)
-                    .setKycKey(structurallyInvalidKey)
+                    .setKycKey(ecdsaKey)
                     .setKeyVerificationMode(TokenKeyValidation.NO_VALIDATION)
                     .freezeWith(testEnv.client)
                     .sign(kycKey)
@@ -2470,7 +2470,7 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThatExceptionOfType(PrecheckStatusException.class).isThrownBy(() -> {
                 new TokenUpdateTransaction()
                     .setTokenId(tokenId)
-                    .setFreezeKey(structurallyInvalidKey)
+                    .setFreezeKey(ecdsaKey)
                     .setKeyVerificationMode(TokenKeyValidation.NO_VALIDATION)
                     .freezeWith(testEnv.client)
                     .sign(freezeKey)
@@ -2481,7 +2481,7 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThatExceptionOfType(PrecheckStatusException.class).isThrownBy(() -> {
                 new TokenUpdateTransaction()
                     .setTokenId(tokenId)
-                    .setPauseKey(structurallyInvalidKey)
+                    .setPauseKey(ecdsaKey)
                     .setKeyVerificationMode(TokenKeyValidation.NO_VALIDATION)
                     .freezeWith(testEnv.client)
                     .sign(pauseKey)
@@ -2492,7 +2492,7 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThatExceptionOfType(PrecheckStatusException.class).isThrownBy(() -> {
                 new TokenUpdateTransaction()
                     .setTokenId(tokenId)
-                    .setSupplyKey(structurallyInvalidKey)
+                    .setSupplyKey(ecdsaKey)
                     .setKeyVerificationMode(TokenKeyValidation.NO_VALIDATION)
                     .freezeWith(testEnv.client)
                     .sign(supplyKey)
@@ -2503,7 +2503,7 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThatExceptionOfType(PrecheckStatusException.class).isThrownBy(() -> {
                 new TokenUpdateTransaction()
                     .setTokenId(tokenId)
-                    .setFeeScheduleKey(structurallyInvalidKey)
+                    .setFeeScheduleKey(ecdsaKey)
                     .setKeyVerificationMode(TokenKeyValidation.NO_VALIDATION)
                     .freezeWith(testEnv.client)
                     .sign(feeScheduleKey)
@@ -2514,7 +2514,7 @@ void cannotUpdateAllLowerPrivilegeKeysWhenUpdatingKeysToStructurallyInvalidKeysS
             assertThatExceptionOfType(PrecheckStatusException.class).isThrownBy(() -> {
                 new TokenUpdateTransaction()
                     .setTokenId(tokenId)
-                    .setMetadataKey(structurallyInvalidKey)
+                    .setMetadataKey(ecdsaKey)
                     .setKeyVerificationMode(TokenKeyValidation.NO_VALIDATION)
                     .freezeWith(testEnv.client)
                     .sign(metadataKey)