Skip to content

Commit

Permalink
verify correct message format when sign a message
Browse files Browse the repository at this point in the history
  • Loading branch information
0xmaayan committed Mar 29, 2024
1 parent 7d257b7 commit 46f8e40
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 59 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ All notable changes to the Aptos TypeScript SDK will be captured in this file. T
# Unreleased

- [`Breaking`] Change `getOwnerAddress` and `getTargetAddress` return type to `AccountAddress`
- Add `message` input type verification on `sign` and `verifySignature` functions and convert it into a correct type if needed
- [`Breaking`] Change `fromString` to `fromHexString` on `Hex` class

# 1.11.0 (2024-03-26)

Expand Down
12 changes: 7 additions & 5 deletions src/core/crypto/ed25519.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { CKDPriv, deriveKey, HARDENED_OFFSET, isValidHardenedPath, mnemonicToSee
import { PrivateKey } from "./privateKey";
import { AccountPublicKey, VerifySignatureArgs } from "./publicKey";
import { Signature } from "./signature";
import { convertMessage } from "./utils";

/**
* Represents the public key of an Ed25519 key pair.
Expand Down Expand Up @@ -52,16 +53,16 @@ export class Ed25519PublicKey extends AccountPublicKey {

/**
* Verifies a signed data with a public key
* @param args.message a signed message
* @param args.message a signed message as a Hex string or Uint8Array
* @param args.signature the signature of the message
*/
verifySignature(args: VerifySignatureArgs): boolean {
const { message, signature } = args;
if (!(signature instanceof Ed25519Signature)) {
return false;
}

const messageBytes = Hex.fromHexInput(message).toUint8Array();
const messageToVerify = convertMessage(message);
const messageBytes = Hex.fromHexInput(messageToVerify).toUint8Array();
const signatureBytes = signature.toUint8Array();
const publicKeyBytes = this.key.toUint8Array();
return nacl.sign.detached.verify(messageBytes, signatureBytes, publicKeyBytes);
Expand Down Expand Up @@ -213,11 +214,12 @@ export class Ed25519PrivateKey extends Serializable implements PrivateKey {
/**
* Sign the given message with the private key.
*
* @param message in HexInput format
* @param message a message as a Hex string or Uint8Array
* @returns Signature
*/
sign(message: HexInput): Ed25519Signature {
const messageBytes = Hex.fromHexInput(message).toUint8Array();
const messageToSign = convertMessage(message);
const messageBytes = Hex.fromHexInput(messageToSign).toUint8Array();
const signatureBytes = nacl.sign.detached(messageBytes, this.signingKeyPair.secretKey);
return new Ed25519Signature(signatureBytes);
}
Expand Down
8 changes: 5 additions & 3 deletions src/core/crypto/secp256k1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { isValidBIP44Path, mnemonicToSeed } from "./hdKey";
import { PrivateKey } from "./privateKey";
import { PublicKey, VerifySignatureArgs } from "./publicKey";
import { Signature } from "./signature";
import { convertMessage } from "./utils";

/**
* Represents the Secp256k1 ecdsa public key
Expand Down Expand Up @@ -46,8 +47,8 @@ export class Secp256k1PublicKey extends PublicKey {
if (!(signature instanceof Secp256k1Signature)) {
return false;
}

const messageBytes = Hex.fromHexInput(message).toUint8Array();
const messageToVerify = convertMessage(message);
const messageBytes = Hex.fromHexInput(messageToVerify).toUint8Array();
const messageSha3Bytes = sha3_256(messageBytes);
const signatureBytes = signature.toUint8Array();
return secp256k1.verify(signatureBytes, messageSha3Bytes, this.key.toUint8Array());
Expand Down Expand Up @@ -169,7 +170,8 @@ export class Secp256k1PrivateKey extends Serializable implements PrivateKey {
* @returns Signature
*/
sign(message: HexInput): Secp256k1Signature {
const messageBytes = Hex.fromHexInput(message);
const messageToSign = convertMessage(message);
const messageBytes = Hex.fromHexInput(messageToSign);
const messageHashBytes = sha3_256(messageBytes.toUint8Array());
const signature = secp256k1.sign(messageHashBytes, this.key.toUint8Array());
return new Secp256k1Signature(signature.toCompactRawBytes());
Expand Down
24 changes: 24 additions & 0 deletions src/core/crypto/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { HexInput } from "../../types";
import { Hex } from "../hex";

/**
* Helper function to convert a message to sign or to verify to a valid message input
*
* @param message a message as a string or Uint8Array
*
* @returns a valid HexInput - Hex string or Uint8Array
*/
export const convertMessage = (message: HexInput): HexInput => {
// if message is of type string, verify it is a valid Hex string
if (typeof message === "string") {
const isValid = Hex.isValid(message);
// If message is not a valid Hex string, convert it into a Buffer
if (!isValid.valid) {
return Buffer.from(message, "utf8");
}
// If message is a valid Hex string, return it
return message;
}
// message is a Uint8Array
return message;
};
8 changes: 4 additions & 4 deletions src/core/hex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export enum HexInvalidReason {
* with a leading 0x prefix, regardless of what the input format was.
*
* These are some other ways to chain the functions together:
* - `Hex.fromString({ hexInput: "0x1f" }).toUint8Array()`
* - `Hex.fromHexString({ hexInput: "0x1f" }).toUint8Array()`
* - `new Hex([1, 3]).toStringWithoutPrefix()`
*/
export class Hex {
Expand Down Expand Up @@ -95,7 +95,7 @@ export class Hex {
*
* @returns Hex
*/
static fromString(str: string): Hex {
static fromHexString(str: string): Hex {
let input = str;

if (input.startsWith("0x")) {
Expand Down Expand Up @@ -132,7 +132,7 @@ export class Hex {
*/
static fromHexInput(hexInput: HexInput): Hex {
if (hexInput instanceof Uint8Array) return new Hex(hexInput);
return Hex.fromString(hexInput);
return Hex.fromHexString(hexInput);
}

// ===
Expand All @@ -150,7 +150,7 @@ export class Hex {
*/
static isValid(str: string): ParsingResult<HexInvalidReason> {
try {
Hex.fromString(str);
Hex.fromHexString(str);
return { valid: true };
} catch (error: any) {
return {
Expand Down
28 changes: 19 additions & 9 deletions tests/unit/account.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,18 @@ describe("Account", () => {

describe("sign and verify", () => {
it("signs a message with single signer Secp256k1 scheme and verifies successfully", () => {
const { privateKey: privateKeyBytes, address, signatureHex, messageEncoded } = secp256k1TestObject;
const { privateKey: privateKeyBytes, address, signatureHex, messageEncoded, stringMessage } = secp256k1TestObject;
const privateKey = new Secp256k1PrivateKey(privateKeyBytes);
const accountAddress = AccountAddress.from(address);
const secpAccount = Account.fromPrivateKey({ privateKey, address: accountAddress });
const signature = secpAccount.sign(messageEncoded);
expect(signature.signature.toString()).toEqual(signatureHex);
expect(secpAccount.verifySignature({ message: messageEncoded, signature })).toBeTruthy();
// verifies an encoded message
const signature1 = secpAccount.sign(messageEncoded);
expect(signature1.signature.toString()).toEqual(signatureHex);
expect(secpAccount.verifySignature({ message: messageEncoded, signature: signature1 })).toBeTruthy();
// verifies a string message
const signature2 = secpAccount.sign(stringMessage);
expect(signature2.signature.toString()).toEqual(signatureHex);
expect(secpAccount.verifySignature({ message: stringMessage, signature: signature2 })).toBeTruthy();
});

it("signs a message with single signer ed25519 scheme and verifies successfully", () => {
Expand All @@ -184,14 +189,19 @@ describe("Account", () => {
expect(edAccount.verifySignature({ message: messageEncoded, signature })).toBeTruthy();
});

it("derives the correct account from a legacy ed25519 private key", () => {
const { privateKey: privateKeyBytes, address, signedMessage, message } = ed25519;
it("signs a message with a legacy ed25519 scheme and verifies successfully", () => {
const { privateKey: privateKeyBytes, address, signatureHex, messageEncoded, stringMessage } = ed25519;
const privateKey = new Ed25519PrivateKey(privateKeyBytes);
const accountAddress = AccountAddress.from(address);
const legacyEdAccount = Account.fromPrivateKey({ privateKey, address: accountAddress, legacy: true });
const signature = legacyEdAccount.sign(message);
expect(signature.toString()).toEqual(signedMessage);
expect(legacyEdAccount.verifySignature({ message, signature })).toBeTruthy();
// verifies an encoded message
const signature1 = legacyEdAccount.sign(messageEncoded);
expect(signature1.toString()).toEqual(signatureHex);
expect(legacyEdAccount.verifySignature({ message: messageEncoded, signature: signature1 })).toBeTruthy();
// verifies a string message
const signature2 = legacyEdAccount.sign(stringMessage);
expect(signature2.toString()).toEqual(signatureHex);
expect(legacyEdAccount.verifySignature({ message: stringMessage, signature: signature2 })).toBeTruthy();
});
});

Expand Down
31 changes: 15 additions & 16 deletions tests/unit/ed25519.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ describe("Ed25519PublicKey", () => {

it("should verify the signature correctly", () => {
const pubKey = new Ed25519PublicKey(ed25519.publicKey);
const signature = new Ed25519Signature(ed25519.signedMessage);
const signature = new Ed25519Signature(ed25519.signatureHex);

// Verify with correct signed message
expect(pubKey.verifySignature({ message: ed25519.message, signature })).toBe(true);
expect(pubKey.verifySignature({ message: ed25519.messageEncoded, signature })).toBe(true);

// Verify with incorrect signed message
const incorrectSignedMessage =
Expand All @@ -43,7 +43,7 @@ describe("Ed25519PublicKey", () => {
const invalidSignature = new Ed25519Signature(incorrectSignedMessage);
expect(
pubKey.verifySignature({
message: ed25519.message,
message: ed25519.messageEncoded,
signature: invalidSignature,
}),
).toBe(false);
Expand Down Expand Up @@ -111,8 +111,8 @@ describe("PrivateKey", () => {

it("should sign the message correctly", () => {
const privateKey = new Ed25519PrivateKey(ed25519.privateKey);
const signedMessage = privateKey.sign(ed25519.message);
expect(signedMessage.toString()).toEqual(ed25519.signedMessage);
const signedMessage = privateKey.sign(ed25519.messageEncoded);
expect(signedMessage.toString()).toEqual(ed25519.signatureHex);
});

it("should serialize correctly", () => {
Expand Down Expand Up @@ -184,9 +184,9 @@ describe("PrivateKey", () => {
describe("Signature", () => {
it("should create an instance correctly without error", () => {
// Create from string
const signatureStr = new Ed25519Signature(ed25519.signedMessage);
const signatureStr = new Ed25519Signature(ed25519.signatureHex);
expect(signatureStr).toBeInstanceOf(Ed25519Signature);
expect(signatureStr.toString()).toEqual(ed25519.signedMessage);
expect(signatureStr.toString()).toEqual(ed25519.signatureHex);

// Create from Uint8Array
const signatureValue = new Uint8Array(Ed25519Signature.LENGTH);
Expand All @@ -203,28 +203,27 @@ describe("Signature", () => {
});

it("should serialize correctly", () => {
const signature = new Ed25519Signature(ed25519.signedMessage);
const signature = new Ed25519Signature(ed25519.signatureHex);
const serializer = new Serializer();
signature.serialize(serializer);

const expectedUint8Array = new Uint8Array([
64, 197, 222, 158, 64, 172, 0, 179, 113, 205, 131, 177, 193, 151, 250, 91, 102, 91, 116, 73, 179, 60, 211, 205,
211, 5, 187, 120, 34, 46, 6, 166, 113, 164, 150, 37, 171, 154, 234, 138, 3, 157, 75, 183, 14, 39, 87, 104, 8, 77,
98, 176, 148, 188, 27, 49, 150, 79, 35, 87, 183, 193, 175, 126, 13,
64, 158, 101, 61, 86, 160, 146, 71, 87, 11, 177, 116, 163, 137, 232, 91, 146, 38, 171, 213, 196, 3, 234, 108, 80,
75, 56, 102, 38, 161, 69, 21, 140, 212, 239, 214, 111, 197, 224, 113, 192, 225, 149, 56, 169, 106, 5, 221, 189,
162, 77, 60, 81, 225, 230, 169, 218, 204, 107, 177, 206, 119, 92, 206, 7,
]);
expect(serializer.toUint8Array()).toEqual(expectedUint8Array);
});

it("should deserialize correctly", () => {
const serializedSignature = new Uint8Array([
64, 197, 222, 158, 64, 172, 0, 179, 113, 205, 131, 177, 193, 151, 250, 91, 102, 91, 116, 73, 179, 60, 211, 205,
211, 5, 187, 120, 34, 46, 6, 166, 113, 164, 150, 37, 171, 154, 234, 138, 3, 157, 75, 183, 14, 39, 87, 104, 8, 77,
98, 176, 148, 188, 27, 49, 150, 79, 35, 87, 183, 193, 175, 126, 13,
64, 158, 101, 61, 86, 160, 146, 71, 87, 11, 177, 116, 163, 137, 232, 91, 146, 38, 171, 213, 196, 3, 234, 108, 80,
75, 56, 102, 38, 161, 69, 21, 140, 212, 239, 214, 111, 197, 224, 113, 192, 225, 149, 56, 169, 106, 5, 221, 189,
162, 77, 60, 81, 225, 230, 169, 218, 204, 107, 177, 206, 119, 92, 206, 7,
]);
const deserializer = new Deserializer(serializedSignature);
const signature = Ed25519Signature.deserialize(deserializer);

expect(signature.toString()).toEqual(ed25519.signedMessage);
expect(signature.toString()).toEqual(ed25519.signatureHex);
});

it("should serialize and deserialize correctly", () => {
Expand Down
10 changes: 6 additions & 4 deletions tests/unit/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ export const ed25519 = {
publicKey: "0xde19e5d1880cac87d57484ce9ed2e84cf0f9599f12e7cc3a52e4e7657a763f2c",
authKey: "0x978c213990c4833df71548df7ce49d54c759d6b6d932de22b24d56060b7af2aa",
address: "0x978c213990c4833df71548df7ce49d54c759d6b6d932de22b24d56060b7af2aa",
message: "0x7777",
signedMessage:
"0xc5de9e40ac00b371cd83b1c197fa5b665b7449b33cd3cdd305bb78222e06a671a49625ab9aea8a039d4bb70e275768084d62b094bc1b31964f2357b7c1af7e0d",
messageEncoded: "68656c6c6f20776f726c64",
stringMessage: "hello world",
signatureHex:
"0x9e653d56a09247570bb174a389e85b9226abd5c403ea6c504b386626a145158cd4efd66fc5e071c0e19538a96a05ddbda24d3c51e1e6a9dacc6bb1ce775cce07",
};

export const multiEd25519PkTestObject = {
Expand Down Expand Up @@ -76,7 +77,8 @@ export const secp256k1TestObject = {
"0x04acdd16651b839c24665b7e2033b55225f384554949fef46c397b5275f37f6ee95554d70fb5d9f93c5831ebf695c7206e7477ce708f03ae9bb2862dc6c9e033ea",
address: "0x5792c985bc96f436270bd2a3c692210b09c7febb8889345ceefdbae4bacfe498",
authKey: "0x5792c985bc96f436270bd2a3c692210b09c7febb8889345ceefdbae4bacfe498",
messageEncoded: "68656c6c6f20776f726c64", // "hello world"
messageEncoded: "68656c6c6f20776f726c64",
stringMessage: "hello world",
signatureHex:
"0xd0d634e843b61339473b028105930ace022980708b2855954b977da09df84a770c0b68c29c8ca1b5409a5085b0ec263be80e433c83fcf6debb82f3447e71edca",
};
Expand Down
18 changes: 9 additions & 9 deletions tests/unit/hex.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,19 @@ test("converts hex bytes input into hex data", () => {
});

test("converts hex string input into hex data", () => {
const hex = Hex.fromString(mockHex.withPrefix);
const hex = Hex.fromHexString(mockHex.withPrefix);
expect(hex instanceof Hex).toBeTruthy();
expect(hex.toUint8Array()).toEqual(mockHex.bytes);
});

test("accepts hex string input without prefix", () => {
const hex = Hex.fromString(mockHex.withoutPrefix);
const hex = Hex.fromHexString(mockHex.withoutPrefix);
expect(hex instanceof Hex).toBeTruthy();
expect(hex.toUint8Array()).toEqual(mockHex.bytes);
});

test("accepts hex string with prefix", () => {
const hex = Hex.fromString(mockHex.withPrefix);
const hex = Hex.fromHexString(mockHex.withPrefix);
expect(hex instanceof Hex).toBeTruthy();
expect(hex.toUint8Array()).toEqual(mockHex.bytes);
});
Expand All @@ -61,23 +61,23 @@ test("converts hex bytes to string without 0x prefix", () => {
});

test("throws when parsing invalid hex char", () => {
expect(() => Hex.fromString("0xzyzz")).toThrow(
expect(() => Hex.fromHexString("0xzyzz")).toThrow(
// eslint-disable-next-line quotes
'Hex string contains invalid hex characters: hex string expected, got non-hex character "zy" at index 0',
);
});

test("throws when parsing hex of length zero", () => {
expect(() => Hex.fromString("0x")).toThrow(
expect(() => Hex.fromHexString("0x")).toThrow(
"Hex string is too short, must be at least 1 char long, excluding the optional leading 0x.",
);
expect(() => Hex.fromString("")).toThrow(
expect(() => Hex.fromHexString("")).toThrow(
"Hex string is too short, must be at least 1 char long, excluding the optional leading 0x.",
);
});

test("throws when parsing hex of invalid length", () => {
expect(() => Hex.fromString("0x1")).toThrow("Hex string must be an even number of hex characters.");
expect(() => Hex.fromHexString("0x1")).toThrow("Hex string must be an even number of hex characters.");
});

test("isValid returns true when parsing valid string", () => {
Expand All @@ -95,7 +95,7 @@ test("isValid returns false when parsing hex of invalid length", () => {
});

test("compares equality with equals as expected", () => {
const hexOne = Hex.fromString("0x11");
const hexTwo = Hex.fromString("0x11");
const hexOne = Hex.fromHexString("0x11");
const hexTwo = Hex.fromHexString("0x11");
expect(hexOne.equals(hexTwo)).toBeTruthy();
});
8 changes: 4 additions & 4 deletions tests/unit/multiEd25519.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,12 @@ describe("MultiEd25519Signature", () => {
it("should serializes to bytes correctly", async () => {
const edSigsArray = [];
for (let i = 0; i < multiEd25519SigTestObject.signatures.length; i += 1) {
edSigsArray.push(new Ed25519Signature(Hex.fromString(multiEd25519SigTestObject.signatures[i]).toUint8Array()));
edSigsArray.push(new Ed25519Signature(Hex.fromHexString(multiEd25519SigTestObject.signatures[i]).toUint8Array()));
}

const multisig = new MultiEd25519Signature({
signatures: edSigsArray,
bitmap: Hex.fromString(multiEd25519SigTestObject.bitmap).toUint8Array(),
bitmap: Hex.fromHexString(multiEd25519SigTestObject.bitmap).toUint8Array(),
});

expect(Hex.fromHexInput(multisig.toUint8Array()).toStringWithoutPrefix()).toEqual(
Expand All @@ -153,12 +153,12 @@ describe("MultiEd25519Signature", () => {
it("should deserializes from bytes correctly", async () => {
const edSigsArray = [];
for (let i = 0; i < multiEd25519SigTestObject.signatures.length; i += 1) {
edSigsArray.push(new Ed25519Signature(Hex.fromString(multiEd25519SigTestObject.signatures[i]).toUint8Array()));
edSigsArray.push(new Ed25519Signature(Hex.fromHexString(multiEd25519SigTestObject.signatures[i]).toUint8Array()));
}

const multisig = new MultiEd25519Signature({
signatures: edSigsArray,
bitmap: Hex.fromString(multiEd25519SigTestObject.bitmap).toUint8Array(),
bitmap: Hex.fromHexString(multiEd25519SigTestObject.bitmap).toUint8Array(),
});

const serializer = new Serializer();
Expand Down
Loading

0 comments on commit 46f8e40

Please sign in to comment.