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

@@ -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

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

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

docker/prod/root/usr/lib/ImageMagick-7.1.1/config-Q16HDRI/policy.xml

@@ -1,21 +0,0 @@
-<policymap>
-    <policy domain="resource" name="memory" value="256MiB" />
-    <policy domain="resource" name="list-length" value="128" />
-    <policy domain="resource" name="width" value="10KP" />
-    <policy domain="resource" name="height" value="10KP" />
-    <policy domain="resource" name="map" value="512MiB" />
-    <policy domain="resource" name="area" value="40MP" />
-    <policy domain="resource" name="disk" value="1GiB" />
-    <policy domain="resource" name="file" value="768" />
-    <policy domain="resource" name="thread" value="2" />
-    <policy domain="path" rights="none" pattern="@*" />
-    <policy domain="coder" rights="none" pattern="*" />
-    <policy domain="coder" rights="read | write" pattern="{APNG,AVIF,GIF,HEIC,JPEG,JSON,JXL,PNG,WEBP,MP4,WEBM,TMP,PAM}" />
-    <policy domain="delegate" rights="none" pattern="*" />
-    <policy domain="delegate" rights="execute" pattern="ffmpeg" />
-    <policy domain="filter" rights="none" pattern="*" />
-    <policy domain="module" rights="none" pattern="*" />
-    <policy domain="module" rights="read | write" pattern="{APNG,AVIF,GIF,HEIC,JPEG,JSON,JXL,PNG,WEBP,TMP,PAM,PNM,VIDEO}" />
-        <!-- indirect reads not permitted -->
-    <policy domain="system" name="precision" value="6" />
-</policymap>

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}";
         };
       });

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]

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.

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<u16>,
     #[serde(skip_serializing_if = "Option::is_none")]
     max_area: Option<u32>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    memory: Option<u32>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    map: Option<u32>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    disk: Option<u32>,
 }
 
 impl Magick {
     fn set(self) -> Option<Self> {
-        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<u16>,
-    /// 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<u32>,
+    /// 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<u32>,
+    /// 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<u32>,
+    /// 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<u32>,
 
     /// The maximum width, in pixels, for uploaded images
     #[arg(long)]

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,
         }
     }
 }

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)]

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#"<policymap>
-    <policy domain="resource" name="width" value="{width}" />
-    <policy domain="resource" name="height" value="{height}" />
-    <policy domain="resource" name="area" value="{area}" />
+    <policy domain="resource" name="width" value="{width}P" />
+    <policy domain="resource" name="height" value="{height}P" />
+    <policy domain="resource" name="area" value="{area}P" />
+    <policy domain="resource" name="list-length" value="{frames}" />
     <policy domain="resource" name="time" value="{timeout}" />
-    <policy domain="resource" name="memory" value="256MiB" />
-    <policy domain="resource" name="list-length" value="128" />
-    <policy domain="resource" name="map" value="512MiB" />
-    <policy domain="resource" name="disk" value="1GiB" />
+    <policy domain="resource" name="memory" value="{memory}MiB" />
+    <policy domain="resource" name="map" value="{map}MiB" />
+    <policy domain="resource" name="disk" value="{disk}MiB" />
     <policy domain="resource" name="file" value="768" />
     <policy domain="resource" name="thread" value="2" />
+    <policy domain="cache" name="memory-map" value="anonymous"/>
+    <policy domain="cache" name="synchronize" value="true"/>
     <policy domain="path" rights="none" pattern="@*" />
     <policy domain="coder" rights="none" pattern="*" />
     <policy domain="coder" rights="read | write" pattern="{{APNG,AVIF,GIF,HEIC,JPEG,JSON,JXL,PNG,WEBP,MP4,WEBM,TMP,PAM}}" />
     <policy domain="delegate" rights="none" pattern="*" />
-    <policy domain="delegate" rights="execute" pattern="ffmpeg" />
+    <policy domain="delegate" rights="execute" pattern="FFMPEG" />
     <policy domain="filter" rights="none" pattern="*" />
     <policy domain="module" rights="none" pattern="*" />
     <policy domain="module" rights="read | write" pattern="{{APNG,AVIF,GIF,HEIC,JPEG,JSON,JXL,PNG,WEBP,TMP,PAM,PNM,VIDEO}}" />
         <!-- indirect reads not permitted -->
+    <policy domain="system" name="max-memory-request" value="256MiB"/>
+    <policy domain="system" name="memory-map" value="anonymous"/>
     <policy domain="system" name="precision" value="6" />
 </policymap>"#
     )