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

check timestamps of BLS session sigs #512

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Conversation

glitch003
Copy link
Collaborator

A user found a bug where the SDK was sending an expired BLS session sig. I discovered that we aren't checking the expiration when we are checking if we need to resign the session key. We do this check for ECDSA, because it's built into SIWE, but we can't use SIWE verification for the BLS session sig, because SIWE doesn't understand BLS.

So we're checking the sig already, and this PR just copy/pastes the timestamp verification code from the SIWE repo here: https://github.com/spruceid/siwe/blob/main/packages/siwe/lib/client.ts#L289

This makes it so that we check the expiration and issued at time, and then throw an error if they're not valid. The calling function (checkNeedToResignSessionKey()) catches errors and will try to re-sign if the SIWE is expired.

@joshLong145 joshLong145 self-requested a review June 26, 2024 17:00
joshLong145
joshLong145 previously approved these changes Jul 1, 2024
Copy link
Contributor

@MaximusHaximus MaximusHaximus left a comment

Choose a reason for hiding this comment

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

👍 LGTM -- :shipit: 🚢

@glitch003 glitch003 merged commit 97ff442 into master Jul 2, 2024
4 checks passed
@glitch003 glitch003 deleted the fix/session-sig-re-signing branch July 2, 2024 04:20
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.

3 participants