IMG-56: Security instance (#1524)

* Security instance

* Security -> Checker

config.go

@@ -6,6 +6,7 @@ import (
 	"github.com/imgproxy/imgproxy/v3/fetcher"
 	processinghandler "github.com/imgproxy/imgproxy/v3/handlers/processing"
 	streamhandler "github.com/imgproxy/imgproxy/v3/handlers/stream"
+	"github.com/imgproxy/imgproxy/v3/security"
 	"github.com/imgproxy/imgproxy/v3/server"
 	"github.com/imgproxy/imgproxy/v3/workers"
 )
@@ -24,6 +25,7 @@ type Config struct {
 	Fetcher        fetcher.Config
 	Handlers       HandlerConfigs
 	Server         server.Config
+	Security       security.Config
 }
 
 // NewDefaultConfig creates a new default configuration
@@ -37,7 +39,8 @@ func NewDefaultConfig() Config {
 			Processing: processinghandler.NewDefaultConfig(),
 			Stream:     streamhandler.NewDefaultConfig(),
 		},
-		Server: server.NewDefaultConfig(),
+		Server:   server.NewDefaultConfig(),
+		Security: security.NewDefaultConfig(),
 	}
 }
 
@@ -75,5 +78,9 @@ func LoadConfigFromEnv(c *Config) (*Config, error) {
 		return nil, err
 	}
 
+	if _, err := security.LoadConfigFromEnv(&c.Security); err != nil {
+		return nil, err
+	}
+
 	return c, nil
 }

handlers/processing/handler.go

@@ -25,6 +25,7 @@ type HandlerContext interface {
 	FallbackImage() auximageprovider.Provider
 	WatermarkImage() auximageprovider.Provider
 	ImageDataFactory() *imagedata.Factory
+	Security() *security.Checker
 }
 
 // Handler handles image processing requests
@@ -102,7 +103,7 @@ func (h *Handler) newRequest(
 	}
 
 	// verify the signature (if any)
-	if err = security.VerifySignature(signature, path); err != nil {
+	if err = h.Security().VerifySignature(signature, path); err != nil {
 		return "", nil, nil, ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategorySecurity))
 	}
 
@@ -129,7 +130,7 @@ func (h *Handler) newRequest(
 	monitoring.SetMetadata(ctx, mm)
 
 	// verify that image URL came from the valid source
-	err = security.VerifySourceURL(imageURL)
+	err = h.Security().VerifySourceURL(imageURL)
 	if err != nil {
 		return "", nil, mm, ierrors.Wrap(err, 0, ierrors.WithCategory(handlers.CategorySecurity))
 	}

imagedata/errors.go

@@ -2,11 +2,26 @@ package imagedata
 
 import (
 	"fmt"
+	"net/http"
 
 	"github.com/imgproxy/imgproxy/v3/fetcher"
 	"github.com/imgproxy/imgproxy/v3/ierrors"
 )
 
+type FileSizeError struct{}
+
+func newFileSizeError() error {
+	return ierrors.Wrap(
+		FileSizeError{},
+		1,
+		ierrors.WithStatusCode(http.StatusUnprocessableEntity),
+		ierrors.WithPublicMessage("Invalid source image"),
+		ierrors.WithShouldReport(false),
+	)
+}
+
+func (e FileSizeError) Error() string { return "Source image file is too big" }
+
 func wrapDownloadError(err error, desc string) error {
 	return ierrors.Wrap(
 		fetcher.WrapError(err), 0,

imagedata/factory.go

@@ -11,7 +11,6 @@ import (
 	"github.com/imgproxy/imgproxy/v3/asyncbuffer"
 	"github.com/imgproxy/imgproxy/v3/fetcher"
 	"github.com/imgproxy/imgproxy/v3/imagetype"
-	"github.com/imgproxy/imgproxy/v3/security"
 )
 
 // Factory represents ImageData factory
@@ -101,7 +100,7 @@ func (f *Factory) sendRequest(ctx context.Context, url string, opts DownloadOpti
 		return req, nil, h, err
 	}
 
-	res, err = security.LimitResponseSize(res, opts.MaxSrcFileSize)
+	res, err = limitResponseSize(res, opts.MaxSrcFileSize)
 	if err != nil {
 		if res != nil {
 			res.Body.Close()

security/response_limit.go → imagedata/response_limit.go

@@ -1,4 +1,4 @@
-package security
+package imagedata
 
 import (
 	"io"
@@ -28,11 +28,11 @@ func (lr *hardLimitReadCloser) Close() error {
 	return lr.r.Close()
 }
 
-// LimitResponseSize limits the size of the response body to MaxSrcFileSize (if set).
+// limitResponseSize limits the size of the response body to MaxSrcFileSize (if set).
 // First, it tries to use Content-Length header to check the limit.
 // If Content-Length is not set, it limits the size of the response body by wrapping
 // body reader with hard limit reader.
-func LimitResponseSize(r *http.Response, limit int) (*http.Response, error) {
+func limitResponseSize(r *http.Response, limit int) (*http.Response, error) {
 	if limit == 0 {
 		return r, nil
 	}

imgproxy.go

@@ -14,6 +14,7 @@ import (
 	"github.com/imgproxy/imgproxy/v3/imagedata"
 	"github.com/imgproxy/imgproxy/v3/memory"
 	"github.com/imgproxy/imgproxy/v3/monitoring/prometheus"
+	"github.com/imgproxy/imgproxy/v3/security"
 	"github.com/imgproxy/imgproxy/v3/server"
 	"github.com/imgproxy/imgproxy/v3/workers"
 )
@@ -39,6 +40,7 @@ type Imgproxy struct {
 	fetcher          *fetcher.Fetcher
 	imageDataFactory *imagedata.Factory
 	handlers         ImgproxyHandlers
+	security         *security.Checker
 	config           *Config
 }
 
@@ -66,6 +68,11 @@ func New(ctx context.Context, config *Config) (*Imgproxy, error) {
 		return nil, err
 	}
 
+	security, err := security.New(&config.Security)
+	if err != nil {
+		return nil, err
+	}
+
 	imgproxy := &Imgproxy{
 		workers:          workers,
 		fallbackImage:    fallbackImage,
@@ -73,6 +80,7 @@ func New(ctx context.Context, config *Config) (*Imgproxy, error) {
 		fetcher:          fetcher,
 		imageDataFactory: idf,
 		config:           config,
+		security:         security,
 	}
 
 	imgproxy.handlers.Health = healthhandler.New()
@@ -187,3 +195,7 @@ func (i *Imgproxy) WatermarkImage() auximageprovider.Provider {
 func (i *Imgproxy) ImageDataFactory() *imagedata.Factory {
 	return i.imageDataFactory
 }
+
+func (i *Imgproxy) Security() *security.Checker {
+	return i.security
+}

options/processing_options.go

@@ -2,6 +2,7 @@ package options
 
 import (
 	"encoding/base64"
+	"fmt"
 	"net/http"
 	"slices"
 	"strconv"
@@ -120,6 +121,31 @@ type ProcessingOptions struct {
 }
 
 func NewProcessingOptions() *ProcessingOptions {
+	// NOTE: This is temporary hack until ProcessingOptions does not have Factory
+	securityCfg, err := security.LoadConfigFromEnv(nil)
+	if err != nil {
+		fmt.Println(err)
+	}
+
+	// NOTE: This is a temporary workaround for logrus bug that deadlocks
+	// if log is used within another log (issue 1448)
+	if len(securityCfg.Salts) == 0 {
+		securityCfg.Salts = [][]byte{[]byte("logrusbugworkaround")}
+	}
+
+	if len(securityCfg.Keys) == 0 {
+		securityCfg.Keys = [][]byte{[]byte("logrusbugworkaround")}
+	}
+	// END OF WORKAROUND
+
+	security, err := security.New(securityCfg)
+	if err != nil {
+		fmt.Println(err)
+	}
+
+	securityOptions := security.NewOptions()
+	// NOTE: This is temporary hack until ProcessingOptions does not have Factory
+
 	po := ProcessingOptions{
 		ResizingType:      ResizeFit,
 		Width:             0,
@@ -151,7 +177,7 @@ func NewProcessingOptions() *ProcessingOptions {
 		SkipProcessingFormats: append([]imagetype.Type(nil), config.SkipProcessingFormats...),
 		UsedPresets:           make([]string, 0, len(config.Presets)),
 
-		SecurityOptions: security.DefaultOptions(),
+		SecurityOptions: securityOptions,
 
 		// Basically, we need this to update ETag when `IMGPROXY_QUALITY` is changed
 		defaultQuality: config.Quality,

processing/processing.go

@@ -215,7 +215,7 @@ func checkImageSize(
 		return width, height, nil
 	}
 
-	err := security.CheckDimensions(width, height, frames, secops)
+	err := secops.CheckDimensions(width, height, frames)
 
 	return width, height, err
 }

security/checker.go

@@ -0,0 +1,22 @@
+package security
+
+// Checker represents the security package instance
+type Checker struct {
+	config *Config
+}
+
+// New creates a new Security instance
+func New(config *Config) (*Checker, error) {
+	if err := config.Validate(); err != nil {
+		return nil, err
+	}
+
+	return &Checker{
+		config: config,
+	}, nil
+}
+
+// NewOptions creates a new security.Options instance
+func (s *Checker) NewOptions() Options {
+	return s.config.DefaultOptions
+}

security/config.go

@@ -0,0 +1,90 @@
+package security
+
+import (
+	"fmt"
+	"regexp"
+
+	"github.com/imgproxy/imgproxy/v3/config"
+	"github.com/imgproxy/imgproxy/v3/ensure"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// Config is the package-local configuration
+type Config struct {
+	AllowSecurityOptions bool             // Whether to allow security-related processing options in URLs
+	AllowedSources       []*regexp.Regexp // List of allowed source URL patterns (empty = allow all)
+	Keys                 [][]byte         // List of the HMAC keys
+	Salts                [][]byte         // List of the HMAC salts
+	SignatureSize        int              // Size of the HMAC signature in bytes
+	TrustedSignatures    []string         // List of trusted signature sources
+	DefaultOptions       Options          // Default security options
+}
+
+// NewDefaultConfig returns a new Config instance with default values.
+func NewDefaultConfig() Config {
+	return Config{
+		DefaultOptions: Options{
+			MaxSrcResolution:            50000000,
+			MaxSrcFileSize:              0,
+			MaxAnimationFrames:          1,
+			MaxAnimationFrameResolution: 0,
+			MaxResultDimension:          0,
+		},
+		AllowSecurityOptions: false,
+		SignatureSize:        32,
+	}
+}
+
+// LoadConfigFromEnv overrides configuration variables from environment
+func LoadConfigFromEnv(c *Config) (*Config, error) {
+	c = ensure.Ensure(c, NewDefaultConfig)
+
+	c.AllowSecurityOptions = config.AllowSecurityOptions
+	c.AllowedSources = config.AllowedSources
+	c.Keys = config.Keys
+	c.Salts = config.Salts
+	c.SignatureSize = config.SignatureSize
+	c.TrustedSignatures = config.TrustedSignatures
+
+	c.DefaultOptions.MaxSrcResolution = config.MaxSrcResolution
+	c.DefaultOptions.MaxSrcFileSize = config.MaxSrcFileSize
+	c.DefaultOptions.MaxAnimationFrames = config.MaxAnimationFrames
+	c.DefaultOptions.MaxAnimationFrameResolution = config.MaxAnimationFrameResolution
+	c.DefaultOptions.MaxResultDimension = config.MaxResultDimension
+
+	return c, nil
+}
+
+// Validate validates the configuration
+func (c *Config) Validate() error {
+	if c.DefaultOptions.MaxSrcResolution <= 0 {
+		return fmt.Errorf("max src resolution should be greater than 0, now - %d", c.DefaultOptions.MaxSrcResolution)
+	}
+
+	if c.DefaultOptions.MaxSrcFileSize < 0 {
+		return fmt.Errorf("max src file size should be greater than or equal to 0, now - %d", c.DefaultOptions.MaxSrcFileSize)
+	}
+
+	if c.DefaultOptions.MaxAnimationFrames <= 0 {
+		return fmt.Errorf("max animation frames should be greater than 0, now - %d", c.DefaultOptions.MaxAnimationFrames)
+	}
+
+	if len(c.Keys) != len(c.Salts) {
+		return fmt.Errorf("number of keys and number of salts should be equal. Keys: %d, salts: %d", len(c.Keys), len(c.Salts))
+	}
+
+	if len(c.Keys) == 0 {
+		log.Warning("No keys defined, so signature checking is disabled")
+	}
+
+	if len(c.Salts) == 0 {
+		log.Warning("No salts defined, so signature checking is disabled")
+	}
+
+	if c.SignatureSize < 1 || c.SignatureSize > 32 {
+		return fmt.Errorf("signature size should be within 1 and 32, now - %d", c.SignatureSize)
+	}
+
+	return nil
+}

security/errors.go

@@ -9,7 +9,6 @@ import (
 
 type (
 	SignatureError       string
-	FileSizeError        struct{}
 	ImageResolutionError string
 	SecurityOptionsError struct{}
 	SourceURLError       string
@@ -27,18 +26,6 @@ func newSignatureError(msg string) error {
 
 func (e SignatureError) Error() string { return string(e) }
 
-func newFileSizeError() error {
-	return ierrors.Wrap(
-		FileSizeError{},
-		1,
-		ierrors.WithStatusCode(http.StatusUnprocessableEntity),
-		ierrors.WithPublicMessage("Invalid source image"),
-		ierrors.WithShouldReport(false),
-	)
-}
-
-func (e FileSizeError) Error() string { return "Source image file is too big" }
-
 func newImageResolutionError(msg string) error {
 	return ierrors.Wrap(
 		ImageResolutionError(msg),

security/image_size.go

@@ -1,17 +0,0 @@
-package security
-
-func CheckDimensions(width, height, frames int, opts Options) error {
-	frames = max(frames, 1)
-
-	if frames > 1 && opts.MaxAnimationFrameResolution > 0 {
-		if width*height > opts.MaxAnimationFrameResolution {
-			return newImageResolutionError("Source image frame resolution is too big")
-		}
-	} else {
-		if width*height*frames > opts.MaxSrcResolution {
-			return newImageResolutionError("Source image resolution is too big")
-		}
-	}
-
-	return nil
-}

security/options.go

@@ -4,6 +4,7 @@ import (
 	"github.com/imgproxy/imgproxy/v3/config"
 )
 
+// Security options (part of processing options)
 type Options struct {
 	MaxSrcResolution            int
 	MaxSrcFileSize              int
@@ -12,16 +13,7 @@ type Options struct {
 	MaxResultDimension          int
 }
 
-func DefaultOptions() Options {
-	return Options{
-		MaxSrcResolution:            config.MaxSrcResolution,
-		MaxSrcFileSize:              config.MaxSrcFileSize,
-		MaxAnimationFrames:          config.MaxAnimationFrames,
-		MaxAnimationFrameResolution: config.MaxAnimationFrameResolution,
-		MaxResultDimension:          config.MaxResultDimension,
-	}
-}
-
+// NOTE: This function is a part of processing option, we'll move it in the next PR
 func IsSecurityOptionsAllowed() error {
 	if config.AllowSecurityOptions {
 		return nil
@@ -29,3 +21,20 @@ func IsSecurityOptionsAllowed() error {
 
 	return newSecurityOptionsError()
 }
+
+// CheckDimensions checks if the given dimensions are within the allowed limits
+func (o *Options) CheckDimensions(width, height, frames int) error {
+	frames = max(frames, 1)
+
+	if frames > 1 && o.MaxAnimationFrameResolution > 0 {
+		if width*height > o.MaxAnimationFrameResolution {
+			return newImageResolutionError("Source image frame resolution is too big")
+		}
+	} else {
+		if width*height*frames > o.MaxSrcResolution {
+			return newImageResolutionError("Source image resolution is too big")
+		}
+	}
+
+	return nil
+}

security/signature.go

@@ -5,16 +5,14 @@ import (
 	"crypto/sha256"
 	"encoding/base64"
 	"slices"
-
-	"github.com/imgproxy/imgproxy/v3/config"
 )
 
-func VerifySignature(signature, path string) error {
-	if len(config.Keys) == 0 || len(config.Salts) == 0 {
+func (s *Checker) VerifySignature(signature, path string) error {
+	if len(s.config.Keys) == 0 || len(s.config.Salts) == 0 {
 		return nil
 	}
 
-	if slices.Contains(config.TrustedSignatures, signature) {
+	if slices.Contains(s.config.TrustedSignatures, signature) {
 		return nil
 	}
 
@@ -23,8 +21,8 @@ func VerifySignature(signature, path string) error {
 		return newSignatureError("Invalid signature encoding")
 	}
 
-	for i := 0; i < len(config.Keys); i++ {
-		if hmac.Equal(messageMAC, signatureFor(path, config.Keys[i], config.Salts[i], config.SignatureSize)) {
+	for i := 0; i < len(s.config.Keys); i++ {
+		if hmac.Equal(messageMAC, signatureFor(path, s.config.Keys[i], s.config.Salts[i], s.config.SignatureSize)) {
 			return nil
 		}
 	}

security/signature_test.go

@@ -6,60 +6,78 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/imgproxy/imgproxy/v3/config"
+	"github.com/imgproxy/imgproxy/v3/testutil"
 )
 
 type SignatureTestSuite struct {
-	suite.Suite
+	testutil.LazySuite
+
+	config   testutil.LazyObj[*Config]
+	security testutil.LazyObj[*Checker]
+}
+
+func (s *SignatureTestSuite) SetupSuite() {
+	s.config, _ = testutil.NewLazySuiteObj(
+		s,
+		func() (*Config, error) {
+			c := NewDefaultConfig()
+			return &c, nil
+		},
+	)
+
+	s.security, _ = testutil.NewLazySuiteObj(
+		s,
+		func() (*Checker, error) {
+			return New(s.config())
+		},
+	)
 }
 
 func (s *SignatureTestSuite) SetupTest() {
 	config.Reset()
 
-	config.Keys = [][]byte{[]byte("test-key")}
-	config.Salts = [][]byte{[]byte("test-salt")}
+	s.config().Keys = [][]byte{[]byte("test-key")}
+	s.config().Salts = [][]byte{[]byte("test-salt")}
 }
 
 func (s *SignatureTestSuite) TestVerifySignature() {
-	err := VerifySignature("oWaL7QoW5TsgbuiS9-5-DI8S3Ibbo1gdB2SteJh3a20", "asd")
+	err := s.security().VerifySignature("oWaL7QoW5TsgbuiS9-5-DI8S3Ibbo1gdB2SteJh3a20", "asd")
 	s.Require().NoError(err)
 }
 
 func (s *SignatureTestSuite) TestVerifySignatureTruncated() {
-	config.SignatureSize = 8
+	s.config().SignatureSize = 8
 
-	err := VerifySignature("oWaL7QoW5Ts", "asd")
+	err := s.security().VerifySignature("oWaL7QoW5Ts", "asd")
 	s.Require().NoError(err)
 }
 
 func (s *SignatureTestSuite) TestVerifySignatureInvalid() {
-	err := VerifySignature("oWaL7QoW5Ts", "asd")
+	err := s.security().VerifySignature("oWaL7QoW5Ts", "asd")
 	s.Require().Error(err)
 }
 
 func (s *SignatureTestSuite) TestVerifySignatureMultiplePairs() {
-	config.Keys = append(config.Keys, []byte("test-key2"))
-	config.Salts = append(config.Salts, []byte("test-salt2"))
+	s.config().Keys = append(s.config().Keys, []byte("test-key2"))
+	s.config().Salts = append(s.config().Salts, []byte("test-salt2"))
 
-	err := VerifySignature("jYz1UZ7j1BCdSzH3pZhaYf0iuz0vusoOTdqJsUT6WXI", "asd")
+	err := s.security().VerifySignature("jYz1UZ7j1BCdSzH3pZhaYf0iuz0vusoOTdqJsUT6WXI", "asd")
 	s.Require().NoError(err)
 
-	err = VerifySignature("oWaL7QoW5TsgbuiS9-5-DI8S3Ibbo1gdB2SteJh3a20", "asd")
+	err = s.security().VerifySignature("oWaL7QoW5TsgbuiS9-5-DI8S3Ibbo1gdB2SteJh3a20", "asd")
 	s.Require().NoError(err)
 
-	err = VerifySignature("dtLwhdnPPis", "asd")
+	err = s.security().VerifySignature("dtLwhdnPPis", "asd")
 	s.Require().Error(err)
 }
 
 func (s *SignatureTestSuite) TestVerifySignatureTrusted() {
-	config.TrustedSignatures = []string{"truested"}
-	defer func() {
-		config.TrustedSignatures = []string{}
-	}()
+	s.config().TrustedSignatures = []string{"truested"}
 
-	err := VerifySignature("truested", "asd")
+	err := s.security().VerifySignature("truested", "asd")
 	s.Require().NoError(err)
 
-	err = VerifySignature("untrusted", "asd")
+	err = s.security().VerifySignature("untrusted", "asd")
 	s.Require().Error(err)
 }
 

security/source.go

@@ -1,15 +1,13 @@
 package security
 
-import (
-	"github.com/imgproxy/imgproxy/v3/config"
-)
-
-func VerifySourceURL(imageURL string) error {
-	if len(config.AllowedSources) == 0 {
+// VerifySourceURL checks if the given imageURL is allowed based on
+// the configured AllowedSources.
+func (s *Checker) VerifySourceURL(imageURL string) error {
+	if len(s.config.AllowedSources) == 0 {
 		return nil
 	}
 
-	for _, allowedSource := range config.AllowedSources {
+	for _, allowedSource := range s.config.AllowedSources {
 		if allowedSource.MatchString(imageURL) {
 			return nil
 		}