Code Monkey home page Code Monkey logo

Comments (14)

ploxiln avatar ploxiln commented on June 11, 2024 2

When you get redirected to /oauth2/sign_in or /oauth2/start and eventually to /oauth2/callback, you get responses directly from oauth2_proxy, so it can set two separate cookies with two separate response headers, and the browser gets that response and those headers directly.

In the refresh case, nginx is getting some response headers from the auth_request, and then making the request to the actual application, and then adding to the response from the actual application. It's this part that it seems can't work for two Set-Cookie headers.

from oauth2-proxy.

marratj avatar marratj commented on June 11, 2024 1

Hi @nniikkoollaaii

name always needs to be substituted with the actual cookie name (which can only contain letters, numbers and underscores due to how nginx adresses cookie names as variables).

So if you don't set a custom cookie name in oauth2_proxy (thus keeping the default _oauth2_proxy), the configuration should look like this:

nginx.ingress.kubernetes.io/configuration-snippet: |
    auth_request_set $varname_set_by_you $upstream_cookie__oauth2_proxy_1;

    access_by_lua_block {
    if ngx.var.varname_set_by_you ~= "" then
        ngx.header["Set-Cookie"] = "_oauth2_proxy_1=" .. ngx.var.varname_set_by_you .. ngx.var.auth_cookie:match("(; .*)")
    end
    }

This puts the content of the upstream cookie named _oauth2_proxy_1 into the variable varname_set_by_you and, if the content of it is non-empty, adds the response header Set-Cookie _oauth2_proxy_1 with the content of the variable/upstream cookie.

In the meanwhile, we ourselves switched to using a relatively current commit from master which includes #155

This stores the session info on the server side in a redis cache, therefore only a single small client cookie is required (around 100 bytes) as this only stores the key for accessing the server-side session info. I suggest you give that one a try as this eliminates all the issues with big and split cookies.

from oauth2-proxy.

brianv0 avatar brianv0 commented on June 11, 2024

I was just about to open this issue.

I've been trying to debug it for quite a while in the context of Kubernetes.

The core issue is that nginx seems to drop the additional Set-Cookie header, so the cookie actually ends up getting corrupted, because _oauth_proxy-0 is properly set, but _oauth_proxy-1 is omitted. The ends up corrupting the cookie when it's joined, as it contains the last (3960-xxx) bytes).

So, what happens, is this:
Request comes in before refresh time after first issuance - everything is fine.
Request comes in after refresh time, refresh is made, cookies are set, response continues as normal.
Request comes in after refreshed token, cookie signature fails to validate, user is redirected to login.

In my case, my cookies are 4300 bytes, so just over the limit for splitting. Also in my case, I have access to the nginx logs and I do not see anything funny, but I don't have access to the ingress controller so I can't run tcpdump to wire trace this.

I think it may be fixable by fiddling proxy_headers_hash_bucket_size, but I'm not sure. There was some additional weirdness I didn't understand where the sizes of the previous _oauth2_proxy-0 and current _oath2_proxy-0 after the refresh seem to be different sizes as reported by chrome, but that might just be a chrome idiosyncrasy.

I'm not sure, but it may also be related to the fact that it is using a - in the name.

This is blocking me from my work, so for my purposes, this week I'm looking at implementing a server-side storage of large cookies, where I will actually issue the hash of what-would-be the cookie value as the cookie, and use that hash as the handle to lookup the cookie on the filesystem for now (though the filesystem could be replaced with a database/redis). This, understandably, makes things more complicated because now oauth2_proxy has a dependency on a storage element where previously it required none (thanks to cookies), and there will need to be some sort of daemon which cleans up dead session, but it has the side effect that the request size, should drop 4kB - as the cookies will be ~128 bytes vs 4300. That might be desirable for Single-Page Applications in large token environments.

from oauth2-proxy.

ploxiln avatar ploxiln commented on June 11, 2024

I think the issue is that when you use nginx auth_request, to get cookie refresh working you need:

    auth_request_set $auth_cookie $upstream_http_set_cookie;
    add_header Set-Cookie $auth_cookie;

but that just gets and sets a single header, these variables and directives were not designed for multiple cookie headers.

This cookie splitting was implemented because of the default limit in nginx, which is around 4KiB - but you could increase the limit in nginx with proxy_buffer_size and remove the splitting from the oauth2_proxy code, and get back to one huge response header.

Whether you do that or not, I think browsers will generally send the (next) request with all cookies in a single Cookie header, so the next limit you'll come up against is the 8KiB default in large_client_header_buffers.

I also agree that avoiding huge cookies, somehow, is a good idea. An in-process cache, or optional memcached, may be useful for some cases. Just my 2c.

from oauth2-proxy.

marratj avatar marratj commented on June 11, 2024

I thought about just raising the proxy_buffer_size, but this is not helping in this case, as browsers themselves usually have a hard limit on Cookie size of 4KiB.

In my case I specifically need to extract an additional claim from our id_token for the backend, but as our tokens are too large to fit into a single cookie, I most probably still need to change oauth2_proxy to extract my required claim and add it as a header for auth_request.

from oauth2-proxy.

ploxiln avatar ploxiln commented on June 11, 2024

I see, you're right, browsers limit to ~ 4KiB per cookie.

from oauth2-proxy.

brianv0 avatar brianv0 commented on June 11, 2024

in rootfs/etc/nginx/template/nginx.tmpl:

            auth_request        {{ $authPath }};
            auth_request_set    $auth_cookie $upstream_http_set_cookie;
            add_header          Set-Cookie $auth_cookie;

So I think this may have something to do with it, however, this ONLY happens on refresh. Which means the first time you Set-Cookie, you get both for some reason. It's when you refresh that you only get one. I really don't know why, but maybe it has something to do with the fact that your cookies should have been cleared (and thus, not sent in the request) when you are first logging in, and, when refreshing, you do send the cookies in the request but only get one of the cookies back.

from oauth2-proxy.

brianv0 avatar brianv0 commented on June 11, 2024

Ah! got it, that's exactly the issue.

Now.. if there's a fix, it's probably going to need to go in nginx or maybe it can be handled with an access_by_lua script.

from oauth2-proxy.

marratj avatar marratj commented on June 11, 2024

After two days worth of swearing at nginx, I finally figured out a config that works, albeit with two caveats:

  1. The cookie split separator must not be -, but instead _ (underscore), as nginx cannot address cookie variables where the cookie has a non-alphanumeric (save for _) name. So this means a slight change in oauth2_proxy's code, but I think this is a very tiny PR worth considering.
  2. It's (just a bit) boilerplate, as you need to know how many parts your cookie will have

This is a config that works for me:

       # required as always 
        auth_request_set $auth_cookie $upstream_http_set_cookie;
        add_header Set-Cookie $auth_cookie;

       # extracts additional cookie's content into a new variable
        auth_request_set $name_upstream_1 $upstream_cookie_name_1;
        
       # This extracts the Path, Domain, Expires, whatever values after the original Set-Cookie header content and copies it to the second part
        if ($auth_cookie ~* "(; Path=.*)") {
            set $name_0 $auth_cookie; 
            set $name_1 "name_1=$nameg_upstream_1$1";
        }
        
       # This just sends both cookies if the second one has been set by the upstream oauth2_proxy, else it will just send the header defined above with $auth_request
        if ($name_upstream_1) {
            add_header Set-Cookie $name_0;
            add_header Set-Cookie $name_1;
        }

And now comes the big, big disappointment:

This works just fine in a normal nginx installation. I tried the same with ingress-nginx on Kubernetes, but there seems to be some bug which prevents evaluation of variables set by auth_request_set, so there currently is no way to do this with ingress-nginx (I spent quite some time today figuring this out): kubernetes/ingress-nginx#3716

from oauth2-proxy.

JoelSpeed avatar JoelSpeed commented on June 11, 2024

The cookie split separator must not be -, but instead _ (underscore), as nginx cannot address cookie variables where the cookie has a non-alphanumeric (save for _) name. So this means a slight change in oauth2_proxy's code, but I think this is a very tiny PR worth considering.

Since this feature hasn't been released yet I think we should fix this, are you willing to raise a PR?

It's (just a bit) boilerplate, as you need to know how many parts your cookie will have

I think for most people this will be predictable to an extent

This works just fine in a normal nginx installation. I tried the same with ingress-nginx on Kubernetes, but there seems to be some bug which prevents evaluation of variables set by auth_request_set, so there currently is no way to do this with ingress-nginx (I spent quite some time today figuring this out): kubernetes/ingress-nginx#3716

This is a real shame and I'm rather intrigued as to why this is, hopefully it get's fixed and then maybe we can add your workaround to our docs somewhere?

from oauth2-proxy.

marratj avatar marratj commented on June 11, 2024

@JoelSpeed I will raise a PR, since I'm directly benefitting from it and it's really small.

Also, I found a way around the ingress-nginx 'bug' with access_by_lua_block as suggested by @brianv0

This actually works with ingress-nginx, so this should also work with any other nginx with the Lua module.

nginx.ingress.kubernetes.io/auth-response-headers: Authorization
nginx.ingress.kubernetes.io/auth-signin: https://$host/oauth2/start?rd=$request_uri
nginx.ingress.kubernetes.io/auth-url: https://$host/oauth2/auth
nginx.ingress.kubernetes.io/configuration-snippet: |
  auth_request_set $name_upstream_1 $upstream_cookie_name_1;

  access_by_lua_block {
    if ngx.var.name_upstream_1 ~= "" then
      ngx.header["Set-Cookie"] = "name_1=" .. ngx.var.name_upstream_1 .. ngx.var.auth_cookie:match("(; Path=.*)")
    end
  }

from oauth2-proxy.

marratj avatar marratj commented on June 11, 2024

Fixed with #34

from oauth2-proxy.

nniikkoollaaii avatar nniikkoollaaii commented on June 11, 2024

@marratj could you share your nginx Setup / Version pls?
Or could someone explain why "name_1" in "$upstream_cookie_name_1" should match "_oauth2_proxy_1"?

I'm having problems setting up above provided solution.

I tried both

nginx.ingress.kubernetes.io/rewrite-target: /$2
nginx.ingress.kubernetes.io/ssl-redirect: "true"
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
nginx.ingress.kubernetes.io/auth-url: "http://oauth-proxy.<namespace>.svc.cluster.local:4180/oauth2/auth"
nginx.ingress.kubernetes.io/auth-signin: "https://my.domain.com/oauth2/start?rd=$request_uri"
nginx.ingress.kubernetes.io/auth-response-headers: Authorization
nginx.ingress.kubernetes.io/configuration-snippet: |
  auth_request_set $name_upstream_1 $upstream_cookie_name_1;

  access_by_lua_block {
    if ngx.var.name_upstream_1 ~= "" then
      ngx.header["Set-Cookie"] = "name_1=" .. ngx.var.name_upstream_1 .. ngx.var.auth_cookie:match("(; Path=.*)")
    end
  }

and

nginx.ingress.kubernetes.io/rewrite-target: /$2
nginx.ingress.kubernetes.io/ssl-redirect: "true"
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
nginx.ingress.kubernetes.io/auth-url: "http://oauth-proxy.<namespace>.svc.cluster.local:4180/oauth2/auth"
nginx.ingress.kubernetes.io/auth-signin: "https://my.domain.com/oauth2/start?rd=$request_uri"
nginx.ingress.kubernetes.io/auth-response-headers: Authorization
nginx.ingress.kubernetes.io/configuration-snippet: |
  auth_request_set $auth_cookie_name_upstream_1 $upstream_cookie_auth_cookie_name_1;

  if ($auth_cookie ~* "(; .*)") {
      set $auth_cookie_name_0 $auth_cookie;
      set $auth_cookie_name_1 "auth_cookie_name_1=$auth_cookie_name_upstream_1$1";
  }

  if ($auth_cookie_name_upstream_1) {
      add_header Set-Cookie $auth_cookie_name_0;
      add_header Set-Cookie $auth_cookie_name_1;
  }

from Pusher Docs

curl -v --cookie "_oauth2_proxy_0=xxx; _oauth2_proxy_1=yyy" https://my.domain.com/oauth2/auth

is returning two Set-Cookie-Headers

but using

curl -v --cookie "_oauth2_proxy_0=xxx; _oauth2_proxy_1=yyy" https://my.domain.com/protected/resource

with auth_request only returns the first Set-Cookie-Header

I'm seeing both configuration applied in nginx.conf in my ingress-nginx-controller pod

every help is very welcome

from oauth2-proxy.

Xeonios avatar Xeonios commented on June 11, 2024

People in this topic is made my day. Thanks a lot!
@marratj thank you for nginx config explanation too!

from oauth2-proxy.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.