diff --git a/CHANGELOG.md b/CHANGELOG.md index da882b319..ac8e4b2ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ A breaking change will get clearly marked in this log. ## Unreleased +### Add + +- Adds [client domain verification](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md#verifying-the-client-domain) to the SEP-10 helper functions ([#720](https://github.com/stellar/js-stellar-sdk/pull/720)): + - `Utils.buildChallengeTx()` + - `Utils.readChallengeTx()` + - `Utils.verifyChallengeTxSigners()` ## [v9.0.0](https://github.com/stellar/js-stellar-sdk/compare/v9.0.0-beta.1...v9.0.0) diff --git a/src/utils.ts b/src/utils.ts index 2019f7cbd..2ec15bb71 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -49,6 +49,8 @@ export namespace Utils { networkPassphrase: string, webAuthDomain: string, memo: string | null = null, + clientDomain: string | null = null, + clientSigningKey: string | null = null, ): string { if (clientAccountID.startsWith("M") && memo) { throw Error("memo cannot be used if clientAccountID is a muxed account"); @@ -88,6 +90,19 @@ export namespace Utils { }), ); + if (clientDomain) { + if (!clientSigningKey) { + throw Error("clientSigningKey is required if clientDomain is provided"); + } + builder.addOperation( + Operation.manageData({ + name: `client_domain`, + value: clientDomain, + source: clientSigningKey, + }), + ); + } + if (memo) { builder.addMemo(Memo.id(memo)); } @@ -284,7 +299,7 @@ export namespace Utils { "The transaction has operations that are not of type 'manageData'", ); } - if (op.source !== serverAccountID) { + if (op.source !== serverAccountID && op.name !== "client_domain") { throw new InvalidSep10ChallengeError( "The transaction has operations that are unrecognized", ); @@ -533,6 +548,18 @@ export namespace Utils { ); } + let clientSigningKey; + for (const op of tx.operations) { + if (op.type === "manageData" && op.name === "client_domain") { + if (clientSigningKey) { + throw new InvalidSep10ChallengeError( + "Found more than one client_domain operation", + ); + } + clientSigningKey = op.source; + } + } + // Verify all the transaction's signers (server and client) in one // hit. We do this in one hit here even though the server signature was // checked in the ReadChallengeTx to ensure that every signature and signer @@ -541,16 +568,38 @@ export namespace Utils { serverKP.publicKey(), ...Array.from(clientSigners), ]; + if (clientSigningKey) { + allSigners.push(clientSigningKey); + } const signersFound: string[] = gatherTxSigners(tx, allSigners); + let serverSignatureFound = false; + let clientSigningKeySignatureFound = false; + for (const signer of signersFound) { + if (signer === serverKP.publicKey()) { + serverSignatureFound = true; + } + if (signer === clientSigningKey) { + clientSigningKeySignatureFound = true; + } + } + // Confirm we matched a signature to the server signer. - if (signersFound.indexOf(serverKP.publicKey()) === -1) { + if (!serverSignatureFound) { throw new InvalidSep10ChallengeError( "Transaction not signed by server: '" + serverKP.publicKey() + "'", ); } + // Confirm we matched a signature to the client domain's signer + if (clientSigningKey && !clientSigningKeySignatureFound) { + throw new InvalidSep10ChallengeError( + "Transaction not signed by the source account of the 'client_domain' " + + "ManageData operation", + ); + } + // Confirm we matched at least one given signer with the transaction signatures if (signersFound.length === 1) { throw new InvalidSep10ChallengeError( @@ -567,6 +616,10 @@ export namespace Utils { // Remove the server public key before returning signersFound.splice(signersFound.indexOf(serverKP.publicKey()), 1); + if (clientSigningKey) { + // Remove the client domain public key public key before returning + signersFound.splice(signersFound.indexOf(clientSigningKey), 1); + } return signersFound; } diff --git a/test/unit/utils_test.js b/test/unit/utils_test.js index 9dfda9f13..5f9fa7c11 100644 --- a/test/unit/utils_test.js +++ b/test/unit/utils_test.js @@ -95,6 +95,7 @@ describe('Utils', function() { it('returns challenge which follows SEP0010 spec', function() { let keypair = StellarSdk.Keypair.random(); + let clientSigningKeypair = StellarSdk.Keypair.random(); const challenge = StellarSdk.Utils.buildChallengeTx( keypair, @@ -102,20 +103,23 @@ describe('Utils', function() { "testanchor.stellar.org", 300, StellarSdk.Networks.TESTNET, - "testanchor.stellar.org" + "testanchor.stellar.org", + null, + "testdomain", + clientSigningKeypair.publicKey() ); const transaction = new StellarSdk.Transaction(challenge, StellarSdk.Networks.TESTNET); expect(transaction.sequence).to.eql("0"); expect(transaction.source).to.eql(keypair.publicKey()); - expect(transaction.operations.length).to.eql(2); + expect(transaction.operations.length).to.eql(3); const { maxTime, minTime } = transaction.timeBounds; expect(parseInt(maxTime) - parseInt(minTime)).to.eql(300); - const [ operation1, operation2 ] = transaction.operations; + const [ operation1, operation2, operation3 ] = transaction.operations; expect(operation1.name).to.eql("testanchor.stellar.org auth"); expect(operation1.source).to.eql("GBDIT5GUJ7R5BXO3GJHFXJ6AZ5UQK6MNOIDMPQUSMXLIHTUNR2Q5CFNF"); @@ -127,6 +131,11 @@ describe('Utils', function() { expect(operation2.source).to.eql(keypair.publicKey()); expect(operation2.type).to.eql("manageData"); expect(operation2.value.toString()).to.eql("testanchor.stellar.org"); + + expect(operation3.name).to.eql("client_domain"); + expect(operation3.source).to.eql(clientSigningKeypair.publicKey()); + expect(operation3.type).to.eql("manageData"); + expect(operation3.value.toString()).to.eql("testdomain"); }); it('uses the passed-in timeout', function() { @@ -169,6 +178,24 @@ describe('Utils', function() { ); }); + it("throws an error if clientSigningKey is not passed", function() { + expect(() => + StellarSdk.Utils.buildChallengeTx( + StellarSdk.Keypair.random(), + StellarSdk.Keypair.random().publicKey(), + "testanchor.stellar.org", + 600, + StellarSdk.Networks.TESTNET, + "testanchor.stellar.org", + null, + "testdomain", + null + ) + ).to.throw( + /clientSigningKey is required if clientDomain is provided/ + ); + }); + }); describe("Utils.readChallengeTx", function() { @@ -1424,6 +1451,48 @@ describe('Utils', function() { memo: null }); }); + + it("validates a challenge containing a 'client_domain' manageData operation", () => { + let serverKP = StellarSdk.Keypair.random(); + let clientKP = StellarSdk.Keypair.random(); + const serverAccount = new StellarSdk.Account(serverKP.publicKey(), "-1"); + let clientSigningKeypair = StellarSdk.Keypair.random(); + + const transaction = new StellarSdk.TransactionBuilder( + serverAccount, + txBuilderOpts, + ) + .addOperation( + StellarSdk.Operation.manageData({ + source: clientKP.publicKey(), + name: "testanchor.stellar.org auth", + value: randomBytes(48).toString("base64"), + }), + ) + .addOperation( + StellarSdk.Operation.manageData({ + source: clientSigningKeypair.publicKey(), + name: "client_domain", + value: "testdomain" + }), + ) + .setTimeout(30) + .build(); + + transaction.sign(serverKP); + const challenge = transaction + .toEnvelope() + .toXDR("base64") + .toString(); + + StellarSdk.Utils.readChallengeTx( + challenge, + serverKP.publicKey(), + StellarSdk.Networks.TESTNET, + "testanchor.stellar.org", + "testanchor.stellar.org" + ); + }); }); describe("Utils.verifyChallengeTxThreshold", function() { @@ -2387,6 +2456,155 @@ describe('Utils', function() { /No verifiable client signers provided, at least one G... address must be provided/, ); }); + + it("validates challenges containing client domain signers", () => { + const serverKP = StellarSdk.Keypair.random(); + const clientKP = StellarSdk.Keypair.random(); + const clientSigningKey = StellarSdk.Keypair.random(); + const challenge = StellarSdk.Utils.buildChallengeTx( + serverKP, + clientKP.publicKey(), + "SDF", + 300, + StellarSdk.Networks.TESTNET, + "testanchor.stellar.org", + null, + "testdomain", + clientSigningKey.publicKey() + ); + + clock.tick(200); + + const transaction = new StellarSdk.Transaction( + challenge, + StellarSdk.Networks.TESTNET + ); + + transaction.sign(clientKP); + transaction.sign(clientSigningKey); + + const signedChallenge = transaction + .toEnvelope() + .toXDR("base64") + .toString(); + + const signersFound = StellarSdk.Utils.verifyChallengeTxSigners( + signedChallenge, + serverKP.publicKey(), + StellarSdk.Networks.TESTNET, + [clientKP.publicKey()], + "SDF", + "testanchor.stellar.org" + ); + + expect(signersFound.indexOf(clientSigningKey.publicKey())).to.eql(-1); + }); + + it("throws an error if a challenge with a client_domain operation doesn't have a matching signature", () => { + const serverKP = StellarSdk.Keypair.random(); + const clientKP = StellarSdk.Keypair.random(); + const clientSigningKeypair = StellarSdk.Keypair.random(); + const challenge = StellarSdk.Utils.buildChallengeTx( + serverKP, + clientKP.publicKey(), + "SDF", + 300, + StellarSdk.Networks.TESTNET, + "testanchor.stellar.org", + null, + "testdomain", + clientSigningKeypair.publicKey() + ); + + clock.tick(200); + + const transaction = new StellarSdk.Transaction( + challenge, + StellarSdk.Networks.TESTNET + ); + + transaction.sign(clientKP); + + const signedChallenge = transaction + .toEnvelope() + .toXDR("base64") + .toString(); + + expect(() => + StellarSdk.Utils.verifyChallengeTxSigners( + signedChallenge, + serverKP.publicKey(), + StellarSdk.Networks.TESTNET, + [clientKP.publicKey()], + "SDF", + "testanchor.stellar.org" + ) + ).to.throw( + StellarSdk.InvalidSep10ChallengeError, + /Transaction not signed by the source account of the 'client_domain' ManageData operation/ + ); + }); + + it("throws an error if a challenge has multiple client_domain operations", () => { + const serverKP = StellarSdk.Keypair.random(); + const clientKP = StellarSdk.Keypair.random(); + const clientSigningKeypair = StellarSdk.Keypair.random(); + + const serverAccount = new StellarSdk.Account(serverKP.publicKey(), "-1"); + + const transaction = new StellarSdk.TransactionBuilder( + serverAccount, + txBuilderOpts, + ) + .addOperation( + StellarSdk.Operation.manageData({ + source: clientKP.publicKey(), + name: "testanchor.stellar.org auth", + value: randomBytes(48).toString("base64"), + }), + ) + .addOperation( + StellarSdk.Operation.manageData({ + source: clientSigningKeypair.publicKey(), + name: "client_domain", + value: "testdomain" + }), + ) + .addOperation( + StellarSdk.Operation.manageData({ + source: clientSigningKeypair.publicKey(), + name: "client_domain", + value: "testdomain2" + }), + ) + .setTimeout(30) + .build(); + + clock.tick(200); + + transaction.sign(serverKP); + transaction.sign(clientKP); + transaction.sign(clientSigningKeypair); + + const signedChallenge = transaction + .toEnvelope() + .toXDR("base64") + .toString(); + + expect(() => + StellarSdk.Utils.verifyChallengeTxSigners( + signedChallenge, + serverKP.publicKey(), + StellarSdk.Networks.TESTNET, + [clientKP.publicKey()], + "testanchor.stellar.org", + "testanchor.stellar.org" + ) + ).to.throw( + StellarSdk.InvalidSep10ChallengeError, + /Found more than one client_domain operation/ + ); + }); }); describe('Utils.verifyTxSignedBy', function() {