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

Allow optional localhost network access to sandboxed derivations #1646

Merged
merged 1 commit into from Oct 30, 2017

Conversation

copumpkin
Copy link
Member

This will allow bind and connect to 127.0.0.1, which can reduce purity/security (if you're running a vulnerable service on localhost) but is also needed for a ton of test suites, so I'm leaving it turned off by default but allowing certain derivations to turn it on as needed.

It also allows DNS resolution of arbitrary hostnames but I haven't found a way to avoid that. In principle I'd just want to allow resolving localhost but that doesn't seem to be possible.

I don't think this belongs under build-use-sandbox = relaxed because we want it on Hydra and I don't think it's the end of the world.

cc @edolstra @domenkozar @shlevy

@copumpkin copumpkin force-pushed the optional-sandbox-local-network branch from 0e589a4 to b8966bd Compare October 30, 2017 16:33
@copumpkin
Copy link
Member Author

Whoops, bug, please don't merge

This will allow bind and connect to 127.0.0.1, which can reduce purity/
security (if you're running a vulnerable service on localhost) but is
also needed for a ton of test suites, so I'm leaving it turned off by
default but allowing certain derivations to turn it on as needed.

It also allows DNS resolution of arbitrary hostnames but I haven't found
a way to avoid that. In principle I'd just want to allow resolving
localhost but that doesn't seem to be possible.

I don't think this belongs under `build-use-sandbox = relaxed` because we
want it on Hydra and I don't think it's the end of the world.
@copumpkin copumpkin force-pushed the optional-sandbox-local-network branch from b8966bd to 4a4a009 Compare October 30, 2017 17:11
@copumpkin
Copy link
Member Author

Okay, this is now safe to merge

@edolstra edolstra merged commit 197922e into NixOS:master Oct 30, 2017
@shlevy
Copy link
Member

shlevy commented Oct 30, 2017

We can't use network namespaces? :(

@shlevy
Copy link
Member

shlevy commented Oct 30, 2017

Oh, darwin only

@copumpkin
Copy link
Member Author

If only... 😄 Linux doesn't need to worry about this crap because it already has network namespaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants