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

Remove trusted proxies #2229

Merged

Conversation

kermieisinthehouse
Copy link
Collaborator

Trusted proxies in its current form is near useless, and perhaps encourages bad practices.

It allows a connection from a proxy on the public internet to forward requests into stash. 99% of reverse proxies are running on the same host, and the other 1% are a load balancer or bastion host running on the same network or VPC. I can't figure a use case for this, and I thought the same when I was implementing it. It only came up because of a 'what if' on code review, but I think we should thin it down.

@kermieisinthehouse kermieisinthehouse added backend Pull requests that update Go code improvement Something needed tweaking. labels Jan 10, 2022
@kermieisinthehouse kermieisinthehouse added this to the Version 0.13.0 milestone Jan 10, 2022
@WithoutPants
Copy link
Collaborator

This is a nuclear approach. Normally, we'd mark it as deprecated and warn users about the pending removal. How certain are you that this is not being used? I think we're better off giving users the tools they need to tailor security to their needs.

@kermieisinthehouse
Copy link
Collaborator Author

I'm fairly convinced that the few people that have set the option don't understand why they did or are doing so needlessly.

Most people set up a reverse proxy like this, where the reverse proxy is on the same network as the stash host:

+-----------------------------+
|                             |          +-----------------------------+
|       Public Internet       |          |                             |
|                             |          |        Private Network      |
|                             |          |                             |
|       +-------------+       |          |                             |
|       |             |       |     +----+----+         +---------+    |
|       |             |       |     |    |    |         |         |    |
|       |   Clients   +-------+----->  Proxy  +--------->  Stash  |    |
|       |             |       |     |    |    |         |         |    |
|       |             |       |     +----+----+         +---------+    |
|       +-------------+       |          |                             |
|                             |          |                             |
|                             |          |                             |
|                             |          +-----------------------------+
+-----------------------------+

However, the trusted proxies setting is to allow people to create setups like this, where the proxy is accessing stash through the internet. This is an exotic setup, tricky to get right, and requires you to set up firewall rules to allow the proxy to do this:

+--------------------------------------------------------------------------+
|       Public Internet                                                    |
|                                                                          |
|                                                                          |
|        +-------------+                                                   |
|        |             |                                                   |
|        |             |                                                   |
|        |   Clients   |                                                   |
|        |             |                                                   |
|        |             |                                                   |
|        +-----+-------+                                                   |   
|              |                                                           |                                       
|              |                     +---------------------------------+   |
|              |                     |   Firewall that only allows     |   |
|              |                     |    connections from Proxy's IP  |   |
|              |                     |                                 |   |
|         +----v----+                |          +---------+            |   |
|         |         |                |          |         |            |   |
|         |  Proxy  +--------------------------->  Stash  |            |   |
|         |         |                |          |         |            |   |
|         +---------+                |          +---------+            |   |
|                                    |                                 |   |
|                                    |                                 |   |
|                                    |                                 |   |
|                                    |                                 |   |
|                                    +---------------------------------+   |
|                                                                          |
+--------------------------------------------------------------------------+

@kermieisinthehouse
Copy link
Collaborator Author

FYI, if somebody does have this setup, they can simply not pass X-forwarded-for, and retain the old functionality as a workaround

@WithoutPants WithoutPants merged commit def9ad8 into stashapp:develop Feb 2, 2022
@kermieisinthehouse kermieisinthehouse deleted the deprecate-trusted-proxies branch February 2, 2022 23:19
DogmaDragon pushed a commit to stashapp/Stash-Docs that referenced this pull request Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Pull requests that update Go code improvement Something needed tweaking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants