MediaBrowser / Emby

Emby Server is a personal media server with apps on just about every device.
https://emby.media
GNU General Public License v2.0
4.18k stars 811 forks source link

Emby 4.7.12.0 severely breaks Reverse Proxy support #3783

Closed roop90 closed 1 year ago

roop90 commented 1 year ago

Hi,

i can't follow why you added those changes:

This absolutely breaks my docker setup with caddy <-> emby (or any reverse proxy). Now, even if specified in the config file, I'm always considered "external". To avoid admin lock-out I've rolled back to 4.7.11.0, which works fine.

Why this change and what's the strategy when using a reverse proxy?

Cheers, roop

softworkz commented 1 year ago

Just take a look at the forums: https://emby.media/community/ https://emby.media/support/articles/advisory-23-05.html

More than a thousand servers got infected by a malware due to this.

It was conceptually wrong to allow a remote endpoint to indicate whether it's in the local network or not. In combination with "don't require password in the local network", this allowed any remote client to say "hey, I'm on the local network I don't need a password",

roop90 commented 1 year ago

Oh, that's unfortunate.

Can you confirm, that a public server with a properly set admin password is not affected by this malware?

My scenario breaks due to this option (users -> admin user):

"Allow remote connections to this Emby Server. If unchecked, all remote connections will be blocked."

As I use Caddy as a reverse proxy, I'm now completely locked out, unless I check this option right?

Will the emby team improve on this or what's the roadmap? Wouldn't it make much more sense to make passwords mandatory for admin users?

I mean, this update was done to fix a security issue provoked by users not setting an admin password. People should know what they are doing, if they open their stuff to the internet.

Cheers, roop

softworkz commented 1 year ago

Can you confirm, that a public server with a properly set admin password is not affected by this malware?

No. When you have set an admin password but have enabled "don't require password on local network", you are affected as well.

"Allow remote connections to this Emby Server. If unchecked, all remote connections will be blocked."

As I use Caddy as a reverse proxy, I'm now completely locked out, unless I check this option right?

Disabling "Allow remote connections" doesn't make you more secure. It makes you less secure when you let proxy headers decide about local vs. non-local connections.

You should allow remote connections. All connections through your reverse proxy will be considered as remote connections (which is good).

Local connections must be real local connections and not go through the RP.

Xitro01 commented 1 year ago

Just take a look at the forums: https://emby.media/community/ https://emby.media/support/articles/advisory-23-05.html

More than a thousand servers got infected by a malware due to this.

It was conceptually wrong to allow a remote endpoint to indicate whether it's in the local network or not. In combination with "don't require password in the local network", this allowed any remote client to say "hey, I'm on the local network I don't need a password",

Nice to finally see a security advisory, although I already reported this issue in 2021: https://github.com/MediaBrowser/Emby/issues/3784

So really hate to read "Starting Mid-May 2023, a hacker managed to infiltrate private user-hosted instances of Emby Server which were accessible via public internet and had an insecure configuration for administrative user accounts." as this could've been prevented 2 years earlier.

roop90 commented 1 year ago

Hi,

I was thinking about a possible fix.

The software Nextcloud does provide the option to define trusted proxies in their configuration: [https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/reverse_proxy_configuration.html]() Only if the request comes from such a trusted proxy / IP, the 'X-Forwarded-For' header is allowed. Otherwise the header is ignored (or maybe the connection is closed).

Maybe you could implement something similar? This way the attack surface is closed, while an admin can still choose to setup a reverse proxy.

Greetings, roop

LukePulverenti commented 1 year ago

Hi,

I was thinking about a possible fix.

The software Nextcloud does provide the option to define trusted proxies in their configuration: https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/reverse_proxy_configuration.html Only if the request comes from such a trusted proxy / IP, the 'X-Forwarded-For' header is allowed. Otherwise it is ignored if not even blocked right away.

Maybe you could implement something similar? This way the attack surface is closed while an admin can still choose to setup a reverse proxy.

Greetings, roop

Hi, yes there will be config options coming soon in the beta channel. Thanks.

gmoss-eschercloud commented 1 year ago

The Problem here is that people setting up reverse proxy's don't know or understand that apps like caddy/traefik have already got trusted header forwarding options in place to secure things like this. what worse is that the path you have taken is to completely block this which breaks so much that people are reverting back to 4.7.11.0 I too have had to drop version so that network limiting works I also have multiple local networks and because of this change all remote networks now have no limit.

This should have been a toggle button or should have only been limited on the authentication part. also maybe help inform users on how to setup a reverse proxy with trustedIp for forward headers.

At this point I fell like removing my premium sub and looking for something else.

My setup is running in a Kubernetes cluster with traefik so I have no option but to have a reverse proxy and the network it runs on is not the local network.

Lat31320 commented 1 year ago

Hello,

Although I understand the requirement to have maximum security by default, I don't understand why the modification you made does not allow (for someone who understands how things work) to apply the settings he wish.

My public reverse proxy works in an authenticated way and, moreover, my Emby administration account asks for a password in all circumstances. The only unauthenticated account only has limited, read-only access to certain (non personnal) contents.

Originally, an authenticated reverse proxy (plus a password also required in LAN) was precisely to overcome the weaknesses of Emby server (such as access to information, without authentication, by "url guessing "). Now you explain to everyone that an rproxy is dangerous?

roop90 commented 1 year ago

Hello,

Although I understand the requirement to have maximum security by default, I don't understand why the modification you made does not allow (for someone who understands how things work) to apply the settings he wish.

My public reverse proxy works in an authenticated way and, moreover, my Emby administration account asks for a password in all circumstances. The only unauthenticated account only has limited, read-only access to certain (non personnal) contents.

Originally, an authenticated reverse proxy (plus a password also required in LAN) was precisely to overcome the weaknesses of Emby server (such as access to information, without authentication, by "url guessing "). Now you explain to everyone that an rproxy is dangerous?

As far as I understood the issue, it's a huge problem when not using a reverse proxy. Which I guess a lot of users do.

In that scenario a bad actor would send those headers containing some common private ip addresses directly to Emby. As Emby can't tell if it's a reverse proxy or something else, it will accept the header and thinks the attacker is a local machine. Therefore granting access based on the configuration for internal / private clients. If a admin account can be accessed internally without a password it can therefore be accessed by the attacker. Bingo. They are in.

In my opinion it's a unfortunate combination of sub optimal design and inexperienced (maybe lazy) users.

Greetings, roop

LukePulverenti commented 1 year ago

The next beta build will have a server option to control this so you’ll be able to configure it however you like.

softworkz commented 1 year ago

Although I understand the requirement to have maximum security by default, I don't understand why the modification you made does not allow (for someone who understands how things work) to apply the settings he wish.

Just as a side note: It has turned out that even users who are deemed to be rather knowledgeable, didn't have correct configurations for their reverse proxies.

softworkz commented 1 year ago

The next beta build will have a server option to control this so you’ll be able to configure it however you like.

Yes, but it should also be noted - which is the more important part of the story - that this doesn't matter (much) anymore.

The distinction between local and non-local networks is being removed - at least with regards to security related matters (like login and password requirements).

LukePulverenti commented 1 year ago

This new option I'm referring to is in 4.8.0.41+. Thanks guys.

joekrill commented 1 year ago

The distinction between local and non-local networks is being removed - at least with regards to security related matters (like login and password requirements).

I noticed this when I upgraded to 4.8.0.40 yesterday - I can no longer switch between users without being asked for my password (regardless of this new header setting). I use a secure password, which is long and complicated, so I basically lost the ability to easily do this. I'm guessing this is due to this line I see in the changelog?

Deprecate in-network password option (replacement feature coming soon)

What is this "replacement feature"? Will we get back the ability to switch between users locally without having to enter a password every time?

LukePulverenti commented 1 year ago

The distinction between local and non-local networks is being removed - at least with regards to security related matters (like login and password requirements).

I noticed this when I upgraded to 4.8.0.40 yesterday - I can no longer switch between users without being asked for my password (regardless of this new header setting). I use a secure password, which is long and complicated, so I basically lost the ability to easily do this. I'm guessing this is due to this line I see in the changelog?

Deprecate in-network password option (replacement feature coming soon)

What is this "replacement feature"? Will we get back the ability to switch between users locally without having to enter a password every time?

Yes you'll get that back.