Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#854 closed enhancement (wontfix)

Add inherited keyword for altering directive inheritance

Reported by: Haravikk@… Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.9.x
Keywords: configuration Cc:
uname -a: Linux localhost 3.16.0-53-generic #72~14.04.1-Ubuntu SMP Fri Nov 6 18:17:23 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.8.0
built with OpenSSL 1.0.1f 6 Jan 2014
TLS SNI support enabled
configure arguments: --with-cc-opt='-g -O2 -fPIE -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2' --with-ld-opt='-Wl,-Bsymbolic-functions -fPIE -pie -Wl,-z,relro -Wl,-z,now' --prefix=/usr/share/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-debug --with-pcre-jit --with-ipv6 --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_addition_module --with-http_dav_module --with-http_geoip_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module --with-http_spdy_module --with-http_sub_module --with-http_xslt_module --with-mail --with-mail_ssl_module --add-module=/build/buildd/nginx-1.8.0/debian/modules/nginx-auth-pam --add-module=/build/buildd/nginx-1.8.0/debian/modules/nginx-dav-ext-module --add-module=/build/buildd/nginx-1.8.0/debian/modules/nginx-echo --add-module=/build/buildd/nginx-1.8.0/debian/modules/nginx-upstream-fair --add-module=/build/buildd/nginx-1.8.0/debian/modules/ngx_http_substitutions_filter_module

Description

I wanted to set a few basic headers for all of my HTTP responses in nginx, however the current behaviour of add_header is that it is inherited from http only if a server block has no headers of its own.

I'd like to propose the addition of an inherited (or similar) keyword for the add_header directive that will allow it to be explicitly inherited by nested blocks in spite of its normal behaviour.

However, it is likely that this override may be useful for other directives that aren't inherited by default as well. The only one that springs to mind is try_files which I include in most location blocks except those that are to forbid access (where I put a deny instead).

In my particular use case I was hoping to add the X-Frame-Options: DENY header to all http responses, thus preventing all content on my sites from being placed in iframes by default (in supported browsers). This is a useful way to avoid legitimate pages being used for phishing/click-jacking in modern browsers (i.e- attacks where the user is interacting with the actual, full-featured site, but what they do is captured by the page it is embedded within). Unfortunately most of my server blocks have a custom header of some kind, particularly SSL server blocks (which have an HSTS header) so add_header under http has no effect.

It's fairly trivial in this case I admit, but I think that allowing users to override inheritence behaviour when they know they want to is useful.

Change History (3)

comment:1 by Haravikk@…, 4 years ago

I just wanted to add some other examples:

Strict-Transport-Security is a very important header to declare for secure sites, and it'd be nice to be able to just declare it once in my secure server block, without having to repeat it in any location block with custom headers.

Content-Security-Policy is a particularly awkward header; it'd be great to be able to set a global policy (e.g- default-src: 'none'; then add onto this with script-src, etc. That said, it is a somewhat uniquely difficult header to manage, as it's possible that inheriting a policy and adding to it won't work because added policies can only be more restrictive, not less, if they declare the same elements, which means that the best way to handle it is still to override and completely replace it; not sure if there is any good solution to this particular one, except perhaps that if an inherited keyword is added then perhaps an override keyword to complement it would still be useful (so I could still choose to override an already set, inherited header completely)?

comment:2 by Maxim Dounin, 4 years ago

Component: documentationnginx-core
Resolution: wontfix
Status: newclosed

Practice shows that better approach for special headers is to use special directives to control them, with independent inheritance. E.g., this is how we handle Cache-Control / Expires with the expires directive.

While introducing a special flag to copy list of headers from previous level is possible, I don't think the result will be more convenient than what we have now. And, as your follow-up comments show, this approach may easily result in a bloated "inheritance editor" instead.

It's also worth noting that it's not really possible to introduce such a flag as a generic mechanism. It will require code for all directives to support the flag.

comment:3 by aeris@…, 3 years ago

I reopen this ticket because I have the same trouble with add_header current behavior.

Today, you want to have global headers for security, applied everywhere in every case:

http {
    add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
    add_header X-Frame-Options "DENY" always;
    add_header X-XSS-Protection "1; mode=block" always;
    add_header X-Content-Type-Options "nosniff" always;
    add_header Content-Security-Policy "default-src 'none'" always;
}

Then, you want specific vhost headers, for security too, and some redefining global ones:

server {
  add_header Public-Key-Pins "max-age=5184000; pin-sha256=\"xxx=\"; pin-sha256=\"yyy=\";" always;
  add_header Content-Security-Policy "default-src 'none'; style-src 'self'; script-src 'self'; img-src 'self'; font-src 'self';" always;
}

Then, you want specific location headers, for example:

location /assets/ {
   add_header Cache-Control public always;
}

location /admin/ {
   add_header Content-Security-Policy "default-src 'self'; style-src 'unsafe-inline' 'self'; script-src 'unsafe-inline' 'self';" always;
}

The current behavior of nginx force the user to mentally do the inheritance, with risk of error, and with a difficult way to ensure the result (curling some random location to roughly see if correct).

And there is some security risk in case of misunderstanding of the behavior or external config change. In my case, doing an upgrade of a vendor nginx vhost config just add a new header on the / location, and so drop all HSTS, HPKP, CSP or others security protection.

You can always split your config on multiple snippet files but you have to simulate all the inheritance with a snippets/global.conf included by all snippets/vhost-x.conf, included by site-availables/vhost-x.conf in server and each location section. But this is not an ideal workaround and you're always at risk (specially in case of vendor config) of location/vhost not including snippet or not the right.

If you use config generator, simulating inheritance is hard and you have to do a lot of work to be able to propagate headers down to locations, with lots of code and lots of config. And even in this case, there is no simple way to ensure the resulting behavior.

Some headers are difficult to "inherit" too. CSP for example, if included inside a snippet, can't be overwritten (both snippet and overwritten value will be present at the end).

I think it will be better, safer and simpler to have a directive directly in nginx to allow inheritance, at least for add_header which is the only inheritance trouble I met.
An add_header option inherit like always will do the job, for example add_header Strict-Transport-Security XXX inherit;. An overwrite option will be cool too for CSP (add_header Content-Security-Policy XXX overwrite;).

Last edited 3 years ago by aeris@… (previous) (diff)
Note: See TracTickets for help on using tickets.