Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: verify legitimate claim↔︎redeem by assigning a random phrase during email validation #399

Closed
wants to merge 8 commits into from

Conversation

natevw
Copy link
Contributor

@natevw natevw commented Jan 27, 2023

This branch fulfills the key piece of issue #347, adding a "validation phrase" that serves as a human-friendly nonce — enabling definitive verification of the validity of an email request.

(The other request metadata mentioned can help savvy users investigate any abuse further, but the phrase links one specific request to one specific email, completely out of the control of an attacker.)

Problem summary

The idea here is to avoid malicious account validation: currently, Eve can generate a new space keypair and request delegation against Alice's email address. Alice may not be able to tell if the request is legitimate (or may follow the activation link simply out of curiousity/confusion) and grant Eve access!

Solution provided

  1. The requesting UI and the resulting validation emails will both show a short nonce phrase like carat tender electronic tee guessed. [At the moment can preview phrases via http://localhost:8787/phrase-test?bits=50 but that should likely get dropped before this PR is merged….]
  2. The email will prompt something like "does this phrase match the one displayed in the app you are registering?"
  3. The user can then check to make sure that the email validation they are approving matches the request they made themselves.

The Agent.registerSpace and Agent.recover methods in this repo's @web3-storage/access/agent package now take an (optional but strongly encouraged!) callback via opts.handlePhrase which will be called when the API provides a match_phrase in its response to each method's respective invocation. The w3ui components and any protocol documentation for third-party clients will need updating to support this on the requestor side!

So this is a somewhat breaking change, but:

  • until the email templates are updated, this won't lead to user confusion
  • even then, anyone testing can just ignore the nonce phrase and say "yes it matches"
  • this overall strategy (plaintext phrase) should also be compatible with any CLI or screen reader usage

Note also that the phrase generation is done at an MVP or proof-of-concept level. I've generated a small word back and left the DEFAULT_ENTROPY for generateNoncePhrase correspondingly low as well. Next steps here would be:

  • it might not be a bad idea to increase the number of words so that the phrase entropy can be increased. (I'm not terribly worried about this though since iiuc we're really just fighting the chance of very lucky random collisions on a few malicious attempts — if not already emailing should be generally rate-limited so presumably an attacker would only get a few tries per email address anyway just to prevent spam/abuse generally)
  • at least review the existing word list and simply drop any unwanted entries (potentially offensive, overly negative, weird characters/punctuation)
  • rather than just removing unwanted words, marketing could come up with a whole set of desirable "on-brand" words for enhanced onboarding vibes ✨
  • or, could just find an existing npm word list package (or whole nonce phrase generator!) rather than maintaining a wordlist in this repo

TODO

Here:

  • phrase generation
  • return phrase back to client
  • ^^^ check that above actually is working?
  • show {{ match_phrase }} in HTML+plain emails
  • update access-client here to parse out response
  • update templates in postmark itself (also catch up plaintext after fix: align postmark/welcome.txt with .html version #403 merged)

Over in UI:

  • update w3ui low-level keyring stuff to handle phrase
  • update w3ui high-level examples/components to display phrase

This is tracked in storacha/w3ui#307, and isn't a total blocker since you can either wait to update the templates in Postmark and/or just have the user click through even without being able to see the phrase.

@natevw natevw marked this pull request as draft January 27, 2023 03:34
@natevw natevw changed the title [wip] Generate a random phrase for email validation feat: generate a random phrase for email validation Jan 27, 2023
@natevw
Copy link
Contributor Author

natevw commented Jan 27, 2023

I tried asking ChatGPT for some better words but it wasn't a whole lot of help 😂

Screenshot of ChatGPT convo asking for cool p2p fileshare words and getting back only things like GnuTella and Retroshare, asking for 500 happy internet words and getting just 10 words about streaming and expression

@natevw natevw marked this pull request as ready for review January 27, 2023 22:58
@natevw natevw changed the title feat: generate a random phrase for email validation feat: link email validation request↔︎approval via random phrase Jan 27, 2023
@natevw natevw changed the title feat: link email validation request↔︎approval via random phrase feat: verify legitimate claim↔︎redeem by assigning a random phrase during email validation Jan 27, 2023
@travis travis self-assigned this Feb 8, 2023
@travis
Copy link
Member

travis commented Feb 9, 2023

moving to #432 to merge main and fix tests

@travis travis closed this Feb 9, 2023
Peeja pushed a commit to storacha/upload-service that referenced this pull request Jan 17, 2025
Closes storacha#399

---------

Co-authored-by: Travis Vachon <travis@dag.house>
Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
Peeja pushed a commit to storacha/upload-service that referenced this pull request Jan 29, 2025
Closes storacha#399

---------

Co-authored-by: Travis Vachon <travis@dag.house>
Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants