refactor: add missing docs and add explicit NoKeysProvidedException

This commit is contained in:
Harsh Shandilya 2023-05-18 13:10:53 +05:30
parent 6b0e4feed6
commit a24d02052f
No known key found for this signature in database
6 changed files with 49 additions and 14 deletions

View file

@ -38,7 +38,4 @@ public interface KeyManager<Key, KeyIdentifier> {
* as an identifier for the cryptographic identity tied to this key. * as an identifier for the cryptographic identity tied to this key.
*/ */
public suspend fun getKeyId(key: Key): KeyIdentifier? public suspend fun getKeyId(key: Key): KeyIdentifier?
/** Given a [fileName], return whether this instance can handle it. */
public fun canHandle(fileName: String): Boolean
} }

View file

@ -37,5 +37,8 @@ public sealed class CryptoHandlerException(message: String? = null, cause: Throw
/** The passphrase provided for decryption was incorrect. */ /** The passphrase provided for decryption was incorrect. */
public class IncorrectPassphraseException(cause: Throwable) : CryptoHandlerException(null, cause) 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. */ /** An unexpected error that cannot be mapped to a known type. */
public class UnknownError(cause: Throwable) : CryptoHandlerException(null, cause) public class UnknownError(cause: Throwable) : CryptoHandlerException(null, cause)

View file

@ -8,7 +8,10 @@ package app.passwordstore.crypto
import java.util.Locale import java.util.Locale
import java.util.regex.Pattern import java.util.regex.Pattern
/** Supertype for valid identifiers of GPG keys. */
public sealed class GpgIdentifier { public sealed class GpgIdentifier {
/** A [GpgIdentifier] that represents either a long key ID or a fingerprint. */
public data class KeyId(val id: Long) : GpgIdentifier() { public data class KeyId(val id: Long) : GpgIdentifier() {
override fun toString(): String { override fun toString(): String {
return convertKeyIdToHex(id) 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() { public data class UserId(val email: String) : GpgIdentifier() {
override fun toString(): String { override fun toString(): String {
return email return email
@ -45,6 +52,9 @@ public sealed class GpgIdentifier {
private const val HEX_32_STRING_LENGTH = 8 private const val HEX_32_STRING_LENGTH = 8
private const val TRUNCATED_FINGERPRINT_LENGTH = 16 private const val TRUNCATED_FINGERPRINT_LENGTH = 16
/**
* Attempts to parse an untyped String identifier into a concrete subtype of [GpgIdentifier].
*/
@Suppress("ReturnCount") @Suppress("ReturnCount")
public fun fromString(identifier: String): GpgIdentifier? { public fun fromString(identifier: String): GpgIdentifier? {
if (identifier.isEmpty()) return null if (identifier.isEmpty()) return null

View file

@ -28,6 +28,10 @@ public object KeyUtils {
return KeyId(keyRing.publicKey.keyID) 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? { public fun tryGetEmail(key: PGPKey): UserId? {
val keyRing = tryParseKeyring(key) ?: return null val keyRing = tryParseKeyring(key) ?: return null
return UserId(keyRing.publicKey.userIDs.next()) return UserId(keyRing.publicKey.userIDs.next())

View file

@ -36,6 +36,7 @@ constructor(
private val keyDir = File(filesDir, KEY_DIR_NAME) private val keyDir = File(filesDir, KEY_DIR_NAME)
/** @see KeyManager.addKey */
override suspend fun addKey(key: PGPKey, replace: Boolean): Result<PGPKey, Throwable> = override suspend fun addKey(key: PGPKey, replace: Boolean): Result<PGPKey, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
@ -72,6 +73,7 @@ constructor(
} }
} }
/** @see KeyManager.removeKey */
override suspend fun removeKey(identifier: GpgIdentifier): Result<Unit, Throwable> = override suspend fun removeKey(identifier: GpgIdentifier): Result<Unit, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
@ -84,6 +86,7 @@ constructor(
} }
} }
/** @see KeyManager.getKeyById */
override suspend fun getKeyById(id: GpgIdentifier): Result<PGPKey, Throwable> = override suspend fun getKeyById(id: GpgIdentifier): Result<PGPKey, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
@ -119,6 +122,7 @@ constructor(
} }
} }
/** @see KeyManager.getAllKeys */
override suspend fun getAllKeys(): Result<List<PGPKey>, Throwable> = override suspend fun getAllKeys(): Result<List<PGPKey>, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
@ -129,14 +133,9 @@ constructor(
} }
} }
/** @see KeyManager.getKeyById */
override suspend fun getKeyId(key: PGPKey): GpgIdentifier? = 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.
override fun canHandle(fileName: String): Boolean {
return fileName.endsWith(KEY_EXTENSION)
}
/** Checks if [keyDir] exists and attempts to create it if not. */ /** Checks if [keyDir] exists and attempts to create it if not. */
private fun keyDirExists(): Boolean { private fun keyDirExists(): Boolean {
return keyDir.exists() || keyDir.mkdirs() return keyDir.exists() || keyDir.mkdirs()

View file

@ -7,6 +7,7 @@ package app.passwordstore.crypto
import app.passwordstore.crypto.errors.CryptoHandlerException import app.passwordstore.crypto.errors.CryptoHandlerException
import app.passwordstore.crypto.errors.IncorrectPassphraseException import app.passwordstore.crypto.errors.IncorrectPassphraseException
import app.passwordstore.crypto.errors.NoKeysProvidedException
import app.passwordstore.crypto.errors.UnknownError import app.passwordstore.crypto.errors.UnknownError
import com.github.michaelbull.result.Result import com.github.michaelbull.result.Result
import com.github.michaelbull.result.mapError import com.github.michaelbull.result.mapError
@ -29,6 +30,13 @@ import org.pgpainless.util.Passphrase
public class PGPainlessCryptoHandler @Inject constructor() : public class PGPainlessCryptoHandler @Inject constructor() :
CryptoHandler<PGPKey, PGPEncryptOptions, PGPDecryptOptions> { CryptoHandler<PGPKey, PGPEncryptOptions, PGPDecryptOptions> {
/**
* 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( public override fun decrypt(
keys: List<PGPKey>, keys: List<PGPKey>,
passphrase: String, passphrase: String,
@ -37,7 +45,9 @@ public class PGPainlessCryptoHandler @Inject constructor() :
options: PGPDecryptOptions, options: PGPDecryptOptions,
): Result<Unit, CryptoHandlerException> = ): Result<Unit, CryptoHandlerException> =
runCatching { runCatching {
require(keys.isNotEmpty()) if (keys.isEmpty()) {
throw NoKeysProvidedException
}
val keyringCollection = val keyringCollection =
keys keys
.map { key -> PGPainless.readKeyRing().secretKeyRing(key.contents) } .map { key -> PGPainless.readKeyRing().secretKeyRing(key.contents) }
@ -56,10 +66,17 @@ public class PGPainlessCryptoHandler @Inject constructor() :
.mapError { error -> .mapError { error ->
when (error) { when (error) {
is WrongPassphraseException -> IncorrectPassphraseException(error) is WrongPassphraseException -> IncorrectPassphraseException(error)
is CryptoHandlerException -> error
else -> UnknownError(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( public override fun encrypt(
keys: List<PGPKey>, keys: List<PGPKey>,
plaintextStream: InputStream, plaintextStream: InputStream,
@ -67,7 +84,9 @@ public class PGPainlessCryptoHandler @Inject constructor() :
options: PGPEncryptOptions, options: PGPEncryptOptions,
): Result<Unit, CryptoHandlerException> = ): Result<Unit, CryptoHandlerException> =
runCatching { runCatching {
require(keys.isNotEmpty()) if (keys.isEmpty()) {
throw NoKeysProvidedException
}
val publicKeyRings = val publicKeyRings =
keys.mapNotNull(KeyUtils::tryParseKeyring).mapNotNull { keyRing -> keys.mapNotNull(KeyUtils::tryParseKeyring).mapNotNull { keyRing ->
when (keyRing) { when (keyRing) {
@ -77,9 +96,11 @@ public class PGPainlessCryptoHandler @Inject constructor() :
} }
} }
require(keys.size == publicKeyRings.size) { 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 publicKeyRingCollection = PGPPublicKeyRingCollection(publicKeyRings)
val encryptionOptions = EncryptionOptions().addRecipients(publicKeyRingCollection) val encryptionOptions = EncryptionOptions().addRecipients(publicKeyRingCollection)
val producerOptions = 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 { public override fun canHandle(fileName: String): Boolean {
return fileName.split('.').lastOrNull() == "gpg" return fileName.substringAfterLast('.', "") == "gpg"
} }
} }