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

Prevent ASF from accepting requests from public network if IPCPassword is not set #2371

Closed
ezhevita opened this issue Jul 10, 2021 · 1 comment · Fixed by #2372
Closed

Prevent ASF from accepting requests from public network if IPCPassword is not set #2371

ezhevita opened this issue Jul 10, 2021 · 1 comment · Fixed by #2372
Labels
✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 🟢 Low priority Issues marked with this label are actively being worked on if nothing serious is on the list. 🙏 Wishlist Issues marked with this label are wishlisted. We'd like to make them happen but they're not crucial.

Comments

@ezhevita
Copy link
Member

ezhevita commented Jul 10, 2021

Enhancement

Purpose

User may accidentally forget to set IPCPassword while server being exposed publicly to the web which creates a security breach.

Solution

Implementing this suggestion will reduce risk of attacking from external network. IPC server should not respond to any requests (or respond with 403) from public network if IPCPassword isn't set (unless user decides to override this behaviour with config property or program argument).

Why currently available solutions are not sufficient?

There is no available solution that makes sure that user doesn't accidentally expose their IPC server publicly.

Does your suggestion fall into ASF scope?

Yes.

Is your suggestion abiding to Steam guidelines?

Yes.

Additional info

We do have already KnownNetworks property in IPC.config, which may be used as well, and if it's not defined, use private address spaces defined by RFC1918.

@ezhevita ezhevita added ✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 👀 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered. labels Jul 10, 2021
@JustArchi
Copy link
Member

This is much less "useful" than you may think, indeed, in ASF standalone IPC server scenario, this can drastically reduce the risk and will be the core reason why I'm all in into this suggestion and I consider it a very good enhancement idea.

In practice however, people set up some kind of reverse-proxy in front of ASF, which depending on the user's stupidity, may or may not present itself as user from 127.0.0.1 regardless.

There is not much we can do in this regard, but I totally agree that if the request really arrives from outside of what we can classify as LAN, we should definitely refuse to honor such request as a security measure.

@JustArchi JustArchi added 🙏 Wishlist Issues marked with this label are wishlisted. We'd like to make them happen but they're not crucial. 🟢 Low priority Issues marked with this label are actively being worked on if nothing serious is on the list. and removed 👀 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered. labels Jul 10, 2021
JustArchi added a commit that referenced this issue Jul 10, 2021
JustArchi added a commit that referenced this issue Jul 12, 2021
* Closes #2371

* Change the default to no known networks

* Address @vital7 note

* Handle both IPv4 and IPv6 when mapped

This follows ASP.NET Core logic

* Refactor forwarded headers usage
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 🟢 Low priority Issues marked with this label are actively being worked on if nothing serious is on the list. 🙏 Wishlist Issues marked with this label are wishlisted. We'd like to make them happen but they're not crucial.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants