Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sdk/src/main/java/com/hedera/hashgraph/sdk/Key.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions sdk/src/main/java/com/hedera/hashgraph/sdk/PublicKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
Expand Down
27 changes: 13 additions & 14 deletions sdk/src/main/java/com/hedera/hashgraph/sdk/PublicKeyECDSA.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down