Merge pull request #43 from stellar/mackey

Add configurable payload authentication using a MAC

config_bridge_example.toml

@@ -4,6 +4,7 @@ port = 8001
 horizon = "https://horizon-testnet.stellar.org"
 network_passphrase = "Test SDF Network ; September 2015"
 api_key = ""
+mac_key = ""
 
 [[assets]]
 code="USD"

readme_bridge.md

@@ -45,6 +45,7 @@ The `config_bridge.toml` file must be present in a working directory. Here is an
   * `receive` - URL of the webhook where requests will be sent when a new payment is sent to the receiving account. The bridge server will keep calling the receive callback indefinitely until 200 OK status is returned by it. **WARNING** The bridge server can send multiple requests to this webhook for a single payment! You need to be prepared for it. See: [Security](#security).
   * `error` - URL of the webhook where requests will be sent when there is an error with an incoming payment
 * `log_format` - set to `json` for JSON logs
+* `mac_key` - a stellar secret key used to add MAC headers to a payment notification.
 
 Check [`config_bridge_example.toml`](./config_bridge_example.toml).
 
@@ -393,6 +394,12 @@ name | description
 
 Respond with `200 OK` when processing succeeded. Any other status code will be considered an error.
 
+#### Payload Authentication
+
+When the `mac_key` configuration value is set, the bridge server will attach HTTP headers to each payment notification that allow the receiver to verify that the notification is not forged.  A header named `X_PAYLOAD_MAC` that contains a base64-encoded MAC value will be included. This MAC is derived by calculating the HMAC-SHA256 of the raw request body using the decoded value of the `mac_key` configuration option as the key.
+
+This MAC can be used on the receiving side of the notification to verify that the payment notifications was generated from the bridge server, rather than from some other actor, to increase security.
+
 ## Security
 
 * This server must be set up in an isolated environment (ex. AWS VPC). Please make sure your firewall is properly configured 

src/github.com/stellar/gateway/bridge/config/config.go

@@ -13,6 +13,7 @@ type Config struct {
 	Horizon           string
 	Compliance        string
 	LogFormat         string `mapstructure:"log_format"`
+	MACKey            string `mapstructure:"mac_key"`
 	APIKey            string `mapstructure:"api_key"`
 	NetworkPassphrase string `mapstructure:"network_passphrase"`
 	Assets            []Asset

src/github.com/stellar/gateway/listener/payment_listener.go

@@ -1,27 +1,32 @@
 package listener
 
 import (
+	"crypto/hmac"
+	"crypto/sha256"
 	"encoding/json"
-	"errors"
 	"io/ioutil"
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"time"
 
+	"encoding/base64"
+
 	"github.com/Sirupsen/logrus"
 	"github.com/stellar/gateway/bridge/config"
 	"github.com/stellar/gateway/db"
 	"github.com/stellar/gateway/db/entities"
 	"github.com/stellar/gateway/horizon"
-	"github.com/stellar/gateway/net"
 	"github.com/stellar/gateway/protocols/compliance"
 	"github.com/stellar/gateway/protocols/memo"
+	"github.com/stellar/go/strkey"
+	"github.com/stellar/go/support/errors"
 )
 
 // PaymentListener is listening for a new payments received by ReceivingAccount
 type PaymentListener struct {
-	client        net.HTTPClientInterface
+	client        HTTP
 	config        *config.Config
 	entityManager db.EntityManagerInterface
 	horizon       horizon.HorizonInterface
@@ -30,6 +35,12 @@ type PaymentListener struct {
 	now           func() time.Time
 }
 
+// HTTP represents an http client that a payment listener can use to make HTTP
+// requests.
+type HTTP interface {
+	Do(req *http.Request) (resp *http.Response, err error)
+}
+
 const callbackTimeout = 60 * time.Second
 
 // NewPaymentListener creates a new PaymentListener
@@ -55,7 +66,7 @@ func NewPaymentListener(
 }
 
 // Listen starts listening for new payments
-func (pl PaymentListener) Listen() (err error) {
+func (pl *PaymentListener) Listen() (err error) {
 	accountID := pl.config.Accounts.ReceivingAccountID
 
 	_, err = pl.horizon.LoadAccount(accountID)
@@ -102,7 +113,7 @@ func (pl PaymentListener) Listen() (err error) {
 	return
 }
 
-func (pl PaymentListener) onPayment(payment horizon.PaymentResponse) (err error) {
+func (pl *PaymentListener) onPayment(payment horizon.PaymentResponse) (err error) {
 	pl.log.WithFields(logrus.Fields{"id": payment.ID}).Info("New received payment")
 
 	id, err := strconv.ParseInt(payment.ID, 10, 64)
@@ -162,7 +173,7 @@ func (pl PaymentListener) onPayment(payment horizon.PaymentResponse) (err error)
 
 	// Request extra_memo from compliance server
 	if pl.config.Compliance != "" && payment.Memo.Type == "hash" {
-		resp, err := pl.client.PostForm(
+		resp, err := pl.postForm(
 			pl.config.Compliance+"/receive",
 			url.Values{"memo": {string(payment.Memo.Value)}},
 		)
@@ -211,7 +222,7 @@ func (pl PaymentListener) onPayment(payment horizon.PaymentResponse) (err error)
 		route = payment.Memo.Value
 	}
 
-	resp, err := pl.client.PostForm(
+	resp, err := pl.postForm(
 		pl.config.Callbacks.Receive,
 		url.Values{
 			"id":         {payment.ID},
@@ -254,11 +265,54 @@ func (pl PaymentListener) onPayment(payment horizon.PaymentResponse) (err error)
 	return nil
 }
 
-func (pl PaymentListener) isAssetAllowed(code string, issuer string) bool {
+func (pl *PaymentListener) isAssetAllowed(code string, issuer string) bool {
 	for _, asset := range pl.config.Assets {
 		if asset.Code == code && asset.Issuer == issuer {
 			return true
 		}
 	}
 	return false
 }
+
+func (pl *PaymentListener) postForm(
+	url string,
+	form url.Values,
+) (*http.Response, error) {
+
+	strbody := form.Encode()
+
+	req, err := http.NewRequest("POST", url, strings.NewReader(strbody))
+	if err != nil {
+		return nil, errors.Wrap(err, "configure http request failed")
+	}
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	if pl.config.MACKey != "" {
+		rawMAC, err := pl.getMAC(pl.config.MACKey, []byte(strbody))
+		if err != nil {
+			return nil, errors.Wrap(err, "getMAC failed")
+		}
+
+		encMAC := base64.StdEncoding.EncodeToString(rawMAC)
+		req.Header.Set("X_PAYLOAD_MAC", encMAC)
+	}
+
+	resp, err := pl.client.Do(req)
+	if err != nil {
+		return nil, errors.Wrap(err, "http request errored")
+	}
+
+	return resp, nil
+}
+
+func (pl *PaymentListener) getMAC(key string, raw []byte) ([]byte, error) {
+
+	rawkey, err := strkey.Decode(strkey.VersionByteSeed, pl.config.MACKey)
+	if err != nil {
+		return nil, errors.Wrap(err, "invalid MAC key")
+	}
+
+	macer := hmac.New(sha256.New, rawkey)
+	macer.Write(raw)
+	return macer.Sum(nil), nil
+}

src/github.com/stellar/gateway/listener/payment_listener_test.go

@@ -1,8 +1,14 @@
 package listener
 
 import (
+	"crypto/hmac"
+	"crypto/sha256"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"testing"
 	"time"
@@ -15,7 +21,10 @@ import (
 	"github.com/stellar/gateway/net"
 	"github.com/stellar/gateway/protocols/compliance"
 	"github.com/stellar/gateway/protocols/memo"
+	"github.com/stellar/go/strkey"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 )
 
 func TestPaymentListener(t *testing.T) {
@@ -38,13 +47,14 @@ func TestPaymentListener(t *testing.T) {
 		},
 	}
 
-	paymentListener, _ := NewPaymentListener(
+	paymentListener, err := NewPaymentListener(
 		config,
 		mockEntityManager,
 		mockHorizon,
 		mockRepository,
 		mocks.Now,
 	)
+	require.NoError(t, err)
 
 	paymentListener.client = mockHTTPClient
 
@@ -163,18 +173,10 @@ func TestPaymentListener(t *testing.T) {
 			mockHorizon.On("LoadMemo", &operation).Return(nil).Once()
 
 			mockHTTPClient.On(
-				"PostForm",
-				"http://receive_callback",
-				url.Values{
-					"id":         {"1"},
-					"from":       {"GBIHSMPXC2KJ3NJVHEYTG3KCHYEUQRT45X6AWYWXMAXZOAX4F5LFZYYQ"},
-					"route":      {"testing"},
-					"amount":     {"200"},
-					"asset_code": {"USD"},
-					"memo_type":  {"text"},
-					"memo":       {"testing"},
-					"data":       {""},
-				},
+				"Do",
+				mock.MatchedBy(func(req *http.Request) bool {
+					return req.URL.String() == "http://receive_callback"
+				}),
 			).Return(
 				net.BuildHTTPResponse(503, "ok"),
 				nil,
@@ -203,18 +205,10 @@ func TestPaymentListener(t *testing.T) {
 			mockEntityManager.On("Persist", &dbPayment).Return(nil).Once()
 
 			mockHTTPClient.On(
-				"PostForm",
-				"http://receive_callback",
-				url.Values{
-					"id":         {"1"},
-					"from":       {"GBIHSMPXC2KJ3NJVHEYTG3KCHYEUQRT45X6AWYWXMAXZOAX4F5LFZYYQ"},
-					"route":      {"testing"},
-					"amount":     {"200"},
-					"asset_code": {"USD"},
-					"memo_type":  {"text"},
-					"memo":       {"testing"},
-					"data":       {""},
-				},
+				"Do",
+				mock.MatchedBy(func(req *http.Request) bool {
+					return req.URL.String() == "http://receive_callback"
+				}),
 			).Return(
 				net.BuildHTTPResponse(200, "ok"),
 				nil,
@@ -241,18 +235,10 @@ func TestPaymentListener(t *testing.T) {
 			mockEntityManager.On("Persist", &dbPayment).Return(nil).Once()
 
 			mockHTTPClient.On(
-				"PostForm",
-				"http://receive_callback",
-				url.Values{
-					"id":         {"1"},
-					"from":       {"GBIHSMPXC2KJ3NJVHEYTG3KCHYEUQRT45X6AWYWXMAXZOAX4F5LFZYYQ"},
-					"route":      {""},
-					"amount":     {"200"},
-					"asset_code": {"USD"},
-					"memo_type":  {""},
-					"memo":       {""},
-					"data":       {""},
-				},
+				"Do",
+				mock.MatchedBy(func(req *http.Request) bool {
+					return req.URL.String() == "http://receive_callback"
+				}),
 			).Return(
 				net.BuildHTTPResponse(200, "ok"),
 				nil,
@@ -303,27 +289,20 @@ func TestPaymentListener(t *testing.T) {
 			responseString, _ := json.Marshal(response)
 
 			mockHTTPClient.On(
-				"PostForm",
-				"http://compliance/receive",
-				url.Values{"memo": {"b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"}},
+				"Do",
+				mock.MatchedBy(func(req *http.Request) bool {
+					return req.URL.String() == "http://compliance/receive"
+				}),
 			).Return(
 				net.BuildHTTPResponse(200, string(responseString)),
 				nil,
 			).Once()
 
 			mockHTTPClient.On(
-				"PostForm",
-				"http://receive_callback",
-				url.Values{
-					"id":         {"1"},
-					"from":       {"GBIHSMPXC2KJ3NJVHEYTG3KCHYEUQRT45X6AWYWXMAXZOAX4F5LFZYYQ"},
-					"route":      {"jed*stellar.org"},
-					"amount":     {"200"},
-					"asset_code": {"USD"},
-					"memo_type":  {"hash"},
-					"memo":       {"b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"},
-					"data":       {string(authString)},
-				},
+				"Do",
+				mock.MatchedBy(func(req *http.Request) bool {
+					return req.URL.String() == "http://receive_callback"
+				}),
 			).Return(
 				net.BuildHTTPResponse(200, "ok"),
 				nil,
@@ -338,3 +317,49 @@ func TestPaymentListener(t *testing.T) {
 		})
 	})
 }
+
+func TestPostForm_MACKey(t *testing.T) {
+	validKey := "SABLR5HOI2IUOYB27TR4TO7HWDJIGSRJTT4UUTXXZOFVVPGQKJ5ME43J"
+	rawkey, err := strkey.Decode(strkey.VersionByteSeed, validKey)
+	require.NoError(t, err)
+
+	handler := http.NewServeMux()
+	handler.HandleFunc("/no_mac", func(w http.ResponseWriter, req *http.Request) {
+		assert.Empty(t, req.Header.Get("X_PAYLOAD_MAC"), "unexpected MAC present")
+	})
+	handler.HandleFunc("/mac", func(w http.ResponseWriter, req *http.Request) {
+		body, err := ioutil.ReadAll(req.Body)
+		require.NoError(t, err)
+
+		macer := hmac.New(sha256.New, rawkey)
+		macer.Write(body)
+		rawExpected := macer.Sum(nil)
+		encExpected := base64.StdEncoding.EncodeToString(rawExpected)
+
+		assert.Equal(t, encExpected, req.Header.Get("X_PAYLOAD_MAC"), "MAC is wrong")
+	})
+
+	srv := httptest.NewServer(handler)
+	defer srv.Close()
+
+	cfg := &config.Config{}
+	pl, err := NewPaymentListener(cfg, nil, nil, nil, nil)
+	require.NoError(t, err)
+
+	// no mac if the key is not set
+	_, err = pl.postForm(srv.URL+"/no_mac", url.Values{"foo": []string{"base"}})
+	require.NoError(t, err)
+
+	// generates a valid mac if a key is set.
+	cfg.MACKey = validKey
+	_, err = pl.postForm(srv.URL+"/mac", url.Values{"foo": []string{"base"}})
+	require.NoError(t, err)
+
+	// errors is the key is invalid
+	cfg.MACKey = "broken"
+	_, err = pl.postForm(srv.URL+"/mac", url.Values{"foo": []string{"base"}})
+
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), "invalid MAC key")
+	}
+}

src/github.com/stellar/gateway/mocks/main.go

@@ -58,6 +58,12 @@ func (m *MockHTTPClient) PostForm(url string, data url.Values) (resp *http.Respo
 	return a.Get(0).(*http.Response), a.Error(1)
 }
 
+// Do is a mocking a method
+func (m *MockHTTPClient) Do(req *http.Request) (resp *http.Response, err error) {
+	a := m.Called(req)
+	return a.Get(0).(*http.Response), a.Error(1)
+}
+
 // MockHorizon ...
 type MockHorizon struct {
 	mock.Mock