Skip to content
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

Rewrite security section #692

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Rewrite security section #692

wants to merge 4 commits into from

Conversation

npm1
Copy link
Collaborator

@npm1 npm1 commented Jan 15, 2025

Improves the security section by adding a brief introduction and mostly talking about threats and attacks as well as the mitigations for each. See #652


Preview | Diff

@npm1
Copy link
Collaborator Author

npm1 commented Jan 15, 2025

Can you take a look @philsmart

@npm1 npm1 mentioned this pull request Jan 16, 2025
5 tasks
Copy link
Contributor

@philsmart philsmart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @npm1, the new structuring of the threats looks good to me, thanks. I've added some comments to the PR (mostly clarifications and suggestions where mitigations might layer or overlap).

if we want to conform to the suggested Security Consideration section structuring in security-questionnaire#181, I think the spec might still need a 'Security Assumptions' section, but would ask for @simoneonofri's advice on that. If it does, some ideas (wording/terminology is probably poor):

  • The user agent is not, itself, acting maliciously and therefore the FedCM API is well-behaved.
    • While the site may behave maliciously if compromised, the user agent implementing FedCM will not.
    • Therefore, fetches to HTTP endpoints are performed through this well-behaved user agent (either via FedCM, or if attempted directly via nefarious JavaScript).
      • Otherwise, we would have to assume the attacker has access to public IdP HTTP endpoints e.g. using a different user agent which allows the user/scripts to set security headers etc.
  • An attacker will not have physical access to a victim user's active session on their user agent.
    • Probably an implicit assumption anyway.
  • (If IdP Registration API is in scope) There is no guarantee of pre-established trust (technical or otherwise) between RPs and IdPs. For example, an attacker can register their own IdP to use at RPs that support the IdP registration API.
  • The messages carried via FedCM are based on an established, secure, identity protocol.
  • Communication between FedCM and the IdP occurs over a secure channel e.g. TLS.
  • ...

Maybe @simoneonofri and @TallTed would be kind enough to give this PR are scan as well, before it goes out for wider review.

spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Show resolved Hide resolved
spec/index.bs Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Show resolved Hide resolved
@simoneonofri
Copy link
Contributor

Thank you. I will review it this week, and the security assumptions make sense.

One of the reasons security assumption exists is to put the scope.

It is a good point that we assume the UA is 'good' to understand whether we are considering a malicious extension in scope or not.

And in general it should me useful to find a mitigation to that point.

@npm1
Copy link
Collaborator Author

npm1 commented Jan 21, 2025

I do not want to add obvious things to the spec just for the sake of adding them. Pretty much all web platform APIs assume that the 'UA is good', no? Is there any example of a spec where it considers the UA as an attacker?

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not exhaustive, but these tweaks should make some parts clearer.

spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small fixes

spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
Copy link
Contributor

@philsmart philsmart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left two further minor text adjustment suggestions. Otherwise, this all looks good to me.

spec/index.bs Show resolved Hide resolved
spec/index.bs Show resolved Hide resolved
spec/index.bs Outdated
<!-- ============================================================ -->

An attacker who has some knowledge about the user via previous tracking could try to impersonate a
real [=IDP=] by registering itself as a FedCM provider and invoking the FedCM API. However, this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the rename makes sense.

spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated Show resolved Hide resolved
spec/index.bs Outdated
that the request was originated by the FedCM browser rather than sent by a websites trying to run an
XSS attack. An [=IDP=] needs to check for this header's value in the credentialed requests it
receives, which ensures that the request was initiated by the user agent, based on the FedCM API.
This protects the new [=IDP=] endpoints from unauthorized fetches.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text additions LGTM

spec/index.bs Show resolved Hide resolved
spec/index.bs Show resolved Hide resolved
@npm1
Copy link
Collaborator Author

npm1 commented Jan 27, 2025

I think I have addressed the comments, ptal

@philsmart
Copy link
Contributor

LGTM, thanks.

@@ -2559,49 +2559,65 @@ The [=remote end steps=] are:
This section provides a few of the security considerations for the FedCM API. Note that there is a
separate section for [[#privacy]] considerations.

In FedCM, there are various assets that need to be protected. All of the endpoints need basic
protections, and the credentialed ones need protections from various kinds of attacks and threats.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protections, and the credentialed ones need protections from various kinds of attacks and threats.
protections, and the credentialed ones need protections from various kinds of attacks and threats.
<!-- ============================================================ -->
## Security Assumptions ## {#security-assumptions}
<!-- ============================================================ -->
The design of FedCM relies on several key security assumptions:
<!-- ============================================================ -->
### Trusted User Agent ### {#trusted-user-agent}
<!-- ============================================================ -->
The user agent (i.e., the browser) is assumed to be a trusted entity. It is expected to faithfully
enforce same-origin policies, execute CSP and CORS checks, and present a secure, non-forgeable UI
that users can trust, and to not contains or executes malicious third parties scripts. The browser
is responsible for mediating the flow and preventing unauthorized access to credentials.
<!-- ============================================================ -->
### Secure Network and Transport ### {#secure-network-transport}
<!-- ============================================================ -->
All network requests — especially those carrying sensitive information such as tokens — occur over
secure channels (e.g. HTTPS). The specification assumes that the transport layer prevents
man-in-the-middle and other network attacks.
<!-- ============================================================ -->
### Proper Implementation of Security Headers ### {#proper-implementation-headers}
<!-- ============================================================ -->
[=IDP=] and [=RP=] implement and enforce appropriate security headers (such as CSP, CORS,
and the mandatory use of the `Sec-Fetch-Dest: webidentity` header). These headers are key to
distinguishing browser-initiated FedCM flows from arbitrary requests.
<!-- ============================================================ -->
### IDP and RP Integrity ### {#proper-implementation-headers}
<!-- ============================================================ -->
IDP and RP endpoints are implemented correctly and do not contain vulnerabilities that could
allow an attacker to bypass the FedCM flow or extract sensitive data.
<!-- ============================================================ -->
## Threats and Attacks ## {#security-threats-aattacks}
<!-- ============================================================ -->

Adding some assumptions. I have been thinking about this for a long time, and it makes sense that some assumptions may seem obvious to us (precisely because they are things we take for granted), but they are useful in defining the scope of the specification, even “outside” of what is the environment we consider within the trust boundaries.

Copy link
Contributor

@simoneonofri simoneonofri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all the effort in the Security Consideration section.

I've just proposed some assumptions and a formatting for the pairs threats/mitigations

<!-- ============================================================ -->
## Content Security Policy ## {#content-security-policy}
## Cross-Site Scripting ## {#cross-site-scripting}
<!-- ============================================================ -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**

API calls to a legitimate [=IDP=]. If the call is successful, this would introduce some browser UI
where all parties might look legitimate to the user. If the user logs in using the FedCM API in this
scenario, their credentials are then received by the malicious script, which can then send it to its
own server. The protection against this attack is to sanitize all user input to prevent script
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
own server. The protection against this attack is to sanitize all user input to prevent script
own server.
**Mitigation**:
The protection against this attack is to sanitize all user input to prevent script

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
own server. The protection against this attack is to sanitize all user input to prevent script
own server.
**Mitigation**
The protection against this attack is to sanitize all user input to prevent script

scenario, their credentials are then received by the malicious script, which can then send it to its
own server. The protection against this attack is to sanitize all user input to prevent script
injection, and to use [[!CSP]] to restrict the execution of inline scripts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**


A script could also instead use the FedCM API to request credentials from some [=IDP=] that is not
approved by the [=RP=]. If the user went through this flow, these credentials could also be stolen,
or the [=RP=]'s reputation could be harmed. An additional protection for this scenario is that the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
or the [=RP=]'s reputation could be harmed. An additional protection for this scenario is that the
or the [=RP=]'s reputation could be harmed.
**Mitigation**:
An additional protection for this scenario is that the

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
or the [=RP=]'s reputation could be harmed. An additional protection for this scenario is that the
or the [=RP=]'s reputation could be harmed.
**Mitigation**
An additional protection for this scenario is that the


<!-- ============================================================ -->
## Sec-Fetch-Dest Header ## {#sec-fetch-dest-header}
## Unauthorized Fetching ## {#unauthorized-fetching}
<!-- ============================================================ -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**

## Clickbait ## {#clickbait}
<!-- ============================================================ -->

An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by
**Threat**:
An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by
**Threat**
An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by

visible in the UI. For example, they could enter text such as "Click here to win $100!". As this
requires the attacker to control the [=IDP=], the gain would be minimal, i.e. access to user
tracking within the [=RP=]. No sensitive information would be shared with the attacker in this case.
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some
**Mitigation**:
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some
**Mitigation**
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some

## Clickjacking ## {#clickjacking}
<!-- ============================================================ -->

An attacker could surface UI to try to force the user into accidentally consenting to the FedCM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An attacker could surface UI to try to force the user into accidentally consenting to the FedCM
**Threat**:
An attacker could surface UI to try to force the user into accidentally consenting to the FedCM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An attacker could surface UI to try to force the user into accidentally consenting to the FedCM
**Threat**
An attacker could surface UI to try to force the user into accidentally consenting to the FedCM

user information from a real [=IDP=]. For example, an attacker could have a button 'click here 4
times' which would surface the FedCM dialog the first time it is clicked. If the timing and the
location of this button works out, an unsuspecting user would click on their account in the FedCM
dialog when it appears. There are a couple of possible mitigations to this. One is the user agent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dialog when it appears. There are a couple of possible mitigations to this. One is the user agent
dialog when it appears.
**Mitigation**:
There are a couple of possible mitigations to this. One is the user agent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dialog when it appears. There are a couple of possible mitigations to this. One is the user agent
dialog when it appears.
**Mitigation**
There are a couple of possible mitigations to this. One is the user agent

enhanced protection. To enable this, we impose the requirement that the [=IDP=] explicitly consents
to this sharing taking place by using the "cors" [=request/mode=] when fetching this endpoint. This
also helps with servers that may accidentally ignore the <a http-header>Sec-Fetch-Dest</a>: they
cannot ignore CORS, as without it the fetch will fail.

<!-- ============================================================ -->
## Browser Surface Impersonation ## {#browser-surface-impersonation}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on this, for some reason I cannot propose the threat/mitigation schema

@TallTed
Copy link
Contributor

TallTed commented Feb 4, 2025

For better readability, I strongly advise that **Mitigation**: and similar headings either drop the colons (preferred), e.g., **Mitigation**, or include the colons in the bold, e.g., **Mitigation:**.

@simoneonofri
Copy link
Contributor

Hi Ted, it is ok for me as you proposed

<!-- ============================================================ -->
## Content Security Policy ## {#content-security-policy}
## Cross-Site Scripting ## {#cross-site-scripting}
<!-- ============================================================ -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**

API calls to a legitimate [=IDP=]. If the call is successful, this would introduce some browser UI
where all parties might look legitimate to the user. If the user logs in using the FedCM API in this
scenario, their credentials are then received by the malicious script, which can then send it to its
own server. The protection against this attack is to sanitize all user input to prevent script
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
own server. The protection against this attack is to sanitize all user input to prevent script
own server.
**Mitigation**
The protection against this attack is to sanitize all user input to prevent script

scenario, their credentials are then received by the malicious script, which can then send it to its
own server. The protection against this attack is to sanitize all user input to prevent script
injection, and to use [[!CSP]] to restrict the execution of inline scripts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**


A script could also instead use the FedCM API to request credentials from some [=IDP=] that is not
approved by the [=RP=]. If the user went through this flow, these credentials could also be stolen,
or the [=RP=]'s reputation could be harmed. An additional protection for this scenario is that the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
or the [=RP=]'s reputation could be harmed. An additional protection for this scenario is that the
or the [=RP=]'s reputation could be harmed.
**Mitigation**
An additional protection for this scenario is that the


<!-- ============================================================ -->
## Sec-Fetch-Dest Header ## {#sec-fetch-dest-header}
## Unauthorized Fetching ## {#unauthorized-fetching}
<!-- ============================================================ -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Threat**

real [=IDP=] by pretending to be an [=IDP=] and invoking the FedCM API. However, this does not grant
much more power to the attacker: if the user logs in via FedCM to the attacker, the attacker can now
track the user more easily within the [=RP=], but the attacker was already tracking the user in
order to surface a meaningful dialog. That said, [=IDP=] impersonation would still be poor UX for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
order to surface a meaningful dialog. That said, [=IDP=] impersonation would still be poor UX for
order to surface a meaningful dialog.
**Mitigation**
That said, [=IDP=] impersonation would still be poor UX for

## Clickbait ## {#clickbait}
<!-- ============================================================ -->

An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by
**Threat**
An attacker controlled [=IDP=] could attempt to trick the user into going through the FedCM UI by

visible in the UI. For example, they could enter text such as "Click here to win $100!". As this
requires the attacker to control the [=IDP=], the gain would be minimal, i.e. access to user
tracking within the [=RP=]. No sensitive information would be shared with the attacker in this case.
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some
**Mitigation**
To mitigate this, the user agent can add [=IDP=]-independent text to ensure that the user has some

## Clickjacking ## {#clickjacking}
<!-- ============================================================ -->

An attacker could surface UI to try to force the user into accidentally consenting to the FedCM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An attacker could surface UI to try to force the user into accidentally consenting to the FedCM
**Threat**
An attacker could surface UI to try to force the user into accidentally consenting to the FedCM

user information from a real [=IDP=]. For example, an attacker could have a button 'click here 4
times' which would surface the FedCM dialog the first time it is clicked. If the timing and the
location of this button works out, an unsuspecting user would click on their account in the FedCM
dialog when it appears. There are a couple of possible mitigations to this. One is the user agent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dialog when it appears. There are a couple of possible mitigations to this. One is the user agent
dialog when it appears.
**Mitigation**
There are a couple of possible mitigations to this. One is the user agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants