Move signature verification to middleware

Mak Muftic · Matija Petrunic · commit e16d176dbf48 · 2021-01-04T11:23:23.000+01:00
diff --git a/internal/constants/signaturestats.go b/internal/constants/signaturestats.go
@@ -1,3 +1,3 @@
-package controllers
+package constants
 
 const StatsSignedData = "loadbalancer-request"
diff --git a/internal/controllers/api.go b/internal/controllers/api.go
@@ -9,19 +9,16 @@ type ApiController struct {
 	whitelistEnabled bool
 	repositories     repositories.Repos
 	actions          actions.Actions
-	privateKey       string
 }
 
 func NewApiController(
 	whitelistEnabled bool,
 	repositories repositories.Repos,
 	actions actions.Actions,
-	privateKey string,
 ) *ApiController {
 	return &ApiController{
 		whitelistEnabled: whitelistEnabled,
 		repositories:     repositories,
 		actions:          actions,
-		privateKey:       privateKey,
 	}
 }
diff --git a/internal/controllers/metrics_test.go b/internal/controllers/metrics_test.go
@@ -245,7 +245,7 @@ func TestApiController_SaveMetricsHandler(t *testing.T) {
 				MetricsRepo:  &metricsRepoMock,
 				RecordRepo:   &recordRepoMock,
 				DowntimeRepo: &downtimeRepoMock,
-			}, nil, "")
+			}, nil)
 
 			handler := http.HandlerFunc(apiController.SaveMetricsHandler)
 
diff --git a/internal/controllers/ping_test.go b/internal/controllers/ping_test.go
@@ -106,7 +106,7 @@ func TestApiController_PingHandler(t *testing.T) {
 				MetricsRepo:  &metricsRepoMock,
 				RecordRepo:   &recordRepoMock,
 				DowntimeRepo: &downtimeRepoMock,
-			}, nil, "")
+			}, nil)
 			handler := http.HandlerFunc(apiController.PingHandler)
 
 			// create test request and populate context
diff --git a/internal/controllers/register_test.go b/internal/controllers/register_test.go
@@ -146,7 +146,7 @@ func TestApiController_RegisterHandler(t *testing.T) {
 				MetricsRepo:  &metricsRepoMock,
 				RecordRepo:   &recordRepoMock,
 				DowntimeRepo: &downtimeRepoMock,
-			}, nil, "")
+			}, nil)
 
 			handler := http.HandlerFunc(apiController.RegisterHandler)
 
diff --git a/internal/controllers/rpc_test.go b/internal/controllers/rpc_test.go
@@ -56,7 +56,7 @@ func TestApiController_RPCHandler(t *testing.T) {
 	apiController := NewApiController(false, repositories.Repos{
 		NodeRepo:   &nodeRepoMock,
 		RecordRepo: &recordRepoMock,
-	}, actionsMockObject, "")
+	}, actionsMockObject)
 
 	handler := http.HandlerFunc(apiController.RPCHandler)
 
@@ -165,7 +165,7 @@ func TestApiController_BatchRPCHandler(t *testing.T) {
 	apiController := NewApiController(false, repositories.Repos{
 		NodeRepo:   &nodeRepoMock,
 		RecordRepo: &recordRepoMock,
-	}, actionsMockObject, "")
+	}, actionsMockObject)
 
 	handler := http.HandlerFunc(apiController.RPCHandler)
 
@@ -244,7 +244,7 @@ func TestApiController_RPCHandler_InvalidBody(t *testing.T) {
 	actionsMockObject := new(actionMocks.Actions)
 	actionsMockObject.On("PenalizeNode", mock.Anything, mock.Anything).Return()
 
-	apiController := NewApiController(false, repositories.Repos{}, actionsMockObject, "")
+	apiController := NewApiController(false, repositories.Repos{}, actionsMockObject)
 
 	handler := http.HandlerFunc(apiController.RPCHandler)
 
diff --git a/internal/controllers/stats.go b/internal/controllers/stats.go
@@ -13,8 +13,6 @@ import (
 	"github.com/NodeFactoryIo/vedran/internal/models"
 	"github.com/NodeFactoryIo/vedran/internal/stats"
 
-	"github.com/NodeFactoryIo/go-substrate-rpc-client/signature"
-	"github.com/ethereum/go-ethereum/common/hexutil"
 	muxhelpper "github.com/gorilla/mux"
 	log "github.com/sirupsen/logrus"
 )
@@ -50,19 +48,8 @@ type LoadbalancerStatsRequest struct {
 	TotalReward string `json:"total_reward"`
 }
 
-// handler for `POST /api/v1/stats`
+// handler for `POST /api/v1/stats` - signature verification in middleware
 func (c *ApiController) StatisticsHandlerAllStatsForLoadbalancer(w http.ResponseWriter, r *http.Request) {
-	verified, httpStatusCode, err := verifySignatureInHeader(r, c.privateKey)
-	if err != nil {
-		http.Error(w, http.StatusText(httpStatusCode), httpStatusCode)
-		return
-	}
-	if !verified {
-		log.Errorf("Invalid request signature")
-		http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
-		return
-	}
-
 	totalRewardAsFloat, err := getTotalRewardFromRequest(r)
 	if err != nil {
 		log.Error(err)
@@ -110,25 +97,6 @@ func (c *ApiController) StatisticsHandlerAllStatsForLoadbalancer(w http.Response
 	})
 }
 
-func verifySignatureInHeader(r *http.Request, privateKey string) (bool, int, error) {
-	sig := r.Header.Get("X-Signature")
-	if sig == "" {
-		log.Error("Missing signature header")
-		return false, http.StatusBadRequest, nil
-	}
-	sigInBytes, err := hexutil.Decode(sig)
-	if err != nil {
-		log.Errorf("Unable to decode signature, because of: %v", err)
-		return false, http.StatusBadRequest, err
-	}
-	verified, err := signature.Verify([]byte(StatsSignedData), sigInBytes, privateKey)
-	if err != nil {
-		log.Errorf("Failed to verify signature, because %v", err)
-		return false, http.StatusInternalServerError, err
-	}
-	return verified, 0, err
-}
-
 func getTotalRewardFromRequest(r *http.Request) (float64, error) {
 	var statsRequest LoadbalancerStatsRequest
 	reqBody, err := ioutil.ReadAll(r.Body)
diff --git a/internal/controllers/stats_test.go b/internal/controllers/stats_test.go
@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"github.com/NodeFactoryIo/go-substrate-rpc-client/signature"
 	"github.com/NodeFactoryIo/vedran/internal/configuration"
+	"github.com/NodeFactoryIo/vedran/internal/constants"
+	"github.com/NodeFactoryIo/vedran/internal/middleware"
 	"github.com/NodeFactoryIo/vedran/internal/models"
 	"github.com/NodeFactoryIo/vedran/internal/repositories"
 	mocks "github.com/NodeFactoryIo/vedran/mocks/repositories"
@@ -135,7 +137,7 @@ func TestApiController_StatisticsHandlerAllStats(t *testing.T) {
 				RecordRepo:   &recordRepoMock,
 				DowntimeRepo: &downtimeRepoMock,
 				PayoutRepo:   &payoutRepoMock,
-			}, nil, "")
+			}, nil)
 			handler := http.HandlerFunc(apiController.StatisticsHandlerAllStats)
 			req, _ := http.NewRequest("GET", "/api/v1/stats", bytes.NewReader(nil))
 			rr := httptest.NewRecorder()
@@ -225,7 +227,7 @@ func TestApiController_StatisticsHandlerAllStatsForLoadbalancer(t *testing.T) {
 			requestContent: `{"total_reward":"1000000"}`,
 			//
 			secret:        "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
-			signatureData: StatsSignedData,
+			signatureData: constants.StatsSignedData,
 		},
 		{
 			name:          "missing signature, 400 bad request",
@@ -260,7 +262,7 @@ func TestApiController_StatisticsHandlerAllStatsForLoadbalancer(t *testing.T) {
 			nodeNumberOfPings:    float64(8640),
 			//
 			secret:        "",
-			signatureData: StatsSignedData,
+			signatureData: constants.StatsSignedData,
 		},
 		{
 			name:          "invalid signature, 400 bad request",
@@ -303,7 +305,7 @@ func TestApiController_StatisticsHandlerAllStatsForLoadbalancer(t *testing.T) {
 			payoutRepoFindLatestPayoutError: errors.New("db-error"),
 			secret:                          "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
 			requestContent:                  `{"total_reward":"1000000"}`,
-			signatureData:                   StatsSignedData,
+			signatureData:                   constants.StatsSignedData,
 		},
 	}
 	configuration.Config.Fee = 0.1
@@ -357,8 +359,12 @@ func TestApiController_StatisticsHandlerAllStatsForLoadbalancer(t *testing.T) {
 				DowntimeRepo: &downtimeRepoMock,
 				PayoutRepo:   &payoutRepoMock,
 				FeeRepo:      &feeRepoMock,
-			}, nil, test.secret)
-			handler := http.HandlerFunc(apiController.StatisticsHandlerAllStatsForLoadbalancer)
+			}, nil)
+
+			handler := middleware.VerifySignatureMiddleware(
+				http.HandlerFunc(apiController.StatisticsHandlerAllStatsForLoadbalancer),
+				test.secret,
+			)
 
 			req, _ := http.NewRequest("POST", "/api/v1/stats", bytes.NewReader([]byte(test.requestContent)))
 
@@ -486,7 +492,7 @@ func TestApiController_StatisticsHandlerStatsForNode(t *testing.T) {
 				RecordRepo:   &recordRepoMock,
 				DowntimeRepo: &downtimeRepoMock,
 				PayoutRepo:   &payoutRepoMock,
-			}, nil, "")
+			}, nil)
 			type ContextKey string
 			req, _ := http.NewRequest("GET", "/api/v1/stats/node/1", bytes.NewReader(nil))
 			req = req.WithContext(context.WithValue(req.Context(), ContextKey(test.contextKey), "1"))
diff --git a/internal/controllers/ws_test.go b/internal/controllers/ws_test.go
@@ -86,7 +86,7 @@ func TestApiController_WSHandler(t *testing.T) {
 			apiController := NewApiController(false, repositories.Repos{
 				NodeRepo:   &nodeRepoMock,
 				RecordRepo: &recordRepoMock,
-			}, actionsMockObject, "")
+			}, actionsMockObject)
 
 			// start test loadbalancer ws server
 			router := mm.NewRouter()
diff --git a/internal/loadbalancer/server.go b/internal/loadbalancer/server.go
@@ -93,9 +93,9 @@ func StartLoadBalancerServer(
 	// start server
 	log.Infof("Starting vedran load balancer on port :%d...", props.Port)
 	apiController := controllers.NewApiController(
-		props.WhitelistEnabled, *repos, actions.NewActions(), privateKey,
+		props.WhitelistEnabled, *repos, actions.NewActions(),
 	)
-	r := router.CreateNewApiRouter(apiController)
+	r := router.CreateNewApiRouter(apiController, privateKey)
 	prometheus.RecordMetrics(*repos)
 	if props.CertFile != "" {
 		err = http.ListenAndServeTLS(
diff --git a/internal/middleware/signature.go b/internal/middleware/signature.go
@@ -0,0 +1,45 @@
+package middleware
+
+import (
+	"github.com/NodeFactoryIo/go-substrate-rpc-client/signature"
+	"github.com/NodeFactoryIo/vedran/internal/constants"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	log "github.com/sirupsen/logrus"
+	"net/http"
+)
+
+func VerifySignatureMiddleware(next http.Handler, privateKey string) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		verified, httpStatusCode, err := verifySignatureInHeader(r, privateKey)
+		if err != nil {
+			http.Error(w, http.StatusText(httpStatusCode), httpStatusCode)
+			return
+		}
+		if !verified {
+			log.Errorf("Invalid request signature")
+			http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
+			return
+		}
+
+		next.ServeHTTP(w, r)
+	})
+}
+
+func verifySignatureInHeader(r *http.Request, privateKey string) (bool, int, error) {
+	sig := r.Header.Get("X-Signature")
+	if sig == "" {
+		log.Error("Missing signature header")
+		return false, http.StatusBadRequest, nil
+	}
+	sigInBytes, err := hexutil.Decode(sig)
+	if err != nil {
+		log.Errorf("Unable to decode signature, because of: %v", err)
+		return false, http.StatusBadRequest, err
+	}
+	verified, err := signature.Verify([]byte(constants.StatsSignedData), sigInBytes, privateKey)
+	if err != nil {
+		log.Errorf("Failed to verify signature, because %v", err)
+		return false, http.StatusInternalServerError, err
+	}
+	return verified, 0, err
+}
diff --git a/internal/router/router.go b/internal/router/router.go
@@ -5,8 +5,8 @@ import (
 	"github.com/gorilla/mux"
 )
 
-func CreateNewApiRouter(controller *controllers.ApiController) *mux.Router {
+func CreateNewApiRouter(controller *controllers.ApiController, privateKey string) *mux.Router {
 	router := mux.NewRouter()
-	createRoutes(controller, router)
+	createRoutes(controller, router, privateKey)
 	return router
 }
diff --git a/internal/router/routes.go b/internal/router/routes.go
@@ -1,6 +1,7 @@
 package router
 
 import (
+	middleware2 "github.com/NodeFactoryIo/vedran/internal/middleware"
 	"net/http"
 
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -14,7 +15,9 @@ import (
 	"github.com/gorilla/mux"
 )
 
-func createRoute(route string, method string, handler http.HandlerFunc, router *mux.Router, authorized bool) {
+func createRoute(
+	route string, method string, handler http.HandlerFunc, router *mux.Router, authorized bool,
+) {
 	var r *mux.Route
 	if authorized {
 		r = router.Handle(route, auth.AuthMiddleware(handler))
@@ -27,30 +30,54 @@ func createRoute(route string, method string, handler http.HandlerFunc, router *
 	log.Debugf("Created route %s\t%s", method, route)
 }
 
-func createTrackedRoute(route string, method string, handler http.Handler, router *mux.Router) {
+func createTrackedRoute(
+	route string, method string, handler http.Handler, router *mux.Router,
+) {
 	r := router.Handle(route, handler)
 	r.Methods(method)
 	r.Name(route)
 
 	log.Debugf("Created route %s\t%s", method, route)
+}
+
+func createSignatureVerificationRoute(
+	route string, method string, handler http.HandlerFunc, router *mux.Router, privateKey string,
+) {
+	r := router.Handle(route, middleware2.VerifySignatureMiddleware(handler, privateKey))
+	r.Methods(method)
+	r.Name(route)
 
+	log.Debugf("Created route %s\t%s", method, route)
 }
 
-func createRoutes(apiController *controllers.ApiController, router *mux.Router) {
+func createRoutes(apiController *controllers.ApiController, router *mux.Router, privateKey string) {
 	// Create a custom registry for prometheus.
 	mdlw := middleware.New(middleware.Config{
 		Recorder: metrics.NewRecorder(metrics.Config{}),
 	})
 
-	createTrackedRoute("/", "POST", std.Handler("/", mdlw, http.HandlerFunc(apiController.RPCHandler)), router)
-	createTrackedRoute("/ws", "GET", std.Handler("/ws", mdlw, http.HandlerFunc(apiController.WSHandler)), router)
-
-	createRoute("/api/v1/nodes", "POST", apiController.RegisterHandler, router, false)
-	createRoute("/api/v1/nodes/pings", "POST", apiController.PingHandler, router, true)
-	createRoute("/api/v1/nodes/metrics", "PUT", apiController.SaveMetricsHandler, router, true)
-	createRoute("/api/v1/stats", "POST", apiController.StatisticsHandlerAllStatsForLoadbalancer, router, false)
-	createRoute("/api/v1/stats", "GET", apiController.StatisticsHandlerAllStats, router, false)
-	createRoute("/api/v1/stats/node/{id}", "GET", apiController.StatisticsHandlerStatsForNode, router, false)
-	createRoute("/api/v1/stats/lb", "GET", apiController.StatisticsHandlerStatsForLoadBalancer, router, false)
-	createRoute("/metrics", "GET", promhttp.Handler().ServeHTTP, router, false)
+	createTrackedRoute(
+		"/", "POST", std.Handler("/", mdlw, http.HandlerFunc(apiController.RPCHandler)), router)
+	createTrackedRoute(
+		"/ws", "GET", std.Handler("/ws", mdlw, http.HandlerFunc(apiController.WSHandler)), router)
+
+	createSignatureVerificationRoute(
+		"/api/v1/stats", "POST", apiController.StatisticsHandlerAllStatsForLoadbalancer, router, privateKey)
+
+	// authorized
+	createRoute(
+		"/api/v1/nodes/pings", "POST", apiController.PingHandler, router, true)
+	createRoute(
+		"/api/v1/nodes/metrics", "PUT", apiController.SaveMetricsHandler, router, true)
+	// unauthorized
+	createRoute(
+		"/api/v1/nodes", "POST", apiController.RegisterHandler, router, false)
+	createRoute(
+		"/api/v1/stats", "GET", apiController.StatisticsHandlerAllStats, router, false)
+	createRoute(
+		"/api/v1/stats/node/{id}", "GET", apiController.StatisticsHandlerStatsForNode, router, false)
+	createRoute(
+		"/api/v1/stats/lb", "GET", apiController.StatisticsHandlerStatsForLoadBalancer, router, false)
+	createRoute(
+		"/metrics", "GET", promhttp.Handler().ServeHTTP, router, false)
 }
diff --git a/internal/router/routes_test.go b/internal/router/routes_test.go
@@ -25,7 +25,6 @@ func TestApiRouter(t *testing.T) {
 			RecordRepo:  &recordRepoMock,
 		},
 		nil,
-		"",
 	)
 
 	tests := []struct {
@@ -39,7 +38,7 @@ func TestApiRouter(t *testing.T) {
 	}
 
 	router := mux.NewRouter()
-	createRoutes(apiController, router)
+	createRoutes(apiController, router, "")
 
 	for _, test := range tests {
 		t.Run(fmt.Sprintf("%s::%s", test.name, test.url), func(t *testing.T) {
diff --git a/internal/script/payout.go b/internal/script/payout.go

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,3 @@`
`1`		`-package controllers`
	`1`	`+package constants`
`2`	`2`
`3`	`3`	`const StatsSignedData = "loadbalancer-request"`
Original file line number	Diff line number	Diff line change
`@@ -9,19 +9,16 @@ type ApiController struct {`
`9`	`9`	`whitelistEnabled bool`
`10`	`10`	`repositories repositories.Repos`
`11`	`11`	`actions actions.Actions`
`12`		`- privateKey string`
`13`	`12`	`}`
`14`	`13`
`15`	`14`	`func NewApiController(`
`16`	`15`	`whitelistEnabled bool,`
`17`	`16`	`repositories repositories.Repos,`
`18`	`17`	`actions actions.Actions,`
`19`		`- privateKey string,`
`20`	`18`	`) *ApiController {`
`21`	`19`	`return &ApiController{`
`22`	`20`	`whitelistEnabled: whitelistEnabled,`
`23`	`21`	`repositories: repositories,`
`24`	`22`	`actions: actions,`
`25`		`- privateKey: privateKey,`
`26`	`23`	`}`
`27`	`24`	`}`