MinIO Console security headers

[pjuarezd]

…

Community Contribution License

All community contributions in this pull request are licensed to the project maintainers
under the terms of the Apache 2 license.
By creating this pull request I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 license.

Description

New config section browser to store Browser (console) dynamic settings

mc admin config get ALIAS
KEYS:
...
browser               manage Browser HTTP specific features, such as Security headers, etc.
...

mc admin config get ALIAS
browser csp_policy="default-src 'self' 'unsafe-eval' 'unsafe-inline';" hsts_seconds=0 hsts_include_subdomains=on hsts_preload=on referrer_policy=strict-origin-when-cross-origin

Allow set the values for response headers:

• Content-Security-Policy
• Strict-Transport-Security
• Refferer-Policy

Read settings from env variables:

• MINIO_BROWSER_CONTENT_SECURITY_POLICY
• MINIO_BROWSER_HSTS_SECONDS
• MINIO_BROWSER_HSTS_INCLUDE_SUB_DOMAINS
• MINIO_BROWSER_HSTS_PRELOAD
• MINIO_BROWSER_REFERRER_POLICY

Settings have default values:

Content-Security-Policy = default-src 'self' 'unsafe-eval' 'unsafe-inline';
Refferer-Policy = strict-origin-when-cross-origin
Strict-Transport-Security not set

This PR depends on minio/madmin-go#255 to be merge first

Motivation and Context

Application does not respond with security heades, such header help to prevent Cross-Site Scripting attacks or mitigate MITM attacks.

How to test this PR?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Optimization (provides speedup with no functional changes)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• Fixes a regression (If yes, please add commit-id or PR # here)
• Unit tests added/updated
• Internal documentation updated
• Create a documentation update request here

[harshavardhana]

why does it need to be customizable?

[pjuarezd]

why does it need to be customizable?

CSP is required to further restrict where web resources load from (css, js, etc), specially useful to prevent XSS in MinIO Console.

Is a best practice to set a restrictive CSP, also XSS it's been in OWASP Top 10 for many years and CSP is one of the recommended Controls for XSS, see https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#other-controls

We are also having customers requesting this cause vuln showed up in their 3rd party pentests @harshavardhana

[harshavardhana]

why does it need to be customizable?

CSP is required to further restrict where web resources load from (css, js, etc), specially useful to prevent XSS in MinIO Console.

Is a best practice to set a restrictive CSP, also XSS is been in OWASP Top 10 for many years and CSP is one of the recommended for Controls for XSS, see https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#other-controls

We are also having customers requesting this cause vuln showed up in their 3rd party pentests @harshavardhana

We can set standard headers and be done with it. Why does it need to be customized?

[pjuarezd]

why does it need to be customizable?

CSP is required to further restrict where web resources load from (css, js, etc), specially useful to prevent XSS in MinIO Console.
Is a best practice to set a restrictive CSP, also XSS is been in OWASP Top 10 for many years and CSP is one of the recommended for Controls for XSS, see https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#other-controls
We are also having customers requesting this cause vuln showed up in their 3rd party pentests @harshavardhana

We can set standard headers and be done with it. Why does it need to be customized?

That won't work for all cases, becasue of third party services like OpenID, LDAP, etc. Anything console need to interact with from the client (browser) direcly, we'll need to know which domains to trust ie: for a XHR call to OpenID

[harshavardhana]

That won't work for all cases, becasue of third party services like OpenID, LDAP, etc. Anything console need to interact with from the client (browser) direcly, we'll need to know which domains to trust ie: for a XHR call to OpenID

Can you provide examples of all the values that can be configured and what is the value that users must set? in those scenarios?

I don't think adding this like a naked environment variable is going to work, that can take literally any string doesn't make sense.

[klauspost]

So it transfers the value from one env var to another.

Why not just set the correct one from the beginning?

[pjuarezd]

So it transfers the value from one env var to another.

Why not just set the correct one from the beginning?

We are removing all setted env variables for console and then setting only "the right ones", not my design so I can advocate for it, I am just following the established pattern @klauspost

minio/cmd/common-main.go

Lines 212 to 217 in 4a21dce

	for _, cenv := range env.List(consolePrefix) {
	os.Unsetenv(cenv)
	}
	// enable all console environment variables
	minioConfigToConsoleFeatures()

[pjuarezd]

That won't work for all cases, becasue of third party services like OpenID, LDAP, etc. Anything console need to interact with from the client (browser) direcly, we'll need to know which domains to trust ie: for a XHR call to OpenID

Can you provide examples of all the values that can be configured and what is the value that users must set? in those scenarios?

I provided a "starters" example policy as a comment @harshavardhana
default-src 'self' 'unsafe-eval' 'unsafe-inline';

https://github.com/minio/minio/pull/18631/files#diff-4a2ae40b87e0b6d02d439f087737b78754a045535b6c3cf87e2ced69e0c900beR185

Althoug I cannot foresight all scenarios, cause (as the purpose of CSP is,) CSP is to constrain which domain can resources load from, is why we need to allow admin to set this value.

I don't think adding this like a naked environment variable is going to work, that can take literally any string doesn't make sense.

Which would be a preferred way? ie: we could be verify the input is a valid CSP, like by using the mc admin config set tool

[harshavardhana]

We are removing all setted env variables for console and then setting only "the right ones", not my design so I can advocate for it, I am just following the established pattern @klauspost

That is because the Console's own defaults must be removed, or some user mistakenly set something for CONSOLE_ - we want them to use MINIO_ not CONSOLE_.

[harshavardhana]

Is this PR ready, or are we making any more changes?

• Referrer-Policy Totally fine we can add a default value
• Strict-Transport-Security We will need some UI (CLI and/or web) to allow setting the fields values max-age, includeSubdomains, and preload
• Content-Security-Policy We can set a default policy including all domains MinIO is aware of and a possible artisan way to set a handcrafted value (CLI and web)

There are 3 things that are needed - which one is covered in this PR?

[pjuarezd]

Is this PR ready, or are we making any more changes?

WIP, just rebased to resolve conflicts

[pjuarezd]

Is this PR ready, or are we making any more changes?

• Referrer-Policy Totally fine we can add a default value
• Strict-Transport-Security We will need some UI (CLI and/or web) to allow setting the fields values max-age, includeSubdomains, and preload
• Content-Security-Policy We can set a default policy including all domains MinIO is aware of and a possible artisan way to set a handcrafted value (CLI and web)

There are 3 things that are needed - which one is covered in this PR?

This is ready @harshavardhana