From 90b7a0f4f179e3b8df7be2c81a90354ace7955e8 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 19 Sep 2017 15:02:40 +0200 Subject: [PATCH] handle one-octet length headers in public key block reformatting --- .../keychain/pgp/PgpHelper.java | 38 ++++++++++++++++++- .../keychain/pgp/PgpHelperTest.java | 32 +++++++++++++--- 2 files changed, 63 insertions(+), 7 deletions(-) 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 5e7bc6fb6..0ff7d01de 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpHelper.java @@ -44,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 @@ -126,17 +127,50 @@ public class PgpHelper { @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 indexOfPubkeyMaterial = text.indexOf("mQ", indexOfBlock); int indexOfBlockEnd = text.indexOf("-----END PGP PUBLIC KEY BLOCK-----"); - if (indexOfBlock < 0 || indexOfPubkeyMaterial < 0 || indexOfBlockEnd < 0) { + 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"); diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpHelperTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpHelperTest.java index 340dd7ce9..f6956349c 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpHelperTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpHelperTest.java @@ -17,7 +17,7 @@ import static junit.framework.Assert.assertNotNull; @RunWith(KeychainTestRunner.class) public class PgpHelperTest { - static final String INPUT_KEY_BLOCK = "-----BEGIN PGP PUBLIC KEY BLOCK----- Version: GnuPG v2 " + + static final String INPUT_KEY_BLOCK_TWO_OCTET_LENGTH = "-----BEGIN PGP PUBLIC KEY BLOCK----- Version: GnuPG v2 " + "mQENBFnA7Y0BCAC+pdQ1mV9QguWvAyMsKiKkzeP5VxbIDUyQ8OBDFKIrZKZGTzjZ " + "xvZs22j5pXWYlyDi4HU4+nuQmAMo6jxJMKQQkW7Y5AmRYijboLN+lZ8L28bYJC4o " + "PcAnS3xlib2lE7aNK2BRUbctkhahhb2hohAxiXTUdGmfr9sHgZ2+B9sPTfuhrvtI " + @@ -44,10 +44,23 @@ public class PgpHelperTest { "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); + String reformattedKey = PgpHelper.reformatPgpPublicKeyBlock(INPUT_KEY_BLOCK_TWO_OCTET_LENGTH); assertNotNull(reformattedKey); UncachedKeyRing.decodeFromData(reformattedKey.getBytes()); @@ -55,7 +68,8 @@ public class PgpHelperTest { @Test public void reformatPgpPublicKeyBlock_consecutiveKeys() throws Exception { - String reformattedKey = PgpHelper.reformatPgpPublicKeyBlock(INPUT_KEY_BLOCK + INPUT_KEY_BLOCK); + String reformattedKey = PgpHelper.reformatPgpPublicKeyBlock( + INPUT_KEY_BLOCK_TWO_OCTET_LENGTH + INPUT_KEY_BLOCK_TWO_OCTET_LENGTH); assertNotNull(reformattedKey); IteratorWithIOThrow uncachedKeyRingIteratorWithIOThrow = @@ -67,10 +81,18 @@ public class PgpHelperTest { @Test public void reformatPgpPublicKeyBlock_shouldBeIdempotent() throws Exception { - String reformattedKey1 = PgpHelper.reformatPgpPublicKeyBlock(INPUT_KEY_BLOCK); - String reformattedKey2 = PgpHelper.reformatPgpPublicKeyBlock(reformattedKey1); + 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