Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alloc fs: use case-insensitive check for reads of secret/private dir #24125

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

tgross
Copy link
Member

@tgross tgross commented Oct 3, 2024

When using the Client FS APIs, we check to ensure that reads don't traverse into the allocation's secret dir and private dir. But this check can be bypassed on case-insensitive file systems (ex. Windows, macOS, and Linux with obscure ext4 options enabled). This allows a user with read-fs permissions but not alloc-exec permissions to read from the secrets dir.

This changeset updates the check so that it's case-insensitive. This risks false positives for escape (see linked Go issue), but only if a task without filesystem isolation deliberately writes into the task working directory to do so, which is a fail-safe failure mode.

Ref: golang/go#18358
Ref: https://hashicorp.atlassian.net/browse/NET-10664

When using the Client FS APIs, we check to ensure that reads don't traverse into
the allocation's secret dir and private dir. But this check can be bypassed on
case-insensitive file systems (ex. Windows, macOS, and Linux with obscure ext4
options enabled). This allows a user with `read-fs` permissions but not
`alloc-exec` permissions to read from the secrets dir.

This changeset updates the check so that it's case-insensitive. This risks false
positives for escape (see linked Go issue), but only if a task without
filesystem isolation deliberately writes into the task working directory to do
so, which is a fail-safe failure mode.

Ref: golang/go#18358
@tgross tgross force-pushed the b-case-sensitive-template-check branch from cb6b817 to 4f1cbe3 Compare October 3, 2024 17:58
@tgross tgross added backport/ent/1.7.x+ent Changes are backported to 1.7.x+ent backport/1.8.x backport to 1.8.x release line theme/allocation API type/bug theme/security labels Oct 3, 2024
@tgross tgross added this to the 1.9.0 milestone Oct 3, 2024
@tgross tgross marked this pull request as ready for review October 3, 2024 18:00
Copy link
Contributor

@dduzgun-security dduzgun-security left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍 Thank you

@tgross tgross merged commit b7595c6 into main Oct 3, 2024
42 checks passed
@tgross tgross deleted the b-case-sensitive-template-check branch October 3, 2024 18:20
Comment on lines +4 to +5
//go:build !linux
// +build !linux
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BSDs are gonna be sad, but we don't officially support them anyway. Perhaps:

Suggested change
//go:build !linux
// +build !linux
//go:build darwin || windows

(can we drop the old // +build way? it was replaced 3+ years ago)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(can we drop the old // +build way? it was replaced 3+ years ago)

I think so, but we've got roughly a zillion of those so let's do it all in one big sed in its own PR. 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/ent/1.7.x+ent Changes are backported to 1.7.x+ent backport/1.8.x backport to 1.8.x release line theme/allocation API theme/security type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants