-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User identity associated with the RP should not be sent to the IDP before explicit consent is gathered #230
Comments
I don't think this is quite correct. Per this table the referrer would not be sent on the manifest (fedcm.json) https://github.com/fedidcg/FedCM/blob/main/explainer.md#endpoint-properties |
The step |
We did clarify this in the explainer as @npm1 but it hasn't been backported to the spec yet. |
That's quite an interesting attack @bvandersloot-mozilla , thanks for pointing it out! While I agree with @npm1 and @dj2 that the specific attack you suggested is not viable (we need to fix the spec, PR on the way), I think that we should use this as an opportunity to find attacks that are akin to it. For example, I think the the following attack is one that we wouldn't be able to mitigate right now: const cred = await navigator.credentials.get({
federated: {
providers: [{
url: `https://idp.example/${window.location.href}`
}]
}
});
// The following will fetch the manifest JSON file, which will know the origin of the RP :(
const {idToken} = cred.login(); |
Thanks for the info @dj2 and @npm1. Good to see this doesn't get through as diagrammed. @samuelgoto : That is a good catch. This is related to #231. Both are something like link decoration of the IDP API endpoints. |
As you said, there are two attack vectors here:
On link decoration, specifically, I'm wondering if something along the lines of an API that IDPs are required to "declare themselves as IDPs" in a moment that happens before any RPs can use them (and, hence, has to be RP-agnostic) would help. For example: // This gets used across all RPs:
// Has to be called while the user is on idp.example:
FederatedCredential.registerIDP("https://idp.example"); HTML tags would also be an isomorphic formulation: <meta name="fedcm-manifest" data-url="https://idp.example"> WDYT? |
I agree that there are two vectors here, and I think that link decoration is the more concerning one. Timing attacks are definitely the more challenging vector to prevent. Until people begin to talk about removing third-party requests, I'm happy to punt on those :) I think some pre-declaration of the API is a really useful tool. I worry about frequent or just-in-time uses of Another option is to have the account check occur after a user consent is gathered. This would require new structure in the browser because right now the only way to get the account state is to go through the IDP API. This would bind some permission to the RP and IDP pair that permits these requests. This starts to feel a little Storage Access API-like, but hopefully I've conveyed the picture. I think I prefer pre-declaration of the API endpoint though. |
The downside of having account checks after user consent is that it would require two dialogs (first consent, then account chooser), which is a much worse UX. It would be much better if we could find a viable way that did not require that. |
Without having though this through in depth - why not simply disallow path / fragments / query parts for the IDP URL? The only downside I see is that you can't run multiple IDPs on the same eTLD+1, but that might be an issue for some scenarios. |
We had something like this originally where the URL was the IDP origin and the manifest was at |
@dj2: Is there a thread or minutes for this? I'd like to know a little more about the use case and if there is a way to handle it less generally. To make something explicit: an IDP could still smuggle the RDP in on the subdomain unless we restrict the path to only the eTLD+1 and well known URL as @asr-enid implied. |
Here: |
Looking at Tim's comment in that thread it looks similar to exactly the use case we fear: a unique endpoint per RP. I'm not that familiar with Azure AD and how it molds into FedCM. Would the intent be to run one IDP per tenant? A counterproposal could be for the RP to embed the tenant explicitly into the client ID, e.g. |
For most consumer use cases, we'd expect a very small number(N) of IDPs per eTLD+1. While having the manifest under root (N=1) is not very extensible, @samuelgoto came up with an alternative proposal that tries to address this issue via an extra layer of indirection. RP sideThe RP API remains the same: navigator.credentials.get({
federated: {
providers: [{
"url": "https://foo.idp.example/sub",
"clientId": "1234"
}]
}
}); IDP sideUnchangedThe IDP has the same manifest in {
"accounts_endpoint": "/accounts.php",
"client_metadata_endpoint": "/client_metadata.php",
"id_token_endpoint": "/id_token.php",
"revocation_endpoint": "revocation.php"
} ChangedIn addition, in this formulation, the IDP adds (a one time cost) an extra .well-known file under the root of eTLD+1 ( {
"provider_urls": ["https://foo.idp.example/sub/", "https://bar.idp.example/"]
} The idea here is that a browser can check that the list of IDPs in the global .well-known file is sufficiently small to represent IDPs (say, a handful per company) but not RPs (say, thousands). The proposal is to start with N=1, and make this larger as we learn that companies have more than 1 IDP. Notes
Caveat: we propose to start with N = 1 (and incrementally make it larger as needed) because when N becomes large it adds extra entropy that could be abused. We believe N would be able to handle most use cases and that, even if we need a larger N, the abuse would be publicly observable. |
@bvandersloot-mozilla above is another thought that occurred to us as an alternative to the registration alternative we described here. The advantage of this approach is that it: (a) it only requires a one-time configuration cost to add the global .well-known file while (b) fetching the .well-known file dynamically rather than registering it. We think the alternative would be hard because it could get easily out of sync (race conditions, e.g. do we fail if we don't see anything registered before hand?) or attacked (e.g. can the RP collude with the IDP to make the IDP "register" moments before we use it? and if so, would we have to impose some sort of "old enough" criteria?). WDYT? |
With |
The observation that lead to the indirection was that we have heard repeatedly that the teams/servers/codebases/release schedules responsible for running IDPs aren't the same teams/servers/codebases/release schedules responsible for running the etld+1. So, the indirection allows the IDP to move autonomously (e.g. change the configuration), while still being vouched by the etld+1 (which, I expect, needs to happen only one time). FWIW, not having the indirection is a perfectly valid solution to me too (wearing a browser's hat), so they aren't mutually exclusive to me. I'm pretty confident we (wearing an IDP's hat) need the level of indirection, and once the browser has to pay that price, I don't know if not having the indirection buys you much, but nevertheless, they aren't mutually exclusive. My overall argument was more one of a concern over runtime registration, rather than approaches that try to limit the entropy in the .well-known, so I don't feel particularly strongly about the indirection (it just codifies something that we had heard before). Does that make sense? |
I think I have two points to follow up on here:
|
@bvandersloot-mozilla could you elaborate on your attack in 2) a little bit? How would fingerprinting work? One thing we're already aware of is this: since FedCM makes a client metadata request (with client ID & referrer) at about the same time as the account list request (with credentials), the server could try to correlate the two requests based on time and IP address and therefore know that a user was visiting the site even when they decline login. Is the attack you're thinking of more or less the same as this one? |
Part 1 of the change described here: w3c-fedid/FedCM#230 (comment) Bug: 1306997 Change-Id: Ida55651e9782f487a48defbe7e42097f70e0f82e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3569803 Auto-Submit: Christian Biesinger <cbiesinger@chromium.org> Reviewed-by: Yi Gu <yigu@chromium.org> Commit-Queue: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/main@{#989223}
Almost identical! It is a very similar attack, but a little different mechanism. I was thinking of RP Javascript just sending the URI over to the IDP with the Fetch API immediately before creating a credential with the IDP. I just didn't see where in the spec the Metadata endpoint was ever hit, so I didn't want to rely on that. While fingerprinting is hard to solve entirely, we can try to reduce its feasibility and precision. But having a same-moment pair of requests from the same client containing the information we don't want correlated can lead to a very confident link. |
Hmm the spec does seem unclear when the client metadata endpoint gets called. The idea is that it is fetched after the manifest, either concurrently or in sequence with the account list endpoint. The attack is not 100% precise since the server only has the IP address and time to correlate, but yeah, we are aware that this is something we need to solve. |
This seems fundamental to a one-click account chooser. Would it be UX breaking to put a IDP provider chooser before the account chooser? |
One option to fix this that we are exploring is what we've been calling the "push model". In short, in the push model, the IDPs would push the accounts to the browser ahead of time, rather than be pulled dynamically. We think that pushing (as opposed to pulling) would mitigate all of these fingerprinting/timing problems. Because we are expecting the push model to be super complex/involved, we are also trying to understand how important are these timing attacks, compared to other options an attacker would have (e.g. IP, other fingerprinting surface and bounce tracking). Any thoughts on whether the "push model" would work?
Yeah, that could work, but would lead to awkward UX. One of the questions that occurred to us is whether the "IDP chooser" should be in the content area (e.g. only allow the API to be called behind a user gesture) or the browser UI. |
My vision, based on my own priors and what you've said of the push model:
This does incur browser state for each user IDP account. These are rendered legible to the browser, and is more complicated in the browser. However I would be optimistic about the push model from a privacy perspective. There is no communication with the IDP between when top-level authentication is finished and when the user selects them for a given RP.
My first instinct is to make the IDP chooser in the browser UI, and if you are concerned about too much browser UI you could allow the RP to manage account choice in the content area . The user-RP privacy is at risk when an IDP is contacted, so I'd prefer to have that be gated by a browser-user interaction.
w.r.t. IP and other fingerprinting: those are noisy mechanisms that, with work, may be brought back to k-anonymous for useful values of k. The attack here leaks a k=1 identifier (cookie) under reasonable conditions (one login per IP per O(RTT)). As solutions are used to make IP fingerprinting less practical, the privacy gains against all IP fingerprinting would be constrained by this attack because of how it allows precise timing to be added to the mechanism. |
Behind a flag for now. Part 2 of the change described here: w3c-fedid/FedCM#230 (comment) Bug: 1306997 Change-Id: I5b30644ddf59b0d770572ee9b9c70729c55358fb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3572253 Reviewed-by: Danil Somsikov <dsv@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Yi Gu <yigu@chromium.org> Cr-Commit-Position: refs/heads/main@{#991727}
Yeah, that sequence of steps matches more or less what I had in mind too, and the privacy/security trade-offs. It is substantially more complicated (for IDPs), so we've avoided it at first. It seems like we are in agreement that The Push Model would sufficiently mitigate the timing attack problem. Ack?
Do you think that gating behind user gesture is a sufficient mitigation? That is, if we kept the "pull model" but gated it behind a browser-user interaction, do you think that would decrease the attack surface enough to mitigate it? |
I didn't think that it would be that much harder for IDPs. Is there a place I can read where the difficulty arises?
It depends on what you mean by user gesture. If you mean a Browser UI that indicates the particular IDPs, then I think it would. If you mean user activation, then I don't think so. |
To clarify, I'm not saying this is happening. Rather, that it is possible. |
I understand the intent but I'm not sure it's true. As we've seen in other issues, there are many additional use cases that FedCM will need to support. As those use cases get added I highly suspect the impact to RPs will go up. There is also much intrenched deployed code that isn't in active development. Since RPs have to change (even if hopefully minimally) fixing that code can be quite challenging and take some time to figure out. We need to be careful to no underestimate the amount of work required to move to FedCM. |
Just want to note that we addressed the deterministic attacks resulting from the filing of this issue by having a two-tier manifest. We still have the non-deterministic timing attack issue from the credentialed fetch ie #231 |
For those of you following this thread, I just wanted to report back on the progress that @bvandersloot-mozilla and @cbiesinger have been making. Here are a few presentations that we have used to discuss options: I think we are arriving at some convergence, which we expect to close at TPAC in person. |
Just to write down here what we seem to be converging on, it is a variation of what @bvandersloot-mozilla proposed earlier:
To support step (2),
the proposal is to expose a browser API that allows IdPs to tell the browser that the user is logged in (with no UX prompt, a silent store). We've been calling this the "IdP Sign-in Status API", because, well, it is an API that allows IdPs to signal its user's sign-in status. Syntax largely TBD, but, based on what we heard from IdPs, we think it is likely useful to have a HTTP Header API (for IdPs that use a lot of 302s to log the user in and out) and/or a JS API (for IdPs that actually load content/script to log users in). Here is an example of what that could look like: IdP-Sign-In-Status: action=login
IdP-Sign-In-Status: action=logout And in JS: // logs the user in
IdentityProvider.login();
// logs the user out
IdentityProvider.logout(); Every IdP can be in one of three states: To support step (4),
the browser then reads its local storage to determine if the user is logged in to the IdP or not.
This variation doesn't fully prevent the timing attack problem (e.g. step (5) can still incur in a timing attack), but it allows the API to guarantee that UX will always be shown regardless of the case, which substantially limits how it can be abused (e.g. invisibly) by trackers. Importantly, we think this is likely a strict pre-requisite for more privacy preserving options (e.g. using a Web Push API to also cache the user's account metadata) and strikes the right balance between deployment feasibility and privacy protection.
|
Am I right that this assumes that an IDP Sign-In UX would be mandatory here (in case a session expired for example), i.e. could not be controlled by the RP but only a users choice to proceed or not? This seems to be the case - see discussion around "silent" parameters in classical flows (prompt=none). How would one deal with Multi-IDP Scenarios here?
|
Reading through the updated slide deck it seems this step is meant to not occur with real IDPs albeit edge cases like network errors. It might also happen in case the session is timing out though, in which case the IDP will might not have been able to signal a logout via the API which is also mentioned in the deck |
Yes you are right. The essence of this proposal is to prevent "silent tracking". i.e. whenever a timing attack could happen, we show some UI to users (except for the one-time-per-IdP silent tracking). Given that the API is likely called by the IdP, exposing some control to the RP could actually pass it to the IdP or trackers.
If a user has no active session with any of the IdPs, it's up to the RP to specify the order of IdPs. We are still working on UX details such as whether previous session should count w.r.t. the order. e.g. a user has signed in with one IdP in the past and signed out before the API is called. Suggestions are welcome!
This matches our intuition as well. Again, we are still in early UI design stage so suggestions are welcome :) |
Any insight into how often the session would time out without having an ability to call |
Behind a flag for now. Part 2 of the change described here: w3c-fedid/FedCM#230 (comment) Bug: 1306997 Change-Id: I5b30644ddf59b0d770572ee9b9c70729c55358fb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3572253 Reviewed-by: Danil Somsikov <dsv@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Yi Gu <yigu@chromium.org> Cr-Commit-Position: refs/heads/main@{#991727} NOKEYCHECK=True GitOrigin-RevId: eff7aff67a1dc5ea003d94a44b0b563976accd9a
I think this issue solved with all the work we've done in this area (well-known files and IdP login status)? Let me know if we can close |
There is a potential attack that discloses that a user visited an RP to an IDP without explicit consent.
federated
parameter andrequired
mode.There are two features that enable this attack: arbitrary URLs allowed in the manifest and the Referer header being sent with the manifest. Neither is explicitly permitted, but since they are explicitly forbidden in other API call in the spec I assume that they are allowed behaviors. Let me know if I'm misreading this at all!
The text was updated successfully, but these errors were encountered: