Isolate PHP spool reader offsets

eldryoth · eldryoth · commit b9c51937c794 · 2026-07-11T13:39:51.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,7 +29,8 @@ behavior when the change improves security or project direction.
 - Document that the opt-in clock capability exposes full host clock resolution
   and is unsuitable for untrusted plugins colocated with secret-dependent work.
 - Bound complete FastCGI request/response operations with one deadline, retain
-  anonymous request-body spool descriptors, and fix the managed child `PATH`.
+  anonymous request-body spool descriptors with independent positional reader
+  offsets, and fix the managed child `PATH`.
 - Bind managed PHP-FPM spawn to a trusted executable descriptor and terminate
   its complete dedicated process group during shutdown and watchdog recovery.
 
diff --git a/crates/fluxheim-php-fpm/src/request_body.rs b/crates/fluxheim-php-fpm/src/request_body.rs
@@ -1,11 +1,15 @@
 use std::io;
 use std::path::{Path, PathBuf};
+use std::pin::Pin;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicUsize, Ordering};
+use std::task::{Context, Poll};
 
 use zeroize::Zeroizing;
 
 static PHP_REQUEST_BODY_SPOOL_COUNTER: AtomicUsize = AtomicUsize::new(0);
+const PHP_SPOOL_POSITIONAL_READ_BYTES: usize = 64 * 1024;
+type PhpSpoolReadTask = tokio::task::JoinHandle<io::Result<(Vec<u8>, usize)>>;
 
 pub struct PhpRequestBody {
     inner: Arc<PhpRequestBodyInner>,
@@ -18,7 +22,15 @@ enum PhpRequestBodyInner {
 }
 
 struct PhpRequestBodySpool {
-    file: std::fs::File,
+    file: Arc<std::fs::File>,
+}
+
+struct PhpSpoolReader {
+    file: Arc<std::fs::File>,
+    offset: u64,
+    pending: Option<PhpSpoolReadTask>,
+    ready: Vec<u8>,
+    ready_start: usize,
 }
 
 impl PhpRequestBody {
@@ -37,7 +49,9 @@ impl PhpRequestBody {
         let file = file.into_std().await;
         Ok(Self {
             len,
-            inner: Arc::new(PhpRequestBodyInner::Spool(PhpRequestBodySpool { file })),
+            inner: Arc::new(PhpRequestBodyInner::Spool(PhpRequestBodySpool {
+                file: Arc::new(file),
+            })),
         })
     }
 
@@ -56,16 +70,89 @@ impl PhpRequestBody {
             PhpRequestBodyInner::Memory(body) => {
                 Ok(Box::new(fastcgi_client::io::Cursor::new(body.clone())))
             }
-            PhpRequestBodyInner::Spool(spool) => {
-                use std::io::{Seek as _, SeekFrom};
-
-                let mut file = spool.file.try_clone()?;
-                file.seek(SeekFrom::Start(0))?;
-                let file = tokio::fs::File::from_std(file);
-                Ok(Box::new(
-                    fastcgi_client::io::TokioAsyncReadCompatExt::compat(file),
-                ))
+            PhpRequestBodyInner::Spool(spool) => Ok(Box::new(PhpSpoolReader {
+                file: Arc::clone(&spool.file),
+                offset: 0,
+                pending: None,
+                ready: Vec::new(),
+                ready_start: 0,
+            })),
+        }
+    }
+}
+
+impl fastcgi_client::io::AsyncRead for PhpSpoolReader {
+    fn poll_read(
+        self: Pin<&mut Self>,
+        context: &mut Context<'_>,
+        buffer: &mut [u8],
+    ) -> Poll<io::Result<usize>> {
+        use std::future::Future as _;
+
+        let this = self.get_mut();
+        if buffer.is_empty() {
+            return Poll::Ready(Ok(0));
+        }
+        loop {
+            if this.ready_start < this.ready.len() {
+                let available = &this.ready[this.ready_start..];
+                let copied = available.len().min(buffer.len());
+                buffer[..copied].copy_from_slice(&available[..copied]);
+                this.ready_start += copied;
+                if this.ready_start == this.ready.len() {
+                    this.ready.clear();
+                    this.ready_start = 0;
+                }
+                return Poll::Ready(Ok(copied));
+            }
+            if let Some(pending) = this.pending.as_mut() {
+                let result = match Pin::new(pending).poll(context) {
+                    Poll::Pending => return Poll::Pending,
+                    Poll::Ready(result) => result,
+                };
+                this.pending = None;
+                let (bytes, read) = result.map_err(|error| {
+                    io::Error::other(format!("PHP spool read task failed: {error}"))
+                })??;
+                if read > bytes.len() {
+                    return Poll::Ready(Err(io::Error::new(
+                        io::ErrorKind::InvalidData,
+                        "PHP spool positional read returned an invalid length",
+                    )));
+                }
+                let read_len = read;
+                let Ok(read) = u64::try_from(read_len) else {
+                    return Poll::Ready(Err(io::Error::new(
+                        io::ErrorKind::InvalidData,
+                        "PHP spool positional read length is not representable",
+                    )));
+                };
+                let Some(next_offset) = this.offset.checked_add(read) else {
+                    return Poll::Ready(Err(io::Error::new(
+                        io::ErrorKind::InvalidData,
+                        "PHP spool read offset overflow",
+                    )));
+                };
+                this.offset = next_offset;
+                this.ready = bytes;
+                this.ready.truncate(read_len);
+                this.ready_start = 0;
+                if read_len == 0 {
+                    return Poll::Ready(Ok(0));
+                }
+                continue;
             }
+
+            let file = Arc::clone(&this.file);
+            let offset = this.offset;
+            let read_len = buffer.len().min(PHP_SPOOL_POSITIONAL_READ_BYTES);
+            this.pending = Some(tokio::task::spawn_blocking(move || {
+                use std::os::unix::fs::FileExt as _;
+
+                let mut bytes = vec![0_u8; read_len];
+                let read = file.read_at(&mut bytes, offset)?;
+                Ok((bytes, read))
+            }));
         }
     }
 }
diff --git a/crates/fluxheim-php-fpm/src/tests_io_policy.rs b/crates/fluxheim-php-fpm/src/tests_io_policy.rs
@@ -85,6 +85,69 @@ fn php_request_body_spool_replays_and_cleans_up_file() {
     std::fs::remove_dir(&spool_dir).expect("remove spool dir");
 }
 
+#[test]
+fn php_request_body_spool_readers_keep_independent_offsets() {
+    let spool_dir = fluxheim_common::test_support::unique_temp_path("php-fpm-spool-offsets");
+    std::fs::create_dir_all(&spool_dir).expect("spool dir");
+    let runtime = tokio::runtime::Builder::new_current_thread()
+        .enable_io()
+        .build()
+        .expect("test runtime");
+    let expected = (0..(PHP_TEST_SPOOL_BYTES + 17))
+        .map(|index| (index % 251) as u8)
+        .collect::<Vec<_>>();
+    let mut file = runtime
+        .block_on(create_php_request_body_spool_file(&spool_dir))
+        .expect("create spool file");
+    runtime.block_on(async {
+        use tokio::io::AsyncWriteExt as _;
+
+        file.write_all(&expected).await.expect("write spool");
+        file.flush().await.expect("flush spool");
+    });
+    let body = runtime
+        .block_on(PhpRequestBody::spooled(file, expected.len()))
+        .expect("retain anonymous spool file");
+    let mut first = runtime.block_on(body.reader()).expect("first reader");
+    let mut second = runtime.block_on(body.reader()).expect("second reader");
+    let mut first_prefix = vec![0_u8; 1_003];
+    let mut second_prefix = vec![0_u8; 70_001];
+    let mut first_rest = Vec::new();
+    let mut second_rest = Vec::new();
+
+    runtime.block_on(async {
+        use fastcgi_client::io::AsyncReadExt as _;
+
+        first
+            .read_exact(&mut first_prefix)
+            .await
+            .expect("first prefix");
+        second
+            .read_exact(&mut second_prefix)
+            .await
+            .expect("second prefix");
+        first
+            .read_to_end(&mut first_rest)
+            .await
+            .expect("first remainder");
+        second
+            .read_to_end(&mut second_rest)
+            .await
+            .expect("second remainder");
+    });
+
+    first_prefix.extend(first_rest);
+    second_prefix.extend(second_rest);
+    assert_eq!(first_prefix, expected);
+    assert_eq!(second_prefix, expected);
+    drop(first);
+    drop(second);
+    drop(body);
+    std::fs::remove_dir(&spool_dir).expect("remove spool dir");
+}
+
+const PHP_TEST_SPOOL_BYTES: usize = 128 * 1024;
+
 #[test]
 fn php_fpm_stream_chunk_limit_counts_stdout_and_stderr() {
     let mut total = 0;
diff --git a/docs/config-reference.md b/docs/config-reference.md
@@ -3507,8 +3507,10 @@ FastCGI and lets retries replay the same upload without cloning a large memory
 buffer; both spool settings must be configured together. On Unix, the securely
 created spool entry is unlinked immediately and Fluxheim retains its open file
 descriptor for request and retry readers, preventing a crash from leaving a
-named plaintext upload behind. This does not guarantee raw-media erasure; use
-encrypted storage or tmpfs when storage remanence is in scope. When
+named plaintext upload behind. Overlapping readers use independent logical
+offsets with bounded positional disk reads, so one reader cannot reset or
+consume another reader's body position. This does not guarantee raw-media
+erasure; use encrypted storage or tmpfs when storage remanence is in scope. When
 `php.max_request_body_bytes` is set on the same PHP action, the spool threshold
 must be lower than that body limit. Existing spool paths must be directories,
 and existing directories must not be group/world writable. Fluxheim rechecks
diff --git a/docs/php-runtime-support.md b/docs/php-runtime-support.md
@@ -334,8 +334,11 @@ for single-user/rootless deployments.
   writable, and runtime spool creation rechecks permissions before writing
   upload bodies. On Unix, Fluxheim unlinks each securely created spool entry
   immediately and retains only the open descriptor, so crashes do not leave a
-  named plaintext request body behind. Use encrypted storage or tmpfs when raw
-  storage remanence is in scope.
+  named plaintext request body behind. Every request or retry reader maintains
+  an independent logical offset and uses bounded positional reads through the
+  blocking-I/O pool, so overlapping readers cannot reset or consume each
+  other's stream position. Use encrypted storage or tmpfs when raw storage
+  remanence is in scope.
 - Custom FastCGI params in config. Implemented as `[vhosts.php.params]` and
   `[vhosts.routes.php.params]` with protected core CGI params.
 - Path mapping for separate Fluxheim/php-fpm container filesystem roots.
diff --git a/release-notes/RELEASE_NOTES_1.7.8.md b/release-notes/RELEASE_NOTES_1.7.8.md
@@ -48,7 +48,9 @@ general-purpose WASI application hosting.
 - Run each managed PHP-FPM pool in a dedicated process group and terminate the
   complete group on shutdown, failed status checks, and watchdog restarts.
 - Unlink request-body spool files immediately after secure creation while
-  retaining a rewindable descriptor for retry replay.
+  retaining a descriptor for retry replay. Give every reader an independent
+  logical offset backed by bounded positional reads so overlapping readers
+  cannot corrupt each other's request body stream.
 - Replace inherited managed PHP-FPM `PATH` handling with a fixed allowlisted
   search path after clearing the child environment.
 
