From 98bc33db79ab09739cf00b45e041d54e15f9f672 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Fri, 20 Oct 2023 10:02:22 -0400 Subject: [PATCH 1/4] fix(buffers): apply stricter file permissions to buffer data files when possible --- lib/vector-buffers/src/variants/disk_v2/io.rs | 45 ++++++++++++------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/lib/vector-buffers/src/variants/disk_v2/io.rs b/lib/vector-buffers/src/variants/disk_v2/io.rs index deabe4dcf996b..447177829cd3f 100644 --- a/lib/vector-buffers/src/variants/disk_v2/io.rs +++ b/lib/vector-buffers/src/variants/disk_v2/io.rs @@ -3,6 +3,8 @@ use std::{io, path::Path}; use async_trait::async_trait; use tokio::io::{AsyncRead, AsyncWrite}; +const FILE_MODE_OWNER_RW_ONLY: u32 = 0o600; + /// File metadata. pub struct Metadata { pub(crate) len: u64, @@ -129,21 +131,21 @@ impl Filesystem for ProductionFilesystem { type MutableMemoryMap = memmap2::MmapMut; async fn open_file_writable(&self, path: &Path) -> io::Result { - tokio::fs::OpenOptions::new() - .append(true) - .read(true) - .create(true) - .open(path) - .await + let mut open_options = tokio::fs::OpenOptions::new(); + open_options.append(true).read(true).create(true); + + configure_file_open_options_for_write(&mut open_options); + + open_options.open(path).await } async fn open_file_writable_atomic(&self, path: &Path) -> io::Result { - tokio::fs::OpenOptions::new() - .append(true) - .read(true) - .create_new(true) - .open(path) - .await + let mut open_options = tokio::fs::OpenOptions::new(); + open_options.append(true).read(true).create_new(true); + + configure_file_open_options_for_write(&mut open_options); + + open_options.open(path).await } async fn open_file_readable(&self, path: &Path) -> io::Result { @@ -157,11 +159,12 @@ impl Filesystem for ProductionFilesystem { } async fn open_mmap_writable(&self, path: &Path) -> io::Result { - let file = tokio::fs::OpenOptions::new() - .read(true) - .write(true) - .open(path) - .await?; + let mut open_options = tokio::fs::OpenOptions::new(); + open_options.read(true).write(true); + + configure_file_open_options_for_write(&mut open_options); + + let file = open_options.open(path).await?; let std_file = file.into_std().await; unsafe { memmap2::MmapMut::map_mut(&std_file) } } @@ -171,6 +174,14 @@ impl Filesystem for ProductionFilesystem { } } +#[cfg(unix)] +fn configure_file_open_options_for_write(open_options: &mut tokio::fs::OpenOptions) { + open_options.mode(FILE_MODE_OWNER_RW_ONLY); +} + +#[cfg(not(unix))] +fn configure_file_open_options_for_write(_open_options: &mut tokio::fs::OpenOptions) {} + #[async_trait] impl AsyncFile for tokio::fs::File { async fn metadata(&self) -> io::Result { From 630e3180549b0eb9a8245d73d762f101bd9a2f6e Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Fri, 20 Oct 2023 10:35:57 -0400 Subject: [PATCH 2/4] also allow group to read --- lib/vector-buffers/src/variants/disk_v2/io.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vector-buffers/src/variants/disk_v2/io.rs b/lib/vector-buffers/src/variants/disk_v2/io.rs index 447177829cd3f..1e06d71619893 100644 --- a/lib/vector-buffers/src/variants/disk_v2/io.rs +++ b/lib/vector-buffers/src/variants/disk_v2/io.rs @@ -3,7 +3,7 @@ use std::{io, path::Path}; use async_trait::async_trait; use tokio::io::{AsyncRead, AsyncWrite}; -const FILE_MODE_OWNER_RW_ONLY: u32 = 0o600; +const FILE_MODE_OWNER_RW_GROUP_RO: u32 = 0o640; /// File metadata. pub struct Metadata { @@ -176,7 +176,7 @@ impl Filesystem for ProductionFilesystem { #[cfg(unix)] fn configure_file_open_options_for_write(open_options: &mut tokio::fs::OpenOptions) { - open_options.mode(FILE_MODE_OWNER_RW_ONLY); + open_options.mode(FILE_MODE_OWNER_RW_GROUP_RO); } #[cfg(not(unix))] From ba90f442186d6c97c0dcd1e9d8ac77043766d384 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Tue, 24 Oct 2023 11:35:31 -0400 Subject: [PATCH 3/4] clean up based on PR feedback --- lib/vector-buffers/src/variants/disk_v2/io.rs | 84 +++++++++++++------ 1 file changed, 59 insertions(+), 25 deletions(-) diff --git a/lib/vector-buffers/src/variants/disk_v2/io.rs b/lib/vector-buffers/src/variants/disk_v2/io.rs index 1e06d71619893..88b9b547806f0 100644 --- a/lib/vector-buffers/src/variants/disk_v2/io.rs +++ b/lib/vector-buffers/src/variants/disk_v2/io.rs @@ -1,7 +1,10 @@ use std::{io, path::Path}; use async_trait::async_trait; -use tokio::io::{AsyncRead, AsyncWrite}; +use tokio::{ + fs::OpenOptions, + io::{AsyncRead, AsyncWrite}, +}; const FILE_MODE_OWNER_RW_GROUP_RO: u32 = 0o640; @@ -131,40 +134,32 @@ impl Filesystem for ProductionFilesystem { type MutableMemoryMap = memmap2::MmapMut; async fn open_file_writable(&self, path: &Path) -> io::Result { - let mut open_options = tokio::fs::OpenOptions::new(); - open_options.append(true).read(true).create(true); - - configure_file_open_options_for_write(&mut open_options); - - open_options.open(path).await + create_writable_file_options(false) + .append(true) + .open(path) + .await } async fn open_file_writable_atomic(&self, path: &Path) -> io::Result { - let mut open_options = tokio::fs::OpenOptions::new(); - open_options.append(true).read(true).create_new(true); - - configure_file_open_options_for_write(&mut open_options); - - open_options.open(path).await + create_writable_file_options(true) + .append(true) + .open(path) + .await } async fn open_file_readable(&self, path: &Path) -> io::Result { - tokio::fs::OpenOptions::new().read(true).open(path).await + open_readable_file_options().open(path).await } async fn open_mmap_readable(&self, path: &Path) -> io::Result { - let file = tokio::fs::OpenOptions::new().read(true).open(path).await?; + let file = open_readable_file_options().open(path).await?; let std_file = file.into_std().await; unsafe { memmap2::Mmap::map(&std_file) } } async fn open_mmap_writable(&self, path: &Path) -> io::Result { - let mut open_options = tokio::fs::OpenOptions::new(); - open_options.read(true).write(true); - - configure_file_open_options_for_write(&mut open_options); + let file = open_writable_file_options().open(path).await?; - let file = open_options.open(path).await?; let std_file = file.into_std().await; unsafe { memmap2::MmapMut::map_mut(&std_file) } } @@ -174,13 +169,52 @@ impl Filesystem for ProductionFilesystem { } } -#[cfg(unix)] -fn configure_file_open_options_for_write(open_options: &mut tokio::fs::OpenOptions) { - open_options.mode(FILE_MODE_OWNER_RW_GROUP_RO); +/// Builds a set of `OpenOptions` for opening a file as readable/writable. +fn open_writable_file_options() -> OpenOptions { + let mut open_options = OpenOptions::new(); + open_options.read(true).write(true); + + #[cfg(unix)] + { + open_options.mode(FILE_MODE_OWNER_RW_GROUP_RO); + } + + open_options } -#[cfg(not(unix))] -fn configure_file_open_options_for_write(_open_options: &mut tokio::fs::OpenOptions) {} +/// Builds a set of `OpenOptions` for opening a file as readable/writable, creating it if it does +/// not already exist. +/// +/// When `create_atomic` is set to `true`, this ensures that the operation only succeeds if the +/// subsequent call to `open` is able to create the file, ensuring that another process did not +/// create it before us. Otherwise, the normal create mode is configured, which creates the file if +/// it does not exist but does not throw an error if it already did. +/// +/// On Unix platforms, file permissions will be set so that only the owning user of the file can +/// write to it, the owning group can read it, and the file is inaccessible otherwise. +fn create_writable_file_options(create_atomic: bool) -> OpenOptions { + let mut open_options = open_writable_file_options(); + + #[cfg(unix)] + { + open_options.mode(FILE_MODE_OWNER_RW_GROUP_RO); + } + + if create_atomic { + open_options.create_new(true); + } else { + open_options.create(true); + } + + open_options +} + +/// Builds a set of `OpenOptions` for opening a file as readable. +fn open_readable_file_options() -> OpenOptions { + let mut open_options = OpenOptions::new(); + open_options.read(true); + open_options +} #[async_trait] impl AsyncFile for tokio::fs::File { From c4481df9e586efcab14ede88278d600b4a2d8d19 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Fri, 27 Oct 2023 11:34:30 -0400 Subject: [PATCH 4/4] make linter happy --- lib/vector-buffers/src/variants/disk_v2/io.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/vector-buffers/src/variants/disk_v2/io.rs b/lib/vector-buffers/src/variants/disk_v2/io.rs index 88b9b547806f0..e81ec5c80a69b 100644 --- a/lib/vector-buffers/src/variants/disk_v2/io.rs +++ b/lib/vector-buffers/src/variants/disk_v2/io.rs @@ -6,6 +6,7 @@ use tokio::{ io::{AsyncRead, AsyncWrite}, }; +#[cfg(unix)] const FILE_MODE_OWNER_RW_GROUP_RO: u32 = 0o640; /// File metadata.