diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index aae64dc11c..ec78ada7d2 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -21,6 +21,7 @@ import ( "github.com/stellar/go/keypair" "github.com/stellar/go/network" + "github.com/stellar/go/strkey" "github.com/stellar/go/support/errors" "github.com/stellar/go/xdr" ) @@ -499,6 +500,9 @@ func ReadChallengeTx(challengeTx, serverAccountID, network string) (tx Transacti // provided as an argument, and those signatures meet a threshold on the // account. // +// Signers that are not prefixed as an address/account ID strkey (G...) will be +// ignored. +// // Errors will be raised if: // - The transaction is invalid according to ReadChallengeTx. // - No client signatures are found on the transaction. @@ -536,16 +540,15 @@ func VerifyChallengeTxThreshold(challengeTx, serverAccountID, network string, th // to a signer for verification to succeed. If verification succeeds a list of // signers that were found is returned, excluding the server account ID. // +// Signers that are not prefixed as an address/account ID strkey (G...) will be +// ignored. +// // Errors will be raised if: // - The transaction is invalid according to ReadChallengeTx. // - No client signatures are found on the transaction. // - One or more signatures in the transaction are not identifiable as the // server account or one of the signers provided in the arguments. func VerifyChallengeTxSigners(challengeTx, serverAccountID, network string, signers ...string) ([]string, error) { - if len(signers) == 0 { - return nil, errors.New("no signers provided") - } - // Read the transaction which validates its structure. tx, _, err := ReadChallengeTx(challengeTx, serverAccountID, network) if err != nil { @@ -571,13 +574,27 @@ func VerifyChallengeTxSigners(challengeTx, serverAccountID, network string, sign if signer == serverKP.Address() { continue } + // Deduplicate. if _, seen := clientSignersSeen[signer]; seen { continue } + // Ignore non-G... account/address signers. + strkeyVersionByte, strkeyErr := strkey.Version(signer) + if strkeyErr != nil { + continue + } + if strkeyVersionByte != strkey.VersionByteAccountID { + continue + } clientSigners = append(clientSigners, signer) clientSignersSeen[signer] = struct{}{} } + // Don't continue if none of the signers provided are in the final list. + if len(clientSigners) == 0 { + return nil, errors.New("no verifiable signers provided, at least one G... address must be provided") + } + // 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 diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index 5cf0178e03..7857ab8d14 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -1712,6 +1712,52 @@ func TestVerifyChallengeTxThreshold_validServerAndMultipleClientKeyMeetingThresh assert.NoError(t, err) } +func TestVerifyChallengeTxThreshold_validServerAndMultipleClientKeyMeetingThresholdSomeUnusedIgnorePreauthTxHashAndXHash(t *testing.T) { + serverKP := newKeypair0() + clientKP1 := newKeypair1() + clientKP2 := newKeypair2() + clientKP3 := keypair.MustRandom() + preauthTxHash := "TAQCSRX2RIDJNHFIFHWD63X7D7D6TRT5Y2S6E3TEMXTG5W3OECHZ2OG4" + xHash := "XDRPF6NZRR7EEVO7ESIWUDXHAOMM2QSKIQQBJK6I2FB7YKDZES5UCLWD" + unknownSignerType := "?ARPF6NZRR7EEVO7ESIWUDXHAOMM2QSKIQQBJK6I2FB7YKDZES5UCLWD" + txSource := NewSimpleAccount(serverKP.Address(), -1) + opSource := NewSimpleAccount(clientKP1.Address(), 0) + op := ManageData{ + SourceAccount: &opSource, + Name: "testserver auth", + Value: []byte(base64.StdEncoding.EncodeToString(make([]byte, 48))), + } + tx := Transaction{ + SourceAccount: &txSource, + Operations: []Operation{&op}, + Timebounds: NewTimeout(1000), + Network: network.TestNetworkPassphrase, + } + threshold := Threshold(3) + signerSummary := SignerSummary{ + clientKP1.Address(): 1, + clientKP2.Address(): 2, + clientKP3.Address(): 2, + preauthTxHash: 10, + xHash: 10, + unknownSignerType: 10, + } + wantSigners := []string{ + clientKP1.Address(), + clientKP2.Address(), + } + + err := tx.Build() + require.NoError(t, err) + err = tx.Sign(serverKP, clientKP1, clientKP2) + assert.NoError(t, err) + tx64, err := tx.Base64() + require.NoError(t, err) + signersFound, err := VerifyChallengeTxThreshold(tx64, serverKP.Address(), network.TestNetworkPassphrase, threshold, signerSummary) + assert.ElementsMatch(t, wantSigners, signersFound) + assert.NoError(t, err) +} + func TestVerifyChallengeTxThreshold_invalidServerAndMultipleClientKeyNotMeetingThreshold(t *testing.T) { serverKP := newKeypair0() clientKP1 := newKeypair1() @@ -1809,7 +1855,7 @@ func TestVerifyChallengeTxThreshold_invalidNoSigners(t *testing.T) { tx64, err := tx.Base64() require.NoError(t, err) _, err = VerifyChallengeTxThreshold(tx64, serverKP.Address(), network.TestNetworkPassphrase, threshold, signerSummary) - assert.EqualError(t, err, "no signers provided") + assert.EqualError(t, err, "no verifiable signers provided, at least one G... address must be provided") } func TestVerifyChallengeTxThreshold_weightsAddToMoreThan8Bits(t *testing.T) { @@ -2137,6 +2183,38 @@ func TestVerifyChallengeTxSigners_validServerAndClientSignersIgnoresDuplicateSig assert.NoError(t, err) } +func TestVerifyChallengeTxSigners_validIgnorePreauthTxHashAndXHash(t *testing.T) { + serverKP := newKeypair0() + clientKP := newKeypair1() + clientKP2 := newKeypair2() + preauthTxHash := "TAQCSRX2RIDJNHFIFHWD63X7D7D6TRT5Y2S6E3TEMXTG5W3OECHZ2OG4" + xHash := "XDRPF6NZRR7EEVO7ESIWUDXHAOMM2QSKIQQBJK6I2FB7YKDZES5UCLWD" + unknownSignerType := "?ARPF6NZRR7EEVO7ESIWUDXHAOMM2QSKIQQBJK6I2FB7YKDZES5UCLWD" + txSource := NewSimpleAccount(serverKP.Address(), -1) + opSource := NewSimpleAccount(clientKP.Address(), 0) + op := ManageData{ + SourceAccount: &opSource, + Name: "testserver auth", + Value: []byte(base64.StdEncoding.EncodeToString(make([]byte, 48))), + } + tx := Transaction{ + SourceAccount: &txSource, + Operations: []Operation{&op}, + Timebounds: NewTimeout(1000), + Network: network.TestNetworkPassphrase, + } + + err := tx.Build() + require.NoError(t, err) + err = tx.Sign(serverKP, clientKP2) + assert.NoError(t, err) + tx64, err := tx.Base64() + require.NoError(t, err) + signersFound, err := VerifyChallengeTxSigners(tx64, serverKP.Address(), network.TestNetworkPassphrase, clientKP2.Address(), preauthTxHash, xHash, unknownSignerType) + assert.Equal(t, []string{clientKP2.Address()}, signersFound) + assert.NoError(t, err) +} + func TestVerifyChallengeTxSigners_invalidServerAndClientSignersIgnoresDuplicateSignerInError(t *testing.T) { serverKP := newKeypair0() clientKP := newKeypair1() @@ -2221,7 +2299,7 @@ func TestVerifyChallengeTxSigners_invalidServerAndClientSignersFailsSignerSeed(t require.NoError(t, err) signersFound, err := VerifyChallengeTxSigners(tx64, serverKP.Address(), network.TestNetworkPassphrase, clientKP2.Seed()) assert.Empty(t, signersFound) - assert.EqualError(t, err, "signer not address: invalid version byte") + assert.EqualError(t, err, "no verifiable signers provided, at least one G... address must be provided") } func TestVerifyChallengeTxSigners_invalidNoSigners(t *testing.T) { @@ -2248,7 +2326,7 @@ func TestVerifyChallengeTxSigners_invalidNoSigners(t *testing.T) { tx64, err := tx.Base64() require.NoError(t, err) _, err = VerifyChallengeTxSigners(tx64, serverKP.Address(), network.TestNetworkPassphrase) - assert.EqualError(t, err, "no signers provided") + assert.EqualError(t, err, "no verifiable signers provided, at least one G... address must be provided") } func TestVerifyTxSignatureUnsignedTx(t *testing.T) {