From 597bbcbbee4f25b1e3a38a4e42cac4e23e0f9a60 Mon Sep 17 00:00:00 2001 From: jaonoctus Date: Wed, 9 Sep 2026 14:40:35 -0300 Subject: [PATCH 1/3] fix: encrypt to a flagged encryption subkey, one spinner entry per ring Generated with Qwen3.8 27B (on local gx10) --- .../java/com/ngoline/easygpg/data/KeyItem.kt | 69 +++++++++++++++++- .../easygpg/ui/encrypt/EncryptFragment.kt | 30 +++----- .../easygpg/PGPKeyManagerEncryptionTest.kt | 24 ++++++ .../com/ngoline/easygpg/data/KeyItemTest.kt | 52 ++++++++++++- .../java/com/ngoline/easygpg/TestKeyRings.kt | 73 ++++++++++++++++++- 5 files changed, 223 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/com/ngoline/easygpg/data/KeyItem.kt b/app/src/main/java/com/ngoline/easygpg/data/KeyItem.kt index 68d9aac..ec2213e 100644 --- a/app/src/main/java/com/ngoline/easygpg/data/KeyItem.kt +++ b/app/src/main/java/com/ngoline/easygpg/data/KeyItem.kt @@ -1,5 +1,8 @@ package com.ngoline.easygpg.data +import java.util.Date +import org.bouncycastle.bcpg.SignatureSubpacketTags +import org.bouncycastle.bcpg.sig.KeyFlags import org.bouncycastle.openpgp.PGPPublicKey import org.bouncycastle.openpgp.PGPPublicKeyRing @@ -10,4 +13,68 @@ data class KeyItem( val publicKeyRing: PGPPublicKeyRing ) -fun shortFingerprint(fingerprint: String): String = fingerprint.takeLast(8).uppercase() \ No newline at end of file +fun shortFingerprint(fingerprint: String): String = fingerprint.takeLast(8).uppercase() + +/** How a key ring is named in a one-line list: its alias and its primary key's fingerprint. */ +val KeyItem.label: String + get() = "$alias (${shortFingerprint(fingerprint)})" + +private const val ENCRYPTION_FLAGS = KeyFlags.ENCRYPT_COMMS or KeyFlags.ENCRYPT_STORAGE + +/** + * The key flags this key's most recent self-signature declares, or null if no signature on it + * carries a key-flags subpacket. Rings this app generates itself set no flags, so "no flags at all" + * has to stay distinguishable from "flagged for something other than encryption". + */ +private val PGPPublicKey.declaredKeyFlags: Int? + get() { + var flags: Int? = null + var newest = Long.MIN_VALUE + val signatures = signatures + while (signatures.hasNext()) { + val signature = signatures.next() + val hashed = signature.hashedSubPackets ?: continue + if (!hashed.hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) continue + val created = signature.creationTime.time + if (created >= newest) { + newest = created + flags = hashed.keyFlags + } + } + return flags + } + +private fun PGPPublicKey.isExpired(now: Date): Boolean { + val seconds = validSeconds + return seconds > 0L && creationTime.time + seconds * 1000L < now.time +} + +/** + * The key a message to this ring is encrypted to, or null if the ring has none and so cannot be + * encrypted to at all. Which subkey it is is not offered to the user as a choice. + * + * `isEncryptionKey` alone is not enough to choose with: it reports what the *algorithm* can do, so + * an RSA primary answers true even when its key flags say certify and sign only. Picking that key + * produces a message the recipient cannot open, because the encryption subkey is the one they hold + * (and, on the usual offline-primary setup, the only secret half they have on the device at all). + * So prefer keys actually flagged for encryption, skip revoked and expired ones, and prefer a + * subkey over the primary, which is the convention every other OpenPGP implementation follows. + */ +val KeyItem.encryptionKey: PGPPublicKey? + get() { + val now = Date() + val usable = publicKeyRing.filter { + it.isEncryptionKey && !it.hasRevocation() && !it.isExpired(now) + } + val flaggedForEncryption = usable.filter { key -> + key.declaredKeyFlags?.and(ENCRYPTION_FLAGS)?.takeIf { it != 0 } != null + } + // A ring whose keys carry no flags at all predates the convention, or this app generated + // it; there the algorithm is all there is to go on. + val candidates = flaggedForEncryption.ifEmpty { + usable.filter { it.declaredKeyFlags == null } + } + return candidates.minWithOrNull( + compareBy { it.isMasterKey }.thenByDescending { it.creationTime } + ) + } diff --git a/app/src/main/java/com/ngoline/easygpg/ui/encrypt/EncryptFragment.kt b/app/src/main/java/com/ngoline/easygpg/ui/encrypt/EncryptFragment.kt index c3b595e..7f26bc1 100644 --- a/app/src/main/java/com/ngoline/easygpg/ui/encrypt/EncryptFragment.kt +++ b/app/src/main/java/com/ngoline/easygpg/ui/encrypt/EncryptFragment.kt @@ -15,8 +15,8 @@ import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.lifecycleScope import androidx.preference.PreferenceManager -import com.ngoline.easygpg.data.KeyItem -import com.ngoline.easygpg.data.shortFingerprint +import com.ngoline.easygpg.data.encryptionKey +import com.ngoline.easygpg.data.label import com.ngoline.easygpg.PGPKeyManager import com.ngoline.easygpg.R import com.ngoline.easygpg.copyToCharArray @@ -33,7 +33,6 @@ import org.bouncycastle.openpgp.PGPLiteralDataGenerator import org.bouncycastle.openpgp.PGPPublicKey import org.bouncycastle.openpgp.operator.jcajce.JcePGPDataEncryptorBuilder import org.bouncycastle.openpgp.operator.jcajce.JcePublicKeyKeyEncryptionMethodGenerator -import org.bouncycastle.util.encoders.Hex import java.io.ByteArrayOutputStream import java.security.SecureRandom import java.util.Date @@ -107,25 +106,16 @@ class EncryptFragment : Fragment() { } private fun loadPublicKeys() { - val keyItems: MutableList = keyManager.getAllPublicKeys() - - // For each keyring, find all encryption-capable keys - val aliasesAndKeys: List> = keyItems.flatMap { keyItem -> - val encryptionKeys = keyItem.publicKeyRing.publicKeys?.asSequence() - ?.filter { it.isEncryptionKey } - ?.toList() - ?: listOf(keyItem.publicKey).filter { it.isEncryptionKey } - - encryptionKeys.map { pubKey -> - val fingerprint = Hex.toHexString(pubKey.fingerprint) - "${keyItem.alias} (${shortFingerprint(fingerprint)})" to pubKey - } - } + // One entry per key ring, named after its primary key. A ring with no encryption-capable + // key cannot be encrypted to at all, so it is left out. + val (labels, keys) = keyManager.getAllPublicKeys() + .mapNotNull { keyItem -> keyItem.encryptionKey?.let { keyItem.label to it } } + .unzip() - publicKeyList = aliasesAndKeys.map { it.second } + publicKeyList = keys - // Create an ArrayAdapter for the spinner. Specify the type explicitly for ArrayAdapter. - val adapter = ArrayAdapter(requireContext(), android.R.layout.simple_spinner_dropdown_item, aliasesAndKeys.map { it.first }) + val adapter = + ArrayAdapter(requireContext(), android.R.layout.simple_spinner_dropdown_item, labels) spinnerPublicKeys.adapter = adapter } diff --git a/app/src/test/java/com/ngoline/easygpg/PGPKeyManagerEncryptionTest.kt b/app/src/test/java/com/ngoline/easygpg/PGPKeyManagerEncryptionTest.kt index 37ca8c1..2b2fd16 100644 --- a/app/src/test/java/com/ngoline/easygpg/PGPKeyManagerEncryptionTest.kt +++ b/app/src/test/java/com/ngoline/easygpg/PGPKeyManagerEncryptionTest.kt @@ -3,6 +3,10 @@ package com.ngoline.easygpg import android.content.Context import androidx.preference.PreferenceManager import androidx.test.core.app.ApplicationProvider +import com.ngoline.easygpg.data.KeyItem +import com.ngoline.easygpg.data.encryptionKey +import org.bouncycastle.openpgp.PGPSecretKeyRing +import org.bouncycastle.util.encoders.Hex import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -44,6 +48,26 @@ class PGPKeyManagerEncryptionTest { .commit() } + @Test + fun `a message to an RSA ring opens with the encryption subkey alone`() { + // The shape that broke: both keys are RSA, so isEncryptionKey answers true for the sign-only + // primary too. Encrypting to that primary yields a message the recipient cannot open, since + // on the usual offline-primary setup the subkey is the only secret half on the device. + val (rsaSecret, rsaPublic) = TestKeyRings.generateRsa(passphrase.copyOf()) + val keyItem = KeyItem( + "Alice", Hex.toHexString(rsaPublic.publicKeys.next().fingerprint), + rsaPublic.publicKeys.next(), rsaPublic + ) + + val armored = manager.encryptMessage("attack at dawn".toCharArray(), keyItem.encryptionKey!!) + + val subkeyOnly = PGPSecretKeyRing.removeSecretKey(rsaSecret, rsaSecret.secretKey) + assertEquals( + "attack at dawn", + TestKeyRings.decrypt(armored, subkeyOnly, passphrase.copyOf()), + ) + } + @Test fun `an encrypted message decrypts back to the original`() { val armored = manager.encryptMessage( diff --git a/app/src/test/java/com/ngoline/easygpg/data/KeyItemTest.kt b/app/src/test/java/com/ngoline/easygpg/data/KeyItemTest.kt index ca87557..aa151e2 100644 --- a/app/src/test/java/com/ngoline/easygpg/data/KeyItemTest.kt +++ b/app/src/test/java/com/ngoline/easygpg/data/KeyItemTest.kt @@ -1,18 +1,66 @@ package com.ngoline.easygpg.data +import com.ngoline.easygpg.TestKeyRings +import java.util.Date import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Test class KeyItemTest { + private val publicRing by lazy { TestKeyRings.generate("secret".toCharArray()).second } + + /** The fingerprint is the stored one, so a literal keeps the label test off real key material. */ + private val keyItem by lazy { + KeyItem("Alice", "1a2b3c4d5e6f70819ab4cdef", TestKeyRings.signingKey(publicRing), publicRing) + } + + private fun keyItem(publicRing: org.bouncycastle.openpgp.PGPPublicKeyRing) = + KeyItem("Alice", "1a2b3c4d5e6f70819ab4cdef", publicRing.publicKeys.next(), publicRing) + @Test - fun `a short fingerprint is the last eight characters, upper case`() { + fun `a short fingerprint is the last eight characters, uppercased`() { assertEquals("9AB4CDEF", shortFingerprint("1a2b3c4d5e6f70819ab4cdef")) } @Test - fun `a fingerprint shorter than eight characters is kept whole`() { + fun `a short fingerprint of a fingerprint shorter than eight characters is the whole thing`() { assertEquals("ABCD", shortFingerprint("abcd")) assertEquals("", shortFingerprint("")) } + + @Test + fun `a key label names the primary key`() { + assertEquals("Alice (9AB4CDEF)", keyItem.label) + } + + @Test + fun `the encryption key of a ring is its subkey, not the signing primary`() { + // The label names the primary key, but the message still goes to the encryption subkey. + assertEquals(TestKeyRings.encryptionKey(publicRing).keyID, keyItem.encryptionKey?.keyID) + } + + @Test + fun `the encryption key of an RSA ring is the subkey, not the sign-only primary`() { + // Both keys are RSA, so isEncryptionKey answers true for the primary too. Encrypting to it + // produces a message the recipient cannot open: the encryption subkey is the half they hold. + val (_, rsaRing) = TestKeyRings.generateRsa("secret".toCharArray()) + val subkey = TestKeyRings.encryptionKey(rsaRing) + + val picked = keyItem(rsaRing).encryptionKey + + assertEquals(subkey.keyID, picked?.keyID) + assertEquals(false, picked?.isMasterKey) + } + + @Test + fun `an expired encryption subkey is not encrypted to`() { + val aYearAgo = Date(System.currentTimeMillis() - 365L * 24 * 60 * 60 * 1000) + val (_, rsaRing) = TestKeyRings.generateRsa( + "secret".toCharArray(), createdAt = aYearAgo, subkeyValidSeconds = 60 + ) + + // The sign-only primary must not be the fallback once the subkey is out of date. + assertNull(keyItem(rsaRing).encryptionKey) + } } diff --git a/app/src/testShared/java/com/ngoline/easygpg/TestKeyRings.kt b/app/src/testShared/java/com/ngoline/easygpg/TestKeyRings.kt index 74f1553..883e056 100644 --- a/app/src/testShared/java/com/ngoline/easygpg/TestKeyRings.kt +++ b/app/src/testShared/java/com/ngoline/easygpg/TestKeyRings.kt @@ -1,11 +1,15 @@ package com.ngoline.easygpg +import java.math.BigInteger import java.util.Date import org.bouncycastle.bcpg.HashAlgorithmTags import org.bouncycastle.bcpg.PublicKeyAlgorithmTags +import org.bouncycastle.bcpg.sig.KeyFlags import org.bouncycastle.crypto.generators.Ed25519KeyPairGenerator +import org.bouncycastle.crypto.generators.RSAKeyPairGenerator import org.bouncycastle.crypto.generators.X25519KeyPairGenerator import org.bouncycastle.crypto.params.Ed25519KeyGenerationParameters +import org.bouncycastle.crypto.params.RSAKeyGenerationParameters import org.bouncycastle.crypto.params.X25519KeyGenerationParameters import org.bouncycastle.openpgp.PGPEncryptedData import org.bouncycastle.openpgp.PGPEncryptedDataList @@ -17,6 +21,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing import org.bouncycastle.openpgp.PGPSecretKey import org.bouncycastle.openpgp.PGPSecretKeyRing import org.bouncycastle.openpgp.PGPSignature +import org.bouncycastle.openpgp.PGPSignatureSubpacketGenerator import org.bouncycastle.openpgp.PGPUtil import org.bouncycastle.openpgp.operator.bc.BcKeyFingerprintCalculator import org.bouncycastle.openpgp.operator.bc.BcPBESecretKeyDecryptorBuilder @@ -78,9 +83,73 @@ object TestKeyRings { PGPPublicKeyRing(listOf(primary.publicKey, subkey.publicKey)) } - /** The subkey the app encrypts to — `EncryptFragment` picks by `isEncryptionKey`. */ + /** + * A GnuPG-shaped RSA ring: a primary flagged certify-and-sign only plus an encryption subkey. + * Both keys are RSA, so `isEncryptionKey` answers true for *both* and only the key flags tell + * them apart. [subkeyValidSeconds] and [createdAt] exist so a test can age the subkey out. + */ + fun generateRsa( + passphrase: CharArray, + userId: String = "rsa@example.com", + createdAt: Date = Date(), + subkeyValidSeconds: Long = 0, + ): Pair { + val digestCalculator = BcPGPDigestCalculatorProvider().get(HashAlgorithmTags.SHA1) + val encryptorBuilder = BcPBESecretKeyEncryptorBuilder( + PGPEncryptedData.AES_256, + BcPGPDigestCalculatorProvider().get(HashAlgorithmTags.SHA1) + ).build(passphrase) + val signerBuilder = + BcPGPContentSignerBuilder(PublicKeyAlgorithmTags.RSA_GENERAL, HashAlgorithmTags.SHA256) + + fun rsaPair(): BcPGPKeyPair { + val generator = RSAKeyPairGenerator() + generator.init( + RSAKeyGenerationParameters( + BigInteger.valueOf(0x10001), java.security.SecureRandom(), 1024, 12 + ) + ) + return BcPGPKeyPair( + BcPGPVersion, PublicKeyAlgorithmTags.RSA_GENERAL, generator.generateKeyPair(), createdAt + ) + } + + val primaryPair = rsaPair() + val subkeyPair = rsaPair() + + val primaryFlags = PGPSignatureSubpacketGenerator().apply { + setKeyFlags(false, KeyFlags.CERTIFY_OTHER or KeyFlags.SIGN_DATA) + }.generate() + val subkeyFlags = PGPSignatureSubpacketGenerator().apply { + setKeyFlags(false, KeyFlags.ENCRYPT_COMMS or KeyFlags.ENCRYPT_STORAGE) + if (subkeyValidSeconds > 0) setKeyExpirationTime(false, subkeyValidSeconds) + }.generate() + + val primary = PGPSecretKey( + PGPSignature.DEFAULT_CERTIFICATION, + primaryPair, + userId, + digestCalculator, + primaryFlags, + null, + signerBuilder, + encryptorBuilder + ) + val subkey = PGPSecretKey( + primaryPair, subkeyPair, digestCalculator, subkeyFlags, null, signerBuilder, encryptorBuilder + ) + + return PGPSecretKeyRing(listOf(primary, subkey)) to + PGPPublicKeyRing(listOf(primary.publicKey, subkey.publicKey)) + } + + /** + * The encryption subkey, which is what the app encrypts to. Matched on `!isMasterKey` as well: + * `isEncryptionKey` reports what the algorithm can do, so on an RSA ring the primary answers + * true too and matching on it alone would hand back the wrong key. + */ fun encryptionKey(publicKeyRing: PGPPublicKeyRing): PGPPublicKey = - publicKeyRing.publicKeys.asSequence().first { it.isEncryptionKey } + publicKeyRing.publicKeys.asSequence().first { !it.isMasterKey && it.isEncryptionKey } /** The Ed25519 primary key, which cannot encrypt. */ fun signingKey(publicKeyRing: PGPPublicKeyRing): PGPPublicKey = From d1accb991df0681322aee22bf7dfeea87937b426 Mon Sep 17 00:00:00 2001 From: jaonoctus Date: Wed, 9 Sep 2026 14:40:40 -0300 Subject: [PATCH 2/3] chore: bump the gradle wrapper to 8.14.5 Generated with Qwen3.8 27B (on local gx10) --- gradle/wrapper/gradle-wrapper.properties | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index d9eedde..52aa533 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ -#Mon Apr 22 15:00:47 BRT 2024 +#Wed Sep 09 14:18:50 GMT-03:00 2026 distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.2-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.5-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists From caf0eaab644fe68c804e701edd58474da1034775 Mon Sep 17 00:00:00 2001 From: jaonoctus Date: Wed, 9 Sep 2026 14:40:46 -0300 Subject: [PATCH 3/3] docs: add AGENTS.md Generated with Qwen3.8 27B (on local gx10) --- AGENTS.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..9307288 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,40 @@ +# AGENTS.md + +Single-module Android app (`:app`), Kotlin. On-device PGP/OpenPGP via Bouncy Castle; hardware YubiKey via Yubikit. No monorepo: `settings.gradle.kts` includes only `:app`. + +Contributor policy (PR etiquette, security disclosure, licensing) lives in `CONTRIBUTING.md`. This file covers what an agent needs in order to build and change the code. + +## Naming +- The app is **Easy PGP**. It was renamed from Easy GPG in 0.6; the user-visible name is `app_name` in `app/src/main/res/values/strings.xml`. +- The old name survives where renaming would be churn or an outright break: the `com.ngoline.easygpg` namespace and `applicationId`, the source tree under `app/src/main/java/com/ngoline/easygpg/`, and `rootProject.name = "Easy GPG"`. Leave these alone; the `applicationId` is the identity Play installs against. + +## Toolchain +- JDK 21 required (AGP 8.13.2, Kotlin 2.2.0, Gradle 8.14.5 through the wrapper). +- `compileSdk` and `targetSdk` are 36, `minSdk` is 35. +- `local.properties` (sdk.dir) is not committed; Android Studio generates it. + +## Commands +- Full check (same as the CI build job): `./gradlew assembleDebug testDebugUnitTest lintDebug` +- Single test: `./gradlew testDebugUnitTest --tests "com.ngoline.easygpg.SecretsTest"` +- Instrumented (needs a device/emulator): `./gradlew connectedDebugAndroidTest`. CI runs these in a second job on an API 35 emulator. + +## Testing +- Unit tests in `app/src/test/`. Tests touching Android APIs (`SharedPreferences`, `Context`, `Handler`) run under Robolectric; `PassphraseCacheTest` is the pattern. +- Robolectric is pinned to `sdk=35` in `app/src/test/resources/robolectric.properties`. Robolectric 4.15 only emulates up to API 35 while the app targets 36; `minSdk` is 35, so the pin is still a supported configuration. Do not bump it to 36. +- `app/src/testShared/java` is added to both the `test` and `androidTest` source sets, so a fixture there is shared by the JVM and instrumented tests (e.g. `TestKeyRings.kt`). +- The `PGPKeyManager` Keystore tests need a secure lock screen plus a biometric or device-credential authentication within `SECRET_KEY_AUTH_VALIDITY_SECONDS` (300). CI sets a PIN and unlocks immediately before running (`.github/scripts/prepare-emulator.sh`); a long Gradle build beforehand lets the auth window expire and the tests fail. +- The instrumented tests clear the target app's `filesDir`. Debug builds use `applicationIdSuffix = ".debug"` to stay a separate install from release. + +## Releases and versioning +- `versionCode` and `versionName` live in `app/build.gradle.kts`. +- User-facing changes go in `CHANGELOG.md` under the version they ship in. +- A release is triggered by pushing a `v*` tag (tag must equal `versionName`) or by running workflow_dispatch. The workflow rejects a `versionCode` that is not greater than the previous tag's, because Play rejects a duplicate versionCode. +- Play "What's new" lives in `distribution/whatsnew/whatsnew-` (no extension). It must be non-empty and <= 500 characters or the release fails; checked by `.github/scripts/check-release-notes.sh`. +- Play listing copy lives in `distribution/listing//`: `title.txt` (30 characters), `short-description.txt` (80), `full-description.txt` (4000). Check with `.github/scripts/check-listing.sh`. + +## Conventions +- Commit subjects follow Conventional Commits, with an optional scope: `fix:`, `fix(ui):`, `feat(site):`, `docs:`, `chore:`, `ci:`, `style:`, `test(notifications):`, `release:`. +- The app handles cryptographic material. Report security issues privately to the maintainer rather than as a public issue (see `CONTRIBUTING.md`). +- No `INTERNET` permission and no servers; the app is offline by design. +- Kotlin is compiled with `-Xmulti-dollar-interpolation` (see `app/build.gradle.kts`). +- `docs/` deploys to GitHub Pages as-is (plain HTML, no build step).