fix and test for bad certificate version numbers (#1012)

closes #1012

4 files changed, 47 insertions, 7 deletions

diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
index 34c637abf..89fefb767 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java
@@ -34,6 +34,7 @@ import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockPar
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.util.Iterator;
 
 @RunWith(RobolectricTestRunner.class)
@@ -129,4 +130,20 @@ public class UncachedKeyringTest {
         pubRing.extractPublicKeyRing();
     }
 
+    @Test(expected = IOException.class)
+    public void testBrokenVersionCert() throws Throwable {
+        // this is a test for one of the patches we use on top of stock bouncycastle, which
+        // returns an IOException rather than a RuntimeException in case of a bad certificate
+        // version byte
+        readRingFromResource("/test-keys/broken_cert_version.asc");
+    }
+
+    UncachedKeyRing readRingFromResource(String name) throws Throwable {
+        try {
+            return UncachedKeyRing.fromStream(UncachedKeyringTest.class.getResourceAsStream(name)).next();
+        } catch (RuntimeException e) {
+            throw e.getCause();
+        }
+    }
+
 }
diff --git a/OpenKeychain-Test/src/test/resources/test-keys/broken_cert_version.asc b/OpenKeychain-Test/src/test/resources/test-keys/broken_cert_version.asc
new file mode 100644
index 000000000..e2d2abd8e
--- /dev/null
+++ b/OpenKeychain-Test/src/test/resources/test-keys/broken_cert_version.asc
@@ -0,0 +1,17 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mQENBFSl5DIBCADqsGJJ8RhV4Uw6a3Q48QyTMrUtvZquOTlLVaqOdEFZNL5/OBal
+prft9LNkcOkIVA89Hdn715WwkmG2OJiJoQ/ZAKwal1CPGm4Q8kZIM7k57ISJL6J5
+300e7UIznc74XbG7eFNxNcjCM9wG12vW2rFwc+ogJtkBSf0IXukPwtUkRK+H5ufO
+lpqS5NNZfiGbNQCrb+YsGZNRk4QTGR6WGyaIRHlcG8G00VPGNSauTqe/11MO9MoF
+BvPgFeur3nefWunCQ+uDmzIEs8r94gaHu3LWbctd5w5x/o/PDfTSSiO+U8zzXrKC
+4ZpEl5bk7t7jH1hYMLWyO6nn0vWTOMO1EYLBABEBAAG0GGJyb2tlbiBzaWduYXR1
+cmUgdmVyc2lvbokBOMATAQIAIgUCVKXkMgIbAwYLCQgHAwIGFQgCCQoLBBYCAwEC
+HgECF4AACgkQDe00lH/2SnprLggAh64TsdHDfIhTNc1DeJLCuvuHsitAcUdEEnue
+yJjodxboKNSplIwnmb5CpM3P8f736dNaW77Yd6aO4IeAy6cBlxT1tSRkJMsp+cBt
+kBa3lRr+GnWZlLZs3coL2g0t5RbuyYKyQxm2qvgFJGi/7Qfty5nJOW5U1ElT3VT8
+jISNdQdDAIaBsCE+TuyW3VsP3PqnJ7x14K7VhkFuCyvYB9paLcJBnan93R0Ja0Ip
+Cv1pbrNxXp0UELf0RYc2X5C1m6otZ9LKf3PmzxlEkApkb1TZUEBak2Za5p99koZT
++pg/XpZPyawi+gZeYkBAohxRGmzG/a4L+YacAZHbchfN0eG7lg==
+=mxTR
+-----END PGP PUBLIC KEY BLOCK-----
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
index c4cd0b3e5..006af6073 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -143,17 +143,22 @@ public class UncachedKeyRing {
             throw new PgpGeneralException("Object not recognized as PGPKeyRing!");
         }
 
-        UncachedKeyRing ring = parsed.next();
+        try {
+            UncachedKeyRing ring = parsed.next();
 
-        if (parsed.hasNext()) {
-            throw new PgpGeneralException("Expected single keyring in stream, found at least two");
-        }
+            if (parsed.hasNext()) {
+                throw new PgpGeneralException("Expected single keyring in stream, found at least two");
+            }
 
-        return ring;
+            return ring;
 
+        } catch (RuntimeException e) {
+            // yes this is bad style. we should rework this in a better way
+            throw new PgpGeneralException(e.getCause());
+        }
     }
 
-    public static Iterator<UncachedKeyRing> fromStream(final InputStream stream) throws IOException {
+    public static Iterator<UncachedKeyRing> fromStream(final InputStream stream) {
 
         return new Iterator<UncachedKeyRing>() {
 
@@ -190,7 +195,8 @@ public class UncachedKeyRing {
                         mObjectFactory = null;
                     }
                 } catch (IOException e) {
-                    Log.e(Constants.TAG, "IOException while processing stream. ArmoredInputStream CRC check failed?", e);
+                    throw new RuntimeException(e);
+                    // Log.e(Constants.TAG, "IOException while processing stream. ArmoredInputStream CRC check failed?", e);
                 } catch (ArrayIndexOutOfBoundsException e) {
                     Log.e(Constants.TAG, "ArmoredInputStream decode failed, symbol is not in decodingTable!", e);
                 }
diff --git a/extern/spongycastle b/extern/spongycastle
-Subproject 375084d55341b575274e49d9a69fa4cf9356682
+Subproject 1d9ee197d8fcc18dcdd1ae9649a5dc53e910b18