From 622b9970901a7d712dcd784c16a55420bf0b31fb Mon Sep 17 00:00:00 2001 From: Komu Wairagu Date: Mon, 28 Aug 2023 18:20:48 +0300 Subject: [PATCH] issues/92: Add support for http.NewResponseController (#368) - Updates: https://github.com/komuw/ong/issues/92 - This still doesn't fix the issue, there is still https://github.com/komuw/ong/pull/213 for that --- CHANGELOG.md | 3 +++ example/main.go | 2 +- internal/key/key.go | 9 +++++++-- internal/key/key_test.go | 7 +++++++ middleware/auth.go | 9 ++++----- middleware/auth_test.go | 5 ++--- middleware/gzip.go | 12 +++++++++--- middleware/log.go | 12 +++++++++--- middleware/middleware.go | 8 ++++++++ middleware/session.go | 12 +++++++++--- mux/mux_test.go | 2 +- 11 files changed, 60 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6da67719..e570216f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Most recent version is listed first. +# v0.0.77 +- ong/middleware: Add support for http.NewResponseController: https://github.com/komuw/ong/pull/368 + # v0.0.76 - ong/middleware: Configure what percentage of ratelimited or loadshed responses should be logged: https://github.com/komuw/ong/pull/364 diff --git a/example/main.go b/example/main.go index 34dc469d..b21af080 100644 --- a/example/main.go +++ b/example/main.go @@ -27,7 +27,7 @@ func main() { mux.NewRoute( "staticAssets/:file", mux.MethodAll, - middleware.BasicAuth(api.handleFileServer(), "user", "some-long-passwd"), + middleware.BasicAuth(api.handleFileServer(), "user", "some-long-p1sswd"), ), mux.NewRoute( "check/:age/", diff --git a/internal/key/key.go b/internal/key/key.go index b1d4bf5c..1a1bd6a0 100644 --- a/internal/key/key.go +++ b/internal/key/key.go @@ -9,7 +9,12 @@ import ( // IsSecure checks that secretKey has at least some minimum desirable security properties. func IsSecure(secretKey string) error { const ( - minLen = 6 + // Using a password like `4$kplejewjdsnv`(one number, one symbol & length 14) + // would take about 90 million years to crack. + // see: + // - https://www.passwordmonster.com/ + // - https://thesecurityfactory.be/password-cracking-speed/ + minLen = 14 maxLen = 256 expected = 1 ) @@ -28,7 +33,7 @@ func IsSecure(secretKey string) error { if unicode.IsDigit(r) { hasDigit = hasDigit + 1 } - if unicode.IsPunct(r) { + if unicode.IsPunct(r) || unicode.IsSymbol(r) { hasSymbol = hasSymbol + 1 } if unicode.IsLetter(r) { diff --git a/internal/key/key_test.go b/internal/key/key_test.go index e01b3efc..7bf9c40b 100644 --- a/internal/key/key_test.go +++ b/internal/key/key_test.go @@ -29,6 +29,13 @@ func TestCheckSecretKey(t *testing.T) { attest.Ok(t, err) }, }, + { + name: "small secure key is ok", + key: "4$kplejewjdsnv", + check: func(err error) { + attest.Ok(t, err) + }, + }, { name: "bad key", key: "super-h@rd-password", diff --git a/middleware/auth.go b/middleware/auth.go index 5a0c8c7a..adc3ef0f 100644 --- a/middleware/auth.go +++ b/middleware/auth.go @@ -2,18 +2,17 @@ package middleware import ( "crypto/subtle" - "fmt" "net/http" -) -const minPasswdSize = 16 + "github.com/komuw/ong/internal/key" +) // BasicAuth is a middleware that protects wrappedHandler using basic authentication. func BasicAuth(wrappedHandler http.Handler, user, passwd string) http.HandlerFunc { const realm = "enter username and password" - if len(passwd) < minPasswdSize { - panic(fmt.Sprintf("passwd cannot be less than %d in size.", minPasswdSize)) + if err := key.IsSecure(passwd); err != nil { + panic(err) } e := func(w http.ResponseWriter) { diff --git a/middleware/auth_test.go b/middleware/auth_test.go index d0f01e4d..6dcede0d 100644 --- a/middleware/auth_test.go +++ b/middleware/auth_test.go @@ -24,14 +24,14 @@ func TestBasicAuth(t *testing.T) { { // small passwd panics. attest.Panics(t, func() { - BasicAuth(protectedHandler("hello"), "user", strings.Repeat("a", (minPasswdSize-3))) + BasicAuth(protectedHandler("hello"), "user", strings.Repeat("a", 8)) }, ) } msg := "hello" user := "some-user" - passwd := "some-long-passwd" + passwd := "some-long-p1sswd" wrappedHandler := BasicAuth(protectedHandler(msg), user, passwd) tests := []struct { @@ -81,7 +81,6 @@ func TestBasicAuth(t *testing.T) { res := rec.Result() defer res.Body.Close() - attest.True(t, len(passwd) >= minPasswdSize) attest.Equal(t, res.StatusCode, tt.wantCode) }) } diff --git a/middleware/gzip.go b/middleware/gzip.go index ae76fd4f..dfc7621e 100644 --- a/middleware/gzip.go +++ b/middleware/gzip.go @@ -82,6 +82,7 @@ var ( _ http.Pusher = &gzipRW{} _ io.WriteCloser = &gzipRW{} _ io.ReaderFrom = &gzipRW{} + _ httpRespCtrler = &logRW{} // _ http.CloseNotifier = &gzipRW{} // `http.CloseNotifier` has been deprecated sinc Go v1.11(year 2018) ) @@ -220,10 +221,8 @@ func (grw *gzipRW) Hijack() (net.Conn, *bufio.ReadWriter, error) { // ReadFrom implements io.ReaderFrom // It is necessary for the sendfile syscall // https://github.com/caddyserver/caddy/pull/5022 +// https://github.com/caddyserver/caddy/blob/v2.7.4/modules/caddyhttp/responsewriter.go#L45-L49 func (grw *gzipRW) ReadFrom(src io.Reader) (n int64, err error) { - if rf, ok := grw.ResponseWriter.(io.ReaderFrom); ok { - return rf.ReadFrom(src) - } return io.Copy(grw.ResponseWriter, src) } @@ -235,6 +234,13 @@ func (grw *gzipRW) Push(target string, opts *http.PushOptions) error { return fmt.Errorf("ong/middleware: http.Pusher interface is not supported") } +// Unwrap implements http.ResponseController. +// It returns the underlying ResponseWriter, +// which is necessary for http.ResponseController to work correctly. +func (grw *gzipRW) Unwrap() http.ResponseWriter { + return grw.ResponseWriter +} + // shouldGzipReq checks whether the request is eligible to be gzipped. func shouldGzipReq(r *http.Request) bool { // Examples of the `acceptEncodingHeader` are: diff --git a/middleware/log.go b/middleware/log.go index 1925c552..81a4ce37 100644 --- a/middleware/log.go +++ b/middleware/log.go @@ -118,6 +118,7 @@ var ( _ http.Hijacker = &logRW{} _ http.Pusher = &logRW{} _ io.ReaderFrom = &logRW{} + _ httpRespCtrler = &logRW{} // _ http.CloseNotifier = &logRW{} // `http.CloseNotifier` has been deprecated sinc Go v1.11(year 2018) ) @@ -172,9 +173,14 @@ func (lrw *logRW) Push(target string, opts *http.PushOptions) error { // ReadFrom implements io.ReaderFrom // It is necessary for the sendfile syscall // https://github.com/caddyserver/caddy/pull/5022 +// https://github.com/caddyserver/caddy/blob/v2.7.4/modules/caddyhttp/responsewriter.go#L45-L49 func (lrw *logRW) ReadFrom(src io.Reader) (n int64, err error) { - if rf, ok := lrw.ResponseWriter.(io.ReaderFrom); ok { - return rf.ReadFrom(src) - } return io.Copy(lrw.ResponseWriter, src) } + +// Unwrap implements http.ResponseController. +// It returns the underlying ResponseWriter, +// which is necessary for http.ResponseController to work correctly. +func (lrw *logRW) Unwrap() http.ResponseWriter { + return lrw.ResponseWriter +} diff --git a/middleware/middleware.go b/middleware/middleware.go index 3b24e735..87b2f305 100644 --- a/middleware/middleware.go +++ b/middleware/middleware.go @@ -507,3 +507,11 @@ func deleteH(wrappedHandler http.Handler) http.HandlerFunc { wrappedHandler.ServeHTTP(w, r) } } + +// httpRespCtrler represents the interface that has to be implemented for a +// responseWriter to satisfy [http.ResponseController] +// +// https://github.com/golang/go/blob/go1.21.0/src/net/http/responsecontroller.go#L42-L44 +type httpRespCtrler interface { + Unwrap() http.ResponseWriter +} diff --git a/middleware/session.go b/middleware/session.go index 14d32605..1d0a177d 100644 --- a/middleware/session.go +++ b/middleware/session.go @@ -63,6 +63,7 @@ var ( _ http.Hijacker = &sessRW{} _ http.Pusher = &sessRW{} _ io.ReaderFrom = &sessRW{} + _ httpRespCtrler = &logRW{} // _ http.CloseNotifier = &sessRW{} // `http.CloseNotifier` has been deprecated sinc Go v1.11(year 2018) ) @@ -132,9 +133,14 @@ func (srw *sessRW) Push(target string, opts *http.PushOptions) error { // ReadFrom implements io.ReaderFrom // It is necessary for the sendfile syscall // https://github.com/caddyserver/caddy/pull/5022 +// https://github.com/caddyserver/caddy/blob/v2.7.4/modules/caddyhttp/responsewriter.go#L45-L49 func (srw *sessRW) ReadFrom(src io.Reader) (n int64, err error) { - if rf, ok := srw.ResponseWriter.(io.ReaderFrom); ok { - return rf.ReadFrom(src) - } return io.Copy(srw.ResponseWriter, src) } + +// Unwrap implements http.ResponseController. +// It returns the underlying ResponseWriter, +// which is necessary for http.ResponseController to work correctly. +func (srw *sessRW) Unwrap() http.ResponseWriter { + return srw.ResponseWriter +} diff --git a/mux/mux_test.go b/mux/mux_test.go index 51bdf494..2833b2d5 100644 --- a/mux/mux_test.go +++ b/mux/mux_test.go @@ -58,7 +58,7 @@ func TestNewRoute(t *testing.T) { _ = NewRoute( "/api", MethodGet, - middleware.BasicAuth(someMuxHandler("msg"), "some-user", "some-very-very-hard-passwd"), + middleware.BasicAuth(someMuxHandler("msg"), "some-user", "some-very-very-h1rd-passwd"), ) // fails