status-go
status-go copied to clipboard
[Onboarding] Store keys with the 6-digit pincode
It should be possible to use 6-digit pincode instead of a password
Actual behaviour 5 accounts are generated and 5 public keys sent to status-react -> user chooses one -> enters password -> chosen account is stored with the password
Expected behaviour 5 accounts are generated and 5 public keys sent to status-react -> user chooses one -> enters 6-digit pincode -> chosen account is stored with the pincode
Implementation notes There is a pincode doc describing updated account flows: https://docs.google.com/document/d/1IrjvvAuAQbYsr3xqZvXfgAeeyDZRl_x9KRxWnLcwVuI/edit?ts=5cc831fb When using pins instead of a password, Ethereum key pair will have to be encrypted/decrypted using a keypair generated inside device's TEE, as pins do not have enough enthropy. This needs to happen on Go side as having access to it from JS context is insecure due to multiple dependencies.
status-go
should receive an object with 3 methods (initialize
, encrypt
, decrypt
) with native implementations for iOS/Android.
An Android implementation but for JS already exists in our fork of react-native-keychain (https://github.com/status-im/react-native-keychain/commit/43e5512cabb8ee064fd9e503be943dcf2c7d7669), as for iOS, inspiration can be taken from https://github.com/keybase/go-keychain.
Also a nice overview is given here
@gravityblast @siphiuel hmm found this issue in Wrike and don't know how to organize it. Is it still relevant?
Yes it is, i've been working on the status-react counterpart to this.
➤ Nabil Naghdy commented:
We should probably update the date for this then as well
@flexsurfer are you the author of the doc? i have a couple of questions, maybe you can help
- The app (status-go) creates an Ethereum HD key pair (eE, dE) and encrypts it with e0 5. The app (status-go) uses Ethereum encryption key (eE) to encrypt the db.
we cannot use asymmetric key to encrypt database. sqlite doesn't support it in any way, and i don't understand how it can. do you propose to derive secret from eE? what should i use as a key material?
- The app (status-react) creates a key pair in the secure element (e0, d0)
why do we need assymetric key in secure element? as i understand the only purpose of this key is to encrypt/decrypt HD key. if so then symmetric key is well suited for this task. The documentation for API is somewhat confusing but it looks like i can generate random aes key with specified length and then use it for encryption/decryption. SecKeyCreateEncryptedData
behavior changes based on the key type. so if it is possible, i will use symmetric key, is that fine or you see some issues?
hey @dmitryn , no, author is @mandrigin , maybe @siphiuel can answer your question, he researched this document
An Android implementation but for JS already exists in our fork of react-native-keychain (status-im/react-native-keychain@43e5512)
i think this implementation uses different API, that is based on password. as i understood we need to generate random key instead of it. or am i wrong?
how secure element handles authorization? how to make key unavailable to any other application? i am trying to find this info in docs, but maybe someone had experience with this already?
another question that came up while reading docs, is that why do we need to encrypt HD pair at all if later we will decrypt it and store in memory? we can add any blob of data to secure storage and then retrieve it when needed (e.g on login). if pincode is not correct that blob of data can be removed, so all other workflows described in the doc are exactly the same
@dshulyak thanks for reviewing the doc! I'll try to answer some of your concerns to the best of my ability
- The app (status-react) creates a key pair in the secure element (e0, d0)
You're right, symmetric AES encryption should be enough I think
- The app (status-go) uses Ethereum encryption key (eE) to encrypt the db.
It should be derived from Ethereum key pair, not sure how.
@corpetty maybe you will be able to answer @dshulyak 's questions
It should be derived from Ethereum key pair, not sure how.
i will be going for hkdf with sha256 as a hasher and private key as secret material for derivation. let me know if there are concerns about it.
so the last question is pretty much what we will store in secure storage, ethereum key or key that was used to encrypt ethereum key. in my current understanding storing additional key is a redundant step that provides no additional security guarantees. if secure storage is compromised and encryption key is available then attacker can decrypt ethereum key as well.
I made several flow charts to better understand the whole process:
- Create account
Security.SecItemAdd needs to be made with status application group and unique user tag so that this item can be found later.
- Login
Authorization is managed using tag and access groups. Per IOS security guide
Keychain items can only be shared between apps from the same developer. Thisis managed by requiring third-party apps to use access groups with aprefix allocated to them through the Apple Developer Program through application groups.
- Account locking
After several invalid pin attempts we delete key from secure storage, after that user will have to follow recovery process.
- Recovery
Recovery is made 12-word pass phrase that was returned to user during onboarding.
Both security concerns identified in original doc are addressed.
@siphiuel question about pincode. is it a system wide passcode (pincode/fingeprint/face id) or some kind of our pincode? asking to understand what kind of auth options we need to use for ios/adnroid keychain
FYI here's so more guidance Igor left: https://www.notion.so/Encryption-in-a-secure-element-ccb8778651024440a81f4f1e626a4342 Leaving it up to you to judge the relevance:)
@dshulyak it's our pincode same as password, just 6 digits so we'll have it in status-react as a string "123456"
FYI here's so more guidance Igor left: https://www.notion.so/Encryption-in-a-secure-element-ccb8778651024440a81f4f1e626a4342 Leaving it up to you to judge the relevance:)
thank you thats relevant, i just wanted to collect more information, cause it seems like a big change. below is the scope of changes as i see them:
- [ ] new keystore that will use keychain interface for encrypting files before storing them on disk
- [ ] refactor accounts manager to work with new keystore
- [ ] add implementation for keychain interface that will use passwords for auth, as desktop won't be always able to use keychain
- [ ] objective c keychain implementation
- [ ] java keychain implementation
following interface needs to be implemented by java and objective c and provided to golang before node is started:
type Keychain interface {
SecurityLevel() int
CreateKey(auth string) error
DeleteKey(auth string) error
Encrypt(auth string, dec []byte) (enc []byte, err error)
Decrypt(auth string, enc []byte) (dec []byte, err error)
}
auth must be unique identifier for the key. it will be used as a tag for ios and alias for android keychains.
cc @gravityblast
➤ Oskar Thoren commented:
Rachel HamlinCorey Petty What was the outcome of recent call? I'm still confused as to why this is necessary, considering we do the same operation with password right now. It'd be great to have a doc where there's a more clear rationale, considering how this delays v1
➤ Rachel Hamlin commented:
Oskar Thoren it's not in scope! Missed this task. I'll remove it from code freeze. Corey Petty reached out to his community to see if we could find someone, but the assumption is we can't build it in time.
➤ Rachel Hamlin commented:
Okay, here's an example of a Wrike issue that I can't remove from its original placement. I just relocated it to v1 - release (optimistically), but I can't delete it from v1 code freeze. The 'Onboarding' label is grayed out.
➤ Corey Petty commented:
Oskar Thoren Do we actually use TEE where available with current password workflow? I was under the impression we "store" the password there for certain reasons, but encrypt with the password itself, not material generated from the TEE. If this is the case, then the pin was a much more secure flow.
➤ Oskar Thoren commented:
Corey Petty not familiar enough with the implementation details of it unfortunately to answer that question
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.