envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.92k stars 4.8k forks source link

Add configurable verification of HttpOnly cookies in JWTAuthentication filter #7025

Open andrewtikhonov opened 5 years ago

andrewtikhonov commented 5 years ago

Title: Add configurable verification of HttpOnly cookies in JWTAuthentication filter

Problem:

One of the methods to protect against XSS attacks and token theft in web apps is the HttpOnly cookie that is set by the server, and that is not accessible from Javascript. A digest of the cookie value is recorded in the token and is verified by the server when the browser sends a request.

Proposal:

It would be very useful for JWT Authentication filter to extract the value from the cookie, take a sha256 digest and verify that a claim with the corresponding value is in the token.

Headers:

Cookie: secure_key=secure_value_1234567890

Envoy config

verify_secure_cookie: key: "secure_key" transform: [sha256, plaintext] claim_name: "cookie_key"

qiwzhang commented 5 years ago

I need some clarifications: In the JWT token, what claim name should be checked? Maybe also from Envoy config?

For example: a request has following headers:

Authorization: Bearer TOKEN Cooke: secure_key=secure_value_1234567890

After Envoy Jwt_Authn filter verified the TOKEN, it also needs to verify the cookie. If it has following config:

verify_secure_cookie: cookie_key: "secure_key" jwt_claim_key: "jwt_secure_key" transform: sha256

The filter will look for "jwt_claim_key" field in the TOKEN claim, and verify its value is sha256 of cookie value from "secure_key".

Is this correct?

I feel it is better to have a separate filter for this function. The verified payload is written to dynamicMetadata in the stream_info. Other filter can access it to perform such verification.

andrewtikhonov commented 5 years ago

Absolutely correct. Sorry, forgot to add the claim name.

It can be a separate filter, but no one will need to do such a verification somewhere else. It will always be in connection with the token verification, which makes me think it has to be the auth filter.

andrewtikhonov commented 5 years ago

Any thoughts ?

yangminzhu commented 5 years ago

I think these custom operations (e.g. sha256, etc.) seems too specific to be added to jwt filter unless there is a more generic way to specify and support these custom operations and allowing it to be reused in other filters.

Maybe you could try to use a Lua filter (https://github.com/envoyproxy/envoy/tree/v1.10.0/examples/lua) to do this?

andrewtikhonov commented 5 years ago

Sha256 is not specific. It's a means to transform the data so that it cannot be restored. I don't mind having a general config saying "use sha256 for for hash operations" and "use hash transform" in the JWT filter. But that's premature optimisation or premature generalisation to be precise.

Sha256 is already implemented in envoy/common/crypto/utility.cc and is used in /filters/http/common/aws it's just a matter of writing code.

yangminzhu commented 5 years ago

Sorry I did't mean sha256 is specific or there is any problem of supporting sha256 in the code, the problem is how to support this properly in the filter API so that it's not a solution that only works for some specific use cases. For example, what if the other user want to use a hash algorithm other than sha256 or the other user want to do some preprocessing and only use part of the the cookie header to calculate the hash?

andrewtikhonov commented 5 years ago

Yangmin, what am I doing wrong ? I'm doing JWT auth and want payload to be in the dynamicMetadata() (see payload_in_metadata: "my_payload") so that the lua filter after jwt_auth can check the token payload. Before JWT_auth filter I capture some headers using another lua filter so that the arguments can be reconstructed.

Problem: I'm getting "nil", dynamicMetadata() contains only my map with recorded header values and not a trace of token payload.

Logs:

[2019-07-03 17:08:52.833][12][info][lua] [source/extensions/filters/http/lua/lua_filter.cc:535] script log: ------------- Request dynamic_key: envoy.lua
[2019-07-03 17:08:52.833][12][error][lua] [source/extensions/filters/http/lua/lua_filter.cc:541] script log: [string "function envoy_on_request(request_handle)..."]:9: bad argument #1 to 'pairs' (table expected, got nil)
[

Envoy Config:

          - name: envoy.lua
            typed_config:
              "@type": type.googleapis.com/envoy.config.filter.http.lua.v2.Lua
              inline_code: |
                function envoy_on_request(request_handle)
                  local headers = request_handle:headers()
                  local dm = request_handle:streamInfo():dynamicMetadata()

                  dm:set("envoy.lua", ":path", headers:get(":path"))
                  dm:set("envoy.lua", ":scheme", headers:get(":scheme"))
                  dm:set("envoy.lua", ":authority", headers:get(":authority"))
                end
          - name: envoy.filters.http.jwt_authn
            config:
              providers:
                provider1:
                  issuer: "Authentication Service"
                  forward: true
                  remote_jwks:
                    http_uri:
                      uri: http://localhost:8080/.public-jwks
                      cluster: keyserver
                  from_headers:
                  - name: token
                  payload_in_metadata: "my_payload"
              rules:
                 - match:
                     prefix: /s/service1
                   requires:
                     provider_name: provider1
                 - match:
                     prefix: /
          - name: envoy.lua
            typed_config:
              "@type": type.googleapis.com/envoy.config.filter.http.lua.v2.Lua
              inline_code: |
                function envoy_on_request(request_handle)
                  local dynamic = request_handle:streamInfo():dynamicMetadata()

                  for dynamic_key, dynamic_value in pairs(dynamic) do
                    request_handle:logInfo("------------- Request dynamic_key: " .. dynamic_key)
                  end

                  local dm = request_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.jwt_authn")
                  for dm_key, dm_value in pairs(dm) do
                    request_handle:logInfo("------------- Request dm " .. dm_key .. ":" .. dm_value)
                  end
                end
andrewtikhonov commented 5 years ago

It's only there when JWT auth is triggered and successfully completes.

andrewtikhonov commented 5 years ago

How do i implement such changes in c++ ?

qiwzhang commented 5 years ago

@andrewtikhonov I am OK to implement such verification feature in jwt_authn filter.

andrewtikhonov commented 5 years ago

Am I the only one who requested this ?

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

andrewtikhonov commented 5 years ago

Activity

andrewtikhonov commented 5 years ago

Activity

andrewtikhonov commented 5 years ago

Activity

andrewtikhonov commented 5 years ago

Activity

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

vemod commented 4 years ago

Actually this feature is a must when working with JWT and JS frontends. We have our fronend apps (vuejs, reactjs etc) which communicate with backend via gateway. and its common practice to use httponly cookies with jwt so js apps can't steal them. So without this feature the usage of a gateway for jwt validation before passing valid requests to backend services is obsolete.

I tried the lua approach but its also not good/working because of POOR cookie support. there is no way to access cookies in lua except for fetching cookie header and parsing it manually by splitting etc..

I tried using header_to_metadata but this doesn't work as well because header_to_metadata puts the value not into headers but into dynamicMetadata (I suppose). So jwt_authn is not able to read jwt token from dynamicMetadata. am I missing something here?

so i would vote +5 for this feature. please :)

vemod commented 4 years ago

so i fount a workaround for this problem. its not that nice but I think it cleaner then processing cookies in lua. So the solution is to use hybrid approach. this is how it looks like in my solution.

http_filters:
          - name: jwtcookie.filter
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.header_to_metadata.v3.Config
              request_rules:
                - cookie: "MPJWT"
                  on_header_present:
                    key: "x-jwt-token"
                    type: STRING
                  on_header_missing:
                    key: "x-jwt-token"
                    type: STRING
                    value: ' '
          - name: jwtcookie.lua
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
              inline_code: |
                function envoy_on_request(request_handle)
                  local jwt = request_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.header_to_metadata")["x-jwt-token"]
                  if jwt == nil or jwt == '' or jwt == ' ' then
                    return
                  end
                  request_handle:logTrace("Passing jwt value from cookie into authorization header: " .. jwt)
                  request_handle:headers():add("Authorization", "Bearer " .. jwt)
                end
          - name: envoy.filters.http.jwt_authn
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.JwtAuthentication
              providers:
                myProvider1:
                  issuer: "https://myhost.com"
                  audiences: [ "https://myhost.com" ]
                  local_jwks:
                    filename: /etc/envoy/jwks/jwks.json
                  from_headers:
                  - name: Authorization
                    value_prefix: "Bearer "
                  forward: true
                  forward_payload_header: x-jwt-payload
              rules:
                - match:
                    prefix: /
                  requires:
                    provider_name: myProvider1
          - name: envoy.filters.http.router

The order is important

  1. header_to_metadata can read a specific cookie value and store it into metadata (dynamicMetadata).
  2. lua filter puts the value from dynamicMetadata into header
  3. generic jwt validation

I would love to skip step 2. not sure if there is a namespace or a way to make header_to_metadata write the value back into headers. or may be we need a new extensions like header_to_header or header_transformer.

well at least with this solution no need to mess around cookie parsing from a sting (in lua its quite dirty).

a note on on_header_missing: empty values are not allowed for headers in header_to_metadata. thus on_header_missing: " ". or alternatively use this lua code. its matter of taste/style:

function envoy_on_request(request_handle)
   local jwt = request_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.header_to_metadata")
   if jwt["x-jwt-token"] == nil or jwt["x-jwt-token"] == '' then
      return
   end

   request_handle:logTrace("Passing jwt value from cookie into authorization header: " .. jwt["x-jwt-token"])
   request_handle:headers():add("Authorization", "Bearer " .. jwt["x-jwt-token"])
end
vemod commented 4 years ago

btw: cookie/header name is case sensitive in header_to_metadata :(

hellraisercenobit commented 3 years ago

@vemod thx a lot for your "patch" how did you deal with grpc-web withCredentials call? with envoy i got this error:

as been blocked by CORS policy: Response to preflight request doesn't pass access control check: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'. The credentials mode of requests initiated by the XMLHttpRequest is controlled by the withCredentials attribute.

In the response envoy always set the response with Access-Control-Allow-Origin: '*'.. But in the config i have this: allow_origin_string_match:

Any way to force the response header with the origin instead of *?

vemod commented 3 years ago

Any way to force the response header with the origin instead of *?

you can force response headers by setting them in lua like in my example with auth request headers. to do that you need to implement function envoy_on_response(response_handle)

vemod commented 3 years ago

The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'.

have you tried to set allow_credentials? you might need to set that in order correct headers are included or set them in lua

hellraisercenobit commented 3 years ago

@vemod allow_credentials: true into cors config not working at all with something like this on client side using grpc-web:

this.greeterClient = new GreeterServicePromiseClient('http://localhost:8080', null, <any>{
      'withCredentials': true
 }

Can i declare multiple function with inline_code? No luck with this (Access-Control-Allow-Origin is always set to *) maybe something is wrong i'm not familiar with this syntax.

              '@type': type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
              inline_code: |
                function envoy_on_request(request_handle)
                  local jwt = request_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.header_to_metadata")["x-jwt-token"]
                  if jwt == nil or jwt == '' or jwt == ' ' then
                    return
                  end
                  request_handle:logTrace("Passing jwt value from cookie into authorization header: " .. jwt)
                  request_handle:headers():add("Authorization", "Bearer " .. jwt)
                end
                function envoy_on_response(response_handle)
                  response_handle:logTrace("Try to add override response headers")
                  response_handle:headers():add("Access-Control-Allow-Origin", "http://localhost:8080")
                end
hellraisercenobit commented 3 years ago

@vemod my bad the problem was coming from an old chrome cors extension. forgot to uninstall ^^ Your solution is ok ;) thx a lot! Will be better to have a native support 👍 Anyway JWT filter is awesome 😄.