From 4ec064ec9c141ca9a6453402714483ad4c6fa6f8 Mon Sep 17 00:00:00 2001 From: Samuel Hawksby-Robinson Date: Thu, 16 Mar 2023 14:15:07 +0000 Subject: [PATCH] Integrated ChallengeGiver into SenderServer --- server/pairing/challenge_management.go | 37 ++++------- server/pairing/handlers.go | 87 +++----------------------- server/pairing/interfaces.go | 3 - server/pairing/server.go | 17 +++-- 4 files changed, 34 insertions(+), 110 deletions(-) diff --git a/server/pairing/challenge_management.go b/server/pairing/challenge_management.go index 7b059e777..c84904691 100644 --- a/server/pairing/challenge_management.go +++ b/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)) - return &ChallengeError{"error", http.StatusInternalServerError} + cg.logger.Error("getChallenge: hs.cookieStore.Get(r, sessionChallenge)", zap.Error(err)) + 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)) - return &ChallengeError{"error", http.StatusInternalServerError} + cg.logger.Error("getChallenge: _, err = rand.Read(challenge)", zap.Error(err)) + 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)) - return &ChallengeError{"error", http.StatusInternalServerError} + cg.logger.Error("getChallenge: err = s.Save(r, w)", zap.Error(err)) + return nil, &ChallengeError{"error", http.StatusInternalServerError} } } - - 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) + return challenge, nil } type ChallengeTaker struct { diff --git a/server/pairing/handlers.go b/server/pairing/handlers.go index ad4e0735e..5ad773127 100644 --- a/server/pairing/handlers.go +++ b/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 { - logger := hs.GetLogger() +func middlewareChallenge(cg *ChallengeGiver, next http.Handler) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - s, err := hs.GetCookieStore().Get(r, sessionChallenge) - if err != nil { - logger.Error("middlewareChallenge: hs.GetCookieStore().Get(r, sessionChallenge)", zap.Error(err), zap.String("sessionChallenge", sessionChallenge)) - 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) + ce := cg.handleChallengeResponse(w, r) + if ce != nil { + http.Error(w, ce.Text, ce.HttpCode) return } @@ -242,40 +197,18 @@ func middlewareChallenge(hs HandlerServer, next http.Handler) http.HandlerFunc { } } -func handlePairingChallenge(hs HandlerServer) http.HandlerFunc { - logger := hs.GetLogger() +func handlePairingChallenge(cg *ChallengeGiver) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - s, err := hs.GetCookieStore().Get(r, sessionChallenge) - if err != nil { - logger.Error("handlePairingChallenge: hs.GetCookieStore().Get(r, sessionChallenge)", zap.Error(err)) - http.Error(w, "error", http.StatusInternalServerError) + challenge, ce := cg.getChallenge(w, r) + if ce != nil { + http.Error(w, ce.Text, ce.HttpCode) 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 } } diff --git a/server/pairing/interfaces.go b/server/pairing/interfaces.go index 4eb9bbc99..2d3253630 100644 --- a/server/pairing/interfaces.go +++ b/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 { diff --git a/server/pairing/server.go b/server/pairing/server.go index fd148d34a..fb7780238 100644 --- a/server/pairing/server.go +++ b/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), - pairingSendAccount: middlewareChallenge(s, handleSendAccount(s, s.accountMounter)), - pairingSendSyncDevice: middlewareChallenge(s, handlePairingSyncDeviceSend(s, s.rawMessageMounter)), + pairingChallenge: handlePairingChallenge(s.challengeGiver), + pairingSendAccount: middlewareChallenge(s.challengeGiver, handleSendAccount(s, s.accountMounter)), + 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