-
Notifications
You must be signed in to change notification settings - Fork 267
crypto and key-management api for DeepKey: Keystore & PassphraseManager #1104
Conversation
} | ||
}; | ||
self.cache | ||
.insert(id_str.clone(), Arc::new(Mutex::new(secret))); |
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'm uncertain about having a cache here. Part of the point of the passphrase manager is to give users control over "locking" their authorizor capabilities after an expiration timer, but if they authorize something and their authorizor key gets cached here, then we have to coordinate expiring the cache when the passphrase expires?
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.
Hm, I get that.
But we do need to hold some secrets unencrypted indefinitely: agent keys in particular.
I would say we might need to add more complexity around this cache so that agent keys stay in, even after the PassphraseManager locks itself up. But everything else gets cleared from the cache at some point, as a default that applies in particular to seeds.. Maybe dependent on the secret type? Or everything gets cleared except for PRIMARY_KEYBUNDLE_ID
? What do you think?
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 guess I was imagining anything that needed persistence like for the app keys handling it on that end... i.e. keeping a copy of the secret after requesting it. But definitely something we can iterate on to see what works.
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.
Nice 👍 This is starting to come together! Couple questions / change requests above.
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.
Wooo, crypto incoming! lgtm
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 PR
Keystore
implementation indpki
crate which has the functions necessary for crypto to be usable both by DPKI and other apps that want to do secure crypto functions. The work is based on this spec:https://hackmd.io/tVy22HhJQPuHO28gtD_UTQ?both
and this diagram:
https://realtimeboard.com/app/board/o9J_kycnaks=/
What it does not do, and needs to be done in future prs is:
hc keygen
solution