emissary-ingress / emissary

open source Kubernetes-native API gateway for microservices built on the Envoy Proxy
https://www.getambassador.io
Apache License 2.0
4.35k stars 681 forks source link

v0.50.1 AuthService drops all but one `set-cookie` response header #1211

Closed mianelson closed 5 years ago

mianelson commented 5 years ago

Describe the bug When AuthService returns a non-200 response to Ambassador, only one set-cookie header can be sent back to the client, all other set-cookie headers are stripped. All cookies have the same domain, max_age, etc.

To Reproduce Steps to reproduce the behavior:

  1. Setup a v1 (0.50.1) AuthService that adds more than one set-cookie header on auth responses
  2. Send a non-200 response (302 in this case so that the request does not continue upstream) from AuthService with >1 set-cookie header on response
  3. Verify all set-cookie headers are present on response from AuthService as the response passes back through Ambassador; there should be >1 set-cookie header on the response from the AuthService and only 1 set-cookie header on the response that Ambassador filters and sends back to the client

Expected behavior We expect all set-cookie headers to be returned by Ambassador when the AuthService returns a response. Rolling the service back to Ambassador v0.40.2 results in the expected behavior.

Versions (please complete the following information):

Additional context Here is logging we captured of the issue. Notice that x-request-destination is preserved but session is removed in the final response.

[2019-02-07 20:41:58.736][000059][debug][http] [source/common/http/async_client_impl.cc:96] async http request response headers (end_stream=false):
':status', '302'
'server', 'nginx/1.15.0'
'date', 'Thu, 07 Feb 2019 20:41:58 GMT'
'content-type', 'text/html; charset=utf-8'
'content-length', '941'
'connection', 'keep-alive'
'location', 'https://dev-syapse.auth0.com/authorize?response_type=code&client_id=Y4EF4s3mw3IvLKFUUggL9Xgz64g0pH6h&redirect_uri=https%3A%2F%2Fambassador.dev.syapse.com%2Fauthz%2Fv1%2Fauth0%2Fcomplete&scope=openid+profile+email+user_metadata+app_metadata&state=u63YpJcjSweLIFyOQpqfFL1ZvRSLxI&audience=https%3A%2F%2Fdev-syapse.auth0.com%2Fuserinfo&prompt=none'
'set-cookie', 'x-request-destination=https://oncology-web-2.dev.syapse.com/; Domain=.syapse.com; Path=/'
'vary', 'Cookie'
'set-cookie', 'session=.eJyrVopPLC3JMACTOZlJ8cmJOTlJicnZ8UpWShklJQXFVvr6iblJicXFiSn5RXopqWV6xZWJBcWpesn5ufogXVX6ZYZghoE-UKggJ7UkVUkH3djiksSSVJCZpWbGkQVeyVnB5ak-nm6V_oEFhWluPoZRZUHBPhWeSrUAJe00QQ.XFyYFg.8BzD07RYRXiRz8Iz2s-9U3S3XUg; Domain=.syapse.com; HttpOnly; Path=/'
'x-content-type-options', 'nosniff'
'x-frame-options', 'SAMEORIGIN'
'x-xss-protection', '1; mode=block'
'strict-transport-security', 'max-age=15768000; includeSubDomains'
'x-envoy-upstream-service-time', '27'

[2019-02-07 20:41:58.740][000059][debug][client] [source/common/http/codec_client.cc:95] [C141708] response complete
[2019-02-07 20:41:58.740][000059][debug][filter] [source/extensions/filters/http/ext_authz/ext_authz.cc:177] [C141707][S17486080943075874799] ext_authz rejected the request
[2019-02-07 20:41:58.740][000059][debug][http] [source/common/http/conn_manager_impl.cc:1096] [C141707][S17486080943075874799] encoding headers via codec (end_stream=false):
':status', '302'
'content-length', '941'
'content-type', 'text/plain'
'location', 'https://dev-syapse.auth0.com/authorize?response_type=code&client_id=Y4EF4s3mw3IvLKFUUggL9Xgz64g0pH6h&redirect_uri=https%3A%2F%2Fambassador.dev.syapse.com%2Fauthz%2Fv1%2Fauth0%2Fcomplete&scope=openid+profile+email+user_metadata+app_metadata&state=u63YpJcjSweLIFyOQpqfFL1ZvRSLxI&audience=https%3A%2F%2Fdev-syapse.auth0.com%2Fuserinfo&prompt=none'
'set-cookie', 'x-request-destination=https://oncology-web-2.dev.syapse.com/; Domain=.syapse.com; Path=/'
'date', 'Thu, 07 Feb 2019 20:41:58 GMT'
'server', 'envoy'

[2019-02-07 20:41:58.741][000059][debug][pool] [source/common/http/http1/conn_pool.cc:209] [C141708] response complete
[2019-02-07 20:41:58.741][000059][debug][pool] [source/common/http/http1/conn_pool.cc:247] [C141708] moving to ready[2019-02-07 20:41:58.736][000059][debug][http] [source/common/http/async_client_impl.cc:96] async http request response headers (end_stream=false):
':status', '302'
'server', 'nginx/1.15.0'
'date', 'Thu, 07 Feb 2019 20:41:58 GMT'
'content-type', 'text/html; charset=utf-8'
'content-length', '941'
'connection', 'keep-alive'
'location', 'https://dev-syapse.auth0.com/authorize?response_type=code&client_id=Y4EF4s3mw3IvLKFUUggL9Xgz64g0pH6h&redirect_uri=https%3A%2F%2Fambassador.dev.syapse.com%2Fauthz%2Fv1%2Fauth0%2Fcomplete&scope=openid+profile+email+user_metadata+app_metadata&state=u63YpJcjSweLIFyOQpqfFL1ZvRSLxI&audience=https%3A%2F%2Fdev-syapse.auth0.com%2Fuserinfo&prompt=none'
'set-cookie', 'x-request-destination=https://oncology-web-2.dev.syapse.com/; Domain=.syapse.com; Path=/'
'vary', 'Cookie'
'set-cookie', 'session=.eJyrVopPLC3JMACTOZlJ8cmJOTlJicnZ8UpWShklJQXFVvr6iblJicXFiSn5RXopqWV6xZWJBcWpesn5ufogXVX6ZYZghoE-UKggJ7UkVUkH3djiksSSVJCZpWbGkQVeyVnB5ak-nm6V_oEFhWluPoZRZUHBPhWeSrUAJe00QQ.XFyYFg.8BzD07RYRXiRz8Iz2s-9U3S3XUg; Domain=.syapse.com; HttpOnly; Path=/'
'x-content-type-options', 'nosniff'
'x-frame-options', 'SAMEORIGIN'
'x-xss-protection', '1; mode=block'
'strict-transport-security', 'max-age=15768000; includeSubDomains'
'x-envoy-upstream-service-time', '27'

[2019-02-07 20:41:58.740][000059][debug][client] [source/common/http/codec_client.cc:95] [C141708] response complete
[2019-02-07 20:41:58.740][000059][debug][filter] [source/extensions/filters/http/ext_authz/ext_authz.cc:177] [C141707][S17486080943075874799] ext_authz rejected the request
[2019-02-07 20:41:58.740][000059][debug][http] [source/common/http/conn_manager_impl.cc:1096] [C141707][S17486080943075874799] encoding headers via codec (end_stream=false):
':status', '302'
'content-length', '941'
'content-type', 'text/plain'
'location', 'https://dev-syapse.auth0.com/authorize?response_type=code&client_id=Y4EF4s3mw3IvLKFUUggL9Xgz64g0pH6h&redirect_uri=https%3A%2F%2Fambassador.dev.syapse.com%2Fauthz%2Fv1%2Fauth0%2Fcomplete&scope=openid+profile+email+user_metadata+app_metadata&state=u63YpJcjSweLIFyOQpqfFL1ZvRSLxI&audience=https%3A%2F%2Fdev-syapse.auth0.com%2Fuserinfo&prompt=none'
'set-cookie', 'x-request-destination=https://oncology-web-2.dev.syapse.com/; Domain=.syapse.com; Path=/'
'date', 'Thu, 07 Feb 2019 20:41:58 GMT'
'server', 'envoy'

[2019-02-07 20:41:58.741][000059][debug][pool] [source/common/http/http1/conn_pool.cc:209] [C141708] response complete
[2019-02-07 20:41:58.741][000059][debug][pool] [source/common/http/http1/conn_pool.cc:247] [C141708] moving to ready

Here's the AuthService annotation that we are using:

  annotations:
    getambassador.io/config: |
      ---
      apiVersion: ambassador/v1
      kind: AuthService
      name: authentication
      auth_service: {auth-service-url-and-port}
      path_prefix: "/v1/validate"
      allowed_authorization_headers:
      - "set-cookie"
      - "session"
      ---
      apiVersion: ambassador/v1
      kind: Mapping
      name: authz_mapping
      prefix: /authz/
      service: {auth-service-url-and-port}
      tls: true

We also verified this behavior against normal non-AuthService request/response flows, and did not see Ambassador filtering any response headers.

FrancoCorleone commented 5 years ago

I observed the same behavior. Does it work for you in previous versions?

cornelius-keller commented 5 years ago

We observe the same behavior. It works for us in 0.40 Version .

FrancoCorleone commented 5 years ago

Anyone knows when can we expect fix for that?

gsagula commented 5 years ago

@mianelson @cornelius-keller Thanks for reporting it.

Just to confirm, what you are saying is that given the following response from the authorization server:

 HTTP/1.1 302 
 set-cookie=a; Domain=whatever.com; Path=/'
 set-cookie=b; Domain=whatever.com; Path=/'

Only cookie b gets set?

If correct, that's a bug probably attributed to this logic: https://github.com/envoyproxy/envoy/blob/e135ec80b768ba9c7a415eb66746f04dec431ed7/source/extensions/filters/http/ext_authz/ext_authz.cc#L138

IRC, it's was a bug fixing due to a segfault that we found in the previous implementation.

FYI @FrancoCorleone

FrancoCorleone commented 5 years ago

I have different http code but the rest is correct. I have two cookies and only one is set. I checked by proxing direcetly to auth service bypassing ambassador and both are set so it has to be further than auth service itself

gsagula commented 5 years ago

Yeah, that's the correct behaviour for setting multiple cookies. I will need to get my hands into Envoy just to confirm it, but it seems that the logic described above is what causing the issue.

FrancoCorleone commented 5 years ago

Ok let me know when it's fixed. Thanks!

mianelson commented 5 years ago

@gsagula yes, can confirm as well that that is the behavior we're experiencing. It's been a while since I've worked in C++, but that line you tracked down in ext_authz.cc is where my search ended as well.

ankitpec72 commented 5 years ago

Any update on this issue? When it going to be fixed? We can not update to 50 version because of this issue.

FrancoCorleone commented 5 years ago

Same here. Any updates? @gsagula ?

kflynn commented 5 years ago

@gsagula: RFC 6265 is pretty unequivocal that multiple Set-Cookie headers must be supported, so I'm sad to say that I have to call this one definitely a bug that needs fixing.

gsagula commented 5 years ago

@kflynn I’m aware of the RFC and that it is a bug. However, it’s not up to me to prioritize the fix.

FrancoCorleone commented 5 years ago

@gsagula, any suggested workaround till then as there is no date for a fix planned?

gsagula commented 5 years ago

@FrancoCorleone As I mentioned in https://github.com/datawire/ambassador/issues/1211#issuecomment-463745263, the problem is in Envoy proxy. I don't think that there is a workaround other than just trying to set one cookie instead of multiples if possible. Regarding prioritazation, it is more of quetion for @kflynn.

richarddli commented 5 years ago

Fixed in 0.51.

FrancoCorleone commented 5 years ago

@richarddli Are you sure it is fixed? I am using quay.io/datawire/ambassador:0.51.1 and I still observe that issue.

gsagula commented 5 years ago

@FrancoCorleone Thanks for reporting, I'm looking into that.

FrancoCorleone commented 5 years ago

@gsagula thanks for quick response. Let me know when you find out anything. For any interested parties. Until it works for you, you can easily work around that issue by making inner redirects setting additional cookies. In my case, I have only two cookies so just one additional redirect.

gsagula commented 5 years ago

@FrancoCorleone We found the problem. I will make sure that we get a new image tomorrow. Thanks!

FrancoCorleone commented 5 years ago

Thanks, @gsagula.

woraphol-j commented 5 years ago

@gsagula I can confirm the issue still exists even in the latest version of ambassador (0.52.0). I created a repository to demonstrate the issue here: https://github.com/woraphol-j/ambassador-cookie-stripped-demo Let me know if you have any question.

FrancoCorleone commented 5 years ago

@woraphol-j it worked for me in 0.51.2. @gsagula is something broken again in newer version?

gsagula commented 5 years ago

@FrancoCorleone @woraphol-j It works for me with quay.io/datawire/ambassador:0.52.0:

curl -v -H "requested-cookie: foo, bar, baz" -H "requested-status:307"  http://localhost:61880/get

> GET /get HTTP/1.1
> Host: localhost:61880
> User-Agent: curl/7.54.0
> Accept: */*
> requested-cookie: foo, bar, baz
> requested-status:307
>
< HTTP/1.1 307 Temporary Redirect
< content-length: 1289
< content-type: text/plain
< set-cookie: foo=foo
< set-cookie: bar=bar
< set-cookie: baz=baz
< date: Sun, 24 Mar 2019 17:10:41 GMT

docker-compose:

version: '2'
services:

  # curl -v -H "requested-cookie: foo, bar, baz" -H "requested-status:307"  http://localhost:61880/get  
  ambassador:
    image: quay.io/datawire/ambassador:0.52.0
    ports:
    - 61880:80
    volumes:
    - ./config:/ambassador/ambassador-config
    environment:
    - AMBASSADOR_NO_KUBEWATCH=no_kubewatch
    networks:
    - ambassador

  auth-service:
    image: quay.io/datawire/kat-backend:11
    environment:
    - DEBUG=1
    - BACKEND=true
    networks:
      ambassador:
        aliases:
          - ambassador
    expose:
      - "8080"
    ports:
      - "61898:8080"

  echo-service:
    image: quay.io/datawire/kat-backend:11
    environment:
    - DEBUG=1
    - BACKEND=true
    networks:
      ambassador:
        aliases:
          - ambassador
    expose:
      - "8080"

networks:
  ambassador: {}

config:

---
apiVersion: ambassador/v1
kind: Module
name: ambassador
config:
  use_remote_address: true
---
apiVersion: ambassador/v1
kind: AuthService
name: authentication
auth_service: "auth-service:8080"
path_prefix: "/extauth"
proto: http
allowed_request_headers:
- requested-status
- requested-cookie
allowed_authorization_headers:
- set-cookie  
---
apiVersion: ambassador/v1
kind:  Mapping
name: echo-service
prefix: /
service: "echo-service:8080"
woraphol-j commented 5 years ago

@gsagula Thanks your quick reply. Really appreciated it. However, I tried the exact same configuration as yours and it turned out it DOES NOT work (at least as I expected) as it shows in the result (as I commented):

curl -v -H "requested-cookie: foo, bar, baz" -H "requested-status:200"  http://localhost:61880/get

*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 61880 (#0)
> GET /get HTTP/1.1
> Host: localhost:61880
> User-Agent: curl/7.54.0
> Accept: */*
> requested-cookie: foo, bar, baz
> requested-status:200
>
< HTTP/1.1 200 OK
< set-cookie: foo=foo
< set-cookie: bar=bar
< set-cookie: baz=baz
< date: Mon, 25 Mar 2019 05:44:03 GMT
< content-length: 1122
< content-type: text/plain; charset=utf-8
< x-envoy-upstream-service-time: 1
< server: envoy
<
{
  "backend": "true",
  "request": {
    "headers": {
      "accept": [
        "*/*"
      ],
      "content-length": [
        "0"
      ],
      "requested-cookie": [
        "foo, bar, baz"
      ],
      "requested-status": [
        "200"
      ],
      "set-cookie": [
        "baz=baz"   // *** only the last set-cookie returned from auth-service reaches the echo server
      ],
      "user-agent": [
        "curl/7.54.0"
      ],
      "x-envoy-expected-rq-timeout-ms": [
        "3000"
      ],
      "x-envoy-internal": [
        "true"
      ],
      "x-envoy-original-path": [
        "/get"
      ],
      "x-forwarded-for": [
        "192.168.192.1"
      ],
      "x-forwarded-proto": [
        "http"
      ],
      "x-request-id": [
        "47c74108-e76b-41c4-a829-efc003b945a8"
      ]
    },
    "host": "localhost:61880",
    "method": "GET",
    "tls": {
      "enabled": false
    },
    "url": {
      "fragment": "",
      "host": "",
      "opaque": "",
      "path": "/get",
      "query": {},
      "rawQuery": "",
      "scheme": ""
    }
  },
  "response": {
    "headers": {
      "set-cookie": [ // the final result is correct because the echo server receives the request with requested-cookie: foo, bar, baz
        "foo=foo",
        "bar=bar",
        "baz=baz"
      ]
    }
  }
* Connection #0 to host localhost left intact
}%

Note that when I call the auth-service directly, it returns a response with 3 set-cookies correctly

curl -v -H "requested-cookie: foo, bar, baz" -H "requested-status:200"  http://localhost:61898/get
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 61898 (#0)
> GET /get HTTP/1.1
> Host: localhost:61898
> User-Agent: curl/7.54.0
> Accept: */*
> requested-cookie: foo, bar, baz
> requested-status:200
>
< HTTP/1.1 200 OK
< Set-Cookie: foo=foo
< Set-Cookie: bar=bar
< Set-Cookie: baz=baz
< Date: Mon, 25 Mar 2019 06:00:35 GMT
< Content-Length: 658
< Content-Type: text/plain; charset=utf-8
gsagula commented 5 years ago

I’m not sure what you mean by not working, I can see the 3 set-cookie headers in the response.

< set-cookie: foo=foo

< set-cookie: bar=bar

< set-cookie: baz=baz

-- Gabriel Linden Sagula

woraphol-j commented 5 years ago

Hi @gsagula there is only one set-cookie in the request to the echo server

"set-cookie": [
        "baz=baz"
],

I think the flow is (Please correct me if I am wrong)

  1. A request reaches Ambassador.
  2. Ambassador passes it on to the AuthService with requested-cookie: foo, bar, baz header.
  3. AuthService generates a response with 3 set-cookie headers and send back to AuthService.
  4. Ambassador receives a response from the AuthService with 3 set-cookie headers.
  5. Ambassador passes this on to the echo server, however, it only passes 1 set-cookie (the last one) instead of 3 set-cookie headers. Note that it still passes requested-cookie: foo, bar, baz header.
  6. Because of the passed requested-cookie: foo, bar, baz header, Ambassador still returns the correct result in the end with 3 set-cookie response headers.