Skip to content

Commit

Permalink
exp/services/recoverysigner: allow recoverysigner to sign transaction…
Browse files Browse the repository at this point in the history
…s with operations containing a whitelisted source account (stellar#3227)

### What

Allow recoverysigner to sign transactions with operations containing another source account if it's listed in the config variables.

### Why

The validation below prevents recoverysigner from signing CAP-33 `BeginSponsoringFutureReserves` operations
https://github.com/stellar/go/blob/c2c77fb6066b7a06a0c31835039926ae7d9d3970/exp/services/recoverysigner/internal/serve/account_sign.go#L132-L138

For a `BeginSponsoringFutureReserves` operation we shouldn't be validating its account address but the sponsored id.
  • Loading branch information
marcelosalloum authored Nov 18, 2020
1 parent b361462 commit 15de5ff
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 31 deletions.
4 changes: 2 additions & 2 deletions exp/services/recoverysigner/README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Recovery Signer

This is an incomplete and work-in-progress implementation of the [SEP-30]
Recovery Signer protocol v0.4.0.
Recovery Signer protocol v0.7.0.

A Recovery Signer is a server that can help a user regain control of a Stellar
account if they have lost their secret key. A user registers their account with
Expand Down Expand Up @@ -74,5 +74,5 @@ Flags:
Use "recoverysigner db [command] --help" for more information about a command.
```

[SEP-30]: https://github.com/stellar/stellar-protocol/blob/600c326b210d71ee031d7f3a40ca88191b4cdf9c/ecosystem/sep-0030.md
[SEP-30]: https://github.com/stellar/stellar-protocol/blob/3e05bb668f94793545588106af74699b8d6b02d6/ecosystem/sep-0030.md
[README-Firebase.md]: README-Firebase.md
7 changes: 7 additions & 0 deletions exp/services/recoverysigner/cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@ func (c *ServeCommand) Command() *cobra.Command {
FlagDefault: "recoverysigner",
Required: false,
},
{
Name: "allowed-source-accounts",
Usage: "Stellar account(s) allowed as source accounts in transactions signed for all users in addition to the registered account comma separated (important: these accounts must never be registered accounts and must never have the signer configured that is a signing key used by this server)",
OptType: types.String,
ConfigKey: &opts.AllowedSourceAccounts,
Required: false,
},
}
cmd := &cobra.Command{
Use: "serve",
Expand Down
26 changes: 19 additions & 7 deletions exp/services/recoverysigner/internal/serve/account_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ import (
)

type accountSignHandler struct {
Logger *supportlog.Entry
SigningKeys []*keypair.Full
NetworkPassphrase string
AccountStore account.Store
Logger *supportlog.Entry
SigningKeys []*keypair.Full
NetworkPassphrase string
AccountStore account.Store
AllowedSourceAccounts []*keypair.FromAddress
}

type accountSignRequest struct {
Expand Down Expand Up @@ -141,10 +142,21 @@ func (h accountSignHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if opSourceAccount == nil {
continue
}

if op.GetSourceAccount().GetAccountID() != req.Address.Address() {
l.Info("Operation's source account is not the account.")
badRequest.Render(w)
return
var opHasAllowedAccount bool
for _, sa := range h.AllowedSourceAccounts {
if sa.Address() == op.GetSourceAccount().GetAccountID() {
opHasAllowedAccount = true
break
}
}

if !opHasAllowedAccount {
l.Info("Operation's source account is not the account in the request and not any account that is configured to be allowed.")
badRequest.Render(w)
return
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package serve

import (
"context"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -985,6 +986,159 @@ func TestAccountSign_signingAddressEmailOwnerAuthenticated(t *testing.T) {
assert.JSONEq(t, wantBody, string(body))
}

// Test that when the source account of the operation is listed in the allowed
// source accounts a successful response is returned.
func TestAccountSign_signingAddressEmailOwnerAuthenticatedOpSourceAccountIsAllowedSourceAccount(t *testing.T) {
s := &account.DBStore{DB: dbtest.Open(t).Open()}
s.Add(account.Account{
Address: "GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4",
Identities: []account.Identity{
{
Role: "sender",
AuthMethods: []account.AuthMethod{
{Type: account.AuthMethodTypeEmail, Value: "[email protected]"},
},
},
},
})
h := accountSignHandler{
Logger: supportlog.DefaultLogger,
AccountStore: s,
SigningKeys: []*keypair.Full{
keypair.MustParseFull("SBIB72S6JMTGJRC6LMKLC5XMHZ2IOHZSZH4SASTN47LECEEJ7QEB6EYK"), // GBOG4KF66M4AFRBUHOTJQJRO7BGGFCSGIICTI5BHXHKXCWV2C67QRN5H
keypair.MustParseFull("SBJGZKZ7LU2FQNEFBUOBW4LHCA5BOZCABIJTR7BQIFWQ3P763ZW7MYDD"), // GAPE22DOMALCH42VOR4S3HN6KIZZ643G7D3GNTYF4YOWWXP6UVRAF5JS
},
NetworkPassphrase: network.TestNetworkPassphrase,
AllowedSourceAccounts: []*keypair.FromAddress{
keypair.MustParseAddress("GDR3RJVOHYR5A4RSLZ7D3GOSTPBGD2FY7KJD7ZB7363ROOQHWYDVVULS"),
},
}

tx, err := txnbuild.NewTransaction(
txnbuild.TransactionParams{
SourceAccount: &txnbuild.SimpleAccount{AccountID: "GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4"},
IncrementSequenceNum: true,
Operations: []txnbuild.Operation{
&txnbuild.BeginSponsoringFutureReserves{
SponsoredID: "GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4",
SourceAccount: &txnbuild.SimpleAccount{AccountID: "GDR3RJVOHYR5A4RSLZ7D3GOSTPBGD2FY7KJD7ZB7363ROOQHWYDVVULS"},
},
&txnbuild.SetOptions{
Signer: &txnbuild.Signer{
Address: "GD7CGJSJ5OBOU5KOP2UQDH3MPY75UTEY27HVV5XPSL2X6DJ2VGTOSXEU",
Weight: 20,
},
},
&txnbuild.EndSponsoringFutureReserves{},
},
BaseFee: txnbuild.MinBaseFee,
Timebounds: txnbuild.NewTimebounds(0, 1),
},
)
require.NoError(t, err)
txEnc, err := tx.Base64()
require.NoError(t, err)
t.Log("Tx:", txEnc)

ctx := context.Background()
ctx = auth.NewContext(ctx, auth.Auth{Email: "[email protected]"})
req := fmt.Sprintf(`{"transaction": "%s"}`, txEnc)
r := httptest.NewRequest("POST", "/GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4/sign/GBOG4KF66M4AFRBUHOTJQJRO7BGGFCSGIICTI5BHXHKXCWV2C67QRN5H", strings.NewReader(req))
r = r.WithContext(ctx)

w := httptest.NewRecorder()
m := chi.NewMux()
m.Post("/{address}/sign/{signing-address}", h.ServeHTTP)
m.ServeHTTP(w, r)
resp := w.Result()

require.Equal(t, http.StatusOK, resp.StatusCode)
assert.Equal(t, "application/json; charset=utf-8", resp.Header.Get("Content-Type"))

body, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)

wantBody := `{
"signature": "Tpl/yZoKkahakaX4fSrdIeBLL2oi4uKegs5bLXFj5fG6Rcfe2D4EeSHcjJmmO2ZscuY8pX8+YPo70AvCtfw9Ag==",
"network_passphrase": "Test SDF Network ; September 2015"
}`
assert.JSONEq(t, wantBody, string(body))
}

// Test that when the source account of the operation is not the account sign
// the request is calling sign on a bad request response is returned.
func TestAccountSign_signingAddressEmailOwnerAuthenticatedOpSourceAccountInvalid(t *testing.T) {
s := &account.DBStore{DB: dbtest.Open(t).Open()}
s.Add(account.Account{
Address: "GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4",
Identities: []account.Identity{
{
Role: "sender",
AuthMethods: []account.AuthMethod{
{Type: account.AuthMethodTypeEmail, Value: "[email protected]"},
},
},
},
})
h := accountSignHandler{
Logger: supportlog.DefaultLogger,
AccountStore: s,
SigningKeys: []*keypair.Full{
keypair.MustParseFull("SBIB72S6JMTGJRC6LMKLC5XMHZ2IOHZSZH4SASTN47LECEEJ7QEB6EYK"), // GBOG4KF66M4AFRBUHOTJQJRO7BGGFCSGIICTI5BHXHKXCWV2C67QRN5H
keypair.MustParseFull("SBJGZKZ7LU2FQNEFBUOBW4LHCA5BOZCABIJTR7BQIFWQ3P763ZW7MYDD"), // GAPE22DOMALCH42VOR4S3HN6KIZZ643G7D3GNTYF4YOWWXP6UVRAF5JS
},
NetworkPassphrase: network.TestNetworkPassphrase,
AllowedSourceAccounts: nil,
}

tx, err := txnbuild.NewTransaction(
txnbuild.TransactionParams{
SourceAccount: &txnbuild.SimpleAccount{AccountID: "GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4"},
IncrementSequenceNum: true,
Operations: []txnbuild.Operation{
&txnbuild.BeginSponsoringFutureReserves{
SponsoredID: "GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4",
SourceAccount: &txnbuild.SimpleAccount{AccountID: "GDR3RJVOHYR5A4RSLZ7D3GOSTPBGD2FY7KJD7ZB7363ROOQHWYDVVULS"},
},
&txnbuild.SetOptions{
Signer: &txnbuild.Signer{
Address: "GD7CGJSJ5OBOU5KOP2UQDH3MPY75UTEY27HVV5XPSL2X6DJ2VGTOSXEU",
Weight: 20,
},
},
&txnbuild.EndSponsoringFutureReserves{},
},
BaseFee: txnbuild.MinBaseFee,
Timebounds: txnbuild.NewTimebounds(0, 1),
},
)
require.NoError(t, err)
txEnc, err := tx.Base64()
require.NoError(t, err)
t.Log("Tx:", txEnc)

ctx := context.Background()
ctx = auth.NewContext(ctx, auth.Auth{Email: "[email protected]"})
req := fmt.Sprintf(`{"transaction": "%s"}`, txEnc)
r := httptest.NewRequest("POST", "/GA6HNE7O2N2IXIOBZNZ4IPTS2P6DSAJJF5GD5PDLH5GYOZ6WMPSKCXD4/sign/GBOG4KF66M4AFRBUHOTJQJRO7BGGFCSGIICTI5BHXHKXCWV2C67QRN5H", strings.NewReader(req))
r = r.WithContext(ctx)

w := httptest.NewRecorder()
m := chi.NewMux()
m.Post("/{address}/sign/{signing-address}", h.ServeHTTP)
m.ServeHTTP(w, r)
resp := w.Result()

require.Equal(t, http.StatusBadRequest, resp.StatusCode)
assert.Equal(t, "application/json; charset=utf-8", resp.Header.Get("Content-Type"))

body, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)

wantBody := `{"error": "The request was invalid in some way."}`
assert.JSONEq(t, wantBody, string(body))
}

// Test that when authenticated with a email signing is possible.
func TestAccountSign_signingAddressEmailOtherAuthenticated(t *testing.T) {
s := &account.DBStore{DB: dbtest.Open(t).Open()}
Expand Down
58 changes: 36 additions & 22 deletions exp/services/recoverysigner/internal/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ type Options struct {

AdminPort int
MetricsNamespace string

AllowedSourceAccounts string
}

func Serve(opts Options) {
Expand Down Expand Up @@ -63,15 +65,16 @@ func Serve(opts Options) {
}

type handlerDeps struct {
Logger *supportlog.Entry
NetworkPassphrase string
SigningKeys []*keypair.Full
SigningAddresses []*keypair.FromAddress
AccountStore account.Store
SEP10JWKS jose.JSONWebKeySet
SEP10JWTIssuer string
FirebaseAuthClient *firebaseauth.Client
MetricsRegistry *prometheus.Registry
Logger *supportlog.Entry
NetworkPassphrase string
SigningKeys []*keypair.Full
SigningAddresses []*keypair.FromAddress
AccountStore account.Store
SEP10JWKS jose.JSONWebKeySet
SEP10JWTIssuer string
FirebaseAuthClient *firebaseauth.Client
MetricsRegistry *prometheus.Registry
AllowedSourceAccounts []*keypair.FromAddress
}

func getHandlerDeps(opts Options) (handlerDeps, error) {
Expand Down Expand Up @@ -141,16 +144,26 @@ func getHandlerDeps(opts Options) (handlerDeps, error) {
opts.Logger.Warn("Error registering metric for accounts count: ", err)
}

allowedSourceAccounts := []*keypair.FromAddress{}
for _, addressStr := range strings.Split(opts.AllowedSourceAccounts, ",") {
accountAddress, err := keypair.ParseAddress(addressStr)
if err != nil {
return handlerDeps{}, errors.Wrap(err, "parsing allowed source accounts")
}
allowedSourceAccounts = append(allowedSourceAccounts, accountAddress)
}

deps := handlerDeps{
Logger: opts.Logger,
NetworkPassphrase: opts.NetworkPassphrase,
SigningKeys: signingKeys,
SigningAddresses: signingAddresses,
AccountStore: accountStore,
SEP10JWKS: sep10JWKS,
SEP10JWTIssuer: opts.SEP10JWTIssuer,
FirebaseAuthClient: firebaseAuthClient,
MetricsRegistry: metricsRegistry,
Logger: opts.Logger,
NetworkPassphrase: opts.NetworkPassphrase,
SigningKeys: signingKeys,
SigningAddresses: signingAddresses,
AccountStore: accountStore,
SEP10JWKS: sep10JWKS,
SEP10JWTIssuer: opts.SEP10JWTIssuer,
FirebaseAuthClient: firebaseAuthClient,
MetricsRegistry: metricsRegistry,
AllowedSourceAccounts: allowedSourceAccounts,
}

return deps, nil
Expand Down Expand Up @@ -193,10 +206,11 @@ func handler(deps handlerDeps) http.Handler {
AccountStore: deps.AccountStore,
}.ServeHTTP)
signHandler := accountSignHandler{
Logger: deps.Logger,
SigningKeys: deps.SigningKeys,
NetworkPassphrase: deps.NetworkPassphrase,
AccountStore: deps.AccountStore,
Logger: deps.Logger,
SigningKeys: deps.SigningKeys,
NetworkPassphrase: deps.NetworkPassphrase,
AccountStore: deps.AccountStore,
AllowedSourceAccounts: deps.AllowedSourceAccounts,
}
mux.Post("/sign", signHandler.ServeHTTP)
mux.Post("/sign/{signing-address}", signHandler.ServeHTTP)
Expand Down

0 comments on commit 15de5ff

Please sign in to comment.