From 4a7a51d1472b62a8b1647e751328c771bd84f42d Mon Sep 17 00:00:00 2001 From: asonix Date: Mon, 5 Feb 2024 14:30:01 -0600 Subject: [PATCH] Add more imagemagick security configuration Remove default security policy, since it is overridden anyway Update default value for max_area Inherit animation frame count configuration for list-length Add release document --- defaults.toml | 3 + docker/drone/Dockerfile | 1 - docker/prod/docker-compose.yml | 2 +- .../config-Q16HDRI/policy.xml | 21 ----- flake.nix | 2 - pict-rs.toml | 36 +++++++-- releases/0.5.6.md | 76 +++++++++++++++++++ src/config/commandline.rs | 35 ++++++++- src/config/defaults.rs | 8 +- src/config/file.rs | 6 ++ src/magick.rs | 24 ++++-- 11 files changed, 169 insertions(+), 45 deletions(-) delete mode 100644 docker/prod/root/usr/lib/ImageMagick-7.1.1/config-Q16HDRI/policy.xml create mode 100644 releases/0.5.6.md diff --git a/defaults.toml b/defaults.toml index 55b5b50..4b6802b 100644 --- a/defaults.toml +++ b/defaults.toml @@ -47,6 +47,9 @@ proxy = "7d" max_width = 10000 max_height = 10000 max_area = 40000000 +memory = 256 +map = 512 +disk = 1024 [media.image] max_width = 10000 diff --git a/docker/drone/Dockerfile b/docker/drone/Dockerfile index 8b43239..a21b2f0 100644 --- a/docker/drone/Dockerfile +++ b/docker/drone/Dockerfile @@ -8,7 +8,6 @@ RUN \ chown -R app:app /mnt COPY pict-rs /usr/local/bin/pict-rs -COPY docker/prod/root/ / USER app EXPOSE 6669 diff --git a/docker/prod/docker-compose.yml b/docker/prod/docker-compose.yml index e5e40f5..9fc1727 100644 --- a/docker/prod/docker-compose.yml +++ b/docker/prod/docker-compose.yml @@ -2,7 +2,7 @@ version: '3.3' services: pictrs: - image: asonix/pictrs:0.4 + image: asonix/pictrs:0.5 ports: - "127.0.0.1:8080:8080" restart: always diff --git a/docker/prod/root/usr/lib/ImageMagick-7.1.1/config-Q16HDRI/policy.xml b/docker/prod/root/usr/lib/ImageMagick-7.1.1/config-Q16HDRI/policy.xml deleted file mode 100644 index ea9a599..0000000 --- a/docker/prod/root/usr/lib/ImageMagick-7.1.1/config-Q16HDRI/policy.xml +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - diff --git a/flake.nix b/flake.nix index 7c96b05..9224cf2 100644 --- a/flake.nix +++ b/flake.nix @@ -47,8 +47,6 @@ tokio-console ]; - MAGICK_CONFIGURE_PATH = ./docker/prod/root/usr/lib/ImageMagick-7.1.1/config-Q16HDRI; - RUST_SRC_PATH = "${pkgs.rust.packages.stable.rustPlatform.rustLibSrc}"; }; }); diff --git a/pict-rs.toml b/pict-rs.toml index e28fed2..e27990f 100644 --- a/pict-rs.toml +++ b/pict-rs.toml @@ -267,16 +267,36 @@ max_width = 10000 # process the image. max_height = 10000 -## Optional: maximum area, in pixels, of media that imagemagick will attempt to process +## Optional: maximum area, in pixels, of media that imagemagick will keep in memory at a time # environment variable: PICTRS__MEDIA__MAGICK__MAX_HEIGHT -# default: 40_000_000 +# default: 20_000 # -# This value should be at least as large as the greatest max_area set for images, animations, and -# videos. Any image that exceeds this limit will cause imagemagick to abort processing, which could -# lead to less clear errors, especially on image upload. In order for pict-rs to return helpful -# validation errors for images that don't meet other requirements, imagemagick must be allowed to -# process the image. -max_area = 40000000 +# Images that exceed this area will have their pixels cached to disk +max_area = 20000 + +## Optional: maximum size, in MiB, that imagemagick is allowed to allocate in memory to store pixels +## while processing media. +# environment variable: PICTRS__MEDIA__MAGICK__MEMORY +# default: 256 +# +# If this limit is exceeded, imagemagick will fall back to memory-mapped disk for storing pixels +memory = 256 + +## Optional: maximim size, in MiB, that imagemagick is allowed to allocate in memory-mapped disk to +## store pixels while processing media +# environment variable: PICTRS__MEDIA__MAGICK__MAP +# default: 512 +# +# If this limit is exceeded, imagemagick will fall back to unmapped disk for storing pixels +map = 512 + +## Optional: maximum size, in MiB, that imagemagick is allowed to allocate on disk to store pixels +## while processing media +# environment variable: PICTRS__MEDIA__MAGICK__DISK +# default: 1024 +# +# If this limit is exceeded, media processing will be aborted +disk = 1024 [media.image] diff --git a/releases/0.5.6.md b/releases/0.5.6.md new file mode 100644 index 0000000..d948cf6 --- /dev/null +++ b/releases/0.5.6.md @@ -0,0 +1,76 @@ +# pict-rs 0.5.6 + +## Overview + +pict-rs 0.5.6 adds more configuration for imagemagick security policies and updates the default +value for `max_area` + +### Features + +- [More Imagemagick Configuration](#more-imagemagick-configuration) + + +### Changes + +- [Imagemagick Area Defaults](#imagemagick-area-defaults) +- [Imagemagick Frame Configuration](#imagemagick-frame-configuration) + + +## Upgrade Notes + +There's no significant changes from 0.5.5, so upgrading should be as simple as pulling a new version +of pict-rs. + + +## Descriptions + +### More Imagemagick Configuration + +Three new configuration values have been added to the imagemagick security configuration for +pict-rs: `memory`, `map`, and `disk`. These options describe sizes for three tiers of storage that +imagemagick is allowed to use when processing media. The first is `memory`, this is a simple value +that represents how much RAM imagemagick is allowed to use to store image pixels. If this size is +exceeded, it will start using the next tier of storage for image pixels, which is `map`. `map` +represents space on disk that's mapped into RAM for quicker access. Since it's disk-backed, it can +be larger than `memory`. Finally, if `map` is exceeded, imagemagick will start using the `disk` for +storing pixels without mapping into memory. If the `disk` size is exceeded, media processing is +aborted. + +The configuration for these values can be set via the pict-rs.toml file, via environment variables, +or via the commandline. +```toml +# pict-rs.toml +# values are in MiB +[media.magick] +memory = 256 +map = 512 +disk = 1024 +``` +```bash +# environment variables +# values are in MiB +PICTRS__MEDIA__MAGICK__MEMORY=256 +PICTRS__MEDIA__MAGICK__MAP=512 +PICTRS__MEDIA__MAGICK__DISK=1024 +``` +```bash +# commandline +# values are in MiB +pict-rs run \ + --media-magick-memory 256 \ + --media-magick-map 512 \ + --media-magick-disk 1024 +``` + + +### Imagemagick Area Defaults + +The default value for `max_area` has been decreased from 40 million to 20 thousand. The reason for +this is it doesn't impose a hard limit on the area of uploaded images, it instead imposes a limit on +how much of an image can be held in memory at a time, with the rest of the image residing on disk. + + +### Imagemagick Frame Configuration + +Imagemagick now inherits pict-rs' animation `max_frame_count` value to set it's maximum +`list-length`, which should allow longer animations to be configured. diff --git a/src/config/commandline.rs b/src/config/commandline.rs index 528afe2..035ecbd 100644 --- a/src/config/commandline.rs +++ b/src/config/commandline.rs @@ -70,6 +70,9 @@ impl Args { media_magick_max_width, media_magick_max_height, media_magick_max_area, + media_magick_memory, + media_magick_map, + media_magick_disk, media_image_max_width, media_image_max_height, media_image_max_area, @@ -144,6 +147,9 @@ impl Args { max_width: media_magick_max_width, max_height: media_magick_max_height, max_area: media_magick_max_area, + memory: media_magick_memory, + map: media_magick_map, + disk: media_magick_disk, }; let image_quality = ImageQuality { @@ -658,12 +664,22 @@ struct Magick { max_height: Option, #[serde(skip_serializing_if = "Option::is_none")] max_area: Option, + #[serde(skip_serializing_if = "Option::is_none")] + memory: Option, + #[serde(skip_serializing_if = "Option::is_none")] + map: Option, + #[serde(skip_serializing_if = "Option::is_none")] + disk: Option, } impl Magick { fn set(self) -> Option { - let any_set = - self.max_width.is_some() || self.max_height.is_some() || self.max_area.is_some(); + let any_set = self.max_width.is_some() + || self.max_height.is_some() + || self.max_area.is_some() + || self.memory.is_some() + || self.map.is_some() + || self.disk.is_some(); if any_set { Some(self) @@ -1033,9 +1049,22 @@ struct Run { /// The maximum height, in pixels, for uploaded media that imagemagick will attempt to process #[arg(long)] media_magick_max_height: Option, - /// The maximum area, in pixels, for uploaded media that imagemagick will attempt to process + /// The maximum area, in pixels, for uploaded media that imagemagick will keep in memory at a + /// time. Larger images will be cached to disk. #[arg(long)] media_magick_max_area: Option, + /// The maximum size, in MiB, that imagemagick is allowed to use to store pixels in memory. If + /// this limit is exceeded the pixels are moved to memory-mapped disk + #[arg(long)] + media_magick_memory: Option, + /// The maximum size, in MiB, that imagemagick is allowed to use to store pixels in + /// memory-mapped disk. If this limit is exceeded the pixels are moved to unmapped disk + #[arg(long)] + media_magick_map: Option, + /// The maximum size, in MiB, that imagemagick is allowed to use to store pixels on disk. If + /// this limit is exceeded, the media processing will abort. + #[arg(long)] + media_magick_disk: Option, /// The maximum width, in pixels, for uploaded images #[arg(long)] diff --git a/src/config/defaults.rs b/src/config/defaults.rs index ace57f1..aec1d98 100644 --- a/src/config/defaults.rs +++ b/src/config/defaults.rs @@ -96,6 +96,9 @@ struct MagickDefaults { max_width: u16, max_height: u16, max_area: u32, + memory: u32, + map: u32, + disk: u32, } #[derive(Clone, Debug, serde::Serialize)] @@ -287,7 +290,10 @@ impl Default for MagickDefaults { Self { max_width: 10_000, max_height: 10_000, - max_area: 40_000_000, + max_area: 20_000, + memory: 256, + map: 512, + disk: 1024, } } } diff --git a/src/config/file.rs b/src/config/file.rs index 14a909e..43b53bd 100644 --- a/src/config/file.rs +++ b/src/config/file.rs @@ -230,6 +230,12 @@ pub(crate) struct Magick { pub(crate) max_height: u16, pub(crate) max_area: u32, + + pub(crate) memory: u32, + + pub(crate) map: u32, + + pub(crate) disk: u32, } #[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] diff --git a/src/magick.rs b/src/magick.rs index 05200ff..cf4f433 100644 --- a/src/magick.rs +++ b/src/magick.rs @@ -258,29 +258,37 @@ fn generate_policy(media: &Media) -> String { let width = media.magick.max_width; let height = media.magick.max_height; let area = media.magick.max_area; + let memory = media.magick.memory; + let map = media.magick.map; + let disk = media.magick.disk; + let frames = media.animation.max_frame_count; let timeout = media.process_timeout; format!( r#" - - - + + + + - - - - + + + + + - + + + "# )