IDEA-200735 Password Safe: IDE freezes on opening Settings dialog on Ubuntu

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

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)

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
   }
 

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 {

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)
   }
 

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
   }

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)
   }