This repository has been archived by the owner on Jan 25, 2020. It is now read-only.
[329] Hide custom in memory protected fields #658
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Version 1.6 was released Jun 14, 2015, this was the first version to set the version number. Version 1.6 was also the first version to have upgrade instructions. So prior versions didn't set the version number stored in the configuration. The way the upgrade process was implemented, fresh installs would be forced to run an upgrade. This was fixed so that fresh installs do not run the upgrade. This was manually tested using the simulator with an iPhone XR with iOS 12.1. The configuration settings were manually modified to mimic an installation of version 1.5.2 (e.g. don't set the version but set the pinEnabled property the old way). This triggered the upgrade from version 1.5.2. A fresh install was tested to not perform any upgrades.
Fine grained control was added for remembering the database passwords. This allows some database passwords to be remembered without remembering all database passwords. The on/off switch for remembering passwords was changed to a choice of never, when configured, and always. A new remember password switch is added to the new database and password screens. This switch is only displayed when the "when configured" option for remembering the password is selected. All remembered database passwords are cleared if the never option is selected. A new option was added to the database swipe on the file screen for forgetting individual remembered database passwords. This was manually tested using the simulator with an iPhone XR with iOS 12.1.
In version 1.7.2 and earlier, the PIN could be bypassed by deleting the app and then reinstalling it. When the app is deleted, this deletes the files and configuration, but it does not necessarily delete the keychain data for the app. After reinstalling the app, it would trigger the upgrade path (since "[633] Fix a fresh install to not run an upgrade procedure" was not applied). The upgrade path would clear the PIN and then store the cleared PIN in the keychain. This would overwrite the previous keychain PIN. However any remembered passwords that were stored in the keychain would not be cleared. The same data base could be reloaded and the previously remembered PIN would be used. As a result, this allowed the PIN to be bypassed. This was unintentionally partly fixed by "[633] Fix a fresh install to not run an upgrade procedure" which caused the upgrade to only run if the version actually changed. But the app was still vulnerable if the app was maliciously downgraded to an unfixed version. In order to protect against maliciously downgrading the version, the app uses a different keychain service key for storing the database passwords and keyfiles. That way old versions are unable to read the remembered database passwords and keyfiles. Additionally version information has been added to the keychain. This is used to prevent against a downgrade which clears the PIN followed by an upgrade to a patched version. The new keychain service names for remembered database passwords and keyfiles are deleted when performing the upgrade and also if a version rollback is detected. In order for this fix to work, the version must be incremented on release to a value greater than 1.7.2. Otherwise this will not protect against a downgrade to version 1.7.2. This was manually tested using the simulator with an iPhone XR with iOS 12.1. The app version was set to 1.7.3 for the manual testing. It was tested against rolling back the version to the released version 1.7.2 and ensuring the remembered password was not usable. It was then upgraded to the patched version which cleared the remembered password because a downgrade was detected. The app was also deleted, reinstalled, and reloaded with the same database that had a remembered password. The remembered database password was checked to make sure it was forgotten. Finally the version was updated to 1.7.4 and then it was downgraded to 1.7.3 to make sure the keychain version actually clears the keychain if there are downgrades in the future.
Previously, there was a vulnerability that allowed a malicious user to enable touch ID and and use that to bypass the PIN. This vulnerability required editing a file stored on the device which contained the property for whether or not touch ID was enabled to disabled. The touch ID property has been moved to the keychain pin service which protects against a malicious user editing the setting. This was manually tested using the simulator with an iPhone 6s with iOS 12.1. When changing the touch ID setting, the property file was verified to have the same contents.
Face ID support has been added and the app will prompt with a description the first time face ID is used to authenticate. The app detects if touch ID or face ID are present. The appropriate text is displayed on the settings screen. If no biometrics ID is present, then it a generic biometric ID switch is displayed. The face ID switch has been moved into the section with PIN protection. The face ID text was also fixed to be properly greyed out when the PIN is disabled. Support for iOS prior to 11.0 cannot determine the difference between the touch ID not being present and there not being any enrolled fingerprints, so the generic biometric ID switch is displayed in these cases. This was manually tested using the simulator with an iPhone 5 with iOS 10.0 to validate the generic biometric ID switch is displayed on old devices that do not have any biometric ID. It was tested on the simulator with an iPhone 6s with iOS 10.0 and iOS 12.1 to make sure the touch ID switch is displayed. It was also tested on the simulator with an iPhone XR with iOS 12.1 to make sure the face ID switch is displayed.
The erase data on failure label was renamed and it was fixed to be properly greyed out when the PIN is disabled. The attempts before erasing data was renamed and it was fixed to be properly disabled when the PIN is disabled or when the erase data on failure is disabled. Both settings were moved into the PIN protection block. This was manually tested using the simulator with an iPhone XR with iOS 12.1.
The database names are cleared when erasing the data after entering the wrong PIN too many times. This was manually tested using the simulator with an iPhone XR with iOS 12.1.
The setting title was added to the setting selection picker screen. This was manually tested using the simulator with an iPhone XR with iOS 12.1.
The alphanumeric prompt for setting a PIN was fixed to be "Set PIN" rather than "Enter your PIN to unlock". This was manually tested using the simulator with an iPhone XR with iOS 12.1.
The integrated web browser was fixed so that it properly closes any open pages if the wrong PIN is entered too many times and all data is erased. Additionally cookies and other cached files are erased. Previously the integrated web browser was neither closed nor were the cookies or temporary files erased. This was manually tested using the simulator with an iPhone XR with iOS 12.1.
The attempts remaining count on the PIN screen was fixed so that it correctly reports the number of remaining attempts. Previously, once an incorrect PIN was entered that attempt count would be reported even after the correct PIN was entered. Also previously, when the app was fully closed and reopened it would always appear that the PIN had never been entered incorrectly even if it had been entered incorrectly. Both of these issues were display issues, they did not effect the actual counts used to erase the data. This was manually tested using the simulator with an iPhone XR with iOS 12.1.
A back button has been added to the top of the set PIN screen. This prevents an infinite loop of set PIN screens until the user properly enters the same PIN twice. It allows a user to cancel the action without actually setting a PIN followed by removing it. This was manually tested using the simulator with an iPhone XR with iOS 12.1.
Before deleting a database, the action is confirmed. This confirmation is not used for cases which result in the data being deleted due to a pin failure. This improves the behavior of MiniKeePass#357 "Databases can be deleted without password" which states "It's too easy to delete a database". Although, this change does not prompt for a password like the request asks because there are other ways that the database can be removed without a password (e.g. delete the app, enter the wrong PIN too many times, etc.). This was tested using the simulator with an iPhone XR with iOS 12.1.
The atomic flag was added when saving KeePass v2 database files. This will use a temporary file and atomically replace the database file. The KeePass v1 database save operation was updated to use the DataOutputStream, which is the same as the v2 save implementation. Temporary files are no longer created. Additionally the database file is no longer written out followed by a separate operation which writes the hash in the database file. Finally, the database file is created with the data protection encryption rather than turning data protection encryption on after the file was originally created without it. The FileOutputStream was removed because the KeePass v1 database save operation now uses the DataOutputStream. This was tested manually using the simulator with an iPhone XR with iOS 12.1. Additionally, KeePass v1 and v2 databases that were created with MiniKeePass and they were exported and validated using KeePassX.
The ability to change the database password has been added. A "more" option was added to the database file selection screen for a swipe left action. The "more" option now has the rename, forget password if applicable, and change password. The change password screen requires the user to enter the current password, the remembered password cannot be used when changing the database password. This was tested manually using the simulator with an iPhone XR with iOS 12.1.
The value for custom in memory protected fields are now hidden. The fields now acts the same way as the password field. On the view screen, there is a eye image that displays the value. On the edit screen, there is a wrench image that generates passwords. This was tested manually using the simulator with an iPhone XR with iOS 12.1.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The value for custom in memory protected fields are now hidden. The
fields now acts the same way as the password field. On the view
screen, there is a eye image that displays the value. On the edit
screen, there is a wrench image that generates passwords.
This was tested manually using the simulator with an iPhone XR with
iOS 12.1.
#329