Skip to content
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
18 changes: 11 additions & 7 deletions modules/sdk-coin-xlm/src/getStellarKeys.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import { BitGoBase, ExternalEd25519Signature, InitiateRecoveryOptions } from '@bitgo/sdk-core';
import { BitGoBase, ExternalEd25519Signer, InitiateRecoveryOptions } from '@bitgo/sdk-core';
import * as stellar from 'stellar-sdk';
import { Utils } from './lib';

export async function getStellarKeys(
bitgo: BitGoBase,
params: InitiateRecoveryOptions,
externalBackupSignature?: ExternalEd25519Signature
params: Omit<InitiateRecoveryOptions, 'backupKey'> & { backupKey?: string },
externalBackupSigner?: ExternalEd25519Signer
): Promise<stellar.Keypair[]> {
const keys: stellar.Keypair[] = [];
let userKey = params.userKey;
let backupKey = params.backupKey;

// Stellar's Ed25519 public keys start with a G, while private keys start with an S
const isKrsRecovery = !externalBackupSignature && backupKey.startsWith('G') && !userKey.startsWith('G');
const isUnsignedSweep = !externalBackupSignature && backupKey.startsWith('G') && userKey.startsWith('G');
const hasPublicBackupKey = backupKey !== undefined && backupKey.startsWith('G');
const isKrsRecovery = !externalBackupSigner && hasPublicBackupKey && !userKey.startsWith('G');
const isUnsignedSweep = !externalBackupSigner && hasPublicBackupKey && userKey.startsWith('G');

/**
* Get User Key
Expand Down Expand Up @@ -42,13 +43,16 @@ export async function getStellarKeys(
/**
* Get Backup Key
*/
if (externalBackupSignature) {
if (externalBackupSigner) {
keys.push(
stellar.Keypair.fromPublicKey(
stellar.StrKey.encodeEd25519PublicKey(Buffer.from(externalBackupSignature.publicKey, 'hex'))
stellar.StrKey.encodeEd25519PublicKey(Buffer.from(externalBackupSigner.publicKey, 'hex'))
)
);
} else {
if (!backupKey) {
throw new Error('backupKey is required for non-bitgo recovery when no external backup signer is provided');
}
try {
if (!backupKey.startsWith('S') && !isKrsRecovery && !isUnsignedSweep) {
backupKey = await bitgo.decrypt({
Expand Down
46 changes: 25 additions & 21 deletions modules/sdk-coin-xlm/src/xlm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ import {
checkKrsProvider,
common,
ErrorNoInputToRecover,
assertValidExternalEd25519Signature,
ExternalEd25519Signature,
assertValidExternalEd25519Signer,
ExternalEd25519Signer,
signWithExternalEd25519Signer,
ExtraPrebuildParamsOptions,
InvalidAddressError,
InvalidMemoIdError,
Expand Down Expand Up @@ -66,16 +67,16 @@ interface Memo {

interface InitiateRecoveryOptions {
userKey: string;
backupKey: string;
backupKey?: string;
recoveryDestination: string;
krsProvider?: string;
walletPassphrase?: string;
}

interface RecoveryOptions extends InitiateRecoveryOptions {
rootAddress?: string;
/** Optional pre-made signature for the backup child, whose signature must be over the transaction hash (`tx.hash()`). */
externalBackupSignature?: ExternalEd25519Signature;
/** Signs `tx.hash()` once the network-dependent transaction fields are fixed. */
externalBackupSigner?: ExternalEd25519Signer;
}

interface RecoveryTransaction {
Expand Down Expand Up @@ -740,19 +741,25 @@ export class Xlm extends BaseCoin {
params.userKey = Utils.encodePublicKey(Buffer.from(params.userKey, 'hex'));
}

if (Utils.isValidRootPrivateKey(params.backupKey)) {
params.backupKey = Utils.encodePrivateKey(Buffer.from(params.backupKey.slice(0, 64), 'hex'));
} else if (Utils.isValidRootPublicKey(params.backupKey)) {
params.backupKey = Utils.encodePublicKey(Buffer.from(params.backupKey, 'hex'));
if (params.backupKey) {
if (Utils.isValidRootPrivateKey(params.backupKey)) {
params.backupKey = Utils.encodePrivateKey(Buffer.from(params.backupKey.slice(0, 64), 'hex'));
} else if (Utils.isValidRootPublicKey(params.backupKey)) {
params.backupKey = Utils.encodePublicKey(Buffer.from(params.backupKey, 'hex'));
}
}

const hasExternalBackupSig = Boolean(params.externalBackupSignature);
if (params.externalBackupSignature) {
assertValidExternalEd25519Signature(params.externalBackupSignature);
const externalBackupSigner = params.externalBackupSigner;
if (externalBackupSigner) {
assertValidExternalEd25519Signer(externalBackupSigner);
}
if (!externalBackupSigner && !params.backupKey) {
throw new Error('backupKey is required for non-bitgo recovery when no external backup signer is provided');
}
// Stellar's Ed25519 public keys start with a G, while private keys start with an S
const isKrsRecovery = !hasExternalBackupSig && params.backupKey.startsWith('G') && !params.userKey.startsWith('G');
const isUnsignedSweep = !hasExternalBackupSig && params.backupKey.startsWith('G') && params.userKey.startsWith('G');
const hasPublicBackupKey = params.backupKey !== undefined && params.backupKey.startsWith('G');
Comment thread
pranishnepal marked this conversation as resolved.
const isKrsRecovery = !externalBackupSigner && hasPublicBackupKey && !params.userKey.startsWith('G');
const isUnsignedSweep = !externalBackupSigner && hasPublicBackupKey && params.userKey.startsWith('G');

if (isKrsRecovery) {
checkKrsProvider(this, params.krsProvider);
Expand All @@ -762,7 +769,7 @@ export class Xlm extends BaseCoin {
throw new InvalidAddressError('Invalid destination address!');
}

const [userKey, backupKey] = await getStellarKeys(this.bitgo, params, params.externalBackupSignature);
const [userKey, backupKey] = await getStellarKeys(this.bitgo, params, externalBackupSigner);

if (!params.rootAddress || !stellar.StrKey.isValidEd25519PublicKey(params.rootAddress)) {
throw new Error(`Invalid wallet address: ${params.rootAddress}`);
Expand Down Expand Up @@ -850,12 +857,9 @@ export class Xlm extends BaseCoin {
}

if (!isKrsRecovery && !isUnsignedSweep) {
const externalBackupSignature = params.externalBackupSignature;
if (externalBackupSignature) {
tx.addSignature(
backupKey.publicKey(),
Buffer.from(externalBackupSignature.signature, 'hex').toString('base64')
);
if (externalBackupSigner) {
const signature = await signWithExternalEd25519Signer(externalBackupSigner, tx.hash());
tx.addSignature(backupKey.publicKey(), Buffer.from(signature, 'hex').toString('base64'));
} else {
tx.sign(backupKey);
}
Expand Down
139 changes: 136 additions & 3 deletions modules/sdk-coin-xlm/test/unit/xlm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -558,25 +558,158 @@ describe('XLM:', function () {
.should.be.rejectedWith(RecoveryProviderError);
});

describe('getStellarKeys with external backup signature', function () {
describe('getStellarKeys with external backup signer', function () {
it('builds a pub-only backup Keypair from the external public key', async function () {
const userKeypair = stellar.Keypair.random();
const externalBackupKeypair = stellar.Keypair.random();
const [userKey, backupKey] = await getStellarKeys(
bitgo,
{
userKey: userKeypair.secret(),
backupKey: 'GC3D3ZNNK7GHLMSWJA54DQO6QJUJJF7K6J5JGCEW45ZT6QMKZ6PMUHUM',
recoveryDestination: 'GDDHCKMYYYCVXOSAVMSEIYGYNX74LIAV3ACXYQ6WPMDUF7W3KZNWTHTH',
},
{
publicKey: externalBackupKeypair.rawPublicKey().toString('hex'),
signature: 'ab'.repeat(64),
sign: async () => ({ signature: 'ab'.repeat(64) }),
}
);
userKey.publicKey().should.equal(userKeypair.publicKey());
backupKey.publicKey().should.equal(externalBackupKeypair.publicKey());
});

it('rejects when neither a backupKey nor an external backup signer is provided', async function () {
const userKeypair = stellar.Keypair.random();
await assert.rejects(
() =>
getStellarKeys(bitgo, {
userKey: userKeypair.secret(),
recoveryDestination: 'GDDHCKMYYYCVXOSAVMSEIYGYNX74LIAV3ACXYQ6WPMDUF7W3KZNWTHTH',
}),
/backupKey is required/
);
});
});

describe('recover with an external backup signer', function () {
const userKeypair = stellar.Keypair.fromSecret(userKeychain.prv);
const backupKeypair = stellar.Keypair.fromSecret(backupKeychain.prv);
const publicKey = backupKeypair.rawPublicKey().toString('hex');
const params = {
userKey: userKeychain.prv,
rootAddress: userKeychain.pub,
recoveryDestination: 'GDDHCKMYYYCVXOSAVMSEIYGYNX74LIAV3ACXYQ6WPMDUF7W3KZNWTHTH',
};

beforeEach(function () {
nock.disableNetConnect();
nock('https://horizon-testnet.stellar.org/accounts')
.get('/' + params.rootAddress)
.reply(200, {
sequence: '35995558267060226',
balances: [{ asset_type: 'native', balance: '6500000000' }],
signers: [
{ key: userKeypair.publicKey(), weight: 1, type: 'ed25519_public_key' },
{ key: backupKeypair.publicKey(), weight: 1, type: 'ed25519_public_key' },
],
});

nock('https://horizon-testnet.stellar.org/accounts')
.get('/' + params.recoveryDestination)
.reply(200, { sequence: '35995558267060213', balances: 13131313 });

nock('https://horizon-testnet.stellar.org')
.get('/ledgers')
.query({ order: 'desc', limit: 1 })
.reply(200, {
records: [{ base_reserve_in_stroops: '5000000', base_fee_in_stroops: 100 }],
})
.persist();
});

afterEach(function () {
nock.cleanAll();
nock.enableNetConnect();
});

it('should attach verifiable user and external backup signatures over the transaction hash', async function () {
const signedMessages: Buffer[] = [];
const recovery = await basecoin.recover({
...params,
externalBackupSigner: {
publicKey,
sign: async (message: Uint8Array) => {
signedMessages.push(Buffer.from(message));
return { signature: backupKeypair.sign(Buffer.from(message)).toString('hex') };
},
},
});

const tx = new stellar.Transaction(recovery.txBase64, basecoin.getStellarNetwork());
assert.strictEqual(tx.signatures.length, 2);
assert.ok(userKeypair.verify(tx.hash(), tx.signatures[0].signature()));
assert.ok(backupKeypair.verify(tx.hash(), tx.signatures[1].signature()));
assert.deepStrictEqual(signedMessages, [tx.hash()]);
});

it('should preserve recovery signed with the local backup seed', async function () {
const recovery = await basecoin.recover({ ...params, backupKey: backupKeychain.prv });
const tx = new stellar.Transaction(recovery.txBase64, basecoin.getStellarNetwork());

assert.strictEqual(tx.signatures.length, 2);
assert.ok(userKeypair.verify(tx.hash(), tx.signatures[0].signature()));
assert.ok(backupKeypair.verify(tx.hash(), tx.signatures[1].signature()));
});

for (const { description, signature, error } of [
{ description: 'does not verify', signature: 'ab'.repeat(64), error: /does not verify against its public key/ },
{ description: 'is malformed', signature: 'ab', error: /expected 64 bytes/ },
]) {
it(`should reject an external backup signer whose signature ${description}`, async function () {
await assert.rejects(
() =>
basecoin.recover({
...params,
externalBackupSigner: { publicKey, sign: async () => ({ signature }) },
}),
error
);
});
}

it('should reject an external backup signer with a malformed public key', async function () {
await assert.rejects(
() =>
basecoin.recover({
...params,
externalBackupSigner: {
publicKey: 'not-hex',
sign: async () => ({ signature: 'ab'.repeat(64) }),
},
}),
/expected 32 bytes/
);
});

it('should propagate an external backup signer failure', async function () {
const signingError = new Error('external signer unavailable');
await assert.rejects(
() =>
basecoin.recover({
...params,
externalBackupSigner: {
publicKey,
sign: async () => {
throw signingError;
},
},
}),
(error) => error === signingError
);
});

it('should require a backup key when no external signer is provided', async function () {
await assert.rejects(() => basecoin.recover(params), /backupKey is required/);
});
});

it('should throw RecoveryKeyMismatchError when the user key is not an account signer', async function () {
Expand Down
15 changes: 15 additions & 0 deletions modules/sdk-core/test/unit/bitgo/safe/derivableEd25519Pub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,21 @@ describe('derivableEd25519Pub', function () {
).should.be.rejectedWith(/does not verify against its public key/);
});

it('rejects a signature over mutated bytes without changing the original message', async function () {
const input = Uint8Array.from(message);
await signWithExternalEd25519Signer(
{
publicKey,
sign: async (m: Uint8Array) => {
m[0] ^= 0xff;
return { signature: Buffer.from(nacl.sign.detached(m, keyPair.secretKey)).toString('hex') };
},
},
input
).should.be.rejectedWith(/does not verify against its public key/);
Buffer.from(input).equals(message).should.equal(true);
});

it('signs and returns a verified 64-byte signature', async function () {
const signature = await signWithExternalEd25519Signer(validSigner, message);
signature.should.match(/^[0-9a-f]{128}$/);
Expand Down
Loading