diff options
Diffstat (limited to 'app/src/main/java')
5 files changed, 120 insertions, 48 deletions
diff --git a/app/src/main/java/app/passwordstore/ui/setup/GitServerViewModel.kt b/app/src/main/java/app/passwordstore/ui/setup/GitServerViewModel.kt index f2b8ca30..d549624e 100644 --- a/app/src/main/java/app/passwordstore/ui/setup/GitServerViewModel.kt +++ b/app/src/main/java/app/passwordstore/ui/setup/GitServerViewModel.kt @@ -83,13 +83,16 @@ constructor( /** * Empties the store folder at [root] for a clone, unless it is empty already. Never follows a * symlink: `deleteRecursively()` would, and a `store/link -> ../keys` link would take the PGP - * keys with it. + * keys with it. Returns null, or why it failed as a string resource: then the clone must not + * start. */ - suspend fun clearStore(root: File) { - if (withContext(dispatcherProvider.io()) { cloneTarget(root) } == CloneTarget.Empty) return + suspend fun clearStore(root: File): Int? { + if (withContext(dispatcherProvider.io()) { cloneTarget(root) } == CloneTarget.Empty) return null progress = R.string.delete_directory_progress_text - withContext(dispatcherProvider.io()) { root.deleteWithoutFollowingLinks() } + if (!withContext(dispatcherProvider.io()) { root.deleteWithoutFollowingLinks() }) + return R.string.git_clone_delete_failed progress = R.string.git_clone_progress + return null } /** Set while [finishClone]'s key check runs: the clone's [progress] stays up until it is done. */ diff --git a/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt b/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt index e6000206..ed7fcf37 100644 --- a/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt +++ b/app/src/main/java/app/passwordstore/ui/setup/PgpKeysViewModel.kt @@ -13,11 +13,13 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import app.passwordstore.R import app.passwordstore.crypto.KeyUtils import app.passwordstore.crypto.PGPIdentifier import app.passwordstore.crypto.PGPKey import app.passwordstore.crypto.PGPKeyManager import app.passwordstore.crypto.errors.KeyAlreadyExistsException +import app.passwordstore.crypto.errors.KeyDowngradeException import app.passwordstore.data.repo.PasswordRepository import app.passwordstore.util.coroutines.DispatcherProvider import com.github.michaelbull.result.onSuccess @@ -126,18 +128,23 @@ constructor( } } - /** Reads the key file at [uri] and imports it. */ + /** Reads the key file at [uri] (at most [MAX_KEY_BYTES]) and imports it. */ fun import(uri: Uri, context: Context) { if (busy) return busy = true val resolver = context.applicationContext.contentResolver + val tooLarge = context.getString(R.string.pgp_key_import_too_large) + val downgrade = context.getString(R.string.pgp_key_import_downgrade) viewModelScope.launch { try { val bytes = withContext(dispatcherProvider.io()) { - resolver.openInputStream(uri)?.use { it.readBytes() } + resolver.openInputStream(uri)?.use { + readAtMost(it, MAX_KEY_BYTES) ?: throw IOException(tooLarge) + } } - if (bytes == null) importResult = KeyImport.Failed(null) else add(bytes, replace = false) + if (bytes == null) importResult = KeyImport.Failed(null) + else add(bytes, replace = false, downgrade) } catch (e: IOException) { importResult = KeyImport.Failed(e.message) } catch (e: SecurityException) { @@ -168,22 +175,34 @@ constructor( importResult = null } - /** Imports [bytes]; they are zeroed afterwards unless the replace question keeps them. */ - private suspend fun add(bytes: ByteArray, replace: Boolean) { - val result = keyManager.addKey(PGPKey(bytes), replace) - if (!replace && result.isErr && result.error is KeyAlreadyExistsException) { - pending = bytes - importResult = KeyImport.Exists - return + /** + * Imports [bytes]; they are zeroed afterwards, also on failure or cancellation, unless the + * replace question keeps them. [downgrade] is the message for a public key over a secret one. + */ + private suspend fun add(bytes: ByteArray, replace: Boolean, downgrade: String? = null) { + var keep = false + try { + val result = keyManager.addKey(PGPKey(bytes), replace) + if (!replace && result.isErr && result.error is KeyAlreadyExistsException) { + // A question still open from an earlier pick is superseded: zero its bytes first. + dropPending() + pending = bytes + keep = true + importResult = KeyImport.Exists + return + } + // The ID is read before the bytes are zeroed: the returned key shares them. + importResult = + if (result.isOk) { + val id = withContext(dispatcherProvider.default()) { KeyUtils.tryGetId(result.value) } + KeyImport.Imported(id?.toString().orEmpty()) + } else if (result.error is KeyDowngradeException) { + KeyImport.Failed(downgrade ?: result.error.message) + } else KeyImport.Failed(result.error.message) + if (result.isOk) updateKeySet() + } finally { + if (!keep) bytes.fill(0) } - // The ID is read before the bytes are zeroed: the returned key shares them. - importResult = - if (result.isOk) { - val id = withContext(dispatcherProvider.default()) { KeyUtils.tryGetId(result.value) } - KeyImport.Imported(id?.toString().orEmpty()) - } else KeyImport.Failed(result.error.message) - bytes.fill(0) - if (result.isOk) updateKeySet() } /** @@ -223,3 +242,6 @@ constructor( dropPending() } } + +/** The largest key file imported: far above any real key, small enough to hold in memory. */ +private const val MAX_KEY_BYTES = 1024 * 1024 diff --git a/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt b/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt index 8c7b20a1..ade23aa2 100644 --- a/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt +++ b/app/src/main/java/app/passwordstore/ui/setup/SetupModel.kt @@ -9,8 +9,11 @@ import app.passwordstore.crypto.PGPIdentifier import app.passwordstore.util.settings.AuthMode import java.io.File import java.io.IOException +import java.io.InputStream +import java.nio.channels.Channels import java.nio.file.Files import java.nio.file.LinkOption +import java.nio.file.StandardOpenOption import java.util.Locale /** Why a repository URL is questioned before it is saved, as the old server screen checked it. */ @@ -73,9 +76,17 @@ fun cloneTarget(dir: File): CloneTarget { } } -/** The recipients named by a `.gpg-id` file's [text]; blank and unparsable lines are skipped. */ +/** + * The recipients named by a `.gpg-id` file's [text], read as pass reads it: a `#` starts a comment, + * and a trailing `!` (GnuPG's "this exact subkey") is dropped. Blank and unparsable lines are + * skipped. + */ fun storeRecipients(text: String): List<PGPIdentifier> = - text.lines().map(String::trim).filter(String::isNotEmpty).mapNotNull(PGPIdentifier::fromString) + text + .lines() + .map { it.substringBefore('#').trim().removeSuffix("!") } + .filter(String::isNotEmpty) + .mapNotNull(PGPIdentifier::fromString) /** * A `.gpg-id` naming [keys]: each 16-hex long key ID in uppercase on a line of its own, the format @@ -91,12 +102,34 @@ private const val MAX_GPG_ID_BYTES = 64 * 1024 * than 64 KiB or unreadable. Blocking: call it off the main thread. */ fun readGpgId(dir: File): String? { - val file = File(dir, ".gpg-id") - val path = file.toPath() + val path = File(dir, ".gpg-id").toPath() + if (!Files.isRegularFile(path, LinkOption.NOFOLLOW_LINKS)) return null + // Opened without following a link too: one swapped in after the check is refused. + val options = setOf(StandardOpenOption.READ, LinkOption.NOFOLLOW_LINKS) return try { - if (!Files.isRegularFile(path, LinkOption.NOFOLLOW_LINKS)) null - else if (Files.size(path) > MAX_GPG_ID_BYTES) null else file.readText() + Channels.newInputStream(Files.newByteChannel(path, options)) + .use { readAtMost(it, MAX_GPG_ID_BYTES) } + ?.decodeToString() } catch (_: IOException) { null } } + +/** + * At most [limit] bytes of [input], or null when it holds more. Reads at most [limit] + 1 bytes, + * whatever the stream's size, and zeroes its buffer: the bytes may be a private key. + */ +fun readAtMost(input: InputStream, limit: Int): ByteArray? { + val buffer = ByteArray(limit + 1) + try { + var size = 0 + while (size < buffer.size) { + val read = input.read(buffer, size, buffer.size - size) + if (read < 0) break + size += read + } + return if (size > limit) null else buffer.copyOf(size) + } finally { + buffer.fill(0) + } +} diff --git a/app/src/main/java/app/passwordstore/ui/setup/SshKeyViewModel.kt b/app/src/main/java/app/passwordstore/ui/setup/SshKeyViewModel.kt index d70b06af..bc3e9474 100644 --- a/app/src/main/java/app/passwordstore/ui/setup/SshKeyViewModel.kt +++ b/app/src/main/java/app/passwordstore/ui/setup/SshKeyViewModel.kt @@ -17,10 +17,8 @@ import app.passwordstore.R import app.passwordstore.util.coroutines.DispatcherProvider import app.passwordstore.util.git.sshj.SshKey import dagger.hilt.android.lifecycle.HiltViewModel -import java.io.IOException -import java.security.GeneralSecurityException -import java.security.ProviderException import javax.inject.Inject +import kotlin.coroutines.cancellation.CancellationException import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -83,11 +81,10 @@ class SshKeyViewModel @Inject constructor(private val dispatcherProvider: Dispat withContext(dispatcherProvider.io()) { SshKey.import(uri) } imported = true version++ - } catch (e: IOException) { - failure = e.message.orEmpty() - } catch (e: IllegalArgumentException) { - failure = e.message.orEmpty() - } catch (e: SecurityException) { + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // Any failure of a provider or of sshj, not only I/O: an uncaught one would crash the app. failure = e.message.orEmpty() } finally { busy = false @@ -114,11 +111,10 @@ class SshKeyViewModel @Inject constructor(private val dispatcherProvider: Dispat SshKey.sshPublicKey.orEmpty() } version++ - } catch (e: GeneralSecurityException) { - failure = e.message.orEmpty() - } catch (e: ProviderException) { - failure = e.message.orEmpty() - } catch (e: IOException) { + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // Keystore and provider failures come as many exception types; none may crash the app. failure = e.message.orEmpty() } finally { busy = false diff --git a/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt b/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt index 8d5519a8..06cf5bc8 100644 --- a/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt +++ b/app/src/main/java/app/passwordstore/util/git/sshj/SshKey.kt @@ -29,6 +29,8 @@ import com.github.michaelbull.result.getOrElse import com.github.michaelbull.result.runCatching import java.io.File import java.io.IOException +import java.nio.file.Files +import java.nio.file.StandardCopyOption import java.security.KeyFactory import java.security.KeyPair import java.security.KeyPairGenerator @@ -203,15 +205,20 @@ object SshKey { } private fun delete() { - androidKeystore.deleteEntry(KEYSTORE_ALIAS) if (privateKeyFile.isFile) { privateKeyFile.delete() } + deleteKeyExtras() + type = null + } + + /** Everything of the current key but its private key file. */ + private fun deleteKeyExtras() { + androidKeystore.deleteEntry(KEYSTORE_ALIAS) if (publicKeyFile.isFile) { publicKeyFile.delete() } context.getEncryptedGitPrefs().edit { remove(PreferenceKeys.SSH_KEY_LOCAL_PASSPHRASE) } - type = null } fun import(uri: Uri) { @@ -241,12 +248,23 @@ object SshKey { ) // At this point, we are reasonably confident that we have actually been provided a private - // key and delete the old key. - delete() - // Canonicalize line endings to '\n'. - privateKeyFile.writeText(lines.joinToString("\n")) - + // key. It replaces the old one atomically (an I/O error keeps the old key), then the old key's + // Keystore entry, public key and passphrase go. + val temp = File(privateKeyFile.parentFile, ".${privateKeyFile.name}.tmp") + try { + // Canonicalize line endings to '\n'. + temp.writeText(lines.joinToString("\n")) + Files.move( + temp.toPath(), + privateKeyFile.toPath(), + StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE, + ) + } finally { + temp.delete() + } type = Type.Imported + deleteKeyExtras() } /** Deletes an Ed25519 key wrapped by androidx security-crypto, which can no longer be read. */ |