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

Change message to sign #30

Open
dgrcode opened this issue Sep 17, 2021 · 1 comment
Open

Change message to sign #30

dgrcode opened this issue Sep 17, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@dgrcode
Copy link
Collaborator

dgrcode commented Sep 17, 2021

This is not an MVP feature, but something I think we should consider.

I see the message to be signed something like a CSRF token. If we always send the same message, the same addresses will always produce the same signature, so an eavesdropper will be able to re-authenticate maliciously with the signature they found.

Why we shouldn't worry too much

I don't think this is a big deal because:

  • We won't be dealing with highly sensitive information,
  • All communications will be through https, so eavesdropping will be expensive and definitely not worth it taking into account the information they would get access to if someone is successfully hacked,
  • Even changing the message to sign an eavesdropper would have the JWT token, and complete access until the JWT token is expired.

Why I think this should be implemented at some point

However, I think changing the message to be signed shouldn't be too hard. The only concern here is storing the message to compare it with the signed message. For that, an in-memory dictionary with { [address]: messageToBeSigned } should work completely fine. At least until we have >10k users signing in at the same time (should be fine).

(cc @carletex @sogasg)

@carletex carletex added the enhancement New feature or request label Sep 20, 2021
@dgrcode
Copy link
Collaborator Author

dgrcode commented Sep 24, 2021

From a comment in #48

Nothing to change here, but something to keep in mind. Once we have non-deterministic input to generate the messages (like nonce, timestamp, etc), we'll need a way to store the message even if just in memory because it won't be possible to regenerate the message as we're doing in (currently) line 30:

const trustedMessage = getSignMessageForId(verifyOptions.messageId, verifyOptions);

We also need to update all the messages to sign, not just the one at /sign (we only had that one when the issue was created)

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

No branches or pull requests

2 participants