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

Missing Security Headers #2291

Open
corrideat opened this issue Aug 8, 2024 · 2 comments · May be fixed by #2362
Open

Missing Security Headers #2291

corrideat opened this issue Aug 8, 2024 · 2 comments · May be fixed by #2362
Labels
App:Backend Kind:Enhancement Improvements, new features, performance upgrades, etc. Note:Security

Comments

@corrideat
Copy link
Member

corrideat commented Aug 8, 2024

Problem

The site groupincome.app is missing some important security headers that could mitigate vulnerabilities client-side. This can be verified using https://securityheaders.com.

  • Content-Security-Policy: This is mainly designed to prevent XSS, although it can also be important for privacy (for example, to block loading images from a third party domain). Mostly, it defines origins that are allowed to serve content (plus some additional settings, such as blocking loading the page in a frame)
  • X-Content-Type-Options: AFAIK nosniff is the only permissible (and useful) value. This header instructs the user agent to honour the content-type provided the server, an instruction user agents follow in most circumstances. This is useful to prevent, among other things, stored XSS.
  • Referrer-Policy: This is mostly a privacy-impacting header for users. It tells the user agent under which circumstances to send a referrer header and what information it should contain. Setting it can be useful to prevent destination servers of links in GI from learning the exact URL (which can contain, e.g., a group) the user came from.
  • Permissions-Policy: This is also mostly a privacy-impacting defence-in-depth header. It includes a list of permissions that the website is allowed to request. This way, even if some malicious script is loaded, it cannot access sensitive APIs, such as geolocation.
  • Cross-Origin-Opener-Policy: This is a security-sensitive header that defines whether the site can share a browsing context group with other sites (e.g., via opener).
  • X-Frame-Options: This one is actually set but included here for completeness. It defines whether the site can be loaded in a frame (and is overridden by CSP).
  • Cross-Origin-Embedder-Policy: Blocks resources not explicitly allowed by either CORS or CORP.
  • Cross-Origin-Resource-Policy: Defines who can load the resource. IMHO, not as useful as all the other headers here, but useful at least to prevent hotlinking.

A note on HTTPS and server trust

Regardless of the values of these headers, they are set by the server, which means that their effectiveness depends on trusting the server and that the response reached the client unmodified.

Trusting the server

I think that it's acceptable to have some minimal degree of trust on the server. It'd be a useful exercise to also have a mechanism to relatively easily audit that all artifacts delivered (scripts especially) haven't been tampered with.

Besides the audit mechanism discussed, another (complimentary) option is to use the service worker to control the flow of information from the server to the app. The service worker can verify all subsequent server responses, effectively turning the trust model to TOFU.

Trusting that the response arrived unmodified

This is a tricker requirement. For the most part, HTTPS provides protection (save for the remote possibility of MitM).

Feasibility of MitM

Although Certificate Transparency [1] is not currently required for valid certificates, Chrome requires [2][3] CT as well as Safari [4] require certificate transparency, making it de facto required. AFAIU, this is not an online check, but issuing such proofs of inclusion will require collusion of parties, making it a difficult attack to carry out, since it's practically difficult and detection is likely to be deleterious for all parties involved.

This attack, however unlikely, is also mitigated in a TOFU model using service workers.

[1] https://www.rfc-editor.org/rfc/rfc6962
[2] https://googlechrome.github.io/CertificateTransparency/ct_policy.html "Since Chrome 94, Chrome clients will also attempt to obtain updated CT Log lists from the Component Updater infrastructure several times per day. On a successful update of the CT Log list, Chrome will update the start of its 70 day CT enforcement window to the freshest log_list_timestamp."
[3] GoogleChrome/CertificateTransparency@95694b2 "Chrome requires all publicly-trusted TLS certificates issued after April 30, 2018 to support CT in order to be recognized as valid. This site provides details on what is required. Any questions should be directed to the [email protected] list."
[4] https://support.apple.com/en-us/103214

Solution

  • Content-Security-Policy: default-src 'none'; script-src 'unsafe-eval'; script-src-elem 'self'; script-src-attr 'none'; style-src 'self'; style-src-elem 'self'; style-src-attr 'none'; img-src 'self' blob:; font-src 'self'; connect-src 'self'; media-src 'self'; prefetch-src 'self'; worker-src 'self'; frame-ancestors 'none'; form-action 'self'; upgrade-insecure-requests; manifest-src 'self' (this is the most likely to break things now; it may need changing when sandboxing or federation are implemented)
  • X-Content-Type-Options: nosniff
  • Referrer-Policy: strict-origin-when-cross-origin (send only the origin as referer). Alternatively, no-referrer can be used to block sending referer in all circumstances, which is the most privacy-preserving option.
  • Permissions-Policy: accelerometer=(), ambient-light-sensor=(), autoplay=(), battery=(), camera=(), cross-origin-isolated=(self), display-capture=(), document-domain=(), encrypted-media=(), execution-while-not-rendered=(self), execution-while-out-of-viewport=(self), fullscreen=(self), geolocation=(), gyroscope=(), keyboard-map=(), magnetometer=(), microphone=(), midi=(), navigation-override=(self), payment=(), picture-in-picture=(), publickey-credentials-get=(), screen-wake-lock=(), sync-xhr=(), usb=(), web-share=(self), xr-spatial-tracking=(), clipboard-read=(), clipboard-write=(self), gamepad=(), speaker-selection=() (self-explanatory, list generated using https://www.permissionspolicy.com/)
  • Cross-Origin-Opener-Policy: same-origin (don't share context with potentially untrusted sites)
  • X-Frame-Options: DENY (block loading the page in a frame)
  • Cross-Origin-Embedder-Policy: require-corp (blocks embedding cross-origin resources that don't explicitly allow it).
  • Cross-Origin-Resource-Policy: same-origin (block hotlinking)
@taoeffect taoeffect added Kind:Enhancement Improvements, new features, performance upgrades, etc. App:Backend Note:Security labels Aug 12, 2024
@taoeffect
Copy link
Member

@corrideat while I could add these to reverse proxy config, that would mean that others who are running Group Income might not get these headers as other servers could be configured differently. What do you think about using Hapi to add these headers?

@corrideat
Copy link
Member Author

I think that's reasonable, and also allows for easier testing (as mentioned, CSP and potentially permissions policy could break things if something is missing)

@snowteamer snowteamer linked a pull request Sep 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App:Backend Kind:Enhancement Improvements, new features, performance upgrades, etc. Note:Security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants