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

Restrict internet access from siren container #286

Open
wants to merge 20 commits into
base: stable
Choose a base branch
from

Conversation

magick93
Copy link

@magick93 magick93 commented Dec 16, 2024

Objective

Security harden, specifically by restricting egress traffic from the siren container unless to approved destinations.

Copy link
Member

@rickimoore rickimoore left a comment

Choose a reason for hiding this comment

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

maybe antonD can look at the docker, i just made a small comment

README.md Outdated Show resolved Hide resolved
@antondlr
Copy link
Member

I added some small nitpicks, feel free to revert them!

question; this does not expose over SSL currently, right?
as that would create a new security hole while patching another one:
one of the main issues we faced initially is that a user would have to send their session password in plaintext to Siren.

@antondlr
Copy link
Member

it would also be nice to use the same template for nginx_proxy.conf.template and siren-http{,s}.conf, manipulating the template can be done from docker-assets/docker-entrypoint.sh
(this is by no means a must!)

@magick93
Copy link
Author

@antondlr

question; this does not expose over SSL currently, right?

Yes, the primary objective of this PR is getting the egress restrictions in place.

one of the main issues we faced initially is that a user would have to send their session password in plaintext to Siren.

Preventing the exfiltration of sensitive data (eg, keys) from the container can largely be achieved using egress rules. But from the browser is another another kettle of fish. I

@magick93
Copy link
Author

magick93 commented Dec 16, 2024

@antondlr

it would also be nice to use the same template for nginx_proxy.conf.template and siren-http{,s}.conf, manipulating the template can be done from docker-assets/docker-entrypoint.sh

Yeah I considered that, but I also wanted to avoid changing the existing siren image, and instead try to wrap it in nice warm security blanket.

I would like to explore improving this, and also starting nginx in a container lifecycle hook and then running the container with less perms.

I suggest we explore this in a new issue.

@antondlr
Copy link
Member

Preventing the exfiltration of sensitive data (eg, keys) from the container can largely be achieved using egress rules. But from the browser is another another kettle of fish. I

yeah so, taking a step back here and looking at which dangers actually exist, I think securing browser-siren traffic is paramount because once the SESSION_PASSWORD has leaked, it's game over, and it's a more viable attack vector. I'm definitely not against restricting egress traffic from the container, I think it makes a lot of sense, but not at the cost of sacrificing the current ssl-by-default setup. happy to talk about it / be convinced otherwise!
so the question is... how do we do both?

I also wanted to avoid changing the existing siren image

feel free to go ham on the exiting image :-)

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.

3 participants