nextcloud / spreed

🗨️ Nextcloud Talk – chat, video & audio calls for Nextcloud
https://nextcloud.com/talk
GNU Affero General Public License v3.0
1.6k stars 428 forks source link

Http Header Information Leak #9102

Open ksainc opened 1 year ago

ksainc commented 1 year ago

Description

Hi All,

I was doing some security scans of our NextCloud 25.0.4 installation also tested on NC 26, with an online scan tool and noticed that is was showing information about the structure of our setup just by scanning the login page. I then did a separate page request with insomnia, to see the raw information was being sent to the client and noticed a few items that probably should not be sent to a non authenticated user.

HTTP Response Header

HTTP/2 200 
server: nginx
date: Sun, 19 Mar 2023 21:04:53 GMT
content-type: text/html; charset=UTF-8
content-length: 18135
vary: Accept-Encoding
set-cookie: oclg54wtpcka=gidiobtilv9a7anbig5a6qf29m; path=/; secure; HttpOnly; SameSite=Lax
expires: Sun, 19 Mar 2023 21:05:53 GMT
pragma: no-cache
cache-control: max-age=60
x-request-id: boTH3reSHwVNiSJdR7Lu
content-security-policy: default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' blob:;style-src 'self' 'unsafe-inline';img-src 'self' data: blob: https://*.tile.openstreetmap.org;font-src 'self' data:;connect-src 'self' blob: stun.(sanitized).com:443 ncs1.(sanitized).com wss://ncs1.(sanitized).com;media-src 'self' blob:;frame-src 'self' nc:;child-src blob: 'self';frame-ancestors 'self';worker-src blob: 'self';form-action 'self'
feature-policy: autoplay 'self';camera 'self';fullscreen 'self';geolocation 'none';microphone 'self';payment 'none'
x-robots-tag: noindex, nofollow
strict-transport-security: max-age=31536000; includeSubDomains; preload;
referrer-policy: no-referrer
x-content-type-options: nosniff
x-download-options: noopen
x-frame-options: SAMEORIGIN
x-permitted-cross-domain-policies: none
x-robots-tag: none
x-xss-protection: 1; mode=block

As you can see from the content-security-policy section, the server returns information that the talk app is installed and which servers are being used for STUN, TURN and Signaling. Since this information was provided to a unauthenticated user, it could be used for information gathering for a attack but a bad actor. This information would be very easy to miss use if a vulnerability was discovered in the future in either the coturn server or any of the HPB Signaling services like (Janus, Nats, or Signaling Server).

Although, we have tried to minimize our attack exposure by setting the HPB to respond only to requests to a specific domain, giving up this information to unauthenticated users basically nulls that effort.

I think this information should be stripped from all public facing links like the login page, and only offered once the user is authenticated.

Also I noticed that the installations Instance ID is actually used in the cookie string, and although I am not aware of a attack vector that this could be missed used for at the moment, I'm sure someone out there will find one eventually, so it might be a good idea not to send that to unauthenticated users also, or to have NC uses a separate ID just for cookies.

I know these are minor on the grand scale of things, but I think the less information that is publicly exposed the better. Since in today's world you can hack a security company with an unpatched PLEX server.

Thanks Sebastian

Steps to reproduce

Perform a request to the login page with curl or a program like Insomnia and read the headers.

Expected behavior

Do not send STUN, TURN or Signaling Server information until user is authenticated.

solracsf commented 1 year ago

Think with me and fix me if I'm wrong; CSP is applied to protect your Nextcloud, and you can't enforce it without publishing it (so, the CSP header will output its content, domains, etc.).

If you apply it for logged-in users only (as you can't simply "hide" that header and enforce it at the same time), your Nextcloud would be more vulnerable as any non logged-user would not benefit from this protection.

If the header isn't sent, it can't be read (browsers, etc...) so it can't be enforced.

As you've stated, you've protected your STUN/TURN/HPB and that's the way to go here, each server/service must be protected on it's own. Security through obscurity, even if yeah, it's better to not disclosure what doesn't need to be to, it's not security at all.

ksainc commented 1 year ago

Hi,

Thanks for the comment @solracsf. And I totally agree CSP is needed and very important.

But I don't see what security benefits you would gain by broadcasting CSP with all the server information to non logged in users. If the user is not logged in or opening a link that has been shared with them, that information is not relevant.

And yes obscurity is not security, but it is part of the onion layer approach to security, this is why it is recommended strip all any identifying information from your web servers and mail servers like versions and make, so that the information can not be harvested for an attack later.

nickvergessen commented 1 year ago

But I don't see what security benefits you would gain by broadcasting CSP with all the server information to non logged in users. If the user is not logged in or opening a link that has been shared with them, that information is not relevant.

This is needed in case the guests try to join a call, which is totally possible. The problem is that talk offers integration with various other apps/endpoints, so restricting the population of the header is barely possible and then again also only results in security by obscurity. As soon as you have a link to a shared file, public talk chat, public map, etc it would get populated and would be accessible again.

So yeah, can apply some more snake oil and start a wild and hopefully fast growing allow list for URLs and only inject then, but I fear it will break different things before working properly.