diff --git a/handlers/processing/handler.go b/handlers/processing/handler.go index 3cd57da6..8cbb29d3 100644 --- a/handlers/processing/handler.go +++ b/handlers/processing/handler.go @@ -25,7 +25,7 @@ type HandlerContext interface { FallbackImage() auximageprovider.Provider WatermarkImage() auximageprovider.Provider ImageDataFactory() *imagedata.Factory - Security() *security.Security + Security() *security.Checker } // Handler handles image processing requests diff --git a/imagedata/errors.go b/imagedata/errors.go index c89edb95..77afdae4 100644 --- a/imagedata/errors.go +++ b/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, diff --git a/imagedata/factory.go b/imagedata/factory.go index 525af107..79244f9b 100644 --- a/imagedata/factory.go +++ b/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() diff --git a/security/response_limit.go b/imagedata/response_limit.go similarity index 87% rename from security/response_limit.go rename to imagedata/response_limit.go index 4763ff8c..728c552e 100644 --- a/security/response_limit.go +++ b/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 } diff --git a/imgproxy.go b/imgproxy.go index 3f13e129..c66eeaa1 100644 --- a/imgproxy.go +++ b/imgproxy.go @@ -40,7 +40,7 @@ type Imgproxy struct { fetcher *fetcher.Fetcher imageDataFactory *imagedata.Factory handlers ImgproxyHandlers - security *security.Security + security *security.Checker config *Config } @@ -196,6 +196,6 @@ func (i *Imgproxy) ImageDataFactory() *imagedata.Factory { return i.imageDataFactory } -func (i *Imgproxy) Security() *security.Security { +func (i *Imgproxy) Security() *security.Checker { return i.security } diff --git a/options/processing_options.go b/options/processing_options.go index 6406ff8a..ccbb7b0f 100644 --- a/options/processing_options.go +++ b/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, diff --git a/security/checker.go b/security/checker.go new file mode 100644 index 00000000..ee4270f2 --- /dev/null +++ b/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 +} diff --git a/security/config.go b/security/config.go index 02ae1199..e44b24c6 100644 --- a/security/config.go +++ b/security/config.go @@ -6,72 +6,31 @@ import ( "github.com/imgproxy/imgproxy/v3/config" "github.com/imgproxy/imgproxy/v3/ensure" + log "github.com/sirupsen/logrus" ) -// OptionsConfig represents the configuration for processing limits and security options -type OptionsConfig struct { - MaxSrcResolution int // Maximum allowed source image resolution (width × height) - MaxSrcFileSize int // Maximum allowed source file size in bytes (0 = unlimited) - MaxAnimationFrames int // Maximum number of frames allowed in animated images - MaxAnimationFrameResolution int // Maximum resolution allowed for each frame in animated images (0 = unlimited) - MaxResultDimension int // Maximum allowed dimension (width or height) for result images (0 = unlimited) -} - -// NewDefaultOptionsConfig returns a new OptionsConfig instance with default values -func NewDefaultOptionsConfig() OptionsConfig { - return OptionsConfig{ - MaxSrcResolution: 50000000, - MaxSrcFileSize: 0, - MaxAnimationFrames: 1, - MaxAnimationFrameResolution: 0, - MaxResultDimension: 0, - } -} - -// LoadOptionsConfigFromEnv loads OptionsConfig from global config variables -func LoadOptionsConfigFromEnv(c *OptionsConfig) (*OptionsConfig, error) { - c.MaxSrcResolution = config.MaxSrcResolution - c.MaxSrcFileSize = config.MaxSrcFileSize - c.MaxAnimationFrames = config.MaxAnimationFrames - c.MaxAnimationFrameResolution = config.MaxAnimationFrameResolution - c.MaxResultDimension = config.MaxResultDimension - - return c, nil -} - -// Validate validates the OptionsConfig values -func (c *OptionsConfig) Validate() error { - if c.MaxSrcResolution <= 0 { - return fmt.Errorf("max src resolution should be greater than 0, now - %d", c.MaxSrcResolution) - } - - if c.MaxSrcFileSize < 0 { - return fmt.Errorf("max src file size should be greater than or equal to 0, now - %d", c.MaxSrcFileSize) - } - - if c.MaxAnimationFrames <= 0 { - return fmt.Errorf("max animation frames should be greater than 0, now - %d", c.MaxAnimationFrames) - } - - return nil -} - // Config is the package-local configuration type Config struct { - Options OptionsConfig // Processing limits and security options 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{ - Options: NewDefaultOptionsConfig(), + DefaultOptions: Options{ + MaxSrcResolution: 50000000, + MaxSrcFileSize: 0, + MaxAnimationFrames: 1, + MaxAnimationFrameResolution: 0, + MaxResultDimension: 0, + }, AllowSecurityOptions: false, SignatureSize: 32, } @@ -81,10 +40,6 @@ func NewDefaultConfig() Config { func LoadConfigFromEnv(c *Config) (*Config, error) { c = ensure.Ensure(c, NewDefaultConfig) - if _, err := LoadOptionsConfigFromEnv(&c.Options); err != nil { - return nil, err - } - c.AllowSecurityOptions = config.AllowSecurityOptions c.AllowedSources = config.AllowedSources c.Keys = config.Keys @@ -92,13 +47,27 @@ func LoadConfigFromEnv(c *Config) (*Config, error) { 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 err := c.Options.Validate(); err != nil { - return err + 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) { diff --git a/security/errors.go b/security/errors.go index 3754c26f..929297e7 100644 --- a/security/errors.go +++ b/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), diff --git a/security/options.go b/security/options.go index 98df2211..33a6f5de 100644 --- a/security/options.go +++ b/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,18 +13,7 @@ type Options struct { MaxResultDimension int } -// NOTE: Remove this function in imgproxy v4 -// TODO: Replace this with security.NewOptions() when ProcessingOptions gets config -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 diff --git a/security/security.go b/security/security.go deleted file mode 100644 index 8b347d8f..00000000 --- a/security/security.go +++ /dev/null @@ -1,28 +0,0 @@ -package security - -// Security represents the security package instance -type Security struct { - config *Config -} - -// New creates a new Security instance -func New(config *Config) (*Security, error) { - if err := config.Validate(); err != nil { - return nil, err - } - - return &Security{ - config: config, - }, nil -} - -// NewOptions creates a new security.Options instance -func (s *Security) NewOptions() Options { - return Options{ - MaxSrcResolution: s.config.Options.MaxSrcResolution, - MaxSrcFileSize: s.config.Options.MaxSrcFileSize, - MaxAnimationFrames: s.config.Options.MaxAnimationFrames, - MaxAnimationFrameResolution: s.config.Options.MaxAnimationFrameResolution, - MaxResultDimension: s.config.Options.MaxResultDimension, - } -} diff --git a/security/signature.go b/security/signature.go index 7ebf35af..13d9b787 100644 --- a/security/signature.go +++ b/security/signature.go @@ -7,7 +7,7 @@ import ( "slices" ) -func (s *Security) VerifySignature(signature, path string) error { +func (s *Checker) VerifySignature(signature, path string) error { if len(s.config.Keys) == 0 || len(s.config.Salts) == 0 { return nil } diff --git a/security/signature_test.go b/security/signature_test.go index df0b1fd2..6fda9f09 100644 --- a/security/signature_test.go +++ b/security/signature_test.go @@ -13,7 +13,7 @@ type SignatureTestSuite struct { testutil.LazySuite config testutil.LazyObj[*Config] - security testutil.LazyObj[*Security] + security testutil.LazyObj[*Checker] } func (s *SignatureTestSuite) SetupSuite() { @@ -27,7 +27,7 @@ func (s *SignatureTestSuite) SetupSuite() { s.security, _ = testutil.NewLazySuiteObj( s, - func() (*Security, error) { + func() (*Checker, error) { return New(s.config()) }, ) diff --git a/security/source.go b/security/source.go index 4337ea09..276aa656 100644 --- a/security/source.go +++ b/security/source.go @@ -2,7 +2,7 @@ package security // VerifySourceURL checks if the given imageURL is allowed based on // the configured AllowedSources. -func (s *Security) VerifySourceURL(imageURL string) error { +func (s *Checker) VerifySourceURL(imageURL string) error { if len(s.config.AllowedSources) == 0 { return nil }