diff --git a/platform/credential-store/src/credentialStore.kt b/platform/credential-store/src/credentialStore.kt index 73c5af9736a8..9e512487832b 100644 --- a/platform/credential-store/src/credentialStore.kt +++ b/platform/credential-store/src/credentialStore.kt @@ -6,7 +6,6 @@ import com.intellij.openapi.util.text.StringUtil import com.intellij.util.io.toByteArray import java.nio.CharBuffer import java.security.MessageDigest -import java.security.NoSuchAlgorithmException import java.security.SecureRandom import java.util.* @@ -88,12 +87,10 @@ fun Credentials.serialize(storePassword: Boolean = true) = joinData(userName, if internal val ACCESS_TO_KEY_CHAIN_DENIED = Credentials(null, null as OneTimeString?) fun createSecureRandom(): SecureRandom { - try { - return SecureRandom.getInstanceStrong() - } - catch (e: NoSuchAlgorithmException) { - return SecureRandom() - } + // do not use SecureRandom.getInstanceStrong() + // https://tersesystems.com/blog/2015/12/17/the-right-way-to-use-securerandom/ + // it leads to blocking without any advantages + return SecureRandom() } @Synchronized diff --git a/platform/credential-store/src/kdbx/KdbxHeader.kt b/platform/credential-store/src/kdbx/KdbxHeader.kt index 2c5839a94b6f..1711f389ce09 100644 --- a/platform/credential-store/src/kdbx/KdbxHeader.kt +++ b/platform/credential-store/src/kdbx/KdbxHeader.kt @@ -18,6 +18,7 @@ package com.intellij.credentialStore.kdbx import com.google.common.io.LittleEndianDataInputStream import com.google.common.io.LittleEndianDataOutputStream import com.intellij.credentialStore.generateBytes +import com.intellij.util.ArrayUtilRt import org.bouncycastle.crypto.engines.AESEngine import org.bouncycastle.crypto.io.CipherInputStream import org.bouncycastle.crypto.io.CipherOutputStream @@ -76,7 +77,17 @@ private fun verifyFileVersion(input: LittleEndianDataInputStream): Boolean { return input.readInt() and FILE_VERSION_CRITICAL_MASK <= FILE_VERSION_32 and FILE_VERSION_CRITICAL_MASK } -internal class KdbxHeader(random: SecureRandom) { +internal class KdbxHeader() { + constructor(inputStream: InputStream) : this() { + readKdbxHeader(inputStream) + } + + constructor(random: SecureRandom) : this() { + masterSeed = random.generateBytes(32) + transformSeed = random.generateBytes(32) + encryptionIv = random.generateBytes(16) + protectedStreamKey = createProtectedStreamKey(random) + } /** * The ordinal 0 represents uncompressed and 1 GZip compressed */ @@ -98,11 +109,12 @@ internal class KdbxHeader(random: SecureRandom) { var compressionFlags = CompressionFlags.GZIP private set - private var masterSeed: ByteArray - private var transformSeed: ByteArray + private var masterSeed: ByteArray = ArrayUtilRt.EMPTY_BYTE_ARRAY + private var transformSeed: ByteArray = ArrayUtilRt.EMPTY_BYTE_ARRAY private var transformRounds: Long = 6000 - private var encryptionIv: ByteArray - var protectedStreamKey: ByteArray + private var encryptionIv: ByteArray = ArrayUtilRt.EMPTY_BYTE_ARRAY + var protectedStreamKey: ByteArray = ArrayUtilRt.EMPTY_BYTE_ARRAY + private set private var protectedStreamAlgorithm = ProtectedStreamAlgorithm.SALSA_20 /* these bytes appear in cipher text immediately following the header */ @@ -112,13 +124,6 @@ internal class KdbxHeader(random: SecureRandom) { * on transmission or receipt */ var headerHash: ByteArray? = null - init { - masterSeed = random.generateBytes(32) - transformSeed = random.generateBytes(32) - encryptionIv = random.generateBytes(16) - protectedStreamKey = createProtectedStreamKey(random) - } - /** * Create a decrypted input stream using supplied digest and this header * apply decryption to the passed encrypted input stream @@ -155,7 +160,7 @@ internal class KdbxHeader(random: SecureRandom) { /** * Populate a KdbxHeader from the input stream supplied */ - internal fun readKdbxHeader(inputStream: InputStream) { + private fun readKdbxHeader(inputStream: InputStream) { val digest = sha256MessageDigest() // we do not close this stream, otherwise we lose our place in the underlying stream val digestInputStream = DigestInputStream(inputStream, digest) diff --git a/platform/credential-store/src/kdbx/KeePassDatabase.kt b/platform/credential-store/src/kdbx/KeePassDatabase.kt index 3bfdb2173969..4c5a35ce6607 100644 --- a/platform/credential-store/src/kdbx/KeePassDatabase.kt +++ b/platform/credential-store/src/kdbx/KeePassDatabase.kt @@ -42,19 +42,22 @@ internal val EXPIRY_TIME_ELEMENT_NAME = arrayOf("Times", "ExpiryTime") internal var dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'") -private fun createRandomlyInitializedChaCha7539Engine(secureRandom: SecureRandom): ChaCha7539Engine { +private fun createRandomlyInitializedChaCha7539Engine(secureRandom: SecureRandom): SkippingStreamCipher { val engine = ChaCha7539Engine() + initCipherRandomly(secureRandom, engine) + return engine +} + +private fun initCipherRandomly(secureRandom: SecureRandom, engine: SkippingStreamCipher) { val keyParameter = KeyParameter(secureRandom.generateBytes(32)) engine.init(true, ParametersWithIV(keyParameter, secureRandom.generateBytes(12))) - return engine } // we should on each save change protectedStreamKey for security reasons (as KeeWeb also does) // so, this requirement (is it really required?) can force us to re-encrypt all passwords on save -internal class KeePassDatabase(private val rootElement: Element = createEmptyDatabase(), secureRandom: SecureRandom? = null /* reuse SecureRandom if possible because it is not cheap to create new one */) { - private val secureStringCipher: SkippingStreamCipher by when (secureRandom) { - null -> lazy { createRandomlyInitializedChaCha7539Engine(createSecureRandom()) } - else -> lazyOf(createRandomlyInitializedChaCha7539Engine(secureRandom)) +internal class KeePassDatabase(private val rootElement: Element = createEmptyDatabase()) { + private var secureStringCipher = lazy { + createRandomlyInitializedChaCha7539Engine(createSecureRandom()) } @Volatile @@ -76,12 +79,12 @@ internal class KeePassDatabase(private val rootElement: Element = createEmptyDat } } - internal fun protectValue(value: CharSequence) = StringProtectedByStreamCipher(value, secureStringCipher) + internal fun protectValue(value: CharSequence) = StringProtectedByStreamCipher(value, secureStringCipher.value) @Synchronized fun save(credentials: KeePassCredentials, outputStream: OutputStream) { - val random = createSecureRandom() - val kdbxHeader = KdbxHeader(random) + val secureRandom = createSecureRandom() + val kdbxHeader = KdbxHeader(secureRandom) kdbxHeader.writeKdbxHeader(outputStream) val metaElement = rootElement.getOrCreate("Meta") @@ -92,6 +95,17 @@ internal class KeePassDatabase(private val rootElement: Element = createEmptyDat ProtectedXmlWriter(createSalsa20StreamCipher(kdbxHeader.protectedStreamKey)).printElement(it, rootElement, 0) } + // should we init secureStringCipher if now we have secureRandom? + // on first glance yes, because creating SkippingStreamCipher is very fast and not memory hungry, and creating SecureRandom is a cost operation, + // but no - no need to init because if save called, it means that database is dirty for some reasons already... + // but yes - because maybe database is dirty due to change some unprotected value (url, user name). + if (secureStringCipher.isInitialized()) { + initCipherRandomly(secureRandom, secureStringCipher.value) + } + else { + secureStringCipher = lazyOf(createRandomlyInitializedChaCha7539Engine(secureRandom)) + } + isDirty = false } diff --git a/platform/credential-store/src/kdbx/kdbx.kt b/platform/credential-store/src/kdbx/kdbx.kt index c2d9b27fed5f..3ba88722a013 100644 --- a/platform/credential-store/src/kdbx/kdbx.kt +++ b/platform/credential-store/src/kdbx/kdbx.kt @@ -11,20 +11,18 @@ import org.bouncycastle.crypto.params.ParametersWithIV import java.io.InputStream import java.nio.file.Path import java.security.MessageDigest -import java.security.SecureRandom import java.util.* import java.util.zip.GZIPInputStream // https://gist.github.com/lgg/e6ccc6e212d18dd2ecd8a8c116fb1e45 @Throws(IncorrectMasterPasswordException::class) -internal fun loadKdbx(file: Path, credentials: KeePassCredentials, random: SecureRandom): KeePassDatabase { - return file.inputStream().buffered().use { readKeePassDatabase(credentials, it, random) } +internal fun loadKdbx(file: Path, credentials: KeePassCredentials): KeePassDatabase { + return file.inputStream().buffered().use { readKeePassDatabase(credentials, it) } } -private fun readKeePassDatabase(credentials: KeePassCredentials, inputStream: InputStream, random: SecureRandom): KeePassDatabase { - val kdbxHeader = KdbxHeader(random) - kdbxHeader.readKdbxHeader(inputStream) +private fun readKeePassDatabase(credentials: KeePassCredentials, inputStream: InputStream): KeePassDatabase { + val kdbxHeader = KdbxHeader(inputStream) val decryptedInputStream = kdbxHeader.createDecryptedStream(credentials.key, inputStream) val startBytes = FileUtilRt.loadBytes(decryptedInputStream, 32) @@ -40,7 +38,7 @@ private fun readKeePassDatabase(credentials: KeePassCredentials, inputStream: In element.getChild(KdbxDbElementNames.root)?.let { rootElement -> XmlProtectedValueTransformer(createSalsa20StreamCipher(kdbxHeader.protectedStreamKey)).processEntries(rootElement) } - return KeePassDatabase(element, random) + return KeePassDatabase(element) } internal class KdbxPassword(password: ByteArray) : KeePassCredentials { diff --git a/platform/credential-store/src/keePass/KeePassCredentialStore.kt b/platform/credential-store/src/keePass/KeePassCredentialStore.kt index 848c35b49a23..8ea239a77e81 100644 --- a/platform/credential-store/src/keePass/KeePassCredentialStore.kt +++ b/platform/credential-store/src/keePass/KeePassCredentialStore.kt @@ -81,7 +81,7 @@ internal class KeePassCredentialStore constructor(internal val dbFile: Path, db = when { !isMemoryOnly && dbFile.exists() -> { val masterPassword = masterKeyStorage.load() ?: throw IncorrectMasterPasswordException(isFileMissed = true) - loadKdbx(dbFile, KdbxPassword.createAndClear(masterPassword), createSecureRandom()) + loadKdbx(dbFile, KdbxPassword.createAndClear(masterPassword)) } else -> KeePassDatabase() } @@ -94,13 +94,13 @@ internal class KeePassCredentialStore constructor(internal val dbFile: Path, @Synchronized @TestOnly - fun reload(secureRandom: SecureRandom) { + fun reload() { LOG.assertTrue(!isMemoryOnly) val key = masterKeyStorage.load()!! val kdbxPassword = KdbxPassword(key) key.fill(0) - db = loadKdbx(dbFile, kdbxPassword, secureRandom) + db = loadKdbx(dbFile, kdbxPassword) isNeedToSave.set(false) } diff --git a/platform/credential-store/src/keePass/KeePassFileManager.kt b/platform/credential-store/src/keePass/KeePassFileManager.kt index fbad1ec53f23..81adb437a387 100644 --- a/platform/credential-store/src/keePass/KeePassFileManager.kt +++ b/platform/credential-store/src/keePass/KeePassFileManager.kt @@ -43,7 +43,7 @@ internal open class KeePassFileManager(private val file: Path, // but don't remove other groups val masterPassword = masterKeyFileStorage.load() if (masterPassword != null) { - val db = loadKdbx(file, KdbxPassword.createAndClear(masterPassword), secureRandom.value) + val db = loadKdbx(file, KdbxPassword.createAndClear(masterPassword)) val store = KeePassCredentialStore(file, masterKeyFileStorage, db) store.clear() store.save(masterKeyEncryptionSpec) @@ -98,7 +98,7 @@ internal open class KeePassFileManager(private val file: Path, var masterPassword = MasterKeyFileStorage(possibleMasterKeyFile).load() if (masterPassword != null) { try { - loadKdbx(file, KdbxPassword(masterPassword), secureRandom.value) + loadKdbx(file, KdbxPassword(masterPassword)) } catch (e: IncorrectMasterPasswordException) { LOG.warn("On import \"$file\" found existing master key file \"$possibleMasterKeyFile\" but key is not correct") @@ -108,7 +108,7 @@ internal open class KeePassFileManager(private val file: Path, if (masterPassword == null && !requestMasterPassword("Specify Master Password", contextComponent = contextComponent) { try { - loadKdbx(file, KdbxPassword(it), secureRandom.value) + loadKdbx(file, KdbxPassword(it)) masterPassword = it null } @@ -131,7 +131,7 @@ internal open class KeePassFileManager(private val file: Path, // 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)), secureRandom.value) else KeePassDatabase() + val db = if (file.exists()) loadKdbx(file, KdbxPassword(this.masterKeyFileStorage.load() ?: throw IncorrectMasterPasswordException(isFileMissed = true))) else KeePassDatabase() KeePassCredentialStore(file, this.masterKeyFileStorage, db) } catch (e: IncorrectMasterPasswordException) { @@ -182,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()), secureRandom.value) + val db = loadKdbx(file, createAndClear(current.toByteArrayAndClear())) saveDatabase(file, db, createMasterKey(new), masterKeyFileStorage) return false } diff --git a/platform/credential-store/test/keePass/KeePassFileManagerTest.kt b/platform/credential-store/test/keePass/KeePassFileManagerTest.kt index b2e59a418b4b..a846c2e1a848 100644 --- a/platform/credential-store/test/keePass/KeePassFileManagerTest.kt +++ b/platform/credential-store/test/keePass/KeePassFileManagerTest.kt @@ -164,7 +164,7 @@ internal class KeePassFileManagerTest { } private fun checkStoreAfterSuccessfulImport(store: KeePassCredentialStore) { - store.reload(secureRandomHolder.secureRandom) + store.reload() assertThat(store.dbFile).exists() assertThat(store.masterKeyFile).exists() @@ -187,7 +187,7 @@ internal class KeePassFileManagerTest { // assert that other store not corrupted fsRule.fs.getPath("/other/otherKey").move(fsRule.fs.getPath("/other/${MASTER_KEY_FILE_NAME}")) - otherStore.reload(secureRandomHolder.secureRandom) + otherStore.reload() assertThat(otherStore.get(testCredentialAttributes)!!.password!!.toString()).isEqualTo("p") } @@ -241,14 +241,14 @@ internal class KeePassFileManagerTest { } val kdbxPassword = KdbxPassword("foo".toByteArray()) - var db = loadKdbx(dbFile, kdbxPassword, secureRandomHolder.secureRandom) + var db = loadKdbx(dbFile, kdbxPassword) checkEntry(db) dbFile.outputStream().use { db.save(kdbxPassword, it) } - db = loadKdbx(dbFile, kdbxPassword, secureRandomHolder.secureRandom) + db = loadKdbx(dbFile, kdbxPassword) checkEntry(db) }