storacha-network / w3up

⁂ w3up protocol implementation
https://github.com/storacha-network/specs
Other
55 stars 19 forks source link

access-api: email validation is insecure, currently easy to associate any target email with a malicious identity #333

Open natevw opened 1 year ago

natevw commented 1 year ago

The access-api package's /validate-email endpoint does not provide sufficient authentication or authorization for its anticipated purpose.

Currently the flow works roughly as follows:

  1. The client generates an identity and makes a request to the API to send a validation email
  2. The API sends the email including a link to approve the association between the identity⇔email
  3. When the link is opened, the API sends a message back to the client which allows that association

The problem

The flow as implemented is not secure/trustworthy. There are two significant flaws, closely related but kept separate to highlight the need for improvements on both sides:

In most cases where email login is used, whoever opens the link gains access to the underlying session/identity. But in this case opening the link gives someone else that access! A malicious actor Eve simply provides Alice/Bob's email address — now Eve just needs to wait for Alice's computer to generate a page preview for the link, or for Bob to click the link, to gain access to their account.

[Currently this is not a big deal because there's not really much of any "account" to access, and things like upload lists and such are siloed per-identity rather than per-email. But the anticipated purpose is to link actions done by that identity to the email, otherwise the apps wouldn't be attempting to validate email at all!]

Proposed solution

This came out of a discussion we had at https://filecoinproject.slack.com/archives/C02BZPRS9HP/p1673387411040389 around these concerns (which were also brought up in https://github.com/web3-storage/specs/issues/26 earlier as well). There needs to be some way to "close the loop" and ensure that maliciously requested privileges associated with an email address aren't accidentally approved.

I would propose the following improved email validation flow:

  1. We still allow ± anyone to generate a new identity and request a verification email to an arbitrary address. [Business side will need to consider some sort of throttling/abuse prevention here but that's a general spam thing and doesn't effect this account security issue.]
  2. The API generates a link to an approval dialog page, when fetched has no side effects, just shows options to ACCEPT / REJECT. Note that this link must not be possible to predict based on the original identity — only the email account holder [and their service providers] should have access to the form.
  3. The approval dialog needs to contain sufficient information that the user can make an informed choice whether or not to actually associate the new identity with their email/account — for v0 this could be the raw DID [], for v0.9 this could be a unique pattern/image which the API generates, for v1/v2 the pattern could be uniquely derivable from the identity itself. The user is prompted (in nicer words): do you recognize this request?
  4. Then only if approved, the the approval dialog page issues a PUT/POST request to the API, which can then broadcast the delegated token thing to the original requestor. [Note that this is actually the link/request data that can't be predictable by anyone without the original link; i.e. need to ensure that an attacker can't just skip straight to this step and approve the association without actually proving they have access to the claimed email address]

This approval form both resolves the HTTP/"Safelinks" problem and provides the authenticated end-user a way to review the request more thoroughly before approving. It does not necessarily require any changes to the client library(ies) to just solve the GET vs. PUT/POST problem. But to really solve the authorization side the client library will need to be updated to show something that the user matches up with a copy on the approval form. It could actually be a combo of:

Alternatives considered

The workflow above isn't 100% foolproof! A user could still somehow/someway plow through and approve something they shouldn't have if they were tricked or in a hurry or whatnot. Some potentially more robust solutions might be:

There might be some sort of crypto-ey out-of-the-box-thinking de-centralized solution lurking here too, but we might prefer loading the problem more on the business side to keep the UX simpler!

natevw commented 1 year ago

I'm waiting for more discussion/agreement here but if we proceed in this direction we should also open over at https://github.com/web3-storage/w3ui/issues a corresponding ticket to track the work that would be needed on the client side. But I think the improvements here could be started independently from the client side — splitting the approval link into the GET landing page vs. actual PUT/POST handler and making sure the approval request itself is tied to some sort of temporary nonce or server secret–derived that prevents forgery of the approval itself.

yusefnapora commented 1 year ago

Thanks for the great write up & careful analysis @natevw 👍

I agree that we need to close the loop here, which probably means adding a tiny bit of user friction. Personally I think it's probably fine to require entering an OTP-style code into the client - that feels like the simplest thing, and it doesn't feel surprising as a user since it's pretty similar to 2FA flows that people have probably seen before.

alanshaw commented 1 year ago

@natevw I think the proposed solution is a good and sufficient first step to fix this. Are you interested in working on it? I'd like it to be resolved ASAP. Thanks for raising and thanks especially for proposing a solution.

natevw commented 1 year ago

@alanshaw Yeah, I think this would be a great scope for me to tackle. I'll start proceeding as above but to re-iterate I'm thinking roughly:

  1. Start with splitting the landing page and actual approval (scoped to just this repo)
  2. Make sure the approval request can't be forged by attacker (again just this repo, if even necessary, tbd)
  3. Work out an accessible approach to a "randomart" display that should match between requesting app and approval page (this will touch both repos) — whether just a nonce the API generates or something derived from the underlying DID/UCAN stuff
  4. Potential followup re. OTP-style code — this would again touch both repos and might be more of a v1.5 or v2 thing if the additional security is still desired
hugomrdias commented 1 year ago

@natevw thank you for the feedback and proposal.

I wrote some notes about this here https://purrfect-tracker-45c.notion.site/Email-auth-flow-d33f0715024a47c18a8114ba284e9c07.

I not 100% sure that adding a new step to do a POST is the way to go here but there's definitely improvements to be done.

natevw commented 1 year ago

@hugomrdias re. your:

I not 100% sure that adding a new step to do a POST is the way to go here but there's definitely improvements to be done.

This is what you captured as "Avoid SafeLinks and other auto open email client stuff" in the notes, and is ultimately just an HTTP standards issue.

Any GET handling is expected to be:

essentially read-only; i.e., the client does not request, and does not expect, any state change on the origin server as a result of applying a safe method to a target resource. Likewise, reasonable use of a safe method is not expected to cause any harm, loss of property, or unusual burden on the origin server.

And:

The purpose of distinguishing between safe [i.e. like GET] and unsafe [like PUT/POST] methods is to allow automated retrieval processes (spiders) and cache performance optimization (pre-fetching) to work without fear of causing harm. In addition, it allows a user agent to apply appropriate constraints on the automated use of unsafe methods when processing potentially untrusted content.

And since afaict it doesn't require any changes but to the access-api server itself splitting the process into:

  1. email link opens a page (GET)
  2. that page must submit something (POST) to actually approve

is to me the "low hanging fruit" here and I'm hoping to submit a PR for it asap.

hugomrdias commented 1 year ago

The thing here is that clicking on the email link does not change any state, it works just like any other GET just return whats there already in this case the ucan delegation. Either using websockets or the http response that also includes the delegation as string and QRcode.

natevw commented 1 year ago

This is mostly covered by #348 (→ #398) and the "validation phrase" part of #347 (→ #399). The UI side has a placeholder issue https://github.com/web3-storage/w3ui/issues/307 as well.