Skip to content

Commit

Permalink
Update Ed25519 and ES256K Test Vectors and Ensure Low-S ECDSA Signatu…
Browse files Browse the repository at this point in the history
…res (#375)

* Migrate to using Web5 Test Vectors for Ed25519 and Secp256k1 sign and verify
* Ensure ES256K signatures are low-S form in crypto and crypto-aws-kms
  • Loading branch information
frankhinek authored Jan 16, 2024
1 parent 44c38a1 commit 12f7af5
Show file tree
Hide file tree
Showing 12 changed files with 659 additions and 152 deletions.
10 changes: 9 additions & 1 deletion packages/crypto-aws-kms/src/ecdsa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ export class EcdsaAlgorithm implements
* Hashing the data first ensures that the input to the signing operation is within this limit,
* regardless of the original data size.
*
* Note: The signature returned is normalized to low-S to prevent signature malleability. This
* ensures that the signature can be verified by other libraries that enforce strict verification.
* More information on signature malleability can be found
* {@link @web5/crypto#Secp256k1.adjustSignatureToLowS | here}.
*
* @example
* ```ts
* const ecdsa = new EcdsaAlgorithm({ crypto, kmsClient });
Expand Down Expand Up @@ -214,7 +219,10 @@ export class EcdsaAlgorithm implements
const derSignature = response.Signature;

// Convert the DER encoded signature to a compact R+S signature.
const signature = await Secp256k1.convertDerToCompactSignature({ derSignature });
let signature = await Secp256k1.convertDerToCompactSignature({ derSignature });

// Ensure the signature is in low-S, normalized form to prevent signature malleability.
signature = await Secp256k1.adjustSignatureToLowS({ signature });

return signature;
}
Expand Down
26 changes: 25 additions & 1 deletion packages/crypto-aws-kms/tests/ecdsa.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import type { Jwk } from '@web5/crypto';

import sinon from 'sinon';
import { expect } from 'chai';
import { Convert } from '@web5/common';
import { CreateKeyCommand, DescribeKeyCommand, KMSClient, SignCommand } from '@aws-sdk/client-kms';

import { AwsKmsCrypto } from '../src/api.js';
import { EcdsaAlgorithm } from '../src/ecdsa.js';
import { mockEcdsaSecp256k1 } from './fixtures/mock-ecdsa-secp256k1.js';
import { mockEcdsaSecp256k1, mockSignCommandOutput } from './fixtures/mock-ecdsa-secp256k1.js';

describe('EcdsaAlgorithm', () => {
let crypto: AwsKmsCrypto;
Expand Down Expand Up @@ -85,6 +86,29 @@ describe('EcdsaAlgorithm', () => {
expect(kmsClientStub.send.calledTwice).to.be.true;
});

it('returns normalized, low-s form signatures', async () => {
// Setup.
const mockHighSSignCommandOutput = {
...mockSignCommandOutput,
// Return the DER encoded signature from Wycheproof test case 1, which has a high-s value.
signature: Convert.hex('3046022100813ef79ccefa9a56f7ba805f0e478584fe5f0dd5f567bc09b5123ccbc9832365022100900e75ad233fcc908509dbff5922647db37c21f4afd3203ae8dc4ae7794b0f87').toUint8Array()
};
kmsClientStub.send.withArgs(sinon.match.instanceOf(SignCommand)).resolves(mockHighSSignCommandOutput);
kmsClientStub.send.withArgs(sinon.match.instanceOf(DescribeKeyCommand)).resolves(mockEcdsaSecp256k1.getKeySpec.output);

// Test the method.
const signature = await ecdsa.sign({
algorithm : 'ES256K',
data : new Uint8Array([0, 1, 2, 3, 4]),
keyUri : 'urn:jwk:U01_M3_A9vMLOWixG-rlfC-_f3LLdurttn7c7d3_upU'
});

// Validate the signature returned by EcdsaAlgorithm.sign() has been adjust to low-s form.
expect(signature).to.deep.equal(
Convert.hex('8891914c431baae682defc57fe074c8cb700f790d72e2a51474cca0ee00faa8451e462395b70b51ae6b98d3fadc233ae4db15583e9b75ac32d94a697c71aa426').toUint8Array()
);
});

it('throws an error if an unsupported algorithm is specified', async () => {
// Setup.
const keyUri = 'urn:jwk:U01_M3_A9vMLOWixG-rlfC-_f3LLdurttn7c7d3_upU';
Expand Down
8 changes: 4 additions & 4 deletions packages/crypto/.c8rc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
".js"
],
"include": [
"tests/compiled/src/**"
"tests/compiled/**/src/**"
],
"exclude": [
"tests/compiled/src/index.js",
"tests/compiled/src/types.js",
"tests/compiled/src/types/**"
"tests/compiled/**/src/index.js",
"tests/compiled/**/src/types.js",
"tests/compiled/**/src/types/**"
],
"reporter": [
"cobertura",
Expand Down
70 changes: 70 additions & 0 deletions packages/crypto/src/primitives/secp256k1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,76 @@ import { computeJwkThumbprint, isEcPrivateJwk, isEcPublicJwk } from '../jose/jwk
* ```
*/
export class Secp256k1 {
/**
* Adjusts an ECDSA signature to a normalized, low-S form.
*
* @remarks
* All ECDSA signatures, regardless of the curve, consist of two components, `r` and `s`, both of
* which are integers. The curve's order (the total number of points on the curve) is denoted by
* `n`. In a valid ECDSA signature, both `r` and `s` must be in the range [1, n-1]. However, due
* to the mathematical properties of ECDSA, if `(r, s)` is a valid signature, then `(r, n - s)` is
* also a valid signature for the same message and public key. In other words, for every
* signature, there's a "mirror" signature that's equally valid. For these elliptic curves:
*
* - Low S Signature: A signature where the `s` component is in the lower half of the range,
* specifically less than or equal to `n/2`.
*
* - High S Signature: This is where the `s` component is in the upper half of the range, greater
* than `n/2`.
*
* The practical implication is that a third-party can forge a second valid signature for the same
* message by negating the `s` component of the original signature, without any knowledge of the
* private key. This is known as a "signature malleability" attack.
*
* This type of forgery is not a problem in all systems, but it can be an issue in systems that
* rely on digital signature uniqueness to ensure transaction integrity. For example, in Bitcoin,
* transaction malleability is an issue because it allows for the modification of transaction
* identifiers (and potentially, transactions themselves) after they're signed but before they're
* confirmed in a block. By enforcing low `s` values, the Bitcoin network reduces the likelihood of
* this occurring, making the system more secure and predictable.
*
* For this reason, it's common practice to normalize ECDSA signatures to a low-S form. This
* form is considered standard and preferable in some systems and is known as the "normalized"
* form of the signature.
*
* This method takes a signature, and if it's high-S, returns the normalized low-S form. If the
* signature is already low-S, it's returned unmodified. It's important to note that this
* method does not change the validity of the signature but makes it compliant with systems that
* enforce low-S signatures.
*
* @example
* ```ts
* const signature = new Uint8Array([...]); // Your ECDSA signature
* const adjustedSignature = await Secp256k1.adjustSignatureToLowS({ signature });
* // Now 'adjustedSignature' is in the low-S form.
* ```
*
* @param params - The parameters for the signature adjustment.
* @param params.signature - The ECDSA signature as a `Uint8Array`.
*
* @returns A Promise that resolves to the adjusted signature in low-S form as a `Uint8Array`.
*/
public static async adjustSignatureToLowS({ signature }: {
signature: Uint8Array;
}): Promise<Uint8Array> {
// Convert the signature to a `secp256k1.Signature` object.
const signatureObject = secp256k1.Signature.fromCompact(signature);

if (signatureObject.hasHighS()) {
// Adjust the signature to low-S format if it's high-S.
const adjustedSignatureObject = signatureObject.normalizeS();

// Convert the adjusted signature object back to a byte array.
const adjustedSignature = adjustedSignatureObject.toCompactRawBytes();

return adjustedSignature;

} else {
// Return the unmodified signature if it is already in low-S format.
return signature;
}
}

/**
* Converts a raw private key in bytes to its corresponding JSON Web Key (JWK) format.
*
Expand Down
112 changes: 36 additions & 76 deletions packages/crypto/tests/primitives/ed25519.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import chaiAsPromised from 'chai-as-promised';

import type { Jwk, JwkParamsOkpPrivate } from '../../src/jose/jwk.js';

import ed25519Sign from '../fixtures/test-vectors/ed25519/sign.json' assert { type: 'json' };
import ed25519Verify from '../fixtures/test-vectors/ed25519/verify.json' assert { type: 'json' };
import CryptoEd25519SignTestVector from '../../../../test-vectors/crypto_ed25519/sign.json' assert { type: 'json' };
import ed25519ComputePublicKey from '../fixtures/test-vectors/ed25519/compute-public-key.json' assert { type: 'json' };
import CryptoEd25519VerifyTestVector from '../../../../test-vectors/crypto_ed25519/verify.json' assert { type: 'json' };
import ed25519BytesToPublicKey from '../fixtures/test-vectors/ed25519/bytes-to-public-key.json' assert { type: 'json' };
import ed25519PublicKeyToBytes from '../fixtures/test-vectors/ed25519/public-key-to-bytes.json' assert { type: 'json' };
import ed25519BytesToPrivateKey from '../fixtures/test-vectors/ed25519/bytes-to-private-key.json' assert { type: 'json' };
Expand Down Expand Up @@ -315,17 +315,24 @@ describe('Ed25519', () => {
expect(signature).to.be.instanceOf(Uint8Array);
});

for (const vector of ed25519Sign.vectors) {
it(vector.description, async () => {
const signature = await Ed25519.sign({
key : vector.input.key as Jwk,
data : Convert.hex(vector.input.data).toUint8Array()
});

const signatureHex = Convert.uint8Array(signature).toHex();
expect(signatureHex).to.deep.equal(vector.output);
describe('Web5TestVectorsCryptoEd25519', () => {
it('sign', async () => {
for (const vector of CryptoEd25519SignTestVector.vectors) {
let errorOccurred = false;
try {
const signature = await Ed25519.sign({
key : vector.input.key as Jwk,
data : Convert.hex(vector.input.data).toUint8Array()
});

const signatureHex = Convert.uint8Array(signature).toHex();
expect(signatureHex).to.deep.equal(vector.output, vector.description);

} catch { errorOccurred = true; }
expect(errorOccurred).to.equal(vector.errors, `Expected '${vector.description}' to${vector.errors ? ' ' : ' not '}throw an error`);
}
});
}
});
});

describe('validatePublicKey()', () => {
Expand Down Expand Up @@ -368,70 +375,23 @@ describe('Ed25519', () => {
expect(isValid).to.be.true;
});

it('returns false if the signed data was mutated', async () => {
const data = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]);
let isValid: boolean;

// Generate signature using the private key.
const signature = await Ed25519.sign({ key: privateKey, data });

// Verification should return true with the data used to generate the signature.
isValid = await Ed25519.verify({ key: publicKey, signature, data });
expect(isValid).to.be.true;

// Make a copy and flip the least significant bit (the rightmost bit) in the first byte of the array.
const mutatedData = new Uint8Array(data);
mutatedData[0] ^= 1 << 0;

// Verification should return false if the given data does not match the data used to generate the signature.
isValid = await Ed25519.verify({ key: publicKey, signature, data: mutatedData });
expect(isValid).to.be.false;
});

it('returns false if the signature was mutated', async () => {
const data = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]);
let isValid: boolean;

// Generate a new private key and get its public key.
privateKey = await Ed25519.generateKey();
publicKey = await Ed25519.computePublicKey({ key: privateKey });

// Generate signature using the private key.
const signature = await Ed25519.sign({ key: privateKey, data });

// Make a copy and flip the least significant bit (the rightmost bit) in the first byte of the array.
const mutatedSignature = new Uint8Array(signature);
mutatedSignature[0] ^= 1 << 0;

// Verification should return false if the signature was modified.
isValid = await Ed25519.verify({ key: publicKey, signature: signature, data: mutatedSignature });
expect(isValid).to.be.false;
});

it('returns false with a signature generated using a different private key', async () => {
const data = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]);
const privateKeyA = await Ed25519.generateKey();
const publicKeyA = await Ed25519.computePublicKey({ key: privateKeyA });
const privateKeyB = await Ed25519.generateKey();
let isValid: boolean;

// Generate a signature using private key B.
const signatureB = await Ed25519.sign({ key: privateKeyB, data });

// Verification should return false with the public key from private key A.
isValid = await Ed25519.verify({ key: publicKeyA, signature: signatureB, data });
expect(isValid).to.be.false;
});

for (const vector of ed25519Verify.vectors) {
it(vector.description, async () => {
const isValid = await Ed25519.verify({
key : vector.input.key as Jwk,
signature : Convert.hex(vector.input.signature).toUint8Array(),
data : Convert.hex(vector.input.data).toUint8Array()
});
expect(isValid).to.equal(vector.output);
describe('Web5TestVectorsCryptoEd25519', () => {
it('verify', async () => {
for (const vector of CryptoEd25519VerifyTestVector.vectors) {
let errorOccurred = false;
try {
const isValid = await Ed25519.verify({
key : vector.input.key as Jwk,
signature : Convert.hex(vector.input.signature).toUint8Array(),
data : Convert.hex(vector.input.data).toUint8Array()
});

expect(isValid).to.equal(vector.output);

} catch { errorOccurred = true; }
expect(errorOccurred).to.equal(vector.errors, `Expected '${vector.description}' to${vector.errors ? ' ' : ' not '}throw an error`);
}
});
}
});
});
});
Loading

0 comments on commit 12f7af5

Please sign in to comment.