Merge pull request #3182 from JakeUrban/sep10-v3.0

SEP10 v3.0: Add back homeDomain matching constraint for SEP-10 helpers
stellar · Nov 11, 2020 · 737d545 · 737d545
2 parents a25756d + 886720c
commit 737d545
Show file tree

Hide file tree

Showing 4 changed files with 451 additions and 159 deletions.
diff --git a/exp/services/webauth/internal/serve/token.go b/exp/services/webauth/internal/serve/token.go
@@ -53,22 +53,20 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		homeDomain      string
 	)
 	for _, s := range h.SigningAddresses {
-		for _, domain := range h.HomeDomains {
-			tx, clientAccountID, err = txnbuild.ReadChallengeTx(req.Transaction, s.Address(), h.NetworkPassphrase, domain)
-			if err == nil {
-				signingAddress = s
-				homeDomain = domain
-				break
-			}
-		}
-		if signingAddress != nil {
+		tx, clientAccountID, homeDomain, err = txnbuild.ReadChallengeTx(req.Transaction, s.Address(), h.NetworkPassphrase, h.HomeDomains)
+		if err == nil {
+			signingAddress = s
 			break
 		}
 	}
 	if signingAddress == nil {
 		badRequest.Render(w)
 		return
 	}
+	if homeDomain == "" {
+		badRequest.Render(w)
+		return
+	}
 
 	hash, err := tx.HashHex(h.NetworkPassphrase)
 	if err != nil {
@@ -104,7 +102,7 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if clientAccountExists {
 		requiredThreshold := txnbuild.Threshold(clientAccount.Thresholds.HighThreshold)
 		clientSignerSummary := clientAccount.SignerSummary()
-		signersVerified, err = txnbuild.VerifyChallengeTxThreshold(req.Transaction, signingAddress.Address(), h.NetworkPassphrase, homeDomain, requiredThreshold, clientSignerSummary)
+		signersVerified, err = txnbuild.VerifyChallengeTxThreshold(req.Transaction, signingAddress.Address(), h.NetworkPassphrase, h.HomeDomains, requiredThreshold, clientSignerSummary)
 		if err != nil {
 			l.
 				WithField("signersCount", len(clientSignerSummary)).
@@ -120,7 +118,7 @@ func (h tokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			unauthorized.Render(w)
 			return
 		}
-		signersVerified, err = txnbuild.VerifyChallengeTxSigners(req.Transaction, signingAddress.Address(), h.NetworkPassphrase, homeDomain, clientAccountID)
+		signersVerified, err = txnbuild.VerifyChallengeTxSigners(req.Transaction, signingAddress.Address(), h.NetworkPassphrase, h.HomeDomains, clientAccountID)
 		if err != nil {
 			l.Infof("Failed to verify with account master key as signer.")
 			unauthorized.Render(w)

diff --git a/txnbuild/transaction.go b/txnbuild/transaction.go
@@ -898,103 +898,114 @@ func generateRandomNonce(n int) ([]byte, error) {
 // service's homeDomain passed, malicious web services will not be able to use the
 // challenge transaction POSTed back to the authentication endpoint.
 //
-// The homeDomain field is reserved for future use and not used.
+// The challenge's first Manage Data operation is expected to contain the homeDomain
+// parameter passed to the function.
 //
 // It does not verify that the transaction has been signed by the client or
 // that any signatures other than the servers on the transaction are valid. Use
 // one of the following functions to completely verify the transaction:
 // - VerifyChallengeTxThreshold
 // - VerifyChallengeTxSigners
-func ReadChallengeTx(challengeTx, serverAccountID, network, homeDomain string) (tx *Transaction, clientAccountID string, err error) {
+func ReadChallengeTx(challengeTx, serverAccountID, network string, homeDomains []string) (tx *Transaction, clientAccountID string, matchedHomeDomain string, err error) {
 	parsed, err := TransactionFromXDR(challengeTx)
 	if err != nil {
-		return tx, clientAccountID, errors.Wrap(err, "could not parse challenge")
+		return tx, clientAccountID, matchedHomeDomain, errors.Wrap(err, "could not parse challenge")
 	}
 
 	var isSimple bool
 	tx, isSimple = parsed.Transaction()
 	if !isSimple {
-		return tx, clientAccountID, errors.New("challenge cannot be a fee bump transaction")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("challenge cannot be a fee bump transaction")
 	}
 
 	// Enforce no muxed accounts (at least until we understand their impact)
 	if tx.envelope.SourceAccount().Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 {
 		err = errors.New("invalid source account: only valid Ed25519 accounts are allowed in challenge transactions")
-		return tx, clientAccountID, err
+		return tx, clientAccountID, matchedHomeDomain, err
 	}
 
 	// verify transaction source
 	if tx.SourceAccount().AccountID != serverAccountID {
-		return tx, clientAccountID, errors.New("transaction source account is not equal to server's account")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("transaction source account is not equal to server's account")
 	}
 
 	// verify sequence number
 	if tx.SourceAccount().Sequence != 0 {
-		return tx, clientAccountID, errors.New("transaction sequence number must be 0")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("transaction sequence number must be 0")
 	}
 
 	// verify timebounds
 	if tx.Timebounds().MaxTime == TimeoutInfinite {
-		return tx, clientAccountID, errors.New("transaction requires non-infinite timebounds")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("transaction requires non-infinite timebounds")
 	}
 	currentTime := time.Now().UTC().Unix()
 	if currentTime < tx.Timebounds().MinTime || currentTime > tx.Timebounds().MaxTime {
-		return tx, clientAccountID, errors.Errorf("transaction is not within range of the specified timebounds (currentTime=%d, MinTime=%d, MaxTime=%d)",
+		return tx, clientAccountID, matchedHomeDomain, errors.Errorf("transaction is not within range of the specified timebounds (currentTime=%d, MinTime=%d, MaxTime=%d)",
 			currentTime, tx.Timebounds().MinTime, tx.Timebounds().MaxTime)
 	}
 
 	// verify operation
 	operations := tx.Operations()
 	if len(operations) < 1 {
-		return tx, clientAccountID, errors.New("transaction requires at least one manage_data operation")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("transaction requires at least one manage_data operation")
 	}
 	op, ok := operations[0].(*ManageData)
 	if !ok {
-		return tx, clientAccountID, errors.New("operation type should be manage_data")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("operation type should be manage_data")
 	}
 	if op.SourceAccount == nil {
-		return tx, clientAccountID, errors.New("operation should have a source account")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("operation should have a source account")
+	}
+	for _, homeDomain := range homeDomains {
+		if op.Name == homeDomain+" auth" {
+			matchedHomeDomain = homeDomain
+			break
+		}
 	}
+	if matchedHomeDomain == "" {
+		return tx, clientAccountID, matchedHomeDomain, errors.Errorf("operation key does not match any homeDomains passed (key=%q, homeDomains=%v)", op.Name, homeDomains)
+	}
+
 	clientAccountID = op.SourceAccount.GetAccountID()
 	rawOperations := tx.envelope.Operations()
 	if len(rawOperations) > 0 && rawOperations[0].SourceAccount.Type == xdr.CryptoKeyTypeKeyTypeMuxedEd25519 {
 		err = errors.New("invalid operation source account: only valid Ed25519 accounts are allowed in challenge transactions")
-		return tx, clientAccountID, err
+		return tx, clientAccountID, matchedHomeDomain, err
 	}
 
 	// verify manage data value
 	nonceB64 := string(op.Value)
 	if len(nonceB64) != 64 {
-		return tx, clientAccountID, errors.New("random nonce encoded as base64 should be 64 bytes long")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("random nonce encoded as base64 should be 64 bytes long")
 	}
 	nonceBytes, err := base64.StdEncoding.DecodeString(nonceB64)
 	if err != nil {
-		return tx, clientAccountID, errors.Wrap(err, "failed to decode random nonce provided in manage_data operation")
+		return tx, clientAccountID, matchedHomeDomain, errors.Wrap(err, "failed to decode random nonce provided in manage_data operation")
 	}
 	if len(nonceBytes) != 48 {
-		return tx, clientAccountID, errors.New("random nonce before encoding as base64 should be 48 bytes long")
+		return tx, clientAccountID, matchedHomeDomain, errors.New("random nonce before encoding as base64 should be 48 bytes long")
 	}
 
 	// verify subsequent operations are manage data ops with source account set to server account
 	for _, op := range operations[1:] {
 		op, ok := op.(*ManageData)
 		if !ok {
-			return tx, clientAccountID, errors.New("operation type should be manage_data")
+			return tx, clientAccountID, matchedHomeDomain, errors.New("operation type should be manage_data")
 		}
 		if op.SourceAccount == nil {
-			return tx, clientAccountID, errors.New("operation should have a source account")
+			return tx, clientAccountID, matchedHomeDomain, errors.New("operation should have a source account")
 		}
 		if op.SourceAccount.GetAccountID() != serverAccountID {
-			return tx, clientAccountID, errors.New("subsequent operations are unrecognized")
+			return tx, clientAccountID, matchedHomeDomain, errors.New("subsequent operations are unrecognized")
 		}
 	}
 
 	err = verifyTxSignature(tx, network, serverAccountID)
 	if err != nil {
-		return tx, clientAccountID, err
+		return tx, clientAccountID, matchedHomeDomain, err
 	}
 
-	return tx, clientAccountID, nil
+	return tx, clientAccountID, matchedHomeDomain, nil
 }
 
 // VerifyChallengeTxThreshold verifies that for a SEP 10 challenge transaction
@@ -1015,13 +1026,13 @@ func ReadChallengeTx(challengeTx, serverAccountID, network, homeDomain string) (
 //  - One or more signatures in the transaction are not identifiable as the
 //    server account or one of the signers provided in the arguments.
 //  - The signatures are all valid but do not meet the threshold.
-func VerifyChallengeTxThreshold(challengeTx, serverAccountID, network, homeDomain string, threshold Threshold, signerSummary SignerSummary) (signersFound []string, err error) {
+func VerifyChallengeTxThreshold(challengeTx, serverAccountID, network string, homeDomains []string, threshold Threshold, signerSummary SignerSummary) (signersFound []string, err error) {
 	signers := make([]string, 0, len(signerSummary))
 	for s := range signerSummary {
 		signers = append(signers, s)
 	}
 
-	signersFound, err = VerifyChallengeTxSigners(challengeTx, serverAccountID, network, homeDomain, signers...)
+	signersFound, err = VerifyChallengeTxSigners(challengeTx, serverAccountID, network, homeDomains, signers...)
 	if err != nil {
 		return nil, err
 	}
@@ -1056,9 +1067,9 @@ func VerifyChallengeTxThreshold(challengeTx, serverAccountID, network, homeDomai
 //  - 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, homeDomain string, signers ...string) ([]string, error) {
+func VerifyChallengeTxSigners(challengeTx, serverAccountID, network string, homeDomains []string, signers ...string) ([]string, error) {
 	// Read the transaction which validates its structure.
-	tx, _, err := ReadChallengeTx(challengeTx, serverAccountID, network, homeDomain)
+	tx, _, _, err := ReadChallengeTx(challengeTx, serverAccountID, network, homeDomains)
 	if err != nil {
 		return nil, err
 	}

diff --git a/txnbuild/transaction_challenge_example_test.go b/txnbuild/transaction_challenge_example_test.go
@@ -52,7 +52,7 @@ func ExampleVerifyChallengeTxThreshold() {
 	// Client reads and signs challenge transaction
 	var signedChallengeTx string
 	{
-		tx, txClientAccountID, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "test")
+		tx, txClientAccountID, _, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, []string{"test"})
 		if err != nil {
 			fmt.Println("Error:", err)
 			return
@@ -75,7 +75,7 @@ func ExampleVerifyChallengeTxThreshold() {
 
 	// Server verifies signed challenge transaction
 	{
-		_, txClientAccountID, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "test")
+		_, txClientAccountID, _, err := txnbuild.ReadChallengeTx(challengeTx, serverAccount.Address(), network.TestNetworkPassphrase, []string{"test"})
 		if err != nil {
 			fmt.Println("Error:", err)
 			return
@@ -102,7 +102,7 @@ func ExampleVerifyChallengeTxThreshold() {
 			threshold := txnbuild.Threshold(horizonClientAccount.Thresholds.MedThreshold)
 
 			// Server verifies threshold is met
-			signers, err := txnbuild.VerifyChallengeTxThreshold(signedChallengeTx, serverAccount.Address(), network.TestNetworkPassphrase, "test", threshold, signerSummary)
+			signers, err := txnbuild.VerifyChallengeTxThreshold(signedChallengeTx, serverAccount.Address(), network.TestNetworkPassphrase, []string{"test"}, threshold, signerSummary)
 			if err != nil {
 				fmt.Println("Error:", err)
 				return
@@ -114,7 +114,7 @@ func ExampleVerifyChallengeTxThreshold() {
 			}
 		} else {
 			// Server verifies that master key has signed challenge transaction
-			signersFound, err := txnbuild.VerifyChallengeTxSigners(signedChallengeTx, serverAccount.Address(), network.TestNetworkPassphrase, txClientAccountID)
+			signersFound, err := txnbuild.VerifyChallengeTxSigners(signedChallengeTx, serverAccount.Address(), network.TestNetworkPassphrase, []string{"test"}, txClientAccountID)
 			if err != nil {
 				fmt.Println("Error:", err)
 				return