From de5f2667a5725136934aa925bcf0b2e8442ca57a Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Thu, 18 Oct 2018 12:40:27 +0200 Subject: [PATCH] reuse SecureRandom on keepass store save --- .../src/kdbx/KeePassDatabase.kt | 3 +-- platform/credential-store/src/kdbx/kdbx.kt | 1 + .../src/keePass/KeePassCredentialStore.kt | 19 ++++++++++--------- .../src/keePass/KeePassFileManager.kt | 11 +++++------ .../test/keePass/KeePassFileManagerTest.kt | 6 ++++-- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/platform/credential-store/src/kdbx/KeePassDatabase.kt b/platform/credential-store/src/kdbx/KeePassDatabase.kt index 4c5a35ce6607..f1ce7a12957a 100644 --- a/platform/credential-store/src/kdbx/KeePassDatabase.kt +++ b/platform/credential-store/src/kdbx/KeePassDatabase.kt @@ -82,8 +82,7 @@ internal class KeePassDatabase(private val rootElement: Element = createEmptyDat internal fun protectValue(value: CharSequence) = StringProtectedByStreamCipher(value, secureStringCipher.value) @Synchronized - fun save(credentials: KeePassCredentials, outputStream: OutputStream) { - val secureRandom = createSecureRandom() + fun save(credentials: KeePassCredentials, outputStream: OutputStream, secureRandom: SecureRandom) { val kdbxHeader = KdbxHeader(secureRandom) kdbxHeader.writeKdbxHeader(outputStream) diff --git a/platform/credential-store/src/kdbx/kdbx.kt b/platform/credential-store/src/kdbx/kdbx.kt index 3ba88722a013..8bb7793d11a4 100644 --- a/platform/credential-store/src/kdbx/kdbx.kt +++ b/platform/credential-store/src/kdbx/kdbx.kt @@ -43,6 +43,7 @@ private fun readKeePassDatabase(credentials: KeePassCredentials, inputStream: In internal class KdbxPassword(password: ByteArray) : KeePassCredentials { companion object { + // KdbxPassword hashes value, so, it can be cleared before file write (to reduce time when master password exposed in memory) fun createAndClear(value: ByteArray): KeePassCredentials { val result = KdbxPassword(value) value.fill(0) diff --git a/platform/credential-store/src/keePass/KeePassCredentialStore.kt b/platform/credential-store/src/keePass/KeePassCredentialStore.kt index 5845ceede5ef..6de6158bfe8f 100644 --- a/platform/credential-store/src/keePass/KeePassCredentialStore.kt +++ b/platform/credential-store/src/keePass/KeePassCredentialStore.kt @@ -84,10 +84,11 @@ internal class KeePassCredentialStore constructor(internal val dbFile: Path, } try { + val secureRandom = createSecureRandom() val masterKey = masterKeyStorage.load() val kdbxPassword: KdbxPassword if (masterKey == null) { - val key = generateRandomMasterKey(masterKeyEncryptionSpec, createSecureRandom()) + val key = generateRandomMasterKey(masterKeyEncryptionSpec, secureRandom) kdbxPassword = KdbxPassword(key.value!!) masterKeyStorage.save(key) } @@ -96,7 +97,9 @@ internal class KeePassCredentialStore constructor(internal val dbFile: Path, masterKey.fill(0) } - dbFile.writeSafe { db.save(kdbxPassword, it) } + dbFile.writeSafe { + db.save(kdbxPassword, it, secureRandom) + } dbFile.setOwnerPermissions() } catch (e: Throwable) { @@ -124,12 +127,10 @@ internal class KeePassCredentialStore constructor(internal val dbFile: Path, isNeedToSave.set(db.isDirty) } - /** - * [MasterKey.value] will be cleared on set - */ - fun setMasterPassword(masterKey: MasterKey) { + @TestOnly + fun setMasterPassword(masterKey: MasterKey, secureRandom: SecureRandom) { // KdbxPassword hashes value, so, it can be cleared before file write (to reduce time when master password exposed in memory) - saveDatabase(dbFile, db, masterKey, masterKeyStorage) + saveDatabase(dbFile, db, masterKey, masterKeyStorage, secureRandom) } override fun markDirty() { @@ -149,10 +150,10 @@ internal fun generateRandomMasterKey(masterKeyEncryptionSpec: EncryptionSpec, se return MasterKey(Base64.getEncoder().withoutPadding().encode(bytes), isAutoGenerated = true, encryptionSpec = masterKeyEncryptionSpec) } -internal fun saveDatabase(dbFile: Path, db: KeePassDatabase, masterKey: MasterKey, masterKeyStorage: MasterKeyFileStorage) { +internal fun saveDatabase(dbFile: Path, db: KeePassDatabase, masterKey: MasterKey, masterKeyStorage: MasterKeyFileStorage, secureRandom: SecureRandom) { val kdbxPassword = KdbxPassword(masterKey.value!!) masterKeyStorage.save(masterKey) - dbFile.writeSafe { db.save(kdbxPassword, it) } + dbFile.writeSafe { db.save(kdbxPassword, it, secureRandom) } dbFile.setOwnerPermissions() } diff --git a/platform/credential-store/src/keePass/KeePassFileManager.kt b/platform/credential-store/src/keePass/KeePassFileManager.kt index 81adb437a387..4d39cf58fa84 100644 --- a/platform/credential-store/src/keePass/KeePassFileManager.kt +++ b/platform/credential-store/src/keePass/KeePassFileManager.kt @@ -86,7 +86,7 @@ internal open class KeePassFileManager(private val file: Path, } } else { - saveDatabase(file, KeePassDatabase(), generateRandomMasterKey(masterKeyEncryptionSpec, secureRandom.value), masterKeyFileStorage) + saveDatabase(file, KeePassDatabase(), generateRandomMasterKey(masterKeyEncryptionSpec, secureRandom.value), masterKeyFileStorage, secureRandom.value) } } @@ -130,9 +130,8 @@ internal open class KeePassFileManager(private val file: Path, val contextComponent = event?.getData(PlatformDataKeys.CONTEXT_COMPONENT) // to open old database, key can be required, so, to avoid showing 2 dialogs, check it before - val store = try { - val db = if (file.exists()) loadKdbx(file, KdbxPassword(this.masterKeyFileStorage.load() ?: throw IncorrectMasterPasswordException(isFileMissed = true))) else KeePassDatabase() - KeePassCredentialStore(file, this.masterKeyFileStorage, db) + val db = try { + if (file.exists()) loadKdbx(file, KdbxPassword(this.masterKeyFileStorage.load() ?: throw IncorrectMasterPasswordException(isFileMissed = true))) else KeePassDatabase() } catch (e: IncorrectMasterPasswordException) { // ok, old key is required @@ -140,7 +139,7 @@ internal open class KeePassFileManager(private val file: Path, } return requestMasterPassword("Set Master Password", contextComponent = contextComponent) { - store.setMasterPassword(createMasterKey(it)) + saveDatabase(file, db, createMasterKey(it), masterKeyFileStorage, secureRandom.value) null } } @@ -183,7 +182,7 @@ internal open class KeePassFileManager(private val file: Path, @Suppress("MemberVisibilityCanBePrivate") protected fun doSetNewMasterPassword(current: CharArray, new: CharArray): Boolean { val db = loadKdbx(file, createAndClear(current.toByteArrayAndClear())) - saveDatabase(file, db, createMasterKey(new), masterKeyFileStorage) + saveDatabase(file, db, createMasterKey(new), masterKeyFileStorage, secureRandom.value) return false } diff --git a/platform/credential-store/test/keePass/KeePassFileManagerTest.kt b/platform/credential-store/test/keePass/KeePassFileManagerTest.kt index a846c2e1a848..d0ba98c60258 100644 --- a/platform/credential-store/test/keePass/KeePassFileManagerTest.kt +++ b/platform/credential-store/test/keePass/KeePassFileManagerTest.kt @@ -245,7 +245,7 @@ internal class KeePassFileManagerTest { checkEntry(db) dbFile.outputStream().use { - db.save(kdbxPassword, it) + db.save(kdbxPassword, it, secureRandomHolder.secureRandom) } db = loadKdbx(dbFile, kdbxPassword) @@ -271,7 +271,9 @@ internal class KeePassFileManagerTest { private fun createStore() = createStore(fsRule.fs.getPath("/")) } -internal fun KeePassCredentialStore.setMasterKey(value: String) = setMasterPassword(MasterKey(value.toByteArray(), isAutoGenerated = false, encryptionSpec = defaultEncryptionSpec)) +internal fun KeePassCredentialStore.setMasterKey(value: String) { + setMasterPassword(MasterKey(value.toByteArray(), isAutoGenerated = false, encryptionSpec = defaultEncryptionSpec), KeePassFileManagerTest.secureRandomHolder.secureRandom) +} @Suppress("TestFunctionName") private class TestKeePassFileManager(