jdauphant / ansible-role-nginx

Ansible role to install and manage nginx configuration
655 stars 302 forks source link

Strict-Transport-Security header gets mangled #194

Open apenney opened 7 years ago

apenney commented 7 years ago

Hi, we have the following variable set:

nginx_http_params:
  - server_names_hash_bucket_size 256
  - server_tokens           off
  - etag                    on
  - if_modified_since       before
  - types_hash_max_size     2048

  - add_header X-Frame-Options SAMEORIGIN
  # Add HSTS
  - add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always

And this gets turned into:

   add_header Strict-Transport-Security "max-age=31536000;
       includeSubDomains" always;

Is there a way to escape or exclude a ; from being parsed in templates/site.conf.j2? I assume it's getting hit by:

   {% if v != "" %}{{ v.replace(";",";\n      ").replace(" {"," {\n      ").replace(" }"," \n   }\n") }}{% if v.find('{') == -1%};

To add the \n no matter what. This is causing us to have an incorrect header that breaks Cloudflare.

jdauphant commented 7 years ago

Hello, You have the solution in #135 If you have an idea of pull request to improve the documentation to find the solution, it will be more than welcome.

Also for "nginx_http_params", there is no post treatment with \n, you were meaning "nginx_configs" or "nginx_sites" instead ?

apenney commented 7 years ago

We saw #135 but we couldn't understand how that helped us! Sorry, we might be being stupid, but we weren't sure what to do.

After digging deeper it turns out we have it in nginx_http_params AND the site itself, so we can remove it from the site and leave it in the http_params section and that should fix this. I don't know why we put it in two places!

jdauphant commented 7 years ago

nginx_http_params didn't have post treatment, you will have not trouble to put it here. The solution for nginx_sites is:

nginx_sites:
 test:
     - |
       listen 80;
       server_name www.example.com;
       root "/home/www";
       add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;
mjrsnyder commented 6 years ago

For anyone else who comes across this, here's what I did:

nginx_configs:
  ssl:
    - |
      ssl_certificate_key {{ nginx_ssl_key_path }};
      ssl_certificate {{ nginx_ssl_cert_path }};
      ...
      add_header Strict-Transport-Security "max-age=31536000;includeSubDomains;preload" always;

Switching the whole ssl config over to multiline notation seems to produce the desired result.

smbambling commented 5 years ago

You don't need to move the whole config over to multiline notation you can mix and match. Just make sure to put an ending ; just like you would with the full config for multiline stanzas


nginx_configs:
  account_web_ssl:
    - listen 443 ssl http2
    - listen [::]:443 ssl http2
    - server_name foo.{{ ansible_domain }} foo.example.com
    - |
      add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
    - access_log "{{ nginx_log_dir }}/foo_ssl_access.log" main
    - error_log "{{ nginx_log_dir }}/foo_ssl_error.log" error
    - root "/opt/foo_web"
    - index index.html
    - try_files $uri $uri/ /index.html
    - error_page 404 /404.html