aboutsummaryrefslogtreecommitdiffstats
path: root/crypto/pgpainless
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-11-03 14:44:58 +0100
committerDanilo M. <danix@danix.xyz>2026-11-03 14:44:58 +0100
commit2b73e524d9624897f675ccc14f614fc450a7565f (patch)
tree5f90dd4963e0aaeb2c699d86e1a2acc9dceaf366 /crypto/pgpainless
parent2911783653f699e41a07bb9536460d9d0a1c3b70 (diff)
downloadHardenedPass-compose.tar.gz
HardenedPass-compose.zip
fix(app): address the step 5 review and emulator findingscompose
- A store a cancelled attempt already made for the chosen key (its .gpg-id names exactly that key) no longer counts as occupied: choosing the key again commits and moves on instead of refusing. - SSH key import reads at most 100 kB whatever the provider reports, like the PGP import, instead of reading the stream whole. - PGPKeyManager replaces a key through a temporary file moved over it atomically, so a failed write keeps the old key; listings only read .key files, so a stray temporary file is never taken for a key. - "View generated public SSH key" goes through the settings result launcher, so the preference visibility refreshes afterwards. - A plain start from LaunchActivity replaces the task: an open MainActivity (e.g. one whose store vanished) is no longer left below the new one, and one back leaves the app. - The PGP keys screen's import FAB carries its content description itself; M3 cleared the icon's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Diffstat (limited to 'crypto/pgpainless')
-rw-r--r--crypto/pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt32
-rw-r--r--crypto/pgpainless/src/test/kotlin/app/passwordstore/crypto/PGPKeyManagerTest.kt5
2 files changed, 31 insertions, 6 deletions
diff --git a/crypto/pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt b/crypto/pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt
index ef58ad19..33d61343 100644
--- a/crypto/pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt
+++ b/crypto/pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt
@@ -22,6 +22,8 @@ import com.github.michaelbull.result.Result
import com.github.michaelbull.result.coroutines.runSuspendCatching
import com.github.michaelbull.result.unwrap
import java.io.File
+import java.nio.file.Files
+import java.nio.file.StandardCopyOption
import javax.inject.Inject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.withContext
@@ -51,13 +53,13 @@ constructor(filesDir: String, private val dispatcher: CoroutineDispatcher) :
tryParseKeyring(PGPKey(existingKeyBytes)) ?: throw InvalidKeyException
when {
existingKeyRing is PGPPublicKeyRing && incomingKeyRing is PGPSecretKeyRing -> {
- keyFile.writeBytes(key.contents)
+ writeAtomically(keyFile, key.contents)
return@runSuspendCatching key
}
existingKeyRing is PGPPublicKeyRing && incomingKeyRing is PGPPublicKeyRing -> {
val updatedPublicKey = PGPainless.mergeCertificate(existingKeyRing, incomingKeyRing)
val keyBytes = PGPainless.asciiArmor(updatedPublicKey).encodeToByteArray()
- keyFile.writeBytes(keyBytes)
+ writeAtomically(keyFile, keyBytes)
return@runSuspendCatching key
}
// Never with replace either: it would delete the only copy of the private key.
@@ -69,10 +71,9 @@ constructor(filesDir: String, private val dispatcher: CoroutineDispatcher) :
throw KeyAlreadyExistsException(
tryGetId(key)?.toString() ?: "Failed to retrieve key ID"
)
- if (!keyFile.delete()) throw KeyDeletionFailedException
}
- keyFile.writeBytes(key.contents)
+ writeAtomically(keyFile, key.contents)
key
}
@@ -96,7 +97,7 @@ constructor(filesDir: String, private val dispatcher: CoroutineDispatcher) :
withContext(dispatcher) {
runSuspendCatching {
if (!keyDirExists()) throw KeyDirectoryUnavailableException
- val keyFiles = keyDir.listFiles()
+ val keyFiles = keyDir.listFiles { file -> file.extension == KEY_EXTENSION }
if (keyFiles.isNullOrEmpty()) throw NoKeysAvailableException
val keys = keyFiles.map { file -> PGPKey(file.readBytes()) }
@@ -134,7 +135,7 @@ constructor(filesDir: String, private val dispatcher: CoroutineDispatcher) :
withContext(dispatcher) {
runSuspendCatching {
if (!keyDirExists()) throw KeyDirectoryUnavailableException
- val keyFiles = keyDir.listFiles()
+ val keyFiles = keyDir.listFiles { file -> file.extension == KEY_EXTENSION }
if (keyFiles.isNullOrEmpty()) return@runSuspendCatching emptyList()
keyFiles.map { keyFile -> PGPKey(keyFile.readBytes()) }.toList()
}
@@ -143,6 +144,25 @@ constructor(filesDir: String, private val dispatcher: CoroutineDispatcher) :
/** @see KeyManager.getKeyById */
override suspend fun getKeyId(key: PGPKey): PGPIdentifier? = tryGetId(key)
+ /**
+ * Writes [bytes] to a temporary file next to [file] and moves it over [file] in one step, so a
+ * failed write keeps the key that was there.
+ */
+ private fun writeAtomically(file: File, bytes: ByteArray) {
+ val temp = File(file.parentFile, ".${file.name}.tmp")
+ try {
+ temp.writeBytes(bytes)
+ Files.move(
+ temp.toPath(),
+ file.toPath(),
+ StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.ATOMIC_MOVE,
+ )
+ } finally {
+ temp.delete()
+ }
+ }
+
/** Checks if [keyDir] exists and attempts to create it if not. */
private fun keyDirExists(): Boolean {
return keyDir.exists() || keyDir.mkdirs()
diff --git a/crypto/pgpainless/src/test/kotlin/app/passwordstore/crypto/PGPKeyManagerTest.kt b/crypto/pgpainless/src/test/kotlin/app/passwordstore/crypto/PGPKeyManagerTest.kt
index 81a666f4..587f7239 100644
--- a/crypto/pgpainless/src/test/kotlin/app/passwordstore/crypto/PGPKeyManagerTest.kt
+++ b/crypto/pgpainless/src/test/kotlin/app/passwordstore/crypto/PGPKeyManagerTest.kt
@@ -72,6 +72,11 @@ class PGPKeyManagerTest {
val keyId = keyManager.getKeyId(keyManager.addKey(secretKey, true).unwrap())
assertEquals(KeyId(CryptoConstants.KEY_ID), keyId)
+ // Replaced through a temporary file that does not stay behind.
+ assertEquals(listOf("$keyId.${PGPKeyManager.KEY_EXTENSION}"), keysDir.list()?.toList())
+ // A stray temporary file left by a crash is not listed as a key.
+ File(keysDir, ".$keyId.${PGPKeyManager.KEY_EXTENSION}.tmp").writeText("partial")
+ assertEquals(1, keyManager.getAllKeys().unwrap().size)
}
@Test