[WSLC] Add on-disk alias canonicalization to denied-path overlap check#657
[WSLC] Add on-disk alias canonicalization to denied-path overlap check#657SohamDas2021 wants to merge 7 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| /// `resolve_through_symlinks`. Returns [`PathCanonical::Absent`] only when no | ||
| /// ancestor resolves. | ||
| #[cfg(windows)] | ||
| pub fn canonicalize_allowing_absent_tail(path: &str) -> PathCanonical { |
There was a problem hiding this comment.
The helper handles nonexistent paths by finding the deepest existing ancestor and appending the missing components. While walking backward, file_name() returns nothing for .. , so the algorithm can forget that a preceding component should be cancelled.
For example:
\X\Y..\Z
should normalize to:
\X\Z
but can be reconstructed as:
\X\Y\Z
This matters when is a junction or other alias. Tier 1 folds .. correctly but cannot resolve the alias; Tier 2 resolves the alias but then reconstructs the missing tail incorrectly. It can consequently compare the wrong path and miss a real denied-path overlap.
The fix is to normalize . and .. before searching for an existing ancestor, or explicitly track parent-component cancellations while building the missing tail. A regression test should combine an aliased ancestor, multiple nonexistent components and .. .
There was a problem hiding this comment.
Thanks, fixing!
| /// window also remains between canonicalization and the SDK mount (an alias | ||
| /// could be swapped in between); fully closing it needs handle-based mounting | ||
| /// the WSLC SDK does not expose, so it is likewise accepted. | ||
| pub fn validate_denied_path_overlap( |
There was a problem hiding this comment.
This PR canonicalizes only the paths explicitly listed in the policy. It catches aliases in the mount or deny strings themselves, but not aliases located inside a mounted directory.
Example:
Mounted: C:\project
Denied: C:\secrets
Junction: C:\project\link -> C:\secrets
Canonicalizing the two policy entries produces C:\project and C:\secrets , which do not overlap, so validation succeeds. If WSLC follows the junction inside the mounted tree, the guest can still reach the denied directory through C:\project\link .
The immediate concern is that the new documentation says canonicalization is “closing the alias gap,” while this gap remains. At minimum, the limitation should be documented precisely. Fully enforcing it would require controlling traversal beneath the mount or inspecting reparse points, which is harder, potentially expensive and still subject to races.
There was a problem hiding this comment.
Thanks for the catch! I will document this accordingly.
|
@copilot resolve the merge conflicts in this pull request |
Resolved. The three files with conflicts ( |
📖 Description
The WSLC denied-path overlap validator was lexical-only: it folded case and
./..but could not see on-disk aliases (symlinks, junctions, 8.3 short names,\\?\prefixes), so a deny that resolved into a mounted tree via an alias slipped through and stayed accessible.Added a shared
wxc_common::filesystem_canonicalprimitive that resolves a path to its canonical DOS form via GetFinalPathNameByHandleW, and wire it intovalidate_denied_path_overlapas a second tier: after the lexical pre-check, canonicalize every mount and deny, fail closed on any path that exists but cannot be resolved, then re-run the structuralcontainment compares on the resolved forms.
🔗 References
🔍 Validation
✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(
MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.