Implement the Choose a security key -page #1115
Conversation
| final hiddenKey = securityKey == null ? null : '*' * securityKey.length; | ||
| log.debug('set security key: $hiddenKey'); | ||
| _securityKey = securityKey; | ||
| } |
There was a problem hiding this comment.
Is it practical to store the password in an encrypted form for additional safety?
I understand we are already in a privileged context and an attack there seems unfeasible now, but how knows whether it will change or not in the future.
My understanding is that this will remain in clear text form for a while until we are able to apply the GuidedChoice, moment which has recently been delayed.
There was a problem hiding this comment.
Thanks, very good point! I'll bring it up in today's catch-up meeting.
There was a problem hiding this comment.
Hmm, the page stack keeps the state of all previous pages in memory. The security key is in fact stored in memory multiple times also by the view model and the editing controllers of the two text fields used for entering and confirming the key. Sending the security key to subiquity wouldn't change much because it would just add one more instance of the security key stored in the memory space of a different process.
What we could do is to clear the text fields as soon as the user proceeds to the next page, but would that be annoying if the user takes one step back in the wizard? The security key you just entered would be gone. Furthermore, we could clear the security key stored in the service as soon as the guided choice is sent to subiquity, but the usefulness of this depends on what we decide to do with the text fields. What do you think?
There was a problem hiding this comment.
Wiping out the text fields when proceeding to the next page shouldn't be an issue as long as when navigating backwards, the page view model refreshes itself by asking the values to the service. Wiping out the string from the service class only with the text fields offstage would likely result in not updating them, thus the value copies would still be around for a while.
I couldn't find any package on https://pub.dev that would offer a class able to store strings in an encrypted form and decrypting on read. It would be a little hit in performance, but at least it would be a bit harder for one to find keys and passwords when reading the process's memory. Something like https://docs.oracle.com/html/E28160_01/org/identityconnectors/common/security/GuardedString.html .
flutter_secure_storage seemed quite promising, but an overkill IMHO.
It seems not practical to deal with that issue in this pull request. Making the service encrypt the value and store only the encrypted version could be a solution as long as other copies are not held in memory.
There was a problem hiding this comment.
Wiping out the text fields when proceeding to the next page shouldn't be an issue as long as when navigating backwards, the page view model refreshes itself by asking the values to the service.
Indeed, thanks for opening my eyes! 🙈
There was a problem hiding this comment.
I'll make #951 wipe the security key from the service when the installation begins and the key is no longer required.
There was a problem hiding this comment.
This would be good for now? Ubiquity takes no extra steps to encrypt the key either but we'll bring up the question to the security team.
|
Thank you for bringing it up! I very much agree that we should be considerate when dealing with the key. |
The logic that connects the Choose a security key page with
DiskStorageServicehas been extracted from #951.