aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-11-02 18:27:09 +0100
committerDanilo M. <danix@danix.xyz>2026-11-02 18:27:09 +0100
commit931eb40e8e4deb41edcb1c725e74586ff9f918b2 (patch)
tree3cb0348a771e8bf5a96be5b7c87d860ea5c8e61b
parentf5ca80dfd68d915ee100bd79f4e9e2cc34362b1b (diff)
downloadHardenedPass-931eb40e8e4deb41edcb1c725e74586ff9f918b2.tar.gz
HardenedPass-931eb40e8e4deb41edcb1c725e74586ff9f918b2.zip
fix(crypto): refuse to encrypt when a .gpg-id recipient key is missing
CryptoRepository.encrypt looked up every .gpg-id recipient and dropped the ones it could not find, so an entry saved on a device that lacked one recipient's key was encrypted to the others only. With a store shared between a phone key and a smartcard key, entries created on the phone could no longer be decrypted with the smartcard alone. Fail with the lookup error instead (KeyNotFoundException names the key), so the user imports the missing public key and saves again. PasswordCreationActivity also ignored the Result of encrypt and wrote the output buffer regardless: on any encryption failure it wrote an empty .gpg file, overwriting the entry when editing, and committed it. Show the error and leave the file untouched. CryptoRepositoryTest covers both the missing-recipient failure and the all-present success path, with keys generated by PGPainless (added as an app test dependency). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
-rw-r--r--app/build.gradle.kts1
-rw-r--r--app/src/main/java/app/passwordstore/data/crypto/CryptoRepository.kt11
-rw-r--r--app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt19
-rw-r--r--app/src/main/res/values/strings.xml5
-rw-r--r--app/src/test/java/app/passwordstore/data/crypto/CryptoRepositoryTest.kt84
5 files changed, 112 insertions, 8 deletions
diff --git a/app/build.gradle.kts b/app/build.gradle.kts
index 872f79dd..cd79be78 100644
--- a/app/build.gradle.kts
+++ b/app/build.gradle.kts
@@ -101,5 +101,6 @@ dependencies {
testImplementation(libs.testing.robolectric)
testImplementation(libs.testing.sharedPrefsMock)
+ testImplementation(libs.thirdparty.pgpainless)
testImplementation(libs.bundles.testDependencies)
}
diff --git a/app/src/main/java/app/passwordstore/data/crypto/CryptoRepository.kt b/app/src/main/java/app/passwordstore/data/crypto/CryptoRepository.kt
index eb858006..61b63ea7 100644
--- a/app/src/main/java/app/passwordstore/data/crypto/CryptoRepository.kt
+++ b/app/src/main/java/app/passwordstore/data/crypto/CryptoRepository.kt
@@ -14,6 +14,8 @@ import app.passwordstore.crypto.PGPainlessCryptoHandler
import app.passwordstore.injection.prefs.SettingsPreferences
import app.passwordstore.util.coroutines.DispatcherProvider
import app.passwordstore.util.settings.PreferenceKeys
+import com.github.michaelbull.result.andThen
+import com.github.michaelbull.result.combine
import com.github.michaelbull.result.filterValues
import com.github.michaelbull.result.map
import com.github.michaelbull.result.mapBoth
@@ -64,7 +66,12 @@ constructor(
PGPEncryptOptions.Builder()
.withAsciiArmor(settings.getBoolean(PreferenceKeys.ASCII_ARMOR, false))
.build()
- val keys = identities.map { id -> pgpKeyManager.getKeyById(id) }.filterValues()
- pgpCryptoHandler.encrypt(keys, content, out, encryptionOptions).map { out }
+ // Every recipient in .gpg-id must be present: silently dropping one would write an entry
+ // that the missing key's owner can no longer decrypt.
+ identities
+ .map { id -> pgpKeyManager.getKeyById(id) }
+ .combine()
+ .andThen { keys -> pgpCryptoHandler.encrypt(keys, content, out, encryptionOptions) }
+ .map { out }
}
}
diff --git a/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt b/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt
index b4fd21e5..6971c2e3 100644
--- a/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt
+++ b/app/src/main/java/app/passwordstore/ui/crypto/PasswordCreationActivity.kt
@@ -40,6 +40,7 @@ import app.passwordstore.util.extensions.unsafeLazy
import app.passwordstore.util.extensions.viewBinding
import app.passwordstore.util.settings.DirectoryStructure
import app.passwordstore.util.settings.PreferenceKeys
+import com.github.michaelbull.result.getOrElse
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.onSuccess
import com.github.michaelbull.result.runCatching
@@ -378,10 +379,20 @@ class PasswordCreationActivity : BasePGPActivity() {
runCatching {
val result =
withContext(dispatcherProvider.io()) {
- val outputStream = ByteArrayOutputStream()
- repository.encrypt(gpgIdentifiers, content.byteInputStream(), outputStream)
- outputStream
- }
+ val outputStream = ByteArrayOutputStream()
+ repository.encrypt(gpgIdentifiers, content.byteInputStream(), outputStream)
+ }
+ .getOrElse { e ->
+ logcat(ERROR) { e.asLog("Failed to encrypt password file") }
+ MaterialAlertDialogBuilder(this@PasswordCreationActivity)
+ .setTitle(R.string.password_creation_file_fail_title)
+ .setMessage(
+ getString(R.string.password_creation_encrypt_fail_message, e.message)
+ )
+ .setPositiveButton(android.R.string.ok, null)
+ .show()
+ return@runCatching
+ }
val passwordFile = Paths.get(path)
// If we're not editing, this file should not already exist!
// Additionally, if we were editing and the incoming and outgoing
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index 0584a1f2..61596e58 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -38,8 +38,8 @@
<string name="password_move_error_message">Failed to move %1$s to %2$s</string>
<!-- git commits -->
- <string name="git_commit_add_text">Add generated password for %1$s using Android Password Store.</string>
- <string name="git_commit_edit_text">Edit password for %1$s using Android Password Store.</string>
+ <string name="git_commit_add_text">Add generated password for %1$s using HardenedPass.</string>
+ <string name="git_commit_edit_text">Edit password for %1$s using HardenedPass.</string>
<string name="git_commit_remove_text">Remove %1$s from store.</string>
<string name="git_commit_move_text">Rename %1$s to %2$s.</string>
<string name="git_commit_move_multiple_text">Move multiple passwords to %1$s.</string>
@@ -304,6 +304,7 @@
<!-- Password creation failure -->
<string name="password_creation_file_fail_title">Error</string>
+ <string name="password_creation_encrypt_fail_message">Could not encrypt the password, nothing was saved: %1$s</string>
<string name="password_creation_file_write_fail_message">Failed to write password file to the store, please try again.</string>
<string name="password_creation_file_delete_fail_message">Failed to delete password file %1$s from the store, please delete it manually.</string>
<string name="password_creation_duplicate_error">File already exists, please use a different name</string>
diff --git a/app/src/test/java/app/passwordstore/data/crypto/CryptoRepositoryTest.kt b/app/src/test/java/app/passwordstore/data/crypto/CryptoRepositoryTest.kt
new file mode 100644
index 00000000..73349fd1
--- /dev/null
+++ b/app/src/test/java/app/passwordstore/data/crypto/CryptoRepositoryTest.kt
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ * SPDX-License-Identifier: GPL-3.0-only
+ */
+
+package app.passwordstore.data.crypto
+
+import app.passwordstore.crypto.PGPIdentifier
+import app.passwordstore.crypto.PGPKey
+import app.passwordstore.crypto.PGPKeyManager
+import app.passwordstore.crypto.PGPainlessCryptoHandler
+import app.passwordstore.crypto.errors.KeyNotFoundException
+import app.passwordstore.util.coroutines.DispatcherProvider
+import com.github.ivanshafran.sharedpreferencesmock.SPMockBuilder
+import com.github.michaelbull.result.getError
+import com.github.michaelbull.result.unwrap
+import java.io.ByteArrayOutputStream
+import kotlin.test.Test
+import kotlin.test.assertIs
+import kotlin.test.assertTrue
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.runBlocking
+import org.junit.Rule
+import org.junit.rules.TemporaryFolder
+import org.pgpainless.PGPainless
+
+class CryptoRepositoryTest {
+
+ @get:Rule val temporaryFolder = TemporaryFolder()
+
+ private val dispatchers =
+ object : DispatcherProvider {
+ override fun io() = Dispatchers.Unconfined
+ }
+
+ private fun newKey(uid: String) =
+ PGPKey(PGPainless.asciiArmor(PGPainless.generateKeyRing().modernKeyRing(uid)).toByteArray())
+
+ private suspend fun repository(keys: List<PGPKey>): CryptoRepository {
+ val keyManager = PGPKeyManager(temporaryFolder.root.absolutePath, Dispatchers.Unconfined)
+ keys.forEach { keyManager.addKey(it).unwrap() }
+ return CryptoRepository(
+ keyManager,
+ PGPainlessCryptoHandler(),
+ dispatchers,
+ SPMockBuilder().createSharedPreferences(),
+ )
+ }
+
+ @Test
+ fun encryptFailsWhenARecipientKeyIsMissing() = runBlocking {
+ val repository = repository(listOf(newKey("present <present@example.org>")))
+ val out = ByteArrayOutputStream()
+
+ val result =
+ repository.encrypt(
+ listOf(
+ PGPIdentifier.UserId("present@example.org"),
+ PGPIdentifier.UserId("missing@example.org"),
+ ),
+ "secret".byteInputStream(),
+ out,
+ )
+
+ assertIs<KeyNotFoundException>(result.getError())
+ assertTrue(out.size() == 0, "nothing may be written for a partial recipient set")
+ }
+
+ @Test
+ fun encryptSucceedsWhenAllRecipientKeysArePresent() = runBlocking {
+ val repository = repository(listOf(newKey("a <a@example.org>"), newKey("b <b@example.org>")))
+ val out = ByteArrayOutputStream()
+
+ repository
+ .encrypt(
+ listOf(PGPIdentifier.UserId("a@example.org"), PGPIdentifier.UserId("b@example.org")),
+ "secret".byteInputStream(),
+ out,
+ )
+ .unwrap()
+
+ assertTrue(out.size() > 0)
+ }
+}