Update app access RFD

[kimlisa]

In response to this PR #30321, that changes (reverts) the app authn flow from 3rd party cookie setting to first party cookie setting

rendered

[zmb3]

I realize that you weren't the original author of most of this, so feel free to push back. I just don't personally understand how this works, and I don't feel comfortable being a reviewer of the implementation without understanding this part first.

[kimlisa]

i'm going to try to clarify this even more, like... why do we need two cookies? (idk yet)

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[bl-nero]

Approving with minor suggestions.

[ryanclark]

Could this be simplified?

If POST /v1/webapi/sessions/app returns a signed JWT with a very short expiration, could the UI then redirect to https://dumper.localhost:3080/x-teleport-auth?token=JWT, which then verifies the JWT, grabs the data from the body (SESSION_BEARER_TOKEN, SESSION_ID), sets the right cookie values and then redirects to https://dumper.localhost:3080?

[kimlisa]

Could this be simplified?

If POST /v1/webapi/sessions/app returns a signed JWT with a very short expiration, could the UI then redirect to https://dumper.localhost:3080/x-teleport-auth?token=JWT, which then verifies the JWT, grabs the data from the body (SESSION_BEARER_TOKEN, SESSION_ID), sets the right cookie values and then redirects to https://dumper.localhost:3080?

@ryanclark upon research, it looks like it's generally advised against putting JWT in a query parameter for a GET request

https://community.auth0.com/t/can-i-securely-pass-a-jwt-in-the-url-query-parameters-of-a-get-request/65678

https://community.developer.atlassian.com/t/action-required-deprecating-support-for-passing-connect-jwts-as-a-query-string-parameter-to-atlassian-apis/50540

The proposed implementation in the RFD also puts "sensitive" info as query parameter as well, but there are extra security measures taken like double submit cookie and using POST with content-type == application/json.

That said though... i'm not sure. It does sound simpler. I pinged @jentfoo for his guidance

[jentfoo]

Thank you for pinging me @kimlisa, I was just about to leave a comment as well.

I agree that we should not be putting the token (or any secret) in the query parameter. Another concern with the possible redirect is needing to validate the service we are talking to (and not a potentially malicious service running on the expected port). I see @ryanclark described it using https, however we wont be able to have a trusted certificate for that domain. So we would need to consider how we would validate the service we are talking to on localhost.

[ryanclark]

We put node join tokens in URLs for discover, why couldn't a JWT go in the URL for this?

(and not a potentially malicious service running on the expected port)

Wouldn't the current setup also have this issue?

[jentfoo]

We put node join tokens in URLs for discover

This is not good practice. URL parameters are possible to be easily extracted by browser plugins, and are often logged by proxies or other intermediate services. We should not put sensitive values in the query string, we should instead POST them in the body.

Wouldn't the current setup also have this issue?

Possibly? I have not reviewed this RFD fully, just the recent discussion. I am not sure what we are sending in the current design but the most recent description of sending the auth to an untrusted service is concerning. Is this design captured in this RFD? If not could you point me to it. I would like to review and potentially open an issue if we are trusting user secrets to an unverified endpoint.

[bl-nero]

I agree that we should not be putting the token (or any secret) in the query parameter. Another concern with the possible redirect is needing to validate the service we are talking to (and not a potentially malicious service running on the expected port). I see @ryanclark described it using https, however we wont be able to have a trusted certificate for that domain.

Why do you assume we won't be able to have a trusted certificate? If we didn't, it wouldn't be secure to access it from the browser in the first place; any application deployed in a real-world environment has to have one, after all. Do I misinterpret what you tried to say? Note that I'm assuming here that localhost is just a (perhaps unfortunate) example.

[kimlisa]

To clear things up:

This RFD, was basically approved by someone at doyensec (it looks like he no longer works with doyensec anymore though 🥲), the proposed implementation in this RFD has also been scrutinized by doyensec before because its an implementation we had before (we are reverting back to this previous implementation because of cookie problems)

I'm not discrediting anything anyone's saying, just saying we already had security experts review this before (it just wasn't documented before).

@ryanclark i thought about your approach more.

i think this is what you are proposing. by using JWT, we can make it simpler by getting rid of one of the redirection (the initial state token exchange), is that right? (everything else stays the same: the serving of HTML and using POST inside that HTML).

If so, I could be entirely wrong in my thinking, but I think the way we have it right now proposed in the RFD, is slightly more safer because we can at least check for the original source of the initial request with the state cookie that was created in the initial state token exchange (the double submit cookie)

With JWT, I can imagine someone can steal this token, and say "here it is", and get access to that app. Could that be a possibility?

[jentfoo]

@bl-nero Specifically because of the use of localhost

[ryanclark]

To clear things up:

This RFD, was basically approved by someone at doyensec (it looks like he no longer works with doyensec anymore though 🥲), the proposed implementation in this RFD has also been scrutinized by doyensec before because its an implementation we had before (we are reverting back to this previous implementation because of cookie problems)

I'm not discrediting anything anyone's saying, just saying we already had security experts review this before (it just wasn't documented before).

This is fine and I'm happy to approve it, I was just wondering if we can still get rid of the redirect dance if possible.

@ryanclark i thought about your approach more.

i think this is what you are proposing. by using JWT, we can make it simpler by getting rid of one of the redirection (the initial state token exchange), is that right? (everything else stays the same: the serving of HTML and using POST inside that HTML).

Yeah, we POST the JWT the same way, this way the x-teleport-auth endpoint should be able to redirect to the app and it should be only two redirects

If so, I could be entirely wrong in my thinking, but I think the way we have it right now proposed in the RFD, is slightly more safer because we can at least check for the original source of the initial request with the state cookie that was created in the initial state token exchange (the double submit cookie)

I'm not sure if this is important. The x-teleport-auth endpoint would only ever accept JWTs signed by us

With JWT, I can imagine someone can steal this token, and say "here it is", and get access to that app. Could that be a possibility?

We can sign a JWT that's valid for 15 seconds or something, so even if it's stolen it's useless.

[jentfoo]

@kimlisa I am not trying to raise issues that should hinder us moving forward with this RFD. I just want to highlight potential concerns and try to avoid proliferating these patterns. Specifically putting sensitive data in query parameters, or connecting and communicating sensitive data to an unverified destination.

I will review this more broadly next week and open up issues as security findings if I find any concerns, but it should not hinder moving forward with this RFD.

[bl-nero]

I agree with @jentfoo that this should not stop us from implementing; we can always make another improvement, but as far as I understand, we need to be there in time for the third-party cookie crackdown.