From f20ec1a16afe68dce7d19e8d5e296c5df798299b Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Tue, 18 Mar 2025 10:55:13 -0500 Subject: [PATCH 1/2] Use constant time comparison to check chip secret --- handlers/chip/handler.go | 2 +- handlers/utils.go | 6 ++++++ handlers/utils_test.go | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/handlers/chip/handler.go b/handlers/chip/handler.go index 4aee81572..c4ea45e1d 100644 --- a/handlers/chip/handler.go +++ b/handlers/chip/handler.go @@ -59,7 +59,7 @@ type receivePayload struct { // receiveMessage is our HTTP handler function for incoming events func (h *handler) receive(ctx context.Context, c courier.Channel, w http.ResponseWriter, r *http.Request, payload *receivePayload, clog *courier.ChannelLog) ([]courier.Event, error) { secret := c.StringConfigForKey(courier.ConfigSecret, "") - if payload.Secret != secret { + if !handlers.SecretEqual(payload.Secret, secret) { return nil, handlers.WriteAndLogRequestError(ctx, h, c, w, r, errors.New("secret incorrect")) } diff --git a/handlers/utils.go b/handlers/utils.go index f16400db3..dc01b8cec 100644 --- a/handlers/utils.go +++ b/handlers/utils.go @@ -2,6 +2,7 @@ package handlers import ( "bytes" + "crypto/subtle" "encoding/base64" "fmt" "regexp" @@ -102,3 +103,8 @@ func DecodePossibleBase64(original string) string { func IsURL(s string) bool { return urlRegex.MatchString(s) } + +// SecretEqual checks if an incoming secret matches the expected secret using constant time comparison. +func SecretEqual(in, expected string) bool { + return subtle.ConstantTimeCompare([]byte(in), []byte(expected)) == 1 +} diff --git a/handlers/utils_test.go b/handlers/utils_test.go index 51ef6643e..b5968b058 100644 --- a/handlers/utils_test.go +++ b/handlers/utils_test.go @@ -124,3 +124,9 @@ func TestDecodePossibleBase64(t *testing.T) { assert.Contains(handlers.DecodePossibleBase64("Tm93IGlzDQp0aGUgdGltZQ0KZm9yIGFsbCBnb29kDQpwZW9wbGUgdG8NCnJlc2lzdC4NCg0KSG93IGFib3V0IGhhaWt1cz8NCkkgZmluZCB0aGVtIHRvIGJlIGZyaWVuZGx5Lg0KcmVmcmlnZXJhdG9yDQoNCjAxMjM0NTY3ODkNCiFAIyQlXiYqKCkgW117fS09Xys7JzoiLC4vPD4/fFx+YA0KQUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVphYmNkZWZnaGlqa2xtbm9wcXJzdHV2d3h5eg=="), "I find them to be friendly") assert.Contains(handlers.DecodePossibleBase64(test6), "I received your letter today") } + +func TestSecretEqual(t *testing.T) { + assert.True(t, handlers.SecretEqual("sesame", "sesame")) + assert.False(t, handlers.SecretEqual("sesame", "Sesame")) + assert.False(t, handlers.SecretEqual("sesame", "sesame3")) +} From f52f62a0cdb5c417266d93c1ac1ec8696d3dda33 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Tue, 18 Mar 2025 11:13:41 -0500 Subject: [PATCH 2/2] Use for server auth as well --- handlers/chip/handler.go | 3 ++- handlers/utils.go | 6 ------ handlers/utils_test.go | 6 ------ server.go | 6 ++++-- utils/misc.go | 6 ++++++ utils/misc_test.go | 6 ++++++ 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/handlers/chip/handler.go b/handlers/chip/handler.go index c4ea45e1d..b17bcd2cf 100644 --- a/handlers/chip/handler.go +++ b/handlers/chip/handler.go @@ -8,6 +8,7 @@ import ( "github.com/nyaruka/courier" "github.com/nyaruka/courier/handlers" + "github.com/nyaruka/courier/utils" "github.com/nyaruka/gocommon/jsonx" "github.com/nyaruka/gocommon/urns" ) @@ -59,7 +60,7 @@ type receivePayload struct { // receiveMessage is our HTTP handler function for incoming events func (h *handler) receive(ctx context.Context, c courier.Channel, w http.ResponseWriter, r *http.Request, payload *receivePayload, clog *courier.ChannelLog) ([]courier.Event, error) { secret := c.StringConfigForKey(courier.ConfigSecret, "") - if !handlers.SecretEqual(payload.Secret, secret) { + if !utils.SecretEqual(payload.Secret, secret) { return nil, handlers.WriteAndLogRequestError(ctx, h, c, w, r, errors.New("secret incorrect")) } diff --git a/handlers/utils.go b/handlers/utils.go index dc01b8cec..f16400db3 100644 --- a/handlers/utils.go +++ b/handlers/utils.go @@ -2,7 +2,6 @@ package handlers import ( "bytes" - "crypto/subtle" "encoding/base64" "fmt" "regexp" @@ -103,8 +102,3 @@ func DecodePossibleBase64(original string) string { func IsURL(s string) bool { return urlRegex.MatchString(s) } - -// SecretEqual checks if an incoming secret matches the expected secret using constant time comparison. -func SecretEqual(in, expected string) bool { - return subtle.ConstantTimeCompare([]byte(in), []byte(expected)) == 1 -} diff --git a/handlers/utils_test.go b/handlers/utils_test.go index b5968b058..51ef6643e 100644 --- a/handlers/utils_test.go +++ b/handlers/utils_test.go @@ -124,9 +124,3 @@ func TestDecodePossibleBase64(t *testing.T) { assert.Contains(handlers.DecodePossibleBase64("Tm93IGlzDQp0aGUgdGltZQ0KZm9yIGFsbCBnb29kDQpwZW9wbGUgdG8NCnJlc2lzdC4NCg0KSG93IGFib3V0IGhhaWt1cz8NCkkgZmluZCB0aGVtIHRvIGJlIGZyaWVuZGx5Lg0KcmVmcmlnZXJhdG9yDQoNCjAxMjM0NTY3ODkNCiFAIyQlXiYqKCkgW117fS09Xys7JzoiLC4vPD4/fFx+YA0KQUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVphYmNkZWZnaGlqa2xtbm9wcXJzdHV2d3h5eg=="), "I find them to be friendly") assert.Contains(handlers.DecodePossibleBase64(test6), "I received your letter today") } - -func TestSecretEqual(t *testing.T) { - assert.True(t, handlers.SecretEqual("sesame", "sesame")) - assert.False(t, handlers.SecretEqual("sesame", "Sesame")) - assert.False(t, handlers.SecretEqual("sesame", "sesame3")) -} diff --git a/server.go b/server.go index a02c527c1..a863e0bea 100644 --- a/server.go +++ b/server.go @@ -18,6 +18,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" + "github.com/nyaruka/courier/utils" "github.com/nyaruka/courier/utils/clogs" "github.com/nyaruka/gocommon/httpx" "github.com/nyaruka/gocommon/jsonx" @@ -393,7 +394,8 @@ func (s *server) basicAuthRequired(h http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if s.config.StatusUsername != "" { user, pass, ok := r.BasicAuth() - if !ok || user != s.config.StatusUsername || pass != s.config.StatusPassword { + + if !ok || !utils.SecretEqual(user, s.config.StatusUsername) || !utils.SecretEqual(pass, s.config.StatusPassword) { w.Header().Set("Content-Type", "text/plain") w.Header().Set("WWW-Authenticate", `Basic realm="Authenticate"`) w.WriteHeader(http.StatusUnauthorized) @@ -409,7 +411,7 @@ func (s *server) basicAuthRequired(h http.HandlerFunc) http.HandlerFunc { func (s *server) tokenAuthRequired(h http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { authHeader := r.Header.Get("Authorization") - if !strings.HasPrefix(authHeader, "Bearer ") || authHeader[7:] != s.config.AuthToken { + if !strings.HasPrefix(authHeader, "Bearer ") || !utils.SecretEqual(authHeader[7:], s.config.AuthToken) { w.Header().Set("Content-Type", "text/plain") w.WriteHeader(http.StatusUnauthorized) w.Write([]byte("Unauthorized")) diff --git a/utils/misc.go b/utils/misc.go index baaaf362d..7a81b1a0a 100644 --- a/utils/misc.go +++ b/utils/misc.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/hmac" "crypto/sha256" + "crypto/subtle" "encoding/hex" "net/url" "path" @@ -146,3 +147,8 @@ func MapUpdate[K comparable, V comparable, M ~map[K]V](m1 M, m2 M) { } } } + +// SecretEqual checks if an incoming secret matches the expected secret using constant time comparison. +func SecretEqual(in, expected string) bool { + return subtle.ConstantTimeCompare([]byte(in), []byte(expected)) == 1 +} diff --git a/utils/misc_test.go b/utils/misc_test.go index 1f9c4a163..0b26668fd 100644 --- a/utils/misc_test.go +++ b/utils/misc_test.go @@ -139,3 +139,9 @@ func TestMapUpdate(t *testing.T) { assert.Equal(t, tc.updated, tc.m1) } } + +func TestSecretEqual(t *testing.T) { + assert.True(t, utils.SecretEqual("sesame", "sesame")) + assert.False(t, utils.SecretEqual("sesame", "Sesame")) + assert.False(t, utils.SecretEqual("sesame", "sesame3")) +}