Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Fixed

- **A new file published by `ops_write_file` keeps the directory's inherited grants.** Staging
objects are created with a protected, creator-only DACL, and publication never restored
inheritance for the create path (#420), so the published file carried exactly one explicit ACE.
Any principal that authorizes through the directory — a confined workspace token, another tool,
another user — was cut off, and the sandbox that wrote the file could not unlink it. `publish_new`
now adopts the directory's object-inherit ACEs before the hard link is made, which is after the
bytes are complete, so the private staging DACL keeps its purpose. Overwrites are unchanged: they
still capture and restore the target's own descriptor.
- Withdraw PTC publication and cancel calls when a session leaves its selected preset; keep other
sessions' discovery selections intact and authorize external calls only in their owned wrapper.
- Diagnose the selected discovery/shell services and scoped publication; clarify the host's
Expand Down
1 change: 1 addition & 0 deletions native/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions native/crates/ops-files/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,8 @@ uuid = { version = "1.26", features = ["v4"] }
[target.'cfg(windows)'.dependencies.dsh-ops-windows-metadata]
path = "../../windows-metadata"

[target.'cfg(windows)'.dev-dependencies]
windows-permissions = "=0.2.4"

[target.'cfg(target_os = "macos")'.dependencies.dsh-ops-darwin-metadata]
path = "../../darwin-metadata"
2 changes: 1 addition & 1 deletion native/crates/ops-files/src/file_publication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ pub(super) fn create_sibling(
}

#[cfg(windows)]
fn metadata_error(error: std::io::Error) -> Failure {
pub(super) fn metadata_error(error: std::io::Error) -> Failure {
// The source was opened separately: failures inside capture/restore prove
// metadata cannot be guaranteed, not that the DSH selector denied a write.
Failure::new("METADATA_UNSUPPORTED", error)
Expand Down
86 changes: 86 additions & 0 deletions native/crates/ops-files/src/file_single.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,21 @@ fn publish_new(
return Err(error);
}
}
#[cfg(windows)]
if source.is_none() {
// The private staging DACL exists to hide half-written bytes; it must not outlive
// publication. Adopt the directory's inheritable grants before the link is made so
// the workspace token that owns the new file can still unlink it afterwards.
let parent = checked.parent.try_clone()?.into_std_file();
let child = file.try_clone()?.into_std();
let adopted = dsh_ops_windows_metadata::adopt_parent_dacl(&parent, &child)
.map_err(publication::metadata_error);
drop(child);
if let Err(error) = adopted {
publication::discard_sibling(&checked.parent, &temporary, file)?;
return Err(error);
}
}
#[cfg(target_os = "macos")]
{
// RENAME_EXCL publishes an absent destination without a staging unlink.
Expand Down Expand Up @@ -638,4 +653,75 @@ mod tests {
drop(checked);
std::fs::remove_dir_all(root).unwrap();
}

#[cfg(windows)]
fn fixture() -> std::path::PathBuf {
let root = std::fs::canonicalize(std::env::temp_dir())
.unwrap()
.join(format!("dsh-ops-inherit-{}", uuid::Uuid::new_v4()));
std::fs::create_dir(&root).unwrap();
root
}

#[cfg(windows)]
#[test]
fn new_file_adopts_directory_grants() {
use windows_permissions::{
LocalBox, SecurityDescriptor,
constants::{AceFlags, SeObjectType::SE_FILE_OBJECT, SecurityInformation as Info},
wrappers::{GetNamedSecurityInfo, SetNamedSecurityInfo},
};
let root = fixture();
let creator = windows_permissions::utilities::current_process_sid().unwrap();
// A confined token authorizes through inherited grants, never through the creator,
// so the directory carries a mask the private staging DACL does not name.
let directory: LocalBox<SecurityDescriptor> =
format!("D:(A;OICI;FA;;;{creator})(A;OICI;0x10001;;;S-1-5-32-545)").parse().unwrap();
SetNamedSecurityInfo(&root, SE_FILE_OBJECT, Info::Dacl, None, None, directory.dacl(), None)
.unwrap();
let destination = root.join("published");
publish_new(&destination, b"payload", None, None).unwrap();
let published: LocalBox<SecurityDescriptor> =
GetNamedSecurityInfo(&destination, SE_FILE_OBJECT, Info::Dacl).unwrap();
let granted = published.dacl().expect("a published file has a DACL");
let inherited = (0..granted.len()).any(|index| {
granted.get_ace(index).is_some_and(|ace| {
ace.flags().contains(AceFlags::Inherited)
&& ace.mask().bits() == 0x10001
&& ace.sid().is_some_and(|sid| {
windows_permissions::wrappers::ConvertSidToStringSid(sid)
.is_ok_and(|sid| sid == "S-1-5-32-545")
})
})
});
assert!(
inherited,
"a published file must inherit the directory grants the staging DACL hid: {:?}",
published.as_sddl().map(|text| text.to_string_lossy().into_owned())
);
std::fs::remove_dir_all(root).unwrap();
}

#[cfg(windows)]
#[test]
fn a_directory_without_inheritable_grants_still_publishes_an_accessible_file() {
use windows_permissions::{
LocalBox, SecurityDescriptor,
constants::{SeObjectType::SE_FILE_OBJECT, SecurityInformation as Info},
wrappers::SetNamedSecurityInfo,
};
let root = fixture();
let creator = windows_permissions::utilities::current_process_sid().unwrap();
// Nothing here is inheritable, so adoption has no grant to copy; the publisher must
// still be able to read back and delete what it published.
let locked: LocalBox<SecurityDescriptor> =
format!("D:P(A;;FA;;;{creator})").parse().unwrap();
SetNamedSecurityInfo(&root, SE_FILE_OBJECT, Info::Dacl | Info::ProtectedDacl, None, None, locked.dacl(), None)
.unwrap();
let destination = root.join("published");
publish_new(&destination, b"payload", None, None).unwrap();
std::fs::File::open(&destination).unwrap();
std::fs::remove_file(&destination).unwrap();
std::fs::remove_dir_all(root).unwrap();
}
}
46 changes: 45 additions & 1 deletion native/windows-metadata/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::{
use winapi::shared::ntdef::{OBJECT_ATTRIBUTES, UNICODE_STRING};
use windows_permissions::{
LocalBox, SecurityDescriptor,
constants::{SeObjectType::SE_FILE_OBJECT, SecurityInformation as Info},
constants::{AceFlags, AceType, SeObjectType::SE_FILE_OBJECT, SecurityInformation as Info},
wrappers,
};
use windows_sys::Win32::{
Expand Down Expand Up @@ -188,6 +188,50 @@ pub fn verify_cache(file: &File, write_sid: Option<&str>) -> io::Result<()> {
Ok(())
}

/// Give a just-published object the grants it would have inherited from `parent`.
///
/// Staging objects are created with a protected, creator-only DACL so partial bytes are
/// never readable by anyone else. That protection must not outlive publication: a file
/// left with only the creator's ACE ignores every other grant the directory carries,
/// including the confined token that owns the workspace, so the file cannot even be
/// unlinked by the sandbox that wrote it. Only object-inherit ACEs reach a non-container
/// child; the creator keeps full control so publication never narrows its own access.
pub fn adopt_parent_dacl(parent: &File, child: &File) -> io::Result<()> {
let parent_sd = descriptor(parent)?;
let Some(granted) = parent_sd.dacl() else {
// A null directory DACL grants everyone; the private DACL is the safe reading.
return Ok(());
};
let creator = windows_permissions::utilities::current_process_sid()?;
let mut aces = format!("(A;;FA;;;{creator})");
for index in 0..granted.len() {
let Some(ace) = granted.get_ace(index) else { continue };
if !ace.flags().contains(AceFlags::ObjectInherit) { continue; }
let kind = match ace.ace_type() {
AceType::ACCESS_ALLOWED_ACE_TYPE => 'A',
AceType::ACCESS_DENIED_ACE_TYPE => 'D',
// Object and callback ACEs carry GUID scoping this rebuild would drop.
_ => continue,
};
let Some(sid) = ace.sid() else { continue };
let sid = wrappers::ConvertSidToStringSid(sid)?;
aces.push_str(&format!("({kind};;0x{:x};;;{})", ace.mask().bits(), sid.to_string_lossy()));
}
let mut adopted: LocalBox<SecurityDescriptor> = format!("D:{aces}").parse()?;
// SAFETY: adopted is a live OS-owned descriptor, child is a borrowed live handle.
let result = unsafe {
SetKernelObjectSecurity(
child.as_raw_handle(),
Info::Dacl.bits(),
&mut *adopted as *mut _ as *mut _,
)
};
if result == 0 {
return Err(io::Error::last_os_error());
}
Ok(())
}

/// Captures mandatory security and supported basic metadata. Named streams,
/// extended attributes, EFS/compression/sparse/other special storage fail closed.
pub struct Metadata {
Expand Down