BookStackApp / BookStack

A platform to create documentation/wiki content built with PHP & Laravel
https://www.bookstackapp.com/
MIT License
15.46k stars 1.94k forks source link

Addressing CSP Configuration in BookStack: Resolving Security Scan Findings #5197

Closed mschoon85 closed 1 month ago

mschoon85 commented 2 months ago

Attempted Debugging

Searched GitHub Issues

Describe the Scenario

Hi @ssddanbrown,

Our new BookStack instance is now publicly accessible. Being a government entity, we are obligated to adhere to specific security standards. We employ a particular security scan to identify risks, and the following risk is the only one I was unable to resolve, although I managed to resolve quite a few others. From what I can see, Bookstack is providing the CSP settings. I've tried to reduce the risk by including the following in the /config/nginx/ssl.conf file inside the BookStack container: add_header Content-Security-Policy "default-src 'self'; frame-ancestors 'self'; frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://embed.diagrams.net; script-src 'self' https://ourbookstackURL.nl 'nonce-ournoncenumber' 'strict-dynamic'; object-src 'self'; base-uri 'self'; form-action 'self';";

However, this leads to duplicate CSP settings as indicated by the security scan, implying that the original CSP is derived from Bookstack itself. What actions can we take to mitigate this risk or modify the default CSP settings in BookStack? The scan results below do not include duplicates, as they were obtained without modifying ssl.conf.


Content-Security-Policy: Detailed Report

Verdict:
Your web server does not offer Content-Security-Policy (CSP), or does offer CSP with certain insecure settings.


Technical details:

Web Server IP Address Findings
server IP Retrieved CSP value: ["frame-ancestors 'self'; frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://embed.diagrams.net; script-src http: https: 'nonce-rKI8RE08Qm8HwcEvVVr8Qd1Z' 'strict-dynamic'; object-src 'self'; base-uri 'self'"]

Recommendations:

  1. 'http:' scheme should not be used:
    Explanation:
    Using the http: scheme allows content to be loaded over an insecure connection. This can expose your website to man-in-the-middle attacks, where an attacker intercepts and potentially alters the communication between the browser and the server. Always use https: to ensure that content is delivered securely.

  2. 'https:' scheme without a specific main domain should not be used:
    Explanation:
    Using a general https: scheme without specifying the exact main domain (e.g., https://*.example.com) is insecure because it can allow any subdomain under example.com to be trusted. Instead, specify the exact domains or subdomains that are trusted, such as https://scripts.example.com.

  3. 'default-src' with sufficiently secure value should be defined:
    Explanation:
    The default-src directive sets the default source policy for fetching content such as scripts, stylesheets, and images. If it's not defined or too permissive, attackers may load malicious content. A secure default-src should be either 'none', 'self', or specify trusted domains explicitly.

  4. 'form-action' with sufficiently secure value should be defined:
    Explanation:
    The form-action directive restricts the URLs that can handle form submissions. Without securely defining this, attackers may redirect form submissions to malicious servers. Ensure that the form-action directive only allows trusted URLs, like 'self', to handle form data.

Exact BookStack Version

24.05.3

Log Content

No response

Hosting Environment

Ubuntu 24.04 + Docker

ssddanbrown commented 2 months ago

Hi @mschoon85, We currently don't provide an official means to alter the CSP rules provided by BookStack, outside of a few functionally specific scenarios (Iframe hosts/sources).

From what I understand from this site, it should be fine to add extra CSP headers and the browser will choose the most restrictive.

The tricky part will be the script header, since we use a nonce for these.

It may be possible to alter the existing headers using nginx map's ability but this is not something I've ever played with.

mschoon85 commented 2 months ago

@ssddanbrown It seems that there is an issue. When I introduce an additional or replacement Content Security Policy (CSP) into /config/nginx/ssl.conf, the scan results indicate that both CSPs are being applied simultaneously.

single - bookstack CSP only scan results: Retrieved CSP value: ["frame-ancestors 'self'; frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://embed.diagrams.net; script-src http: https: 'nonce-rKI8RE08Qm8HwcEvVVr8Qd1Z' 'strict-dynamic'; object-src 'self'; base-uri 'self'"]

extra CSP in /config/nginx/ssl.conf scan results: Retrieved CSP value: ["frame-ancestors 'self'; frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://embed.diagrams.net; script-src http: https: 'nonce-9adPREDkGeRYGACRnqZ9H459' 'strict-dynamic'; object-src 'self'; base-uri 'self'", "default-src 'self'; frame-ancestors 'self'; frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://embed.diagrams.net; script-src 'self' https://myurl.nl 'nonce-D4UNqGgnQ5RqAqg09UVFE2I2' 'strict-dynamic'; object-src 'self'; base-uri 'self'; form-action 'self';"]

Would you consider implementing these changes into BookStack to enhance the application's security further?

'http:' scheme should not be used: Explanation: Using the http: scheme allows content to be loaded over an insecure connection. This can expose your website to man-in-the-middle attacks, where an attacker intercepts and potentially alters the communication between the browser and the server. Always use https: to ensure that content is delivered securely.

'https:' scheme without a specific main domain should not be used: Explanation: Using a general https: scheme without specifying the exact main domain (e.g., https://*.example.com) is insecure because it can allow any subdomain under example.com to be trusted. Instead, specify the exact domains or subdomains that are trusted, such as https://scripts.example.com.

'default-src' with sufficiently secure value should be defined: Explanation: The default-src directive sets the default source policy for fetching content such as scripts, stylesheets, and images. If it's not defined or too permissive, attackers may load malicious content. A secure default-src should be either 'none', 'self', or specify trusted domains explicitly.

'form-action' with sufficiently secure value should be defined: Explanation: The form-action directive restricts the URLs that can handle form submissions. Without securely defining this, attackers may redirect form submissions to malicious servers. Ensure that the form-action directive only allows trusted URLs, like 'self', to handle form data.

Here is the security scan we utilize: https://en.internet.nl/site/knowledgebase.sittard-geleen.nl/2946123/ I also discovered this website focusing on CPS itself: https://report-uri.com/home/analyse/https%3A%2F%2Fknowledgebase.sittard-geleen.nl%2F

ssddanbrown commented 2 months ago

the scan results indicate that both CSPs are being applied simultaneously.

Sounds like a potential limitation in the scan logic, in not being able to understand interactions with multiple CSP headers?

Would you consider implementing these changes into BookStack to enhance the application's security further?

Maybe, but only where it would make sense or there's an actual need. Adding extra rules, or tweaking as suggested, will add complexity along with potential compatibility impact with existing use-cases, while likely adding limited benefit.

Again, Happy to consider tweaking rules, but I just want to make sure there's considered value in doing so, relative to the extra support effort & breaking impact, rather than doing so with the aim to pass a specific scan for business reasons.

mschoon85 commented 2 months ago

Would you consider adding a configuration option that allows users to fully customize the CSP from within the BookStack configuration? This could be particularly useful for organizations like mine that need to comply with strict security standards and scans.

I understand the need to limit additional maintenance and support complexity, but I believe a more flexible CSP configuration could be valuable for organizations facing increasingly stringent security requirements.

ssddanbrown commented 2 months ago

I'm not too keen on adding a higher level option since we have existing CSP options that would then potentially (be expected) interplay and add complexity, and I don't want to go down the road of adding options just for the sake of editing headers, which could be done at webserver level.

Could maybe add a logical theme hook instead, but again more to maintain for something which could be done at webserver level.

Did you attempt the nginx map option at all?

mschoon85 commented 2 months ago

Yes, I have tried using the nginx map directive.

I added the following to /config/nginx/nginx.conf:

map $uri $csp {
    default "default-src 'self'; frame-ancestors 'self'; frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://embed.diagrams.net; script-src 'self' https://myurl.nl 'nonce-D4UNqGgnQ5RqAqg09UVFE2I2' 'strict-dynamic'; object-src 'self'; base-uri 'self'; form-action 'self';";
    ~^/special-path "default-src 'none'; script-src 'self';";
}

and to /config/nginx/site-confs/default.conf:

proxy_hide_header Content-Security-Policy;
add_header Content-Security-Policy $csp;

Unfortunately, the default (CSP) for BookStack is still in use.

ssddanbrown commented 2 months ago

Yeah, spent a moment on this and looks like nginx makes this a pain since there's no real way of replacing a header without the headers_more module.

For reference, here's a reduced/simplified example of nonce-respecting map using the existing csp header:

map $sent_http_content_security_policy $altered_csp {
    default "$sent_http_content_security_policy";
    "~nonce-(?<bsnonce>.+?)'" "script-src 'self' 'nonce-$bsnonce' 'strict-dynamic';";
}
mschoon85 commented 2 months ago

@ssddanbrown, was the example you provided effective?

ssddanbrown commented 2 months ago

@mschoon85 It's an example of how you can create a new CSP value using the nonce value from BookStack. It can work using add_header Content-Security-Policy "$altered_csp";, but will still add an extra header unless using the headers_more module.

ssddanbrown commented 1 month ago

Since there's been no further follow up, I'm going to close this support thread off, but feel free to create a feature request for CSP override/customization.