GPG: fix reading unprotected old-format secret keys

Fix code and add a test case. The old code passed on the original input
stream, which has already been consumed.

Bug: 570501
Change-Id: I81f60698ce42443df57e59b1d1ab155574136fa8
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit.gpg.bc.test/tst-rsrc/org/eclipse/jgit/gpg/bc/internal/keys/AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11.asc b/org.eclipse.jgit.gpg.bc.test/tst-rsrc/org/eclipse/jgit/gpg/bc/internal/keys/AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11.asc
new file mode 100644 (file)
index 0000000..f412019
--- /dev/null
+++ b/org.eclipse.jgit.gpg.bc.test/tst-rsrc/org/eclipse/jgit/gpg/bc/internal/keys/AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11.asc
@@ -0,0 +1,30 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mQENBF7EL8wBCADO46xh7nXn7vZ5ow2Zdrp7WTh9BlT2wtaHNKpnKvSoYHjJbbGz
+yF8Jf/qVPuXNbjx2df1lT7zT7x3evcjQoNy80deftCw8ApZB9RMOo3uUIqS2VpO+
+cS9rjTgBRFL6xDv3g4++CE9s+5dKE9gKkwleZ5/tVqUIoHPAIUEjpcPHngi5m2bi
+tSmQUYWLGcliR1E79sJMSzPt1neksqHFMJ1KTEJLAABZ0t3PiBzmycIQWThX3uU/
+lcgnZmmhWCJIqV0yRZqxl61ejUfq+zK0T7MzhAAugqe7D6BM1FRwZRNCHwDQXIvt
+/t3fczTe+x9oTy4qX4MfaP8lHM0223MwGR13ABEBAAG0H0EgVSBUaG9yIDxhLnUu
+dGhvckBleGFtcGxlLm9yZz6JAU4EEwEKADgWIQQILQAv4wNQfEJ6I/NEWemKCmiQ
++wUCXsQvzAIbAwULCQgHAwUVCgkICwUWAgMBAAIeAQIXgAAKCRBEWemKCmiQ+xev
+CACZSWh4xjTgafwGMP9RnReOhubVmfHS+XGlidDQzJDtshDQddPZ3oQwyTe3OgkW
+ZgOrzjrHGsZp3WZmGUZejrKt2Brqp+h+VRujFVcKk4N9A52BkM6OeT9lzBabOpuA
+UaDsNMSFsMcGTTYpB16+sDcyui8LW1jGi1y+8aQa+u1lIk/vVycq8o4htn2Af8xZ
+rAT8peapsjoNjETEs8OQ0al3Q0UX9amW6Rq1zZZ0XtoXDCPTI01EfczDMN+AZoFk
+UYHwSREDFLSh+c+q1HhYp4TqP+2a5Rayna//n7zci1PmSX7zD3iWzV1jEQ3Jm8U3
+DY+P/WLezQdSJIBVCFpCualquQENBF7EL8wBCAC+ef+vNvfu1jl9BXpu6K9PG0I5
+DQfrNtcdPq90O32ipvsYvqGOJX9MHoTyxBPLew+e5UsYb3ex62JyJqdAaqSwYXEN
+MBESZx7yBqBMUvildfh8dowbJeblxCf5KsE4C9uNfg4ApWGD7PjVsUCh47V8VcfG
+ymCxxq80r+4GfFtt/HC+l9fPUnDLuXpAWEM2GPUzcauUoEXxZK6nhstYCRlKlQcK
+Tn+LtCC7SGpYlqvwWBzAnOYP9+eZfSJ897g0AiTEhK0JsBlDAb3UAWHYHkAkVa1+
+oU/UedhPC4j2Q7RzPQFMun6aGkaDrntCxvT7IFiMplPG7iy0JDd6ubrWSzivABEB
+AAGJATYEGAEKACAWIQQILQAv4wNQfEJ6I/NEWemKCmiQ+wUCXsQvzAIbDAAKCRBE
+WemKCmiQ+xoBB/9BAmlHQUmVl/bkwszAcyXkR5HsyA4htMJt+6GKlqftuhLP0SGK
+Il+7GeK6NqNdQXxXG5Wj6dn7ZqWalQRA0evEa6VLH+74zrn0llWfzTPIcP1bHW7l
+uYaOzZ1z/q4FoEGNJxp/jdToZ4970OXLzqY/G/QlMJIlXWCC0EXNYbKCEpOE9uvW
+h4kWe5xeGOmhZylYbzurTDzqEtKy+LZ9f2xNYn6ElcWtwxsxwSY7L9B3eNcCYE46
+Np6uqzPffB9s7PHW46yEL1lQs6ME+9hBGyjeVop+Wg9qkh3YCrp+KY5Vkmdndwkn
+Th4FnTpcCiS06fCVHHC5kelh+H6TgRA+XQ/V
+=WGUq
+-----END PGP PUBLIC KEY BLOCK-----

diff --git a/org.eclipse.jgit.gpg.bc.test/tst-rsrc/org/eclipse/jgit/gpg/bc/internal/keys/AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11.key b/org.eclipse.jgit.gpg.bc.test/tst-rsrc/org/eclipse/jgit/gpg/bc/internal/keys/AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11.key
new file mode 100644 (file)
index 0000000..b8765aa
Binary files /dev/null and b/org.eclipse.jgit.gpg.bc.test/tst-rsrc/org/eclipse/jgit/gpg/bc/internal/keys/AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11.key differ

diff --git a/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java b/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java
index 4eecaf3ab5a03d26f2006078e82f2b0cedd80b99..5e5e303319564ce6546bc00ad9d3ae7e31f638b3 100644 (file)
--- a/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java
+++ b/org.eclipse.jgit.gpg.bc.test/tst/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeysTest.java
@@ -76,9 +76,12 @@ public class SecretKeysTest {
 
                final boolean encrypted;
 
-               TestData(String name, boolean encrypted) {
+               final boolean keyValue;
+
+               TestData(String name, boolean encrypted, boolean keyValue) {
                        this.name = name;
                        this.encrypted = encrypted;
+                       this.keyValue = keyValue;
                }
 
                @Override
@@ -90,10 +93,11 @@ public class SecretKeysTest {
        @Parameters(name = "{0}")
        public static TestData[] initTestData() {
                return new TestData[] {
-                               new TestData("2FB05DBB70FC07CB84C13431F640CA6CEA1DBF8A", false),
-                               new TestData("66CCECEC2AB46A9735B10FEC54EDF9FD0F77BAF9", true),
-                               new TestData("F727FAB884DA3BD402B6E0F5472E108D21033124", true),
-                               new TestData("faked", false) };
+                               new TestData("AFDA8EA10E185ACF8C0D0F8885A0EF61A72ECB11", false, false),
+                               new TestData("2FB05DBB70FC07CB84C13431F640CA6CEA1DBF8A", false, true),
+                               new TestData("66CCECEC2AB46A9735B10FEC54EDF9FD0F77BAF9", true, true),
+                               new TestData("F727FAB884DA3BD402B6E0F5472E108D21033124", true, true),
+                               new TestData("faked", false, true) };
        }
 
        private static byte[] readTestKey(String filename) throws Exception {
@@ -126,9 +130,11 @@ public class SecretKeysTest {
 
        @Test
        public void testKeyRead() throws Exception {
-               byte[] bytes = readTestKey(data.name + ".key");
-               assertEquals('(', bytes[0]);
-               assertEquals(')', bytes[bytes.length - 1]);
+               if (data.keyValue) {
+                       byte[] bytes = readTestKey(data.name + ".key");
+                       assertEquals('(', bytes[0]);
+                       assertEquals(')', bytes[bytes.length - 1]);
+               }
                try (InputStream pubIn = this.getClass()
                                .getResourceAsStream(data.name + ".asc")) {
                        if (pubIn != null) {
@@ -139,7 +145,9 @@ public class SecretKeysTest {
                                try (InputStream in = new BufferedInputStream(this.getClass()
                                                .getResourceAsStream(data.name + ".key"))) {
                                        PGPSecretKey secretKey = SecretKeys.readSecretKey(in,
-                                                       calculatorProvider, () -> "nonsense".toCharArray(),
+                                                       calculatorProvider,
+                                                       data.encrypted ? () -> "nonsense".toCharArray()
+                                                                       : null,
                                                        publicKey);
                                        assertNotNull(secretKey);
                                } catch (PGPException e) {

diff --git a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java
index 1542b8cbcca8a748733614c108b64696311f02ab..269a1ba0f687f87b96bafc61dda120218f001386 100644 (file)
--- a/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java
+++ b/org.eclipse.jgit.gpg.bc/src/org/eclipse/jgit/gpg/bc/internal/keys/SecretKeys.java
@@ -113,13 +113,13 @@ public final class SecretKeys {
                try {
                        if (firstChar == '(') {
                                // Binary format.
-                               if (!matches(data, 4, PROTECTED_KEY)) {
-                                       // Not encrypted binary format.
-                                       return parser.parseSecretKey(in, null, publicKey);
+                               PBEProtectionRemoverFactory decryptor = null;
+                               if (matches(data, 4, PROTECTED_KEY)) {
+                                       // AES/CBC encrypted.
+                                       decryptor = new JcePBEProtectionRemoverFactory(
+                                                       passphraseSupplier.getPassphrase(),
+                                                       calculatorProvider);
                                }
-                               // AES/CBC encrypted.
-                               PBEProtectionRemoverFactory decryptor = new JcePBEProtectionRemoverFactory(
-                                               passphraseSupplier.getPassphrase(), calculatorProvider);
                                try (InputStream sIn = new ByteArrayInputStream(data)) {
                                        return parser.parseSecretKey(sIn, decryptor, publicKey);
                                }