From 5509558eedbcd90f130b32d4e6c41632da291b15 Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Tue, 18 Jan 2022 17:40:16 +0530 Subject: Parameterize key and key identifier types for KeyManager (#1669) --- .../dev/msfjarvis/aps/crypto/GpgIdentifier.kt | 82 ++++++++++++++++++++++ .../kotlin/dev/msfjarvis/aps/crypto/KeyUtils.kt | 13 ++-- .../main/kotlin/dev/msfjarvis/aps/crypto/PGPKey.kt | 12 ++++ .../dev/msfjarvis/aps/crypto/PGPKeyManager.kt | 57 ++++++++------- .../aps/crypto/PGPainlessCryptoHandler.kt | 6 +- .../dev/msfjarvis/aps/crypto/CryptoConstants.kt | 2 +- .../dev/msfjarvis/aps/crypto/GpgIdentifierTest.kt | 41 +++++++++++ .../dev/msfjarvis/aps/crypto/PGPKeyManagerTest.kt | 24 ++++--- .../aps/crypto/PGPainlessCryptoHandlerTest.kt | 4 +- 9 files changed, 191 insertions(+), 50 deletions(-) create mode 100644 crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifier.kt create mode 100644 crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKey.kt create mode 100644 crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifierTest.kt (limited to 'crypto-pgpainless') diff --git a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifier.kt b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifier.kt new file mode 100644 index 00000000..20337d3e --- /dev/null +++ b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifier.kt @@ -0,0 +1,82 @@ +/* + * Copyright © 2014-2022 The Android Password Store Authors. All Rights Reserved. + * SPDX-License-Identifier: GPL-3.0-only + */ + +package dev.msfjarvis.aps.crypto + +import java.util.regex.Pattern + +public sealed class GpgIdentifier { + public data class KeyId(val id: Long) : GpgIdentifier() { + override fun toString(): String { + return java.lang.Long.toHexString(id) + } + } + public data class UserId(val email: String) : GpgIdentifier() { + override fun toString(): String { + return email + } + } + + public companion object { + @OptIn(ExperimentalUnsignedTypes::class) + public fun fromString(identifier: String): GpgIdentifier? { + if (identifier.isEmpty()) return null + // Match long key IDs: + // FF22334455667788 or 0xFF22334455667788 + val maybeLongKeyId = + identifier.removePrefix("0x").takeIf { it.matches("[a-fA-F0-9]{16}".toRegex()) } + if (maybeLongKeyId != null) { + val keyId = maybeLongKeyId.toULong(16) + return KeyId(keyId.toLong()) + } + + // Match fingerprints: + // FF223344556677889900112233445566778899 or 0xFF223344556677889900112233445566778899 + val maybeFingerprint = + identifier.removePrefix("0x").takeIf { it.matches("[a-fA-F0-9]{40}".toRegex()) } + if (maybeFingerprint != null) { + // Truncating to the long key ID is not a security issue since OpenKeychain only + // accepts + // non-ambiguous key IDs. + val keyId = maybeFingerprint.takeLast(16).toULong(16) + return KeyId(keyId.toLong()) + } + + return splitUserId(identifier)?.let { UserId(it) } + } + + private val USER_ID_PATTERN = Pattern.compile("^(.*?)(?: \\((.*)\\))?(?: <(.*)>)?$") + private val EMAIL_PATTERN = Pattern.compile("^\"]*@[^<>\"]*[.]?[^<>\"]*)\"?>?$") + + /** + * Takes a 'Name (Comment) ' user ID in any of its permutations and attempts to extract + * an email from it. + */ + private fun splitUserId(userId: String): String? { + if (userId.isNotEmpty()) { + val matcher = USER_ID_PATTERN.matcher(userId) + if (matcher.matches()) { + var name = if (matcher.group(1)?.isEmpty() == true) null else matcher.group(1) + var email = matcher.group(3) + if (email != null && name != null) { + val emailMatcher = EMAIL_PATTERN.matcher(name) + if (emailMatcher.matches() && email == emailMatcher.group(1)) { + email = emailMatcher.group(1) + name = null + } + } + if (email == null && name != null) { + val emailMatcher = EMAIL_PATTERN.matcher(name) + if (emailMatcher.matches()) { + email = emailMatcher.group(1) + } + } + return email + } + } + return null + } + } +} diff --git a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/KeyUtils.kt b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/KeyUtils.kt index e7a3c387..852ed628 100644 --- a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/KeyUtils.kt +++ b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/KeyUtils.kt @@ -7,17 +7,18 @@ package dev.msfjarvis.aps.crypto import com.github.michaelbull.result.get import com.github.michaelbull.result.runCatching +import dev.msfjarvis.aps.crypto.GpgIdentifier.KeyId import java.util.Locale import org.bouncycastle.openpgp.PGPKeyRing import org.pgpainless.PGPainless -/** Utility methods to deal with PGP [Key]s. */ +/** Utility methods to deal with [PGPKey]s. */ public object KeyUtils { /** * Attempts to parse a [PGPKeyRing] from a given [key]. The key is first tried as a secret key and * then as a public one before the method gives up and returns null. */ - public fun tryParseKeyring(key: Key): PGPKeyRing? { + public fun tryParseKeyring(key: PGPKey): PGPKeyRing? { val secKeyRing = runCatching { PGPainless.readKeyRing().secretKeyRing(key.contents) }.get() if (secKeyRing != null) { return secKeyRing @@ -29,15 +30,15 @@ public object KeyUtils { return null } - /** Parses a [PGPKeyRing] from the given [key] and returns its hex-formatted key ID. */ - public fun tryGetId(key: Key): String? { + /** Parses a [PGPKeyRing] from the given [key] and calculates its long key ID */ + public fun tryGetId(key: PGPKey): KeyId? { val keyRing = tryParseKeyring(key) ?: return null - return convertKeyIdToHex(keyRing.publicKey.keyID) + return KeyId(convertKeyIdToHex(keyRing.publicKey.keyID).toLong(radix = 16)) } /** Convert a [Long] key ID to a formatted string. */ private fun convertKeyIdToHex(keyId: Long): String { - return "0x" + convertKeyIdToHex32bit(keyId shr 32) + convertKeyIdToHex32bit(keyId) + return convertKeyIdToHex32bit(keyId shr 32) + convertKeyIdToHex32bit(keyId) } /** diff --git a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKey.kt b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKey.kt new file mode 100644 index 00000000..2e1311a1 --- /dev/null +++ b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKey.kt @@ -0,0 +1,12 @@ +/* + * Copyright © 2014-2021 The Android Password Store Authors. All Rights Reserved. + * SPDX-License-Identifier: GPL-3.0-only + */ + +package dev.msfjarvis.aps.crypto + +/** + * A simple value class wrapping over a [ByteArray] that can represent a PGP key. The implementation + * details of public/ private parts as well as identities are deferred to [PGPKeyManager]. + */ +@JvmInline public value class PGPKey(public val contents: ByteArray) diff --git a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManager.kt b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManager.kt index 2053ecd7..adcce22b 100644 --- a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManager.kt +++ b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManager.kt @@ -22,11 +22,11 @@ public class PGPKeyManager constructor( filesDir: String, private val dispatcher: CoroutineDispatcher, -) : KeyManager { +) : KeyManager { private val keyDir = File(filesDir, KEY_DIR_NAME) - override suspend fun addKey(key: Key, replace: Boolean): Result = + override suspend fun addKey(key: PGPKey, replace: Boolean): Result = withContext(dispatcher) { runCatching { if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException @@ -36,7 +36,7 @@ constructor( // Check for replace flag first and if it is false, throw an error if (!replace) throw KeyManagerException.KeyAlreadyExistsException( - tryGetId(key) ?: "Failed to retrieve key ID" + tryGetId(key)?.toString() ?: "Failed to retrieve key ID" ) if (!keyFile.delete()) throw KeyManagerException.KeyDeletionFailedException } @@ -47,7 +47,7 @@ constructor( } } - override suspend fun removeKey(key: Key): Result = + override suspend fun removeKey(key: PGPKey): Result = withContext(dispatcher) { runCatching { if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException @@ -61,49 +61,52 @@ constructor( } } - override suspend fun getKeyById(id: String): Result = + override suspend fun getKeyById(id: GpgIdentifier): Result = withContext(dispatcher) { runCatching { if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException val keyFiles = keyDir.listFiles() if (keyFiles.isNullOrEmpty()) throw KeyManagerException.NoKeysAvailableException - - val keys = keyFiles.map { file -> Key(file.readBytes()) } - // Try to parse the key ID as an email - val selector = SelectUserId.byEmail(id) - val userIdMatch = - keys.map { key -> key to tryParseKeyring(key) }.firstOrNull { (_, keyRing) -> - selector.firstMatch(keyRing) != null - } - - if (userIdMatch != null) { - return@runCatching userIdMatch.first - } - - val keyIdMatch = - keys.map { key -> key to tryGetId(key) }.firstOrNull { (_, keyId) -> - keyId == id || keyId == "0x$id" + val keys = keyFiles.map { file -> PGPKey(file.readBytes()) } + + val matchResult = + when (id) { + is GpgIdentifier.KeyId -> { + val keyIdMatch = + keys.map { key -> key to tryGetId(key) }.firstOrNull { (_, keyId) -> + keyId?.id == id.id + } + keyIdMatch?.first + } + is GpgIdentifier.UserId -> { + val selector = SelectUserId.byEmail(id.email) + val userIdMatch = + keys.map { key -> key to tryParseKeyring(key) }.firstOrNull { (_, keyRing) -> + selector.firstMatch(keyRing) != null + } + userIdMatch?.first + } } - if (keyIdMatch != null) { - return@runCatching keyIdMatch.first + if (matchResult != null) { + return@runCatching matchResult } - throw KeyManagerException.KeyNotFoundException(id) + throw KeyManagerException.KeyNotFoundException("$id") } } - override suspend fun getAllKeys(): Result, Throwable> = + override suspend fun getAllKeys(): Result, Throwable> = withContext(dispatcher) { runCatching { if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException val keyFiles = keyDir.listFiles() if (keyFiles.isNullOrEmpty()) return@runCatching emptyList() - keyFiles.map { keyFile -> Key(keyFile.readBytes()) }.toList() + keyFiles.map { keyFile -> PGPKey(keyFile.readBytes()) }.toList() } } - override suspend fun getKeyId(key: Key): String? = tryGetId(key) + override suspend fun getKeyId(key: PGPKey): GpgIdentifier? = tryGetId(key) // TODO: This is a temp hack for now and in future it should check that the GPGKeyManager can // decrypt the file. diff --git a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandler.kt b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandler.kt index 427cd555..637c8586 100644 --- a/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandler.kt +++ b/crypto-pgpainless/src/main/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandler.kt @@ -18,10 +18,10 @@ import org.pgpainless.encryption_signing.ProducerOptions import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector import org.pgpainless.util.Passphrase -public class PGPainlessCryptoHandler @Inject constructor() : CryptoHandler { +public class PGPainlessCryptoHandler @Inject constructor() : CryptoHandler { public override fun decrypt( - privateKey: Key, + privateKey: PGPKey, passphrase: String, ciphertextStream: InputStream, outputStream: OutputStream, @@ -44,7 +44,7 @@ public class PGPainlessCryptoHandler @Inject constructor() : CryptoHandler { } public override fun encrypt( - keys: List, + keys: List, plaintextStream: InputStream, outputStream: OutputStream, ) { diff --git a/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/CryptoConstants.kt b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/CryptoConstants.kt index 6d5eaf01..ede8f08d 100644 --- a/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/CryptoConstants.kt +++ b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/CryptoConstants.kt @@ -10,5 +10,5 @@ object CryptoConstants { const val PLAIN_TEXT = "encryption worthy content" const val KEY_NAME = "John Doe" const val KEY_EMAIL = "john.doe@example.com" - const val KEY_ID = "0x08edf7567183ce27" + const val KEY_ID = 0x08edf7567183ce27 } diff --git a/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifierTest.kt b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifierTest.kt new file mode 100644 index 00000000..3860873c --- /dev/null +++ b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/GpgIdentifierTest.kt @@ -0,0 +1,41 @@ +/* + * Copyright © 2014-2022 The Android Password Store Authors. All Rights Reserved. + * SPDX-License-Identifier: GPL-3.0-only + */ + +package dev.msfjarvis.aps.crypto + +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +class GpgIdentifierTest { + + @Test + fun `parses hexadecimal key id without leading 0x`() { + val identifier = GpgIdentifier.fromString("79E8208280490C77") + assertNotNull(identifier) + assertTrue { identifier is GpgIdentifier.KeyId } + } + + @Test + fun `parses hexadecimal key id`() { + val identifier = GpgIdentifier.fromString("0x79E8208280490C77") + assertNotNull(identifier) + assertTrue { identifier is GpgIdentifier.KeyId } + } + + @Test + fun `parses email as user id`() { + val identifier = GpgIdentifier.fromString("john.doe@example.org") + assertNotNull(identifier) + assertTrue { identifier is GpgIdentifier.UserId } + } + + @Test + fun `parses user@host without TLD`() { + val identifier = GpgIdentifier.fromString("john.doe@example") + assertNotNull(identifier) + assertTrue { identifier is GpgIdentifier.UserId } + } +} diff --git a/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManagerTest.kt b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManagerTest.kt index ed2b00c3..24b27ef8 100644 --- a/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManagerTest.kt +++ b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPKeyManagerTest.kt @@ -2,6 +2,8 @@ package dev.msfjarvis.aps.crypto import com.github.michaelbull.result.unwrap import com.github.michaelbull.result.unwrapError +import dev.msfjarvis.aps.crypto.GpgIdentifier.KeyId +import dev.msfjarvis.aps.crypto.GpgIdentifier.UserId import java.io.File import kotlin.test.AfterTest import kotlin.test.BeforeTest @@ -28,7 +30,7 @@ class PGPKeyManagerTest { private val dispatcher = StandardTestDispatcher() private val scope = TestScope(dispatcher) private val keyManager by unsafeLazy { PGPKeyManager(filesDir.absolutePath, dispatcher) } - private val key = Key(TestUtils.getArmoredPrivateKey()) + private val key = PGPKey(TestUtils.getArmoredPrivateKey()) private fun unsafeLazy(initializer: () -> T) = lazy(LazyThreadSafetyMode.NONE) { initializer.invoke() } @@ -48,7 +50,7 @@ class PGPKeyManagerTest { scope.runTest { // Check if the key id returned is correct val keyId = keyManager.getKeyId(keyManager.addKey(key).unwrap()) - assertEquals(CryptoConstants.KEY_ID, keyId) + assertEquals(KeyId(CryptoConstants.KEY_ID), keyId) // Check if the keys directory have one file assertEquals(1, filesDir.list()?.size) @@ -75,7 +77,7 @@ class PGPKeyManagerTest { keyManager.addKey(key, true).unwrap() val keyId = keyManager.getKeyId(keyManager.addKey(key, true).unwrap()) - assertEquals(CryptoConstants.KEY_ID, keyId) + assertEquals(KeyId(CryptoConstants.KEY_ID), keyId) } @Test @@ -86,7 +88,7 @@ class PGPKeyManagerTest { // Check if the key id returned is correct val keyId = keyManager.getKeyId(keyManager.removeKey(key).unwrap()) - assertEquals(CryptoConstants.KEY_ID, keyId) + assertEquals(KeyId(CryptoConstants.KEY_ID), keyId) // Check if the keys directory have 0 files val keysDir = File(filesDir, PGPKeyManager.KEY_DIR_NAME) @@ -101,7 +103,7 @@ class PGPKeyManagerTest { val keyId = keyManager.getKeyId(key) assertNotNull(keyId) - assertEquals(CryptoConstants.KEY_ID, keyManager.getKeyId(key)) + assertEquals(KeyId(CryptoConstants.KEY_ID), keyManager.getKeyId(key)) // Check returned key id matches the expected id and the created key id val returnedKey = keyManager.getKeyById(keyId).unwrap() @@ -114,7 +116,7 @@ class PGPKeyManagerTest { keyManager.addKey(key).unwrap() val keyId = "${CryptoConstants.KEY_NAME} <${CryptoConstants.KEY_EMAIL}>" - val returnedKey = keyManager.getKeyById(keyId).unwrap() + val returnedKey = keyManager.getKeyById(UserId(keyId)).unwrap() assertEquals(keyManager.getKeyId(key), keyManager.getKeyId(returnedKey)) } @@ -124,7 +126,7 @@ class PGPKeyManagerTest { keyManager.addKey(key).unwrap() val keyId = CryptoConstants.KEY_EMAIL - val returnedKey = keyManager.getKeyById(keyId).unwrap() + val returnedKey = keyManager.getKeyById(UserId(keyId)).unwrap() assertEquals(keyManager.getKeyId(key), keyManager.getKeyId(returnedKey)) } @@ -134,19 +136,19 @@ class PGPKeyManagerTest { // Add key using KeyManager keyManager.addKey(key).unwrap() - val randomKeyId = "0x123456789" + val keyId = KeyId(0x08edf7567183ce44) // Check returned key - val error = keyManager.getKeyById(randomKeyId).unwrapError() + val error = keyManager.getKeyById(keyId).unwrapError() assertIs(error) - assertEquals("No key found with id: $randomKeyId", error.message) + assertEquals("No key found with id: $keyId", error.message) } @Test fun testFindKeysWithoutAdding() = scope.runTest { // Check returned key - val error = keyManager.getKeyById("0x123456789").unwrapError() + val error = keyManager.getKeyById(KeyId(0x08edf7567183ce44)).unwrapError() assertIs(error) assertEquals("No keys were found", error.message) } diff --git a/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandlerTest.kt b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandlerTest.kt index 6ec6ccee..9b4cb664 100644 --- a/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandlerTest.kt +++ b/crypto-pgpainless/src/test/kotlin/dev/msfjarvis/aps/crypto/PGPainlessCryptoHandlerTest.kt @@ -14,8 +14,8 @@ import kotlin.test.assertTrue class PGPainlessCryptoHandlerTest { private val cryptoHandler = PGPainlessCryptoHandler() - private val privateKey = Key(TestUtils.getArmoredPrivateKey()) - private val publicKey = Key(TestUtils.getArmoredPublicKey()) + private val privateKey = PGPKey(TestUtils.getArmoredPrivateKey()) + private val publicKey = PGPKey(TestUtils.getArmoredPublicKey()) @Test fun encryptAndDecrypt() { -- cgit v1.2.3