diff --git a/acme/acme.go b/acme/acme.go index 98e0cd6..ececdc8 100644 --- a/acme/acme.go +++ b/acme/acme.go @@ -20,36 +20,24 @@ func IssueCertificates(cacheDir, email, challengeType string, domains []string, cache := certmagic.NewCache(certmagic.CacheOptions{ GetConfigForCert: func(_ certmagic.Certificate) (*certmagic.Config, error) { return &certmagic.Config{ - RenewalWindowRatio: 0, - MustStaple: false, - OCSP: certmagic.OCSPConfig{}, - Storage: &certmagic.FileStorage{Path: cacheDir}, + Storage: &certmagic.FileStorage{Path: cacheDir}, }, nil }, - OCSPCheckInterval: 0, - RenewCheckInterval: 0, - Capacity: 0, }) cfg := certmagic.New(cache, certmagic.Config{ - RenewalWindowRatio: 0, - MustStaple: false, - OCSP: certmagic.OCSPConfig{}, - Storage: &certmagic.FileStorage{Path: cacheDir}, + Storage: &certmagic.FileStorage{Path: cacheDir}, }) myAcme := certmagic.NewACMEIssuer(cfg, certmagic.ACMEIssuer{ - CA: certmagic.LetsEncryptProductionCA, - TestCA: certmagic.LetsEncryptStagingCA, - Email: email, - Agreed: true, - DisableHTTPChallenge: false, - DisableTLSALPNChallenge: false, - ListenHost: "0.0.0.0", - AltHTTPPort: altHTTPPort, - AltTLSALPNPort: altTLSAlpnPort, - CertObtainTimeout: time.Second * 240, - PreferredChains: certmagic.ChainPreference{}, + CA: certmagic.LetsEncryptProductionCA, + TestCA: certmagic.LetsEncryptStagingCA, + Email: email, + Agreed: true, + ListenHost: "0.0.0.0", + AltHTTPPort: altHTTPPort, + AltTLSALPNPort: altTLSAlpnPort, + CertObtainTimeout: time.Second * 240, }) if !useProduction { diff --git a/attributes/attributes.go b/attributes/attributes.go index 0e41ed7..c6d2c39 100644 --- a/attributes/attributes.go +++ b/attributes/attributes.go @@ -72,7 +72,11 @@ func Get(r *http.Request, key string) any { return nil } - return v.(attrs).get(key) + a, ok := v.(attrs) + if !ok { + return nil + } + return a.get(key) } // Set sets the key to value. It replaces any existing @@ -83,7 +87,11 @@ func Set(r *http.Request, key string, value string) error { return errors.New("unable to find `psr:attributes` context key") } - v.(attrs).set(key, value) + a, ok := v.(attrs) + if !ok { + return errors.New("unexpected type stored under `psr:attributes` context key") + } + a.set(key, value) return nil } diff --git a/handler/request.go b/handler/request.go index c5e7f7a..6ac5801 100644 --- a/handler/request.go +++ b/handler/request.go @@ -24,10 +24,6 @@ const ( // FetchIP extracts the client IP from net/http's RemoteAddr ("host:port" // or a bare IP). Returns the empty string for unparseable input. func FetchIP(pair string, log *slog.Logger) string { - if !strings.ContainsRune(pair, ':') { - return pair - } - addr, _, err := net.SplitHostPort(pair) if err == nil { return addr @@ -44,9 +40,7 @@ func FetchIP(pair string, log *slog.Logger) string { // URI returns the fully-qualified request URI, stripping CR/LF to prevent // header smuggling via the URL. func URI(r *http.Request) string { - uri := r.URL.String() - uri = strings.ReplaceAll(uri, "\n", "") - uri = strings.ReplaceAll(uri, "\r", "") + uri := strings.ReplaceAll(strings.ReplaceAll(r.URL.String(), "\n", ""), "\r", "") if r.URL.Host != "" { return uri @@ -88,9 +82,7 @@ func extractCookies(r *http.Request) map[string]string { // cleanRawQuery strips CR/LF from the URL raw query before exposing it to PHP. func cleanRawQuery(q string) string { - q = strings.ReplaceAll(q, "\n", "") - q = strings.ReplaceAll(q, "\r", "") - return q + return strings.ReplaceAll(strings.ReplaceAll(q, "\n", ""), "\r", "") } // populateBody fills req.Body / req.Parsed based on the request content-type. diff --git a/handler/uploads.go b/handler/uploads.go index 83c2dd5..46a3532 100644 --- a/handler/uploads.go +++ b/handler/uploads.go @@ -2,7 +2,9 @@ package handler import ( "encoding/json" + "errors" "io" + "io/fs" "log/slog" "mime/multipart" "os" @@ -166,8 +168,6 @@ func (f *FileUpload) Open(dir string, forbid, allow map[string]struct{}) error { // exists if file exists. func exists(path string) bool { // path is RR-generated TempFilename, not user-controlled. - if _, err := os.Stat(path); os.IsNotExist(err) { //nolint:gosec // G703 - return false - } - return true + _, err := os.Stat(path) //nolint:gosec // G703 + return !errors.Is(err, fs.ErrNotExist) } diff --git a/middleware/log.go b/middleware/log.go index d63c893..b1a265e 100644 --- a/middleware/log.go +++ b/middleware/log.go @@ -16,6 +16,11 @@ import ( var _ io.ReadCloser = (*wrapper)(nil) var _ http.ResponseWriter = (*wrapper)(nil) +// stripCRLF removes CR/LF to prevent log injection (CWE-117). +func stripCRLF(s string) string { + return strings.ReplaceAll(strings.ReplaceAll(s, "\n", ""), "\r", "") +} + type wrapper struct { io.ReadCloser read int @@ -27,7 +32,6 @@ type wrapper struct { w http.ResponseWriter code int - data []byte } func (w *wrapper) Read(b []byte) (int, error) { @@ -85,7 +89,6 @@ func (w *wrapper) reset() { w.wc = false w.write = 0 w.w = nil - w.data = nil w.ReadCloser = nil } @@ -128,8 +131,7 @@ func (l *lm) Log(next http.Handler, accessLogs bool) http.Handler { } func (l *lm) writeLog(accessLog bool, r *http.Request, bw *wrapper, start time.Time) { - switch accessLog { - case false: + if !accessLog { l.log.Info("http log", "status", bw.code, "method", r.Method, @@ -140,38 +142,31 @@ func (l *lm) writeLog(accessLog bool, r *http.Request, bw *wrapper, start time.T "write_bytes", bw.write, "start", start, "elapsed", time.Since(start).Milliseconds()) - case true: - // external/cwe/cwe-117 - usrA := r.UserAgent() - usrA = strings.ReplaceAll(usrA, "\n", "") - usrA = strings.ReplaceAll(usrA, "\r", "") - - rfr := r.Referer() - rfr = strings.ReplaceAll(rfr, "\n", "") - rfr = strings.ReplaceAll(rfr, "\r", "") - - rq := r.URL.RawQuery - rq = strings.ReplaceAll(rq, "\n", "") - rq = strings.ReplaceAll(rq, "\r", "") - - l.log.Info("http access log", - "read_bytes", bw.read, - "write_bytes", bw.write, - "status", bw.code, - "method", r.Method, - "URI", r.RequestURI, - "URL", r.URL.String(), - "remote_address", r.RemoteAddr, - "query", rq, - "content_len", r.ContentLength, - "host", r.Host, - "user_agent", usrA, - "referer", rfr, - "time_local", time.Now().Format("02/Jan/06:15:04:05 -0700"), - "request_time", time.Now(), - "start", start, - "elapsed", time.Since(start).Milliseconds()) + return } + + // external/cwe/cwe-117 + usrA := stripCRLF(r.UserAgent()) + rfr := stripCRLF(r.Referer()) + rq := stripCRLF(r.URL.RawQuery) + + l.log.Info("http access log", + "read_bytes", bw.read, + "write_bytes", bw.write, + "status", bw.code, + "method", r.Method, + "URI", r.RequestURI, + "URL", r.URL.String(), + "remote_address", r.RemoteAddr, + "query", rq, + "content_len", r.ContentLength, + "host", r.Host, + "user_agent", usrA, + "referer", rfr, + "time_local", start.Format("02/Jan/06:15:04:05 -0700"), + "request_time", start, + "start", start, + "elapsed", time.Since(start).Milliseconds()) } func (l *lm) getW(w http.ResponseWriter) *wrapper { diff --git a/middleware/maxRequest.go b/middleware/maxRequest.go index 6288430..ef0f838 100644 --- a/middleware/maxRequest.go +++ b/middleware/maxRequest.go @@ -6,12 +6,8 @@ import ( func MaxRequestSize(next http.Handler, maxReqSize uint64) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // validating request size - - r2 := r.Clone(r.Context()) - r2.Body = http.MaxBytesReader(w, r2.Body, int64(maxReqSize)) //nolint:gosec - // use max_request_size limit in megabytes - next.ServeHTTP(w, r2) + r.Body = http.MaxBytesReader(w, r.Body, int64(maxReqSize)) //nolint:gosec + next.ServeHTTP(w, r) }) } diff --git a/plugin.go b/plugin.go index 40fdea4..496fe32 100644 --- a/plugin.go +++ b/plugin.go @@ -17,7 +17,6 @@ import ( "github.com/roadrunner-server/http/v6/handler" "github.com/roadrunner-server/http/v6/proxy" "github.com/roadrunner-server/http/v6/servers" - "github.com/roadrunner-server/pool/v2/pool/static_pool" "github.com/roadrunner-server/pool/v2/state/process" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" jprop "go.opentelemetry.io/contrib/propagators/jaeger" @@ -128,12 +127,18 @@ func (p *Plugin) Serve() chan error { p.mu.Lock() defer p.mu.Unlock() - var err error - p.pool, err = p.server.NewPool(context.Background(), p.cfg.Pool, map[string]string{RrMode: RrModeHTTP}, p.log) + // NewPool returns a concrete *static_pool.Pool; assign it to the api.Pool + // interface field only when non-nil so p.pool never holds a typed nil + // (which would make the p.pool != nil guard in Stop spuriously true and + // panic inside Destroy on a nil receiver). + np, err := p.server.NewPool(context.Background(), p.cfg.Pool, map[string]string{RrMode: RrModeHTTP}, p.log) if err != nil { errCh <- err return errCh } + if np != nil { + p.pool = np + } // request queue + worker-facing ConnectRPC server p.queue = proxy.NewQueue(p.cfg.Proxy.InboxSize) @@ -161,14 +166,12 @@ func (p *Plugin) Serve() chan error { p.applyBundledMiddleware() // start all servers - for i := range p.servers { - go func(idx int) { - errSt := p.servers[idx].Serve(p.mdwr, p.cfg.Middleware) - if errSt != nil { + for _, srv := range p.servers { + go func(s servers.InternalServer[any]) { + if errSt := s.Serve(p.mdwr, p.cfg.Middleware); errSt != nil { errCh <- errSt - return } - }(i) + }(srv) } return errCh @@ -201,14 +204,7 @@ func (p *Plugin) Stop(ctx context.Context) error { } if p.pool != nil { - switch pp := p.pool.(type) { - case *static_pool.Pool: - if pp != nil { - pp.Destroy(ctx) - } - default: - // pool is nil, nothing to do - } + p.pool.Destroy(ctx) } doneCh <- struct{}{} @@ -242,8 +238,6 @@ func (p *Plugin) ServeHTTP(w http.ResponseWriter, r *http.Request) { p.handler.ServeHTTP(w, r) p.mu.RUnlock() - _ = r.Body.Close() - if span != nil { span.End() } diff --git a/servers/https/config.go b/servers/https/config.go index 739b96c..e20c429 100644 --- a/servers/https/config.go +++ b/servers/https/config.go @@ -1,11 +1,13 @@ package https import ( + "errors" + "io/fs" "net" "os" "strconv" - "github.com/roadrunner-server/errors" + rrerrors "github.com/roadrunner-server/errors" "github.com/roadrunner-server/http/v6/acme" ) @@ -95,11 +97,11 @@ func (s *SSL) EnableACME() bool { } func (s *SSL) Valid() error { - const op = errors.Op("ssl_valid") + const op = rrerrors.Op("ssl_valid") host, portStr, err := net.SplitHostPort(s.Address) if err != nil { - return errors.E(op, err) + return rrerrors.E(op, err) } if host == "" { s.host = "127.0.0.1" @@ -108,23 +110,23 @@ func (s *SSL) Valid() error { } port, err := strconv.ParseUint(portStr, 10, 16) if err != nil { - return errors.E(op, err) + return rrerrors.E(op, err) } s.Port = int(port) // the user use they own certificates if s.Acme == nil { if _, err := os.Stat(s.Key); err != nil { - if os.IsNotExist(err) { - return errors.E(op, errors.Errorf("key file '%s' does not exists", s.Key)) + if errors.Is(err, fs.ErrNotExist) { + return rrerrors.E(op, rrerrors.Errorf("key file '%s' does not exists", s.Key)) } return err } if _, err := os.Stat(s.Cert); err != nil { - if os.IsNotExist(err) { - return errors.E(op, errors.Errorf("cert file '%s' does not exists", s.Cert)) + if errors.Is(err, fs.ErrNotExist) { + return rrerrors.E(op, rrerrors.Errorf("cert file '%s' does not exists", s.Cert)) } return err @@ -134,8 +136,8 @@ func (s *SSL) Valid() error { // RootCA is optional, but if provided - check it if s.RootCA != "" { if _, err := os.Stat(s.RootCA); err != nil { - if os.IsNotExist(err) { - return errors.E(op, errors.Errorf("root ca path provided, but path '%s' does not exists", s.RootCA)) + if errors.Is(err, fs.ErrNotExist) { + return rrerrors.E(op, rrerrors.Errorf("root ca path provided, but path '%s' does not exists", s.RootCA)) } return err } diff --git a/tlsconf/default_tlsconf.go b/tlsconf/default_tlsconf.go index 3b5d8de..cecad6b 100644 --- a/tlsconf/default_tlsconf.go +++ b/tlsconf/default_tlsconf.go @@ -7,9 +7,6 @@ import ( ) func DefaultTLSConfig() *tls.Config { - var topCipherSuites []uint16 - var defaultCipherSuitesTLS13 []uint16 - hasGCMAsmAMD64 := cpu.X86.HasAES && cpu.X86.HasPCLMULQDQ hasGCMAsmARM64 := cpu.ARM64.HasAES && cpu.ARM64.HasPMULL // Keep in sync with crypto/aes/cipher_s390x.go. @@ -17,10 +14,13 @@ func DefaultTLSConfig() *tls.Config { hasGCMAsm := hasGCMAsmAMD64 || hasGCMAsmARM64 || hasGCMAsmS390X + // CipherSuites only controls TLS 1.0–1.2 suites; Go selects TLS 1.3 suites + // automatically (adding them here is silently ignored). + var cipherSuites []uint16 if hasGCMAsm { // If AES-GCM hardware is provided then priorities AES-GCM // cipher suites. - topCipherSuites = []uint16{ + cipherSuites = []uint16{ tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, @@ -28,15 +28,10 @@ func DefaultTLSConfig() *tls.Config { tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, } - defaultCipherSuitesTLS13 = []uint16{ - tls.TLS_AES_128_GCM_SHA256, - tls.TLS_CHACHA20_POLY1305_SHA256, - tls.TLS_AES_256_GCM_SHA384, - } } else { // Without AES-GCM hardware, we put the ChaCha20-Poly1305 // cipher suites first. - topCipherSuites = []uint16{ + cipherSuites = []uint16{ tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, @@ -44,17 +39,8 @@ func DefaultTLSConfig() *tls.Config { tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, } - defaultCipherSuitesTLS13 = []uint16{ - tls.TLS_CHACHA20_POLY1305_SHA256, - tls.TLS_AES_128_GCM_SHA256, - tls.TLS_AES_256_GCM_SHA384, - } } - defaultCipherSuites := make([]uint16, 0, 22) - defaultCipherSuites = append(defaultCipherSuites, topCipherSuites...) - defaultCipherSuites = append(defaultCipherSuites, defaultCipherSuitesTLS13...) - return &tls.Config{ CurvePreferences: []tls.CurveID{ tls.X25519, @@ -62,7 +48,7 @@ func DefaultTLSConfig() *tls.Config { tls.CurveP384, tls.CurveP521, }, - CipherSuites: defaultCipherSuites, + CipherSuites: cipherSuites, MinVersion: tls.VersionTLS12, } }