-
Notifications
You must be signed in to change notification settings - Fork 54
Duress login fixes #650
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
Duress login fixes #650
Conversation
389b43a
to
e744119
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work! I have some small cleanup suggestions.
const duressSessionKey = decryptChildKey( | ||
stash, | ||
sessionKey, | ||
duressStash.loginId | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed - the root sessionKey
will be fine. On the whiteboard diagram, the core knows how to handle the green arrow (the session key) being on a lower node than the orange arrow (the appId). It just does decryptChildKey
as-needed to handle the difference.
However, the way you are doing it does protect the root account from modifications, since we don't have a root decryption key. If that's the goal, then carry on! That's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Less changes now, means less tests now. I'll save face for this reason and the one you mentioned.
f6ad17d
to
49d6e32
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
none