AirisX / nginx_cookie_flag_module

Module for Nginx which allows to set the flags "HttpOnly", "secure" and "SameSite" for cookies.
BSD 2-Clause "Simplified" License
106 stars 67 forks source link

It's not clear whether this module should work with non-proxied requests or not #5

Closed defanator closed 6 years ago

defanator commented 6 years ago

Example configuration:

-bash-4.2$ cat /etc/nginx/nginx.conf
load_module modules/ngx_http_cookie_flag_filter_module.so;

user nginx;
worker_processes 1;

error_log /var/log/nginx/error.log notice;
pid /var/run/nginx.pid;

events {}

http {
    access_log off;
    root /usr/share/nginx/html;
    index index.html index.htm;

    upstream u {
        server 127.0.0.1:8080;
    }

    server {
        listen 80 default_server;
        server_name localhost;
        set_cookie_flag * HttpOnly;

        location / {
            add_header Set-Cookie "mycookie=from_static";
        }

        location /proxy {
            proxy_pass http://u/;
        }
    }

    server {
        listen 8080 default_server;
        location / {
            add_header Set-Cookie "mycookie=from_upstream";
        }
    }
}
-bash-4.2$ curl -si http://localhost/ | fgrep Set-Cookie
Set-Cookie: mycookie=from_static
-bash-4.2$ curl -si http://localhost/proxy | fgrep Set-Cookie
Set-Cookie: mycookie=from_upstream; HttpOnly

So obviously the module works fine with upstream responses, and doesn't work with "local" add_header directives (which is somewhat expected for experienced nginx users), but it would be good to either have this clarified in documentation, or improve the code, if this behavior is wrong by the author's vision.

Thanks.

AirisX commented 6 years ago

Hi @defanator,

Yes, the key target of this module is set cookie flags for the upstream responses. In real situations we often doesn't know what security settings provides backend software and even we can't affect on it. This module cover this cases.

As for the "local" add_header directives, if we can set a cookie, what prevents us from setting the required flags? For example:

location / {
    add_header Set-Cookie "mycookie=from_static; HttpOnly";
}

But in any case, your idea is clear to me and I'll consider the need for its implementation. Thank you!

defanator commented 6 years ago

@AirisX maybe this change would be enough for now:

diff --git a/README.md b/README.md
index 93c3f2a..5681786 100644
--- a/README.md
+++ b/README.md
@@ -42,7 +42,7 @@ location / {

 ## Description
-This module for Nginx allows to set the flags "**HttpOnly**", "**secure**" and "**SameSite**" for cookies in the "*Set-Cookie*" response headers.
+This module for Nginx allows to set the flags "**HttpOnly**", "**secure**" and "**SameSite**" for cookies in the "*Set-Cookie*" upstream response headers.
 The register of letters for the flags doesn't matter as it will be converted to the correct value. The order of cookie declaration among multiple directives doesn't matter too.
 It is possible to set a default value using symbol "*". In this case flags will be added to the all cookies if no other value for them is overriden.
AirisX commented 6 years ago

@defanator I think, yes.

AirisX commented 6 years ago

@defanator done here