Jigsaw-Code / outline-apps

Outline Client and Manager, developed by Jigsaw. Outline Manager makes it easy to create your own VPN server. Outline Client lets you share access to your VPN with anyone in your network, giving them access to the free and open internet.
https://getoutline.org/
Apache License 2.0
8.38k stars 1.36k forks source link

Add Outline version to http fetch header for dynamic links #2061

Open cornzzy opened 1 month ago

cornzzy commented 1 month ago

Is there an existing issue that is already proposing this?

Application

Outline Client

What are you trying to do? What is your use case?

Dynamic link web service needs to know Outline version to decide to return credentials or not.

Is your feature request related to a problem? Please describe it.

A single client can bring the whole server down for everyone by connecting from an older version.

But that's only for clients without any harmful intentions. There is another security issue, a client with harmful intentions can open dynamic link in browser, decode shadowsocks password, then use it as `ss://` without prefix and take the whole server down for everyone. This proposal is a **very soft fix** for this issue too. I couldn't think of a confident header value for a better fortification. ![image](https://github.com/user-attachments/assets/65f63e4a-15f7-4501-bde8-b48c54490053)

Describe the solution you'd like.

https://github.com/Jigsaw-Code/outline-apps/blob/dff8f19bde8864646e317e2875ab27842570b193/client/src/www/app/outline_server_repository/access_key_serialization.ts#L74

image

Describe alternatives you've considered

Given the current conditions, there is no way to circumvent this issue. I hope it's clear how high a priority this matter is and the significant impact it has on Outline's geographic targets.

sbruens commented 1 month ago

I don't fully understand how an older Outline client can bring down a whole server. Could you give us some more insight in case there's something else we need to look into?

In any case, as I mentioned in #2067, we can add this information to the User-Agent header, which seems reasonable. It'll be easier to do once the config fetching in Go is implemented. Assigning this to @jyyi1 who is leading that work.

cornzzy commented 1 month ago

Here are the key points regarding detection:

  1. I'm not aware of all the changes from 1.10 all the way to 1.13 that can affect detection but purely based on experience, the feedback from a server that have clients with any version from 1.10 to 1.13 is different from a server that's only serving clients using only 1.13.
  2. Prefix is required. With the current situation, it's quite easy to open a dynamic link and use the ss credentials without prefix. Checking the version is a soft fix for this.

So that's the power a single client can have. Once the IP is detected, no prefix gets through it, even if I setup an nginx page on it, it won't open, heck it won't even respond to pings. This situation is in i‌r‌‌a‌n, it's not as severe with my two clients in mainland c‌hi‌na. On a separate note, I hope to see websocket support added soon.

jyyi1 commented 1 month ago

Hi @cornzzy ,

Adding User-Agent header could be a solution. But please note that we have various Outline tools (Client, CLI, SDK utilities, etc.) that accept server keys, each with different User-Agents. Alternatively, I would propose adding a Supported-Outline-Features header (e.g., 'prefix', 'dynamic').

Our keys should be backward compatible, meaning older clients should connect. Neither client nor server should crash in this scenario. A server crashing due to an old key sounds like an issue that needs fixing separately. Can you provide server logs to help us troubleshoot?

cornzzy commented 1 month ago

Sure, Outline has different client apps and there are many ss client apps out there. And the server doesn't crash. There seems to be a misunderstanding.

All I'm saying is the dynamic link web service providing the ss credentials needs to know at least Outline Client version. The goal is to only return ss credentials if version is X even though http header can be set manually outside of outline but still this little header fixes 99% of the problem. I explained here a bit more.

If version is going to be in the header across all Outline Client (GUI) builds that would be awesome and this issue can be closed.

jyyi1 commented 1 month ago

Thanks @cornzzy , we plan to add the User-Agent header to dynamic key requests first. Also, please keep in mind there are many Outline-compatible clients, so please avoid restricting your server to just one Outline Client version.