Added web security headers for nginx configuration #5183
Conversation
pull-request-size
bot
added
the
size/XS
pull-request-size
bot
added
size/S
conf/nginx/st2.conf
Outdated
| @@ -40,6 +40,10 @@ server { | |||
|
|
|||
| add_header Front-End-Https on; | |||
| add_header X-Content-Type-Options nosniff; | |||
| add_header X-Frame-Options DENY always; | |||
| add_header Strict-Transport-Security "max-age=3153600;includeSubDomains"; | |||
| add_header Cache-Control "no-store; no-cache; max-age=0; must-revalidate;"; | |||
There was a problem hiding this comment.
While the security-focused headers are all good, I'm wondering what would be the user impact for the Cache-Control changes here and totally disabling the caching? @ashwini-orchestral what are the specific reasons behind hardening cache-control this way?
WDYT @StackStorm/maintainers @StackStorm/contributors? Any downsides, any behavior/UX changes you could think of?
There was a problem hiding this comment.
Yeah, +1, I don't see much benefit security wise from cache control headers, it would just slow things down if we need to re-fetch all the static resources on each page (re)load...
But others (iframe, stric transport security, etc.) are indeed a good idea 👍
On that note, I believe we need to sync up multiple places (if they don't contain those headers already) - Docker, installer, etc. Or do those places use config from this repo?
There was a problem hiding this comment.
Right, we're pulling in all the places nginx config from the st2 repo (this file).
I think only Puppet is different with the templating @nmaludy ? I guess you'll need to update that
|
On a somewhat related note:
TLS v1.0 and v1.1 are also both past it's prime time now so it would probably be a good idea to just support TLS v1.2 going forward. I don't think it should negatively affect any installations (most everything supports TLS v1.2 these days), but we can document it in ugprade notes and on how to add support for TLS v1.1 back, if needed. |
|
@Kami Good point. |
Only support TLS v1.2 which is the recommended and safe choice at this point.
|
@armab Yeah, I think now is the right time. I will also open docs and upgrade notes entry with that in case it affects someone, but I think that would be quite unlikely. |
| ssl_protocols TLSv1 TLSv1.1 TLSv1.2; | ||
| ssl_protocols TLSv1.2; |
There was a problem hiding this comment.
@ashwini-orchestral Left one change request per team feedback and please also provide a Changelog record for this PR.
|
Added web header settings for possible security issues, X-Frame-Options, Strict-Transport-Security, X-XSS-Protection and server-tokens. |
There was a problem hiding this comment.
Per @ashwini-orchestral, she has tested this nginx configuration change on an st2 install and there's no obvious issue as a result. This change is required to harden st2 deployment.
|
@ashwini-orchestral Can you add an entry in the CHANGELOG? |
Added a few web security headers in the Nginx configuration.
For example - X-Frame-Options Header, Cache-control.
Disable server version information.