From a24d02052f8844497d26274b65abae05148cd86b Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Thu, 18 May 2023 13:10:53 +0530 Subject: refactor: add missing docs and add explicit `NoKeysProvidedException` --- .../kotlin/app/passwordstore/crypto/KeyManager.kt | 3 -- .../passwordstore/crypto/errors/CryptoException.kt | 3 ++ .../app/passwordstore/crypto/GpgIdentifier.kt | 10 +++++++ .../kotlin/app/passwordstore/crypto/KeyUtils.kt | 4 +++ .../app/passwordstore/crypto/PGPKeyManager.kt | 11 ++++---- .../crypto/PGPainlessCryptoHandler.kt | 32 ++++++++++++++++++---- 6 files changed, 49 insertions(+), 14 deletions(-) diff --git a/crypto-common/src/main/kotlin/app/passwordstore/crypto/KeyManager.kt b/crypto-common/src/main/kotlin/app/passwordstore/crypto/KeyManager.kt index cb3df75e..c9db3734 100644 --- a/crypto-common/src/main/kotlin/app/passwordstore/crypto/KeyManager.kt +++ b/crypto-common/src/main/kotlin/app/passwordstore/crypto/KeyManager.kt @@ -38,7 +38,4 @@ public interface KeyManager { * as an identifier for the cryptographic identity tied to this key. */ public suspend fun getKeyId(key: Key): KeyIdentifier? - - /** Given a [fileName], return whether this instance can handle it. */ - public fun canHandle(fileName: String): Boolean } diff --git a/crypto-common/src/main/kotlin/app/passwordstore/crypto/errors/CryptoException.kt b/crypto-common/src/main/kotlin/app/passwordstore/crypto/errors/CryptoException.kt index cb8980bd..6d752964 100644 --- a/crypto-common/src/main/kotlin/app/passwordstore/crypto/errors/CryptoException.kt +++ b/crypto-common/src/main/kotlin/app/passwordstore/crypto/errors/CryptoException.kt @@ -37,5 +37,8 @@ public sealed class CryptoHandlerException(message: String? = null, cause: Throw /** The passphrase provided for decryption was incorrect. */ public class IncorrectPassphraseException(cause: Throwable) : CryptoHandlerException(null, cause) +/** No keys were passed to the encrypt/decrypt operation. */ +public object NoKeysProvidedException : CryptoHandlerException(null, null) + /** An unexpected error that cannot be mapped to a known type. */ public class UnknownError(cause: Throwable) : CryptoHandlerException(null, cause) diff --git a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt index d99213d5..96b9cc88 100644 --- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt +++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt @@ -8,7 +8,10 @@ package app.passwordstore.crypto import java.util.Locale import java.util.regex.Pattern +/** Supertype for valid identifiers of GPG keys. */ public sealed class GpgIdentifier { + + /** A [GpgIdentifier] that represents either a long key ID or a fingerprint. */ public data class KeyId(val id: Long) : GpgIdentifier() { override fun toString(): String { return convertKeyIdToHex(id) @@ -32,6 +35,10 @@ public sealed class GpgIdentifier { } } + /** + * A [GpgIdentifier] that represents the textual name/email combination corresponding to a key. + * Despite the [email] property in this class, the value is not guaranteed to be a valid email. + */ public data class UserId(val email: String) : GpgIdentifier() { override fun toString(): String { return email @@ -45,6 +52,9 @@ public sealed class GpgIdentifier { private const val HEX_32_STRING_LENGTH = 8 private const val TRUNCATED_FINGERPRINT_LENGTH = 16 + /** + * Attempts to parse an untyped String identifier into a concrete subtype of [GpgIdentifier]. + */ @Suppress("ReturnCount") public fun fromString(identifier: String): GpgIdentifier? { if (identifier.isEmpty()) return null diff --git a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt index 47c06c4f..da77bf33 100644 --- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt +++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt @@ -28,6 +28,10 @@ public object KeyUtils { return KeyId(keyRing.publicKey.keyID) } + /** + * Attempts to parse the given [PGPKey] into a [PGPKeyRing] and obtains the [UserId] of the + * corresponding public key. + */ public fun tryGetEmail(key: PGPKey): UserId? { val keyRing = tryParseKeyring(key) ?: return null return UserId(keyRing.publicKey.userIDs.next()) 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 be2ec474..f6ef50b3 100644 --- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt +++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt @@ -36,6 +36,7 @@ constructor( private val keyDir = File(filesDir, KEY_DIR_NAME) + /** @see KeyManager.addKey */ override suspend fun addKey(key: PGPKey, replace: Boolean): Result = withContext(dispatcher) { runSuspendCatching { @@ -72,6 +73,7 @@ constructor( } } + /** @see KeyManager.removeKey */ override suspend fun removeKey(identifier: GpgIdentifier): Result = withContext(dispatcher) { runSuspendCatching { @@ -84,6 +86,7 @@ constructor( } } + /** @see KeyManager.getKeyById */ override suspend fun getKeyById(id: GpgIdentifier): Result = withContext(dispatcher) { runSuspendCatching { @@ -119,6 +122,7 @@ constructor( } } + /** @see KeyManager.getAllKeys */ override suspend fun getAllKeys(): Result, Throwable> = withContext(dispatcher) { runSuspendCatching { @@ -129,14 +133,9 @@ constructor( } } + /** @see KeyManager.getKeyById */ 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. - override fun canHandle(fileName: String): Boolean { - return fileName.endsWith(KEY_EXTENSION) - } - /** 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/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt index 2bf68401..cf29931b 100644 --- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt +++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt @@ -7,6 +7,7 @@ package app.passwordstore.crypto import app.passwordstore.crypto.errors.CryptoHandlerException import app.passwordstore.crypto.errors.IncorrectPassphraseException +import app.passwordstore.crypto.errors.NoKeysProvidedException import app.passwordstore.crypto.errors.UnknownError import com.github.michaelbull.result.Result import com.github.michaelbull.result.mapError @@ -29,6 +30,13 @@ import org.pgpainless.util.Passphrase public class PGPainlessCryptoHandler @Inject constructor() : CryptoHandler { + /** + * Decrypts the given [ciphertextStream] using [PGPainless] and writes the decrypted output to + * [outputStream]. The provided [passphrase] is wrapped in a [SecretKeyRingProtector] and the + * [keys] argument is defensively checked to ensure it has at least one key present. + * + * @see CryptoHandler.decrypt + */ public override fun decrypt( keys: List, passphrase: String, @@ -37,7 +45,9 @@ public class PGPainlessCryptoHandler @Inject constructor() : options: PGPDecryptOptions, ): Result = runCatching { - require(keys.isNotEmpty()) + if (keys.isEmpty()) { + throw NoKeysProvidedException + } val keyringCollection = keys .map { key -> PGPainless.readKeyRing().secretKeyRing(key.contents) } @@ -56,10 +66,17 @@ public class PGPainlessCryptoHandler @Inject constructor() : .mapError { error -> when (error) { is WrongPassphraseException -> IncorrectPassphraseException(error) + is CryptoHandlerException -> error else -> UnknownError(error) } } + /** + * Encrypts the provided [plaintextStream] and writes the encrypted output to [outputStream]. The + * [keys] argument is defensively checked to contain at least one key. + * + * @see CryptoHandler.encrypt + */ public override fun encrypt( keys: List, plaintextStream: InputStream, @@ -67,7 +84,9 @@ public class PGPainlessCryptoHandler @Inject constructor() : options: PGPEncryptOptions, ): Result = runCatching { - require(keys.isNotEmpty()) + if (keys.isEmpty()) { + throw NoKeysProvidedException + } val publicKeyRings = keys.mapNotNull(KeyUtils::tryParseKeyring).mapNotNull { keyRing -> when (keyRing) { @@ -77,9 +96,11 @@ public class PGPainlessCryptoHandler @Inject constructor() : } } require(keys.size == publicKeyRings.size) { - "Failed to parse all keys: keys=${keys.size},parsed=${publicKeyRings.size}" + "Failed to parse all keys: ${keys.size} keys were provided but only ${publicKeyRings.size} were valid" + } + if (publicKeyRings.isEmpty()) { + throw NoKeysProvidedException } - require(publicKeyRings.isNotEmpty()) { "No public keys to encrypt message to" } val publicKeyRingCollection = PGPPublicKeyRingCollection(publicKeyRings) val encryptionOptions = EncryptionOptions().addRecipients(publicKeyRingCollection) val producerOptions = @@ -103,7 +124,8 @@ public class PGPainlessCryptoHandler @Inject constructor() : } } + /** Runs a naive check on the extension for the given [fileName] to check if it is a PGP file. */ public override fun canHandle(fileName: String): Boolean { - return fileName.split('.').lastOrNull() == "gpg" + return fileName.substringAfterLast('.', "") == "gpg" } } -- cgit v1.2.3