diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpHelper.java index 016651c3b..0ff7d01de 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpHelper.java @@ -22,7 +22,10 @@ package org.sufficientlysecure.keychain.pgp; import java.util.regex.Matcher; import java.util.regex.Pattern; +import android.support.annotation.CheckResult; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; +import android.support.annotation.VisibleForTesting; import android.text.TextUtils; import org.sufficientlysecure.keychain.Constants; @@ -41,6 +44,7 @@ public class PgpHelper { public static final Pattern PGP_PUBLIC_KEY = Pattern.compile( ".*?(-----BEGIN PGP PUBLIC KEY BLOCK-----.*?-----END PGP PUBLIC KEY BLOCK-----).*", Pattern.DOTALL); + private static final Pattern KEYDATA_START_PATTERN = Pattern.compile("\\s(m[A-Q])"); /** * Fixing broken PGP MESSAGE Strings coming from GMail/AOSP Mail @@ -104,18 +108,85 @@ public class PgpHelper { } } - public static String getPgpKeyContent(@NonNull CharSequence input) { + public static String getPgpPublicKeyContent(@NonNull CharSequence input) { Log.dEscaped(Constants.TAG, "input: " + input); Matcher matcher = PgpHelper.PGP_PUBLIC_KEY.matcher(input); - if (matcher.matches()) { - String text = matcher.group(1); - text = fixPgpMessage(text); - - Log.dEscaped(Constants.TAG, "input fixed: " + text); - return text; + if (!matcher.matches()) { + return null; } - return null; + + String text = matcher.group(1); + text = fixPgpMessage(text); + text = reformatPgpPublicKeyBlock(text); + + // Log.dEscaped(Constants.TAG, "input fixed: " + text); + return text; + } + + @Nullable + @CheckResult + @VisibleForTesting + /* Reformats a public key block with messed up whitespace. This will strip headers in the process. */ + static String reformatPgpPublicKeyBlock(@NonNull String text) { + StringBuilder reformattedKeyBlocks = new StringBuilder(); + + /* + This method assumes that the base64 encoded public key data always starts with "m[A-Q]". + This holds based on a few assumptions based on the following observations: + + mA encodes 12 bits: 1001 1000 0000 + ... + mP encodes 12 bits: 1001 1000 1111 + mQ encodes 12 bits: 1001 1001 0000 + 1234 5678 + + The first bit is a constant 1, the second is 0 for old packet format. Bits 3 + through 6 encode the packet tag (constant 6 = b0110). Bits 7 and 8 encode the + length type of the packet, with a value of b00 or b01 referring to a 2- or + 3-octet header, respectively. The following four bits are part of the length + header. + + Thus we make the following assumptions: + - The packet uses the old packet format. Since the public key packet tag is available in the old format, + there is no reason to use the new one - implementations *could* do that, however. + - The first packet is a public key. + - The length is encoded as one or two bytes. + - If the length is encoded as one byte, the second character may be A through P (four length bits). + - If the length is encoded as two bytes, the second character is Q. This fixes the first four bits of + the length field to zero, limiting the length to 4096. + */ + + while (!text.isEmpty()) { + int indexOfBlock = text.indexOf("-----BEGIN PGP PUBLIC KEY BLOCK-----"); + int indexOfBlockEnd = text.indexOf("-----END PGP PUBLIC KEY BLOCK-----"); + if (indexOfBlock < 0 || indexOfBlockEnd < 0) { + break; + } + + Matcher matcher = KEYDATA_START_PATTERN.matcher(text); + if (!matcher.find()) { + Log.e(Constants.TAG, "Could not find start of key data!"); + break; + } + int indexOfPubkeyMaterial = matcher.start(1); + + String keyMaterial = text.substring(indexOfPubkeyMaterial, indexOfBlockEnd); + keyMaterial = keyMaterial.replaceAll("\\s+", "\n"); + + reformattedKeyBlocks.append("-----BEGIN PGP PUBLIC KEY BLOCK-----\n"); + reformattedKeyBlocks.append('\n'); + reformattedKeyBlocks.append(keyMaterial); + reformattedKeyBlocks.append("-----END PGP PUBLIC KEY BLOCK-----\n"); + + text = text.substring(indexOfBlockEnd +34).trim(); + } + + if (reformattedKeyBlocks.length() == 0) { + return null; + } + + return reformattedKeyBlocks.toString(); } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysFileFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysFileFragment.java index 9bf106788..2f8a2cc7a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysFileFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysFileFragment.java @@ -24,6 +24,7 @@ import android.net.Uri; import android.os.Bundle; import android.support.annotation.NonNull; import android.support.v4.app.Fragment; +import android.text.TextUtils; import android.view.LayoutInflater; import android.view.Menu; import android.view.MenuInflater; @@ -105,23 +106,29 @@ public class ImportKeysFileFragment extends Fragment { Uri.fromFile(Constants.Path.APP_DIR), "*/*", false, REQUEST_CODE_FILE); return true; case R.id.menu_import_keys_file_paste: - CharSequence clipboardText = ClipboardReflection.getClipboardText(getActivity()); - String sendText = ""; - if (clipboardText != null) { - sendText = clipboardText.toString(); - sendText = PgpHelper.getPgpKeyContent(sendText); - if (sendText == null) { - Notify.create(mActivity, R.string.error_bad_data, Style.ERROR).show(); - } else { - mCallback.loadKeys(new BytesLoaderState(sendText.getBytes(), null)); - } - } + importFromClipboard(); return true; } return super.onOptionsItemSelected(item); } + private void importFromClipboard() { + CharSequence clipboardText = ClipboardReflection.getClipboardText(getActivity()); + if (TextUtils.isEmpty(clipboardText)) { + Notify.create(mActivity, R.string.error_clipboard_empty, Style.ERROR).show(); + return; + } + + String keyText = PgpHelper.getPgpPublicKeyContent(clipboardText); + if (keyText == null) { + Notify.create(mActivity, R.string.error_clipboard_bad, Style.ERROR).show(); + return; + } + + mCallback.loadKeys(new BytesLoaderState(keyText.getBytes(), null)); + } + @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { switch (requestCode) { diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 13484be6e..7756b82e5 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -1648,6 +1648,7 @@ "Original file could not be deleted!" "Clipboard is empty!" "Error copying data to clipboard!" + "Could not read keys from clipboard!" "Error scanning fingerprint!" "Fingerprints did not match!" "Expiry date is in the past!" diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpHelperTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpHelperTest.java new file mode 100644 index 000000000..f6956349c --- /dev/null +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpHelperTest.java @@ -0,0 +1,98 @@ +package org.sufficientlysecure.keychain.pgp; + + +import java.io.ByteArrayInputStream; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.sufficientlysecure.keychain.KeychainTestRunner; +import org.sufficientlysecure.keychain.pgp.UncachedKeyRing.IteratorWithIOThrow; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertNotNull; + + +@SuppressWarnings("WeakerAccess") +@RunWith(KeychainTestRunner.class) +public class PgpHelperTest { + + static final String INPUT_KEY_BLOCK_TWO_OCTET_LENGTH = "-----BEGIN PGP PUBLIC KEY BLOCK----- Version: GnuPG v2 " + + "mQENBFnA7Y0BCAC+pdQ1mV9QguWvAyMsKiKkzeP5VxbIDUyQ8OBDFKIrZKZGTzjZ " + + "xvZs22j5pXWYlyDi4HU4+nuQmAMo6jxJMKQQkW7Y5AmRYijboLN+lZ8L28bYJC4o " + + "PcAnS3xlib2lE7aNK2BRUbctkhahhb2hohAxiXTUdGmfr9sHgZ2+B9sPTfuhrvtI " + + "9cFHZ5/0Wp4pLhLB53gduYeLuw4vVfUd7t0C4IhqB+t5HE+F3lolgya7hXxdH0ji " + + "oFNldCWT2qNdmmehIyY0WaoIrnUm2gVA4LMZ2fQTfsInpec5YT85OZaPEeBs3Opg " + + "3aGxV4mhXOxHkfREJRuYXcqL1s/UERGNprp9ABEBAAG0E01yLiBNYWxmb3JtZWQg " + + "QXNjaWmJAVQEEwEIAD4WIQTLHyFqT4uzqqEXR5Zz6vdoZVkMnwUCWcDtjQIbAwUJ " + + "A8JnAAULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAKCRBz6vdoZVkMnwpcB/985UbR " + + "qxG5pzaLGePdl+Cm6JBra2mC3AfMbJobjHR8YVufDw1tA6qyCMW0emHFi8t/rG8j " + + "tzPIadcl30rOzaGUTF85G5SqbwgAFHddZ01af36F6em0d5tY3+FCclQR7ynFPQlA " + + "+KB9k/M5X91ty6Q3/EAaXst5Uwh1WnKNC1js9RAcYL1s1MXKxg2iMmtE0DvwMAWq " + + "XRR+ADjzqkVdpdrzanTY7b72nuiGfe/H75b7/StIAfyxSZc5BU5535J0wF7Boz4p " + + "A6zRFXTphabmAE9FIKhgj5X7fbU64Hsrc5OkvWt4dF/6VRE4oXgUYwLKaDEH7A0k " + + "32GBGOkmnQGLKuqhuQENBFnA7Y0BCADKxQ1APSraxNKMpJv9vEVcXK3Sr91SYpGY " + + "s/ugYNio2xvIt9Qe2AjYGNSE9+wS6qLRxbbzucIRxl9jbn2QTNbJr7epLVdj3wtL " + + "JlkKsv13iao77Hg9WMvKh+NHpGoIFn4g5LOeYYG0QkZOvdu6b4Eg0RAryTLBh9jB " + + "eMLELkZTFDuQAgOSrVY0XgoURNcaDRtVarnVNBIO1N7/7TNXtmL22wR0wpqh4mKv " + + "vIvhE5itlIrthJHWzTcLDv5BHfyX23wqEpQFEffs1D72k9Ruh60OGgU0XAiVF654 " + + "WjgZCUoscPCLWcDGDOlcN7FpBxMDi1Ao3+7sLOMi9zES0InJ9q8LABEBAAGJATwE " + + "GAEIACYWIQTLHyFqT4uzqqEXR5Zz6vdoZVkMnwUCWcDtjQIbDAUJA8JnAAAKCRBz " + + "6vdoZVkMn3/+B/9B0vrDITV2FpdT+99WVsXJsoiXOsqLfv3WkC0l0RBAcCaUeXxp " + + "EqzyXQ0dVi6RoXu67dSnah6bdgfVnH14hJE8jc30GJ4QEpPD9kAKyodej15ledR5 " + + "sbdjeEfsavn9tvACJ0svfu8YVJjUjJLOj5axXy8wUBm5UvCdZuSL4EjPq7hXdq+j " + + "O/eTJGOfMl6hC4rRxRUbM+piZzbYcQ0lO3R2yPlEwzlO+asM9820V9bkviJUrXiY " + + "c5EX44mwFdhpXuHbRS18DJjCVcMhEsPG6rQ0Qy/6/dafow5HExRBmZl6ZkfjR2Lb " + + "alOZH0SNi47bvn6QKqKgiqT4f9mImyEDtSj/ =2V66 -----END PGP PUBLIC KEY BLOCK-----"; + static final String INPUT_KEY_BLOCK_ONE_OCTET_LENGTH = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + + "\n" + + "mFIEVk2iwRMIKoZIzj0DAQcCAwTBaWEpVYOfZDm85s/zszd4J4CBW8FesYiYQTeX\n" + + "5WMtwXsqrG5/ZcIgHNBzI0EvUbm/oSBFUJNk7RhmOk6MpS2gtAdNci4gRUNDiHkE\n" + + "ExMIACEFAlZNosECGwMFCwkIBwIGFQgJCgsCBBYCAwECHgECF4AACgkQt60Zc7T/\n" + + "SfQTPAD/bZ0ld3UyqAt8oPoHyJduGMkbur5KYoht1w/MMtiogG0BAN8Anhy55kTe\n" + + "H4VmMWxzK9M+kIFPzqEVHOzsuE5nhJOouFYEVk2iwRIIKoZIzj0DAQcCAwSvfTrq\n" + + "kkVeD0cVM8FZwhjTaG+B9wgk7yeoMgjIrSuZLiRjGAYC7Kq+6OiczduoItC2oMuK\n" + + "GpymTF6t+CmQpUfuAwEIB4hhBBgTCAAJBQJWTaLBAhsMAAoJELetGXO0/0n00BwA\n" + + "/2d1w/A4xMwfIFrKDwHeHALUBaIOuhF2AKd/43HujmuLAQDdcWf3h/0zjgBTjSoB\n" + + "bcVr5AE/huKUnwKYa7SP7wzoZg==\n" + + "=ou9N\n" + + "-----END PGP PUBLIC KEY BLOCK-----\n"; + + @Test + public void reformatPgpPublicKeyBlock() throws Exception { + String reformattedKey = PgpHelper.reformatPgpPublicKeyBlock(INPUT_KEY_BLOCK_TWO_OCTET_LENGTH); + + assertNotNull(reformattedKey); + UncachedKeyRing.decodeFromData(reformattedKey.getBytes()); + } + + @Test + public void reformatPgpPublicKeyBlock_consecutiveKeys() throws Exception { + String reformattedKey = PgpHelper.reformatPgpPublicKeyBlock( + INPUT_KEY_BLOCK_TWO_OCTET_LENGTH + INPUT_KEY_BLOCK_TWO_OCTET_LENGTH); + + assertNotNull(reformattedKey); + IteratorWithIOThrow uncachedKeyRingIteratorWithIOThrow = + UncachedKeyRing.fromStream(new ByteArrayInputStream(reformattedKey.getBytes())); + assertNotNull(uncachedKeyRingIteratorWithIOThrow.next()); + assertNotNull(uncachedKeyRingIteratorWithIOThrow.next()); + assertFalse(uncachedKeyRingIteratorWithIOThrow.hasNext()); + } + + @Test + public void reformatPgpPublicKeyBlock_shouldBeIdempotent() throws Exception { + String reformattedKey1 = PgpHelper.reformatPgpPublicKeyBlock(INPUT_KEY_BLOCK_TWO_OCTET_LENGTH); + assertNotNull(reformattedKey1); + + String reformattedKey2 = PgpHelper.reformatPgpPublicKeyBlock(reformattedKey1); + assertEquals(reformattedKey1, reformattedKey2); + } + + @Test + public void reformatPgpPublicKeyBlock_withOneOctetLengthHeader() throws Exception { + String reformattedKey = PgpHelper.reformatPgpPublicKeyBlock(INPUT_KEY_BLOCK_ONE_OCTET_LENGTH); + + assertNotNull(reformattedKey); + UncachedKeyRing.decodeFromData(reformattedKey.getBytes()); + } +} \ No newline at end of file