Integrated ChallengeGiver into SenderServer

server/pairing/challenge_management.go

@@ -58,7 +58,7 @@ func NewChallengeGiver(e *PayloadEncryptor, logger *zap.Logger) (*ChallengeGiver
 func (cg *ChallengeGiver) handleChallengeResponse(w http.ResponseWriter, r *http.Request) *ChallengeError {
 	s, err := cg.cookieStore.Get(r, sessionChallenge)
 	if err != nil {
-		cg.logger.Error("hs.GetCookieStore().Get(r, sessionChallenge)", zap.Error(err))
+		cg.logger.Error("handleChallengeResponse: cg.cookieStore.Get(r, sessionChallenge)", zap.Error(err), zap.String("sessionChallenge", sessionChallenge))
 		return &ChallengeError{"error", http.StatusInternalServerError}
 	}
 
@@ -75,7 +75,7 @@ func (cg *ChallengeGiver) handleChallengeResponse(w http.ResponseWriter, r *http
 
 	c, err := cg.encryptor.decryptPlain(base58.Decode(pc))
 	if err != nil {
-		cg.logger.Error("c, err := hs.DecryptPlain(rc, hs.ek)", zap.Error(err))
+		cg.logger.Error("handleChallengeResponse: cg.encryptor.decryptPlain(base58.Decode(pc))", zap.Error(err), zap.String("pc", pc))
 		return &ChallengeError{"error", http.StatusInternalServerError}
 	}
 
@@ -91,7 +91,8 @@ func (cg *ChallengeGiver) handleChallengeResponse(w http.ResponseWriter, r *http
 		s.Values[sessionBlocked] = true
 		err = s.Save(r, w)
 		if err != nil {
-			cg.logger.Error("err = s.Save(r, w)", zap.Error(err))
+			cg.logger.Error("handleChallengeResponse: err = s.Save(r, w)", zap.Error(err))
+			return &ChallengeError{"error", http.StatusInternalServerError}
 		}
 
 		return &ChallengeError{"forbidden", http.StatusForbidden}
@@ -99,11 +100,11 @@ func (cg *ChallengeGiver) handleChallengeResponse(w http.ResponseWriter, r *http
 	return nil
 }
 
-func (cg *ChallengeGiver) challenge(w http.ResponseWriter, r *http.Request) *ChallengeError {
+func (cg *ChallengeGiver) getChallenge(w http.ResponseWriter, r *http.Request) ([]byte, *ChallengeError) {
 	s, err := cg.cookieStore.Get(r, sessionChallenge)
 	if err != nil {
-		cg.logger.Error("hs.GetCookieStore().Get(r, sessionChallenge)", zap.Error(err))
+		cg.logger.Error("getChallenge: hs.cookieStore.Get(r, sessionChallenge)", zap.Error(err))
-		return &ChallengeError{"error", http.StatusInternalServerError}
+		return nil, &ChallengeError{"error", http.StatusInternalServerError}
 	}
 
 	challenge, ok := s.Values[sessionChallenge].([]byte)
@@ -111,32 +112,18 @@ func (cg *ChallengeGiver) challenge(w http.ResponseWriter, r *http.Request) *Cha
 		challenge = make([]byte, 64)
 		_, err = rand.Read(challenge)
 		if err != nil {
-			cg.logger.Error("_, err = rand.Read(challenge)", zap.Error(err))
+			cg.logger.Error("getChallenge: _, err = rand.Read(challenge)", zap.Error(err))
-			return &ChallengeError{"error", http.StatusInternalServerError}
+			return nil, &ChallengeError{"error", http.StatusInternalServerError}
 		}
 
 		s.Values[sessionChallenge] = challenge
 		err = s.Save(r, w)
 		if err != nil {
-			cg.logger.Error("err = s.Save(r, w)", zap.Error(err))
+			cg.logger.Error("getChallenge: err = s.Save(r, w)", zap.Error(err))
-			return &ChallengeError{"error", http.StatusInternalServerError}
+			return nil, &ChallengeError{"error", http.StatusInternalServerError}
 		}
 	}
-
+	return challenge, nil
-	w.Header().Set("Content-Type", "application/octet-stream")
-	_, err = w.Write(challenge)
-	if err != nil {
-		cg.logger.Error("_, err = w.Write(challenge)", zap.Error(err))
-	}
-	return nil
-}
-
-func (s *BaseServer) GetCookieStore() *sessions.CookieStore {
-	return s.cookieStore
-}
-
-func (s *BaseServer) DecryptPlain(data []byte) ([]byte, error) {
-	return s.encryptor.decryptPlain(data)
 }
 
 type ChallengeTaker struct {

server/pairing/handlers.go

@@ -1,12 +1,9 @@
 package pairing
 
 import (
-	"bytes"
-	"crypto/rand"
 	"io"
 	"net/http"
 
-	"github.com/btcsuite/btcutil/base58"
 	"go.uber.org/zap"
 
 	"github.com/status-im/status-go/signal"
@@ -188,53 +185,11 @@ func handleSendInstallation(hs HandlerServer, pmr PayloadMounterReceiver) http.H
 
 // Challenge middleware and handling
 
-func middlewareChallenge(hs HandlerServer, next http.Handler) http.HandlerFunc {
+func middlewareChallenge(cg *ChallengeGiver, next http.Handler) http.HandlerFunc {
-	logger := hs.GetLogger()
 	return func(w http.ResponseWriter, r *http.Request) {
-		s, err := hs.GetCookieStore().Get(r, sessionChallenge)
+		ce := cg.handleChallengeResponse(w, r)
-		if err != nil {
+		if ce != nil {
-			logger.Error("middlewareChallenge: hs.GetCookieStore().Get(r, sessionChallenge)", zap.Error(err), zap.String("sessionChallenge", sessionChallenge))
+			http.Error(w, ce.Text, ce.HttpCode)
-			http.Error(w, "error", http.StatusInternalServerError)
-			return
-		}
-
-		blocked, ok := s.Values[sessionBlocked].(bool)
-		if ok && blocked {
-			http.Error(w, "forbidden", http.StatusForbidden)
-			return
-		}
-
-		// If the request header doesn't include a challenge don't punish the client, just throw a 403
-		pc := r.Header.Get(sessionChallenge)
-		if pc == "" {
-			http.Error(w, "forbidden", http.StatusForbidden)
-			return
-		}
-
-		c, err := hs.DecryptPlain(base58.Decode(pc))
-		if err != nil {
-			logger.Error("middlewareChallenge: c, err := hs.DecryptPlain(base58.Decode(pc))", zap.Error(err), zap.String("pc", pc))
-			http.Error(w, "error", http.StatusInternalServerError)
-			return
-		}
-
-		// If the challenge is not in the session store don't punish the client, just throw a 403
-		challenge, ok := s.Values[sessionChallenge].([]byte)
-		if !ok {
-			http.Error(w, "forbidden", http.StatusForbidden)
-			return
-		}
-
-		// Only if we have both a challenge in the session store and in the request header
-		// do we entertain blocking the client. Because then we know someone is trying to be sneaky.
-		if !bytes.Equal(c, challenge) {
-			s.Values[sessionBlocked] = true
-			err = s.Save(r, w)
-			if err != nil {
-				logger.Error("middlewareChallenge: err = s.Save(r, w)", zap.Error(err))
-			}
-
-			http.Error(w, "forbidden", http.StatusForbidden)
 			return
 		}
 
@@ -242,40 +197,18 @@ func middlewareChallenge(hs HandlerServer, next http.Handler) http.HandlerFunc {
 	}
 }
 
-func handlePairingChallenge(hs HandlerServer) http.HandlerFunc {
+func handlePairingChallenge(cg *ChallengeGiver) http.HandlerFunc {
-	logger := hs.GetLogger()
 	return func(w http.ResponseWriter, r *http.Request) {
-		s, err := hs.GetCookieStore().Get(r, sessionChallenge)
+		challenge, ce := cg.getChallenge(w, r)
-		if err != nil {
+		if ce != nil {
-			logger.Error("handlePairingChallenge: hs.GetCookieStore().Get(r, sessionChallenge)", zap.Error(err))
+			http.Error(w, ce.Text, ce.HttpCode)
-			http.Error(w, "error", http.StatusInternalServerError)
 			return
 		}
 
-		var challenge []byte
-		challenge, ok := s.Values[sessionChallenge].([]byte)
-		if !ok {
-			challenge = make([]byte, 64)
-			_, err = rand.Read(challenge)
-			if err != nil {
-				logger.Error("handlePairingChallenge: _, err = rand.Read(challenge)", zap.Error(err))
-				http.Error(w, "error", http.StatusInternalServerError)
-				return
-			}
-
-			s.Values[sessionChallenge] = challenge
-			err = s.Save(r, w)
-			if err != nil {
-				logger.Error("handlePairingChallenge: err = s.Save(r, w)", zap.Error(err))
-				http.Error(w, "error", http.StatusInternalServerError)
-				return
-			}
-		}
-
 		w.Header().Set("Content-Type", "application/octet-stream")
-		_, err = w.Write(challenge)
+		_, err := w.Write(challenge)
 		if err != nil {
-			logger.Error("handlePairingChallenge: _, err = w.Write(challenge)", zap.Error(err))
+			cg.logger.Error("handlePairingChallenge: _, err = w.Write(challenge)", zap.Error(err))
 			return
 		}
 	}

server/pairing/interfaces.go

@@ -1,7 +1,6 @@
 package pairing
 
 import (
-	"github.com/gorilla/sessions"
 	"go.uber.org/zap"
 )
 
@@ -28,8 +27,6 @@ type PayloadLocker interface {
 
 type HandlerServer interface {
 	GetLogger() *zap.Logger
-	GetCookieStore() *sessions.CookieStore
-	DecryptPlain([]byte) ([]byte, error)
 }
 
 type ProtobufMarshaler interface {

server/pairing/server.go

@@ -122,6 +122,8 @@ type SenderServer struct {
 	accountMounter      PayloadMounter
 	rawMessageMounter   *RawMessagePayloadMounter
 	installationMounter *InstallationPayloadMounterReceiver
+
+	challengeGiver *ChallengeGiver
 }
 
 // NewSenderServer returns a *SenderServer init from the given *SenderServerConfig
@@ -139,24 +141,30 @@ func NewSenderServer(backend *api.GethStatusBackend, config *SenderServerConfig)
 		return nil, err
 	}
 
+	cg, err := NewChallengeGiver(e, logger)
+	if err != nil {
+		return nil, err
+	}
+
 	return &SenderServer{
 		BaseServer:          bs,
 		accountMounter:      am,
 		rawMessageMounter:   rmm,
 		installationMounter: imr,
+		challengeGiver:      cg,
 	}, nil
 }
 
 func (s *SenderServer) startSendingData() error {
 	s.SetHandlers(server.HandlerPatternMap{
-		pairingChallenge:      handlePairingChallenge(s),
+		pairingChallenge:      handlePairingChallenge(s.challengeGiver),
-		pairingSendAccount:    middlewareChallenge(s, handleSendAccount(s, s.accountMounter)),
+		pairingSendAccount:    middlewareChallenge(s.challengeGiver, handleSendAccount(s, s.accountMounter)),
-		pairingSendSyncDevice: middlewareChallenge(s, handlePairingSyncDeviceSend(s, s.rawMessageMounter)),
+		pairingSendSyncDevice: middlewareChallenge(s.challengeGiver, handlePairingSyncDeviceSend(s, s.rawMessageMounter)),
 		// TODO implement refactor of installation data exchange to follow the send/receive pattern of
 		//  the other handlers.
 		//  https://github.com/status-im/status-go/issues/3304
 		// receive installation data from receiver
-		pairingReceiveInstallation: middlewareChallenge(s, handleReceiveInstallation(s, s.installationMounter)),
+		pairingReceiveInstallation: middlewareChallenge(s.challengeGiver, handleReceiveInstallation(s, s.installationMounter)),
 	})
 	return s.Start()
 }
@@ -240,7 +248,6 @@ func NewReceiverServer(backend *api.GethStatusBackend, config *ReceiverServerCon
 
 func (s *ReceiverServer) startReceivingData() error {
 	s.SetHandlers(server.HandlerPatternMap{
-		pairingChallenge:         handlePairingChallenge(s),
 		pairingReceiveAccount:    handleReceiveAccount(s, s.accountReceiver),
 		pairingReceiveSyncDevice: handleParingSyncDeviceReceive(s, s.rawMessageReceiver),
 		// TODO implement refactor of installation data exchange to follow the send/receive pattern of