From 2bc91bdd45a103fdc09bbac5788076d47a43ec8d Mon Sep 17 00:00:00 2001
From: Kristian Klausen <kristian@klausen.dk>
Date: Sun, 11 Aug 2024 18:22:48 +0200
Subject: [PATCH] Fix missing HSTS header for some URLs due to nginx "directive
 inheritance"[1]

F5/nginx has blogged about this[1] and it is also mentioned in nginx's
documentation[2]:
"There could be several add_header directives. These directives are
inherited from the previous configuration level if and only if there are
no add_header directives defined on the current level. "

The problem occurs when add_header is used in a child context like a
server{} or location{} block. It is solved by moving the HSTS header
into a snippet, which is now included before all add_header lines.

For now the HSTS header is the only global header, but in the future we
may need to add more global headers, like the Alt-Svc header[3] for
HTTP/3.

[1] https://www.f5.com/company/blog/nginx/avoiding-top-10-nginx-configuration-mistakes#directive-inheritance
[2] https://nginx.org/en/docs/http/ngx_http_headers_module.html#add_header
[3] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Alt-Svc

Fix #608
---
 roles/archmanweb/templates/nginx.d.conf.j2          |  1 +
 roles/archweb/templates/ipxe.archlinux.org.j2       |  1 +
 roles/archweb/templates/maintenance-nginx.d.conf.j2 |  2 ++
 roles/archweb/templates/nginx.d.conf.j2             | 10 +++++++---
 roles/archwiki/templates/nginx.d.conf.j2            |  4 ++++
 roles/aurweb/templates/nginx.d.conf.j2              |  2 ++
 roles/fluxbb/templates/nginx.conf.j2                |  2 ++
 roles/matrix/templates/nginx.d.conf.j2              |  1 +
 roles/mirrorsync/templates/nginx.d.conf.j2          |  1 +
 roles/nginx/tasks/main.yml                          |  1 +
 roles/nginx/templates/headers.conf                  |  1 +
 roles/nginx/templates/nginx.conf.j2                 |  1 +
 roles/nginx/templates/sslsettings.conf              |  3 +--
 roles/ping/templates/nginx.d.conf.j2                |  1 +
 roles/rebuilderd/templates/nginx.d.conf.j2          |  5 ++---
 roles/syncrepo/templates/nginx.d.conf.j2            |  1 +
 16 files changed, 29 insertions(+), 8 deletions(-)
 create mode 100644 roles/nginx/templates/headers.conf

diff --git a/roles/archmanweb/templates/nginx.d.conf.j2 b/roles/archmanweb/templates/nginx.d.conf.j2
index 275f38162..3c51fe125 100644
--- a/roles/archmanweb/templates/nginx.d.conf.j2
+++ b/roles/archmanweb/templates/nginx.d.conf.j2
@@ -49,6 +49,7 @@ server {
     # Client-cache for Django's static assets
     location /static/ {
         expires 30d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public";
         alias {{ archmanweb_dir }}/repo/collected_static/;
diff --git a/roles/archweb/templates/ipxe.archlinux.org.j2 b/roles/archweb/templates/ipxe.archlinux.org.j2
index 2df4e1d60..dd6c6f374 100644
--- a/roles/archweb/templates/ipxe.archlinux.org.j2
+++ b/roles/archweb/templates/ipxe.archlinux.org.j2
@@ -41,6 +41,7 @@ server {
     # Cache django's css, js and png files.
     location /static/ {
         expires 30d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public";
         alias /srv/http/archweb/collected_static/;
diff --git a/roles/archweb/templates/maintenance-nginx.d.conf.j2 b/roles/archweb/templates/maintenance-nginx.d.conf.j2
index 6eddee948..e9f743772 100644
--- a/roles/archweb/templates/maintenance-nginx.d.conf.j2
+++ b/roles/archweb/templates/maintenance-nginx.d.conf.j2
@@ -120,6 +120,7 @@ server {
 
     location = /.well-known/matrix/client {
         default_type application/json;
+        include snippets/headers.conf;
         add_header Access-Control-Allow-Origin *;
         return 200 '{"m.homeserver": {"base_url": "https://{{ matrix_domain }}"}, "m.identity_server": {"base_url": "https://matrix.org"} }';
     }
@@ -167,6 +168,7 @@ server {
     # Cache django's css, js and png files.
     location /static/ {
         expires 30d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public";
         alias {{ archweb_dir }}/collected_static/;
diff --git a/roles/archweb/templates/nginx.d.conf.j2 b/roles/archweb/templates/nginx.d.conf.j2
index 8dcd8d7b0..186befa29 100644
--- a/roles/archweb/templates/nginx.d.conf.j2
+++ b/roles/archweb/templates/nginx.d.conf.j2
@@ -42,6 +42,7 @@ server {
     include snippets/letsencrypt.conf;
 
     location /.well-known/ {
+        include snippets/headers.conf;
         add_header Access-Control-Allow-Origin *;
         return 301 https://$server_name$request_uri;
     }
@@ -67,6 +68,7 @@ server {
     ssl_trusted_certificate /etc/letsencrypt/live/{{ archweb_domain }}/chain.pem;
 
     location /.well-known/ {
+        include snippets/headers.conf;
         add_header Access-Control-Allow-Origin *;
         return 301 https://{{ archweb_domain }}{{ domain['redirect']|default('$request_uri') }};
     }
@@ -120,6 +122,7 @@ server {
 
     location = /.well-known/matrix/client {
         default_type application/json;
+        include snippets/headers.conf;
         add_header Access-Control-Allow-Origin *;
         return 200 '{"m.homeserver": {"base_url": "https://{{ matrix_domain }}"}, "m.identity_server": {"base_url": "https://matrix.org"} }';
     }
@@ -169,6 +172,7 @@ server {
     # Cache django's css, js and png files.
     location /static/ {
         expires 30d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public";
         alias {{ archweb_dir }}/collected_static/;
@@ -189,6 +193,7 @@ server {
 
         uwsgi_cache archwebcache;
         uwsgi_cache_revalidate on;
+        include snippets/headers.conf;
         add_header X-Cache-Status $upstream_cache_status;
 
         limit_req zone=rsslimit burst=10 nodelay;
@@ -202,6 +207,7 @@ server {
         uwsgi_cache archwebcache;
         uwsgi_cache_revalidate on;
         uwsgi_cache_key $cache_key;
+        include snippets/headers.conf;
         add_header X-Cache-Status $upstream_cache_status;
 
         limit_req zone=mirrorstatuslimit burst=10 nodelay;
@@ -235,11 +241,9 @@ server {
         uwsgi_cache archwebcache;
         uwsgi_cache_revalidate on;
         uwsgi_cache_key $cache_key;
+        include snippets/headers.conf;
         add_header X-Cache-Status $upstream_cache_status;
 
-        # re-add HSTS (inheritance from sslsettings.conf broken by above header)
-        add_header Strict-Transport-Security $hsts_header always;
-
         limit_req zone=archweblimit burst=10 nodelay;
     }
 }
diff --git a/roles/archwiki/templates/nginx.d.conf.j2 b/roles/archwiki/templates/nginx.d.conf.j2
index 6a6333544..0be7fe318 100644
--- a/roles/archwiki/templates/nginx.d.conf.j2
+++ b/roles/archwiki/templates/nginx.d.conf.j2
@@ -125,6 +125,7 @@ server {
         fastcgi_cache_use_stale updating;
         fastcgi_cache_lock on;
 
+        include snippets/headers.conf;
         add_header X-Cache $upstream_cache_status;
 {% endblock %}
     }
@@ -143,6 +144,7 @@ server {
     # normal PHP FastCGI handler
     location ~ ^/[^/]+\.php$ {
         if ($challenge) {
+            include snippets/headers.conf;
             add_header Set-Cookie "challenge={{ archwiki_nginx_challenge_value }}; SameSite=Strict";
             return 303 $scheme://$server_name/$request_uri;
         }
@@ -165,12 +167,14 @@ server {
     # MediaWiki assets
     location ~ ^/(?:images|resources/(?:assets|lib|src)|(?:skins|extensions)/.+\.(?:css|js|gif|jpg|jpeg|png|svg|wasm)$) {
         expires 30d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public, must-revalidate, proxy-revalidate";
     }
 
     location /images/ {
         # Add the nosniff header to the images folder (required for mw 1.40+)
+        include snippets/headers.conf;
         add_header X-Content-Type-Options nosniff;
     }
 
diff --git a/roles/aurweb/templates/nginx.d.conf.j2 b/roles/aurweb/templates/nginx.d.conf.j2
index 1373b6623..a594ef98c 100644
--- a/roles/aurweb/templates/nginx.d.conf.j2
+++ b/roles/aurweb/templates/nginx.d.conf.j2
@@ -110,6 +110,7 @@ server {
     location ~ \.gz$ {
         root    {{ aurweb_dir }}/archives;
         default_type text/plain;
+        include snippets/headers.conf;
         add_header Content-Encoding gzip;
         expires 5m;
     }
@@ -118,6 +119,7 @@ server {
         rewrite ^/static(/.*)$ $1 break;
 
         expires 7d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public, must-revalidate, proxy-revalidate";
     }
diff --git a/roles/fluxbb/templates/nginx.conf.j2 b/roles/fluxbb/templates/nginx.conf.j2
index f4678cf83..563cfb13e 100644
--- a/roles/fluxbb/templates/nginx.conf.j2
+++ b/roles/fluxbb/templates/nginx.conf.j2
@@ -76,12 +76,14 @@ server {
 
     location ^~ /style/ {
         expires 7d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public, must-revalidate, proxy-revalidate";
     }
 
     location ^~ /img/ {
         expires 7d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public, must-revalidate, proxy-revalidate";
     }
diff --git a/roles/matrix/templates/nginx.d.conf.j2 b/roles/matrix/templates/nginx.d.conf.j2
index a353415c3..85fdc500f 100644
--- a/roles/matrix/templates/nginx.d.conf.j2
+++ b/roles/matrix/templates/nginx.d.conf.j2
@@ -45,6 +45,7 @@ server {
         access_log /var/log/nginx/{{ matrix_domain }}/access.log main;
         access_log /var/log/nginx/{{ matrix_domain }}/access.log.json json_main;
 {% if location.add_cors | default(false) %}
+        include snippets/headers.conf;
         add_header Access-Control-Allow-Origin *;
         add_header Access-Control-Allow-Methods "GET, HEAD, POST, PUT, DELETE, OPTIONS";
         add_header Access-Control-Allow-Headers "X-Requested-With, Content-Type, Authorization, Date";
diff --git a/roles/mirrorsync/templates/nginx.d.conf.j2 b/roles/mirrorsync/templates/nginx.d.conf.j2
index 22604eb57..4eb9cd844 100644
--- a/roles/mirrorsync/templates/nginx.d.conf.j2
+++ b/roles/mirrorsync/templates/nginx.d.conf.j2
@@ -17,6 +17,7 @@ server {
     ssl_certificate_key  /etc/letsencrypt/live/{{ item.value.mirror_domain }}/privkey.pem;
     ssl_trusted_certificate /etc/letsencrypt/live/{{ item.value.mirror_domain }}/chain.pem;
 
+    include snippets/headers.conf;
     add_header X-Served-By "{{ inventory_hostname }}";
 
     autoindex on;
diff --git a/roles/nginx/tasks/main.yml b/roles/nginx/tasks/main.yml
index 5ff618a57..1129b280c 100644
--- a/roles/nginx/tasks/main.yml
+++ b/roles/nginx/tasks/main.yml
@@ -23,6 +23,7 @@
   with_items:
     - letsencrypt.conf
     - sslsettings.conf
+    - headers.conf
   notify:
     - Reload nginx
 
diff --git a/roles/nginx/templates/headers.conf b/roles/nginx/templates/headers.conf
new file mode 100644
index 000000000..d42743037
--- /dev/null
+++ b/roles/nginx/templates/headers.conf
@@ -0,0 +1 @@
+add_header Strict-Transport-Security $hsts_header always;
diff --git a/roles/nginx/templates/nginx.conf.j2 b/roles/nginx/templates/nginx.conf.j2
index 584c26406..29ba71416 100644
--- a/roles/nginx/templates/nginx.conf.j2
+++ b/roles/nginx/templates/nginx.conf.j2
@@ -90,6 +90,7 @@ http {
     access_log syslog:server=unix:/dev/log,nohostname,tag=nginx_http main;
 
     include snippets/sslsettings.conf;
+    include snippets/headers.conf;
 
     include nginx.d/*.conf;
 }
diff --git a/roles/nginx/templates/sslsettings.conf b/roles/nginx/templates/sslsettings.conf
index 1c98ad9ff..43d7f9382 100644
--- a/roles/nginx/templates/sslsettings.conf
+++ b/roles/nginx/templates/sslsettings.conf
@@ -13,10 +13,9 @@ ssl_session_tickets off;
 ssl_stapling on;
 ssl_stapling_verify on;
 
+# See headers.conf for the HSTS add_header line.
 map $scheme $hsts_header {
     https   "max-age=31536000; includeSubdomains; preload";
 }
 
-add_header Strict-Transport-Security $hsts_header always;
-
 resolver 127.0.0.53;
diff --git a/roles/ping/templates/nginx.d.conf.j2 b/roles/ping/templates/nginx.d.conf.j2
index 0149561ca..ea3757426 100644
--- a/roles/ping/templates/nginx.d.conf.j2
+++ b/roles/ping/templates/nginx.d.conf.j2
@@ -26,6 +26,7 @@ server {
     # https://man.archlinux.org/man/NetworkManager.conf.5#CONNECTIVITY_SECTION
     location = /nm-check.txt {
         access_log off;
+        include snippets/headers.conf;
         add_header Cache-Control "max-age=0, must-revalidate";
         return 200 'NetworkManager is online\n';
     }
diff --git a/roles/rebuilderd/templates/nginx.d.conf.j2 b/roles/rebuilderd/templates/nginx.d.conf.j2
index 54ec63890..bd4cc6c5a 100644
--- a/roles/rebuilderd/templates/nginx.d.conf.j2
+++ b/roles/rebuilderd/templates/nginx.d.conf.j2
@@ -30,6 +30,7 @@ server {
     ssl_trusted_certificate /etc/letsencrypt/live/{{ rebuilderd_domain }}/chain.pem;
 
     # Security headers
+    include snippets/headers.conf;
     add_header X-Frame-Options "SAMEORIGIN" always;
     add_header X-Xss-Protection "1; mode=block" always;
     add_header Referrer-Policy "same-origin";
@@ -37,13 +38,11 @@ server {
     add_header Content-Security-Policy "default-src 'self';";
     add_header X-Content-Type-Options "nosniff" always;
 
-    # Apply HSTS header again, since adding a header removes previous headers
-    add_header Strict-Transport-Security $hsts_header;
-
     root {{ rebuilder_website_loc }};
 
     location ~* (css|js|svg)$ {
         expires 30d;
+        include snippets/headers.conf;
         add_header Pragma public;
         add_header Cache-Control "public, must-revalidate, proxy-revalidate";
     }
diff --git a/roles/syncrepo/templates/nginx.d.conf.j2 b/roles/syncrepo/templates/nginx.d.conf.j2
index e5925f837..c5743639e 100644
--- a/roles/syncrepo/templates/nginx.d.conf.j2
+++ b/roles/syncrepo/templates/nginx.d.conf.j2
@@ -19,6 +19,7 @@ server {
     ssl_trusted_certificate /etc/letsencrypt/live/{{ domain }}/chain.pem;
 
 {% if 'geo_mirrors' in group_names and domain == geo_mirror_domain %}
+    include snippets/headers.conf;
     add_header X-Served-By "{{ inventory_hostname }}";
 {% endif %}
 
-- 
GitLab