From 4a827ab71ad5dc819c48a7390c9b1d47da43f71d Mon Sep 17 00:00:00 2001 From: oliha Date: Thu, 8 Aug 2019 17:12:07 -0400 Subject: [PATCH 1/3] verify sep10 challenge --- txnbuild/transaction.go | 86 ++++++++++++++++++++++ txnbuild/transaction_test.go | 134 +++++++++++++++++++++++++++++++++++ 2 files changed, 220 insertions(+) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index 68336c15fe..f8aa66ecf0 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -406,3 +406,89 @@ func (tx *Transaction) SignWithKeyString(keys ...string) error { return tx.Sign(signers...) } + +// VerifyChallengeTx is a factory method that verifies a SEP 10 challenge transaction, +// for use in web authentication. +// More details on SEP 10: https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md +func VerifyChallengeTx(challengeTx, serverAccountId, network string) (bool, error) { + tx, err := TransactionFromXDR(challengeTx) + if err != nil { + return false, err + } + tx.Network = network + + // verify transaction source + if tx.SourceAccount != nil { + if tx.SourceAccount.GetAccountID() != serverAccountId { + return false, errors.New("transaction source account is not equal to server's account") + } + } else { + return false, errors.New("transaction requires a source account") + } + + // verify timebounds + if tx.Timebounds.wasBuilt { + // if timebound is not infinite + if tx.Timebounds.MinTime > 0 && tx.Timebounds.MaxTime > 0 { + currentTime := time.Now().UTC().Unix() + if currentTime < tx.Timebounds.MinTime || currentTime > tx.Timebounds.MaxTime { + return false, errors.New("transaction timebounds has elapsed") + } + } + } else { + return false, errors.New("transaction requires timebounds") + } + + // verify operation + if len(tx.Operations) == 1 { + op, ok := tx.Operations[0].(*ManageData) + if !ok { + return false, errors.New("operation type should be manage_data") + } + if op.SourceAccount == nil { + return false, errors.New("operation should have a source account") + } + // verify signature from operation source + ok, err = verifyTxSignature(tx, op.SourceAccount.GetAccountID()) + if err != nil { + return ok, err + } + } else { + return false, errors.New("transaction requires a single manage_data operation") + } + + // verify signature from server signing key + return verifyTxSignature(tx, serverAccountId) +} + +// verifyTxSignature checks if a transaction has been signed by the provided stellar account. +func verifyTxSignature(tx Transaction, accountID string) (bool, error) { + signerFound := false + txHash, err := tx.Hash() + if err != nil { + return signerFound, err + } + + kp, err := keypair.Parse(accountID) + if err != nil { + return signerFound, err + } + + // find and verify signatures + if tx.xdrEnvelope != nil && len(tx.xdrEnvelope.Signatures) > 0 { + for _, s := range tx.xdrEnvelope.Signatures { + e := kp.Verify(txHash[:], s.Signature) + if e == nil { + signerFound = true + break + } + } + } else { + return signerFound, errors.New("transaction has no signatures") + } + + if !signerFound { + return signerFound, errors.Errorf("transaction not signed by %s", accountID) + } + return signerFound, nil +} diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index 4a690ebd78..ef144a48cf 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -1156,3 +1156,137 @@ func TestSignWithSecretKey(t *testing.T) { assert.NoError(t, err) assert.Equal(t, expected, actual, "base64 xdr should match") } + +func TestVerifyTxSignature(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + txSource := NewSimpleAccount(kp0.Address(), int64(9605939170639897)) + opSource := NewSimpleAccount(kp1.Address(), 0) + createAccount := CreateAccount{ + Destination: "GCCOBXW2XQNUSL467IEILE6MMCNRR66SSVL4YQADUNYYNUVREF3FIV2Z", + Amount: "10", + SourceAccount: &opSource, + } + tx := Transaction{ + SourceAccount: &txSource, + Operations: []Operation{&createAccount}, + Timebounds: NewInfiniteTimeout(), + Network: network.TestNetworkPassphrase, + } + + // verify unsigned tx + err := tx.Build() + assert.NoError(t, err) + ok, err := verifyTxSignature(tx, kp0.Address()) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "transaction has no signatures") + } + assert.Equal(t, false, ok) + + // verify tx with one signature + err = tx.Sign(kp0) + assert.NoError(t, err) + ok, err = verifyTxSignature(tx, kp0.Address()) + assert.NoError(t, err) + assert.Equal(t, true, ok) + + // verify tx with multiple signature + err = tx.Sign(kp1) + assert.NoError(t, err) + ok, err = verifyTxSignature(tx, kp0.Address()) + assert.NoError(t, err) + assert.Equal(t, true, ok) + ok, err = verifyTxSignature(tx, kp1.Address()) + assert.NoError(t, err) + assert.Equal(t, true, ok) + + // verify invalid signer + ok, err = verifyTxSignature(tx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY") + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "transaction not signed by GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY") + } + assert.Equal(t, false, ok) +} + +func TestVerifyChallengeTx(t *testing.T) { + invalidTx := "AAAAACYWIvM98KlTMs0IlQBZ06WkYpZ+gILsQN6ega0++I/sAAAAZAAXeEkAAAABAAAAAAAAAAEAAAAQMkExVjZKNTcwM0c0N1hIWQAAAAEAAAABAAAAACYWIvM98KlTMs0IlQBZ06WkYpZ+gILsQN6ega0++I/sAAAAAQAAAADMSEvcRKXsaUNna++Hy7gWm/CfqTjEA7xoGypfrFGUHAAAAAAAAAACCPHRAAAAAAAAAAABPviP7AAAAEBu6BCKf4WZHPum5+29Nxf6SsJNN8bgjp1+e1uNBaHjRg3rdFZYgUqEqbHxVEs7eze3IeRbjMZxS3zPf/xwJCEI" + + _, err := VerifyChallengeTx(invalidTx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY", network.TestNetworkPassphrase) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "transaction requires timebounds") + } + + kp0 := newKeypair0() + kp1 := newKeypair1() + + // transaction with elapsed timebound + newChallenge, err := BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 1) + assert.NoError(t, err) + time.Sleep(2 * time.Second) + _, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "transaction timebounds has elapsed") + } + + // transaction not signed by client + newChallenge, err = BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 300) + assert.NoError(t, err) + _, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "transaction not signed by "+kp1.Address()) + } + + // valid transaction signed by client + newChallenge, err = BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 300) + assert.NoError(t, err) + newTx, err := TransactionFromXDR(newChallenge) + assert.NoError(t, err) + newTx.Network = network.TestNetworkPassphrase + err = newTx.Sign(kp1) + assert.NoError(t, err) + newChallenge, err = newTx.Base64() + assert.NoError(t, err) + isValid, err := VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + assert.NoError(t, err) + assert.Equal(t, true, isValid, "challenge should be valid") + + // valid transaction with infinite timebound signed by client + newChallenge, err = BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 0) + assert.NoError(t, err) + newTx, err = TransactionFromXDR(newChallenge) + assert.NoError(t, err) + newTx.Network = network.TestNetworkPassphrase + err = newTx.Sign(kp1) + assert.NoError(t, err) + newChallenge, err = newTx.Base64() + assert.NoError(t, err) + isValid, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + assert.NoError(t, err) + assert.Equal(t, true, isValid, "challenge should be valid") + + // invalid operation type + txSource := NewSimpleAccount(kp0.Address(), 0) + opSource := NewSimpleAccount(kp1.Address(), 0) + createAccount := CreateAccount{ + Destination: "GCCOBXW2XQNUSL467IEILE6MMCNRR66SSVL4YQADUNYYNUVREF3FIV2Z", + Amount: "10", + SourceAccount: &opSource, + } + newTx = Transaction{ + SourceAccount: &txSource, + Operations: []Operation{&createAccount}, + Timebounds: NewInfiniteTimeout(), + Network: network.TestNetworkPassphrase, + } + err = newTx.Build() + assert.NoError(t, err) + err = newTx.Sign(kp0, kp1) + assert.NoError(t, err) + newChallenge, err = newTx.Base64() + assert.NoError(t, err) + isValid, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "operation type should be manage_data") + } + assert.Equal(t, false, isValid, "challenge should be invalid") +} From 034ce684f0b1b40ae2171dd42fa780181710df0b Mon Sep 17 00:00:00 2001 From: oliha Date: Fri, 9 Aug 2019 12:33:49 -0400 Subject: [PATCH 2/3] changes from review --- txnbuild/CHANGELOG.md | 3 +- txnbuild/transaction.go | 90 +++++++++---------- txnbuild/transaction_test.go | 168 +++++++++++++++++++++++++++-------- 3 files changed, 173 insertions(+), 88 deletions(-) diff --git a/txnbuild/CHANGELOG.md b/txnbuild/CHANGELOG.md index 638df923e4..b7fdb27617 100644 --- a/txnbuild/CHANGELOG.md +++ b/txnbuild/CHANGELOG.md @@ -5,7 +5,8 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased -* Add `Transaction.BuildChallengeTx` method for building [SEP-10](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md) challenge transaction. +* Add `BuildChallengeTx` function for building [SEP-10](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md) challenge transaction([#1466](https://github.com/stellar/go/issues/1466)). +* Add `VerifyChallengeTx` method for verifying [SEP-10](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md) challenge transaction([#1530](https://github.com/stellar/go/issues/1530)). * Add `TransactionFromXDR` function for building `txnbuild.Transaction` struct from a base64 XDR transaction envelope[#1329](https://github.com/stellar/go/issues/1329). * Fix bug that allowed multiple calls to `Transaction.Build` increment the number of operations in a transaction [#1448](https://github.com/stellar/go/issues/1448). * Add `Transaction.SignWithKeyString` helper method for signing transactions using secret keys as strings.([#1564](https://github.com/stellar/go/issues/1564)) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index f8aa66ecf0..7848fbb3fe 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -211,7 +211,7 @@ func (tx *Transaction) BuildSignEncode(keypairs ...*keypair.Full) (string, error } // BuildChallengeTx is a factory method that creates a valid SEP 10 challenge, for use in web authentication. -// "timebound" is the time duration the transaction should be valid for, O means infinity. +// "timebound" is the time duration the transaction should be valid for. // More details on SEP 10: https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md func BuildChallengeTx(serverSignerSecret, clientAccountID, anchorName, network string, timebound time.Duration) (string, error) { serverKP, err := keypair.Parse(serverSignerSecret) @@ -244,12 +244,12 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, anchorName, network s AccountID: clientAccountID, } - txTimebound := NewInfiniteTimeout() - if timebound > 0 { - currentTime := time.Now().UTC() - maxTime := currentTime.Add(timebound) - txTimebound = NewTimebounds(currentTime.Unix(), maxTime.Unix()) + if timebound == 0 { + return "", errors.New("timebound can not be 0") } + currentTime := time.Now().UTC() + maxTime := currentTime.Add(timebound) + txTimebound := NewTimebounds(currentTime.Unix(), maxTime.Unix()) // Create a SEP 10 compatible response. See // https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md#response @@ -408,9 +408,10 @@ func (tx *Transaction) SignWithKeyString(keys ...string) error { } // VerifyChallengeTx is a factory method that verifies a SEP 10 challenge transaction, -// for use in web authentication. +// for use in web authentication. It can be used by a server to verify that the challenge +// has been signed by the client. // More details on SEP 10: https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0010.md -func VerifyChallengeTx(challengeTx, serverAccountId, network string) (bool, error) { +func VerifyChallengeTx(challengeTx, serverAccountID, network string) (bool, error) { tx, err := TransactionFromXDR(challengeTx) if err != nil { return false, err @@ -418,50 +419,44 @@ func VerifyChallengeTx(challengeTx, serverAccountId, network string) (bool, erro tx.Network = network // verify transaction source - if tx.SourceAccount != nil { - if tx.SourceAccount.GetAccountID() != serverAccountId { - return false, errors.New("transaction source account is not equal to server's account") - } - } else { + if tx.SourceAccount == nil { return false, errors.New("transaction requires a source account") } + if tx.SourceAccount.GetAccountID() != serverAccountID { + return false, errors.New("transaction source account is not equal to server's account") + } // verify timebounds - if tx.Timebounds.wasBuilt { - // if timebound is not infinite - if tx.Timebounds.MinTime > 0 && tx.Timebounds.MaxTime > 0 { - currentTime := time.Now().UTC().Unix() - if currentTime < tx.Timebounds.MinTime || currentTime > tx.Timebounds.MaxTime { - return false, errors.New("transaction timebounds has elapsed") - } - } - } else { - return false, errors.New("transaction requires timebounds") + if tx.Timebounds.MaxTime == TimeoutInfinite { + return false, errors.New("transaction requires non-infinite timebounds") + } + currentTime := time.Now().UTC().Unix() + if currentTime < tx.Timebounds.MinTime || currentTime > tx.Timebounds.MaxTime { + return false, errors.New("transaction is not within range of the specified timebounds") } // verify operation - if len(tx.Operations) == 1 { - op, ok := tx.Operations[0].(*ManageData) - if !ok { - return false, errors.New("operation type should be manage_data") - } - if op.SourceAccount == nil { - return false, errors.New("operation should have a source account") - } - // verify signature from operation source - ok, err = verifyTxSignature(tx, op.SourceAccount.GetAccountID()) - if err != nil { - return ok, err - } - } else { + if len(tx.Operations) != 1 { return false, errors.New("transaction requires a single manage_data operation") } + op, ok := tx.Operations[0].(*ManageData) + if !ok { + return false, errors.New("operation type should be manage_data") + } + if op.SourceAccount == nil { + return false, errors.New("operation should have a source account") + } + // verify signature from operation source + ok, err = verifyTxSignature(tx, op.SourceAccount.GetAccountID()) + if err != nil { + return ok, err + } // verify signature from server signing key - return verifyTxSignature(tx, serverAccountId) + return verifyTxSignature(tx, serverAccountID) } -// verifyTxSignature checks if a transaction has been signed by the provided stellar account. +// verifyTxSignature checks if a transaction has been signed by the provided Stellar account. func verifyTxSignature(tx Transaction, accountID string) (bool, error) { signerFound := false txHash, err := tx.Hash() @@ -475,17 +470,16 @@ func verifyTxSignature(tx Transaction, accountID string) (bool, error) { } // find and verify signatures - if tx.xdrEnvelope != nil && len(tx.xdrEnvelope.Signatures) > 0 { - for _, s := range tx.xdrEnvelope.Signatures { - e := kp.Verify(txHash[:], s.Signature) - if e == nil { - signerFound = true - break - } - } - } else { + if tx.xdrEnvelope == nil { return signerFound, errors.New("transaction has no signatures") } + for _, s := range tx.xdrEnvelope.Signatures { + e := kp.Verify(txHash[:], s.Signature) + if e == nil { + signerFound = true + break + } + } if !signerFound { return signerFound, errors.Errorf("transaction not signed by %s", accountID) diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index ef144a48cf..80ec9206dc 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -740,8 +740,8 @@ func TestManageBuyOfferUpdateOffer(t *testing.T) { func TestBuildChallengeTx(t *testing.T) { kp0 := newKeypair0() - // infinite timebound - txeBase64, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "SDF", network.TestNetworkPassphrase, 0) + // 1 minute timebound + txeBase64, err := BuildChallengeTx(kp0.Seed(), kp0.Address(), "SDF", network.TestNetworkPassphrase, time.Minute) assert.NoError(t, err) var txXDR xdr.TransactionEnvelope err = xdr.SafeUnmarshalBase64(txeBase64, &txXDR) @@ -749,8 +749,8 @@ func TestBuildChallengeTx(t *testing.T) { assert.Equal(t, xdr.SequenceNumber(0), txXDR.Tx.SeqNum, "sequence number should be 0") assert.Equal(t, xdr.Uint32(100), txXDR.Tx.Fee, "Fee should be 100") assert.Equal(t, 1, len(txXDR.Tx.Operations), "number operations should be 1") - assert.Equal(t, xdr.TimePoint(0), xdr.TimePoint(txXDR.Tx.TimeBounds.MinTime), "Min time should be 0") - assert.Equal(t, xdr.TimePoint(0), xdr.TimePoint(txXDR.Tx.TimeBounds.MaxTime), "Max time should be 0") + timeDiff := txXDR.Tx.TimeBounds.MaxTime - txXDR.Tx.TimeBounds.MinTime + assert.Equal(t, int64(60), int64(timeDiff), "time difference should be 300 seconds") op := txXDR.Tx.Operations[0] assert.Equal(t, xdr.OperationTypeManageData, op.Body.Type, "operation type should be manage data") assert.Equal(t, xdr.String64("SDF auth"), op.Body.ManageDataOp.DataName, "DataName should be 'SDF auth'") @@ -766,14 +766,19 @@ func TestBuildChallengeTx(t *testing.T) { assert.Equal(t, xdr.Uint32(100), txXDR1.Tx.Fee, "Fee should be 100") assert.Equal(t, 1, len(txXDR1.Tx.Operations), "number operations should be 1") - timeDiff := txXDR1.Tx.TimeBounds.MaxTime - txXDR1.Tx.TimeBounds.MinTime + timeDiff = txXDR1.Tx.TimeBounds.MaxTime - txXDR1.Tx.TimeBounds.MinTime assert.Equal(t, int64(300), int64(timeDiff), "time difference should be 300 seconds") op1 := txXDR1.Tx.Operations[0] assert.Equal(t, xdr.OperationTypeManageData, op1.Body.Type, "operation type should be manage data") assert.Equal(t, xdr.String64("SDF1 auth"), op1.Body.ManageDataOp.DataName, "DataName should be 'SDF1 auth'") assert.Equal(t, 64, len(*op1.Body.ManageDataOp.DataValue), "DataValue should be 64 bytes") -} + //transaction with infinite timebound + _, err = BuildChallengeTx(kp0.Seed(), kp0.Address(), "sdf", network.TestNetworkPassphrase, 0) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "timebound can not be 0") + } +} func TestHashHex(t *testing.T) { kp0 := newKeypair0() sourceAccount := NewSimpleAccount(kp0.Address(), int64(9605939170639897)) @@ -1157,7 +1162,7 @@ func TestSignWithSecretKey(t *testing.T) { assert.Equal(t, expected, actual, "base64 xdr should match") } -func TestVerifyTxSignature(t *testing.T) { +func TestVerifyTxSignatureUnsignedTx(t *testing.T) { kp0 := newKeypair0() kp1 := newKeypair1() txSource := NewSimpleAccount(kp0.Address(), int64(9605939170639897)) @@ -1179,43 +1184,104 @@ func TestVerifyTxSignature(t *testing.T) { assert.NoError(t, err) ok, err := verifyTxSignature(tx, kp0.Address()) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "transaction has no signatures") + assert.Contains(t, err.Error(), "transaction not signed by GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3") } assert.Equal(t, false, ok) +} +func TestVerifyTxSignatureSingle(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + txSource := NewSimpleAccount(kp0.Address(), int64(9605939170639897)) + opSource := NewSimpleAccount(kp1.Address(), 0) + createAccount := CreateAccount{ + Destination: "GCCOBXW2XQNUSL467IEILE6MMCNRR66SSVL4YQADUNYYNUVREF3FIV2Z", + Amount: "10", + SourceAccount: &opSource, + } + tx := Transaction{ + SourceAccount: &txSource, + Operations: []Operation{&createAccount}, + Timebounds: NewInfiniteTimeout(), + Network: network.TestNetworkPassphrase, + } // verify tx with one signature + err := tx.Build() + assert.NoError(t, err) err = tx.Sign(kp0) assert.NoError(t, err) - ok, err = verifyTxSignature(tx, kp0.Address()) + ok, err := verifyTxSignature(tx, kp0.Address()) assert.NoError(t, err) assert.Equal(t, true, ok) +} +func TestVerifyTxSignatureMultiple(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + txSource := NewSimpleAccount(kp0.Address(), int64(9605939170639897)) + opSource := NewSimpleAccount(kp1.Address(), 0) + createAccount := CreateAccount{ + Destination: "GCCOBXW2XQNUSL467IEILE6MMCNRR66SSVL4YQADUNYYNUVREF3FIV2Z", + Amount: "10", + SourceAccount: &opSource, + } + tx := Transaction{ + SourceAccount: &txSource, + Operations: []Operation{&createAccount}, + Timebounds: NewInfiniteTimeout(), + Network: network.TestNetworkPassphrase, + } // verify tx with multiple signature - err = tx.Sign(kp1) + err := tx.Build() + assert.NoError(t, err) + err = tx.Sign(kp0, kp1) assert.NoError(t, err) - ok, err = verifyTxSignature(tx, kp0.Address()) + ok, err := verifyTxSignature(tx, kp0.Address()) assert.NoError(t, err) assert.Equal(t, true, ok) ok, err = verifyTxSignature(tx, kp1.Address()) assert.NoError(t, err) assert.Equal(t, true, ok) - +} +func TestVerifyTxSignatureInvalid(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + txSource := NewSimpleAccount(kp0.Address(), int64(9605939170639897)) + opSource := NewSimpleAccount(kp1.Address(), 0) + createAccount := CreateAccount{ + Destination: "GCCOBXW2XQNUSL467IEILE6MMCNRR66SSVL4YQADUNYYNUVREF3FIV2Z", + Amount: "10", + SourceAccount: &opSource, + } + tx := Transaction{ + SourceAccount: &txSource, + Operations: []Operation{&createAccount}, + Timebounds: NewInfiniteTimeout(), + Network: network.TestNetworkPassphrase, + } // verify invalid signer - ok, err = verifyTxSignature(tx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY") + err := tx.Build() + assert.NoError(t, err) + err = tx.Sign(kp0, kp1) + assert.NoError(t, err) + ok, err := verifyTxSignature(tx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY") if assert.Error(t, err) { assert.Contains(t, err.Error(), "transaction not signed by GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY") } assert.Equal(t, false, ok) } -func TestVerifyChallengeTx(t *testing.T) { +func TestVerifyChallengeTxInvalid(t *testing.T) { invalidTx := "AAAAACYWIvM98KlTMs0IlQBZ06WkYpZ+gILsQN6ega0++I/sAAAAZAAXeEkAAAABAAAAAAAAAAEAAAAQMkExVjZKNTcwM0c0N1hIWQAAAAEAAAABAAAAACYWIvM98KlTMs0IlQBZ06WkYpZ+gILsQN6ega0++I/sAAAAAQAAAADMSEvcRKXsaUNna++Hy7gWm/CfqTjEA7xoGypfrFGUHAAAAAAAAAACCPHRAAAAAAAAAAABPviP7AAAAEBu6BCKf4WZHPum5+29Nxf6SsJNN8bgjp1+e1uNBaHjRg3rdFZYgUqEqbHxVEs7eze3IeRbjMZxS3zPf/xwJCEI" - _, err := VerifyChallengeTx(invalidTx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY", network.TestNetworkPassphrase) + isValid, err := VerifyChallengeTx(invalidTx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY", network.TestNetworkPassphrase) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "transaction requires timebounds") + assert.Contains(t, err.Error(), "transaction requires non-infinite timebounds") } + assert.Equal(t, false, isValid, "challenge should not be valid") +} +func TestVerifyChallengeTxInvalidTimebound(t *testing.T) { kp0 := newKeypair0() kp1 := newKeypair1() @@ -1223,21 +1289,33 @@ func TestVerifyChallengeTx(t *testing.T) { newChallenge, err := BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 1) assert.NoError(t, err) time.Sleep(2 * time.Second) - _, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + isValid, err := VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "transaction timebounds has elapsed") + assert.Contains(t, err.Error(), "transaction is not within range of the specified timebounds") } + assert.Equal(t, false, isValid, "challenge should not be valid") +} + +func TestVerifyChallengeTxNotSigned(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() // transaction not signed by client - newChallenge, err = BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 300) + newChallenge, err := BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 300) assert.NoError(t, err) - _, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + isValid, err := VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) if assert.Error(t, err) { assert.Contains(t, err.Error(), "transaction not signed by "+kp1.Address()) } + assert.Equal(t, false, isValid, "challenge should not be valid") +} + +func TestVerifyChallengeTxSigned(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() // valid transaction signed by client - newChallenge, err = BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 300) + newChallenge, err := BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 300) assert.NoError(t, err) newTx, err := TransactionFromXDR(newChallenge) assert.NoError(t, err) @@ -1249,20 +1327,11 @@ func TestVerifyChallengeTx(t *testing.T) { isValid, err := VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) assert.NoError(t, err) assert.Equal(t, true, isValid, "challenge should be valid") +} - // valid transaction with infinite timebound signed by client - newChallenge, err = BuildChallengeTx(kp0.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 0) - assert.NoError(t, err) - newTx, err = TransactionFromXDR(newChallenge) - assert.NoError(t, err) - newTx.Network = network.TestNetworkPassphrase - err = newTx.Sign(kp1) - assert.NoError(t, err) - newChallenge, err = newTx.Base64() - assert.NoError(t, err) - isValid, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) - assert.NoError(t, err) - assert.Equal(t, true, isValid, "challenge should be valid") +func TestVerifyChallengeTxInvalidOp(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() // invalid operation type txSource := NewSimpleAccount(kp0.Address(), 0) @@ -1272,21 +1341,42 @@ func TestVerifyChallengeTx(t *testing.T) { Amount: "10", SourceAccount: &opSource, } - newTx = Transaction{ + newTx := Transaction{ SourceAccount: &txSource, Operations: []Operation{&createAccount}, - Timebounds: NewInfiniteTimeout(), + Timebounds: NewTimeout(300), Network: network.TestNetworkPassphrase, } - err = newTx.Build() + err := newTx.Build() assert.NoError(t, err) err = newTx.Sign(kp0, kp1) assert.NoError(t, err) - newChallenge, err = newTx.Base64() + newChallenge, err := newTx.Base64() assert.NoError(t, err) - isValid, err = VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + isValid, err := VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) if assert.Error(t, err) { assert.Contains(t, err.Error(), "operation type should be manage_data") } assert.Equal(t, false, isValid, "challenge should be invalid") } + +func TestVerifyChallengeTxInvalidSource(t *testing.T) { + kp0 := newKeypair0() + kp1 := newKeypair1() + + // transaction with invalid source + newChallenge, err := BuildChallengeTx(kp1.Seed(), kp1.Address(), "sdf", network.TestNetworkPassphrase, 300) + assert.NoError(t, err) + newTx, err := TransactionFromXDR(newChallenge) + assert.NoError(t, err) + newTx.Network = network.TestNetworkPassphrase + err = newTx.Sign(kp1) + assert.NoError(t, err) + newChallenge, err = newTx.Base64() + assert.NoError(t, err) + isValid, err := VerifyChallengeTx(newChallenge, kp0.Address(), network.TestNetworkPassphrase) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "transaction source account is not equal to server's account") + } + assert.Equal(t, false, isValid, "challenge should be valid") +} From 40dedb3ad5ce66ac217fdc18c22d88e9d8455fe8 Mon Sep 17 00:00:00 2001 From: oliha Date: Fri, 9 Aug 2019 14:18:52 -0400 Subject: [PATCH 3/3] fix typo --- txnbuild/transaction.go | 2 +- txnbuild/transaction_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go index 7848fbb3fe..5437645d6f 100644 --- a/txnbuild/transaction.go +++ b/txnbuild/transaction.go @@ -245,7 +245,7 @@ func BuildChallengeTx(serverSignerSecret, clientAccountID, anchorName, network s } if timebound == 0 { - return "", errors.New("timebound can not be 0") + return "", errors.New("timebound cannot be 0") } currentTime := time.Now().UTC() maxTime := currentTime.Add(timebound) diff --git a/txnbuild/transaction_test.go b/txnbuild/transaction_test.go index 80ec9206dc..07b9ce6dd9 100644 --- a/txnbuild/transaction_test.go +++ b/txnbuild/transaction_test.go @@ -776,7 +776,7 @@ func TestBuildChallengeTx(t *testing.T) { //transaction with infinite timebound _, err = BuildChallengeTx(kp0.Seed(), kp0.Address(), "sdf", network.TestNetworkPassphrase, 0) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "timebound can not be 0") + assert.Contains(t, err.Error(), "timebound cannot be 0") } } func TestHashHex(t *testing.T) {