Add tests for GPG identifier parsing (#1319)

This commit is contained in:
Harsh Shandilya 2021-02-15 13:05:09 +05:30 committed by GitHub
parent 7fbe4be711
commit 051d455c9f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 82 additions and 40 deletions

View File

@ -34,6 +34,7 @@ import dev.msfjarvis.aps.ui.dialogs.PasswordGeneratorDialogFragment
import dev.msfjarvis.aps.ui.dialogs.XkPasswordGeneratorDialogFragment
import dev.msfjarvis.aps.util.autofill.AutofillPreferences
import dev.msfjarvis.aps.util.autofill.DirectoryStructure
import dev.msfjarvis.aps.util.crypto.GpgIdentifier
import dev.msfjarvis.aps.util.extensions.base64
import dev.msfjarvis.aps.util.extensions.commitChange
import dev.msfjarvis.aps.util.extensions.getString
@ -50,7 +51,6 @@ import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import me.msfjarvis.openpgpktx.util.OpenPgpApi
import me.msfjarvis.openpgpktx.util.OpenPgpServiceConnection
import me.msfjarvis.openpgpktx.util.OpenPgpUtils
class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnBound {
@ -271,39 +271,6 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
otpImportButton.isVisible = !entry.hasTotp()
}
private sealed class GpgIdentifier {
data class KeyId(val id: Long) : GpgIdentifier()
data class UserId(val email: String) : GpgIdentifier()
}
@OptIn(ExperimentalUnsignedTypes::class)
private fun parseGpgIdentifier(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 GpgIdentifier.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 GpgIdentifier.KeyId(keyId.toLong())
}
return OpenPgpUtils.splitUserId(identifier).email?.let { GpgIdentifier.UserId(it) }
}
/**
* Encrypts the password and the extra content
*/
@ -340,7 +307,7 @@ class PasswordCreationActivity : BasePgpActivity(), OpenPgpServiceConnection.OnB
val gpgIdentifiers = gpgIdentifierFile.readLines()
.filter { it.isNotBlank() }
.map { line ->
parseGpgIdentifier(line) ?: run {
GpgIdentifier.fromString(line) ?: run {
// The line being empty means this is most likely an empty `.gpg-id` file
// we created. Skip the validation so we can make the user add a real ID.
if (line.isEmpty()) return@run

View File

@ -0,0 +1,38 @@
package dev.msfjarvis.aps.util.crypto
import me.msfjarvis.openpgpktx.util.OpenPgpUtils
sealed class GpgIdentifier {
data class KeyId(val id: Long) : GpgIdentifier()
data class UserId(val email: String) : GpgIdentifier()
companion object {
@OptIn(ExperimentalUnsignedTypes::class)
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 OpenPgpUtils.splitUserId(identifier).email?.let { UserId(it) }
}
}
}

View File

@ -0,0 +1,38 @@
package dev.msfjarvis.aps.util.crypto
import kotlin.test.Ignore
import kotlin.test.assertNotNull
import kotlin.test.assertTrue
import kotlin.test.Test
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("aps@msfjarvis.dev")
assertNotNull(identifier)
assertTrue { identifier is GpgIdentifier.UserId }
}
@Test
@Ignore("OpenKeychain can't yet handle these so we don't either")
fun `parses non-email user id`() {
val identifier = GpgIdentifier.fromString("john.doe")
assertNotNull(identifier)
assertTrue { identifier is GpgIdentifier.UserId }
}
}

View File

@ -6,7 +6,6 @@ package me.msfjarvis.openpgpktx.util
import android.content.Context
import android.content.Intent
import android.text.TextUtils
import java.io.Serializable
import java.util.Locale
import java.util.regex.Pattern
@ -64,7 +63,7 @@ public object OpenPgpUtils {
* See SplitUserIdTest for examples.
*/
public fun splitUserId(userId: String): UserId {
if (!TextUtils.isEmpty(userId)) {
if (userId.isNotEmpty()) {
val matcher = USER_ID_PATTERN.matcher(userId)
if (matcher.matches()) {
var name = if (matcher.group(1).isEmpty()) null else matcher.group(1)
@ -95,15 +94,15 @@ public object OpenPgpUtils {
*/
public fun createUserId(userId: UserId): String? {
val userIdBuilder = StringBuilder()
if (!TextUtils.isEmpty(userId.name)) {
if (!userId.name.isNullOrEmpty()) {
userIdBuilder.append(userId.name)
}
if (!TextUtils.isEmpty(userId.comment)) {
if (!userId.comment.isNullOrEmpty()) {
userIdBuilder.append(" (")
userIdBuilder.append(userId.comment)
userIdBuilder.append(")")
}
if (!TextUtils.isEmpty(userId.email)) {
if (!userId.email.isNullOrEmpty()) {
userIdBuilder.append(" <")
userIdBuilder.append(userId.email)
userIdBuilder.append(">")