envoyproxy / envoy

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

OAuth2: tokens set in the cookies are not encrypted #23508

Open ktnrn opened 1 year ago

ktnrn commented 1 year ago

Title: OAuth2 filter sets the tokens in the cookies but not encrypted

Description:

I am using the Oauth2 filter with OIDC scopes to authenticate my web application's users. It sets the access_token, refresh_token, and id_token as the cookies and these tokens are not encrypted. The cookies are also not marked as http_only. Most of the Oauth clients encrypt the tokens when using cookies as storage. i.e. NextAuth etc. The Oauth2 filter must support this. If not, please provide the reasons and the alternate solution to encrypt the tokens. I think most of the SecOps will not allow using the oauth2 filter in the production without this feature.

[optional Relevant Links:]

https://next-auth.js.org/configuration/options#secret

wbpcode commented 1 year ago

Then which type encrypt is you want? IMO, the data security should be ensured by the TLS. There is no other guarantee in oauth2 or oauth2 filter self.

wbpcode commented 1 year ago

cc @snowp

ktnrn commented 1 year ago

This is another example where data is encrypted before storing in the cookie. https://oauth2-proxy.github.io/oauth2-proxy/docs/configuration/session_storage

ktnrn commented 1 year ago

If we agree on this, can we start working on this?

schonbachler commented 1 year ago

I also find it problematic to use this filter with unencrypted cookies with tokens in production.

snowp commented 1 year ago

I don't see any problem with adding support for this, though somebody will have to do the work.

FWIW I believe the filter is being used in production already, maybe @fishcakez can speak to this?

ktnrn commented 1 year ago

Hi All,

Summarizing: We agree that tokens should be encrypted before storing them in the cookie. Since encryption support will require a lot of work, we should at least make the cookie httpOnly so the apps which are already using Oauth2 are not vulnerable to the attacks.

Can someone please help reopen the issue https://github.com/envoyproxy/envoy/issues/24097 so I can send the pull request? Let me know if you have any questions.

Thanks!

On Tue, Nov 29, 2022 at 8:24 AM Snow Pettersen @.***> wrote:

I don't see any problem with adding support for this, though somebody will have to do the work.

FWIW I believe the filter is being used in production already, maybe @fishcakez https://github.com/fishcakez can speak to this?

— Reply to this email directly, view it on GitHub https://github.com/envoyproxy/envoy/issues/23508#issuecomment-1330908506, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGVRDZ46REVDKSGXIQP5EADWKYU3LANCNFSM6AAAAAARFPB6N4 . You are receiving this because you authored the thread.Message ID: @.***>

modulo11 commented 5 months ago

We (@loewenstein, @pbusko, @phil9909, @c0d1ngm0nk3y, @modulo11) would be interested in working on implementing encrypted cookies. Our first idea was to address this generally with an additional filter for encrypting/decrypting specific cookies. Do you think that could be a way to go? Or how can we best discuss that change? Reading the contribution guideline (and extension policy)

All extensions must be sponsored by an existing maintainer.

How can we find one?

phlax commented 5 months ago

@modulo11 your proposal sounds like a good idea to me

can you open a separate ticket to address cookie encryption more generally - once there is a proposal we (maintainers) can discuss who might be willing to sponsor

c0d1ngm0nk3y commented 5 months ago

can you open a separate ticket to address cookie encryption more generally - once there is a proposal we (maintainers) can discuss who might be willing to sponsor

We have opened https://github.com/envoyproxy/envoy/issues/32066

Alexcei88 commented 4 months ago

@ktnrn, Could you explain please why token needs to be encrypted? It will be interested to understand what secure problem is hidden? I suppose that cookies have HttpOnly option.

loewenstein commented 4 months ago

can you open a separate ticket to address cookie encryption more generally - once there is a proposal we (maintainers) can discuss who might be willing to sponsor

We have opened #32066

@phlax there is unfortunately not really much happening in #32066. What would be the next step to get the discussion going and work towards a PR?

mcdafydd commented 3 months ago

@Alexcei88, encrypting the access/ID token cookies could reduce the likelihood of a couple different threat vectors, increasing the cost to a threat actor in some deployments. Here is one example.

Assumptions:

  1. Threat actor only has access to exploit a vulnerability that allows them to steal browser cookies (ie: an endpoint vulnerability or malicious browser extension)
  2. Threat actor wants to impersonate a particular subject OR wants to access a particular resource

By encrypting the tokens, the subject and audience claims would be unavailable to the attacker, so the attacker would need to test every token in some way to identify whether they granted the required access, which would hopefully give the targets more data and time to detect and respond to the situation.