Add tests for PublicKeyParameterValidation

pgpainless-core/src/main/java/org/pgpainless/key/protection/UnlockSecretKey.java

@@ -12,6 +12,7 @@ import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor;
 import org.pgpainless.exception.KeyIntegrityException;
 import org.pgpainless.exception.WrongPassphraseException;
 import org.pgpainless.key.info.KeyInfo;
+import org.pgpainless.key.util.PublicKeyParameterValidationUtil;
 import org.pgpainless.util.Passphrase;
 
 public final class UnlockSecretKey {
@@ -50,6 +51,8 @@ public final class UnlockSecretKey {
             throw new PGPException("Cannot decrypt secret key.");
         }
 
+        PublicKeyParameterValidationUtil.verifyPublicKeyParameterIntegrity(privateKey, secretKey.getPublicKey());
+
         return privateKey;
     }
 

pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java

@@ -4,7 +4,12 @@
 
 package investigations;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 import java.io.IOException;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.NoSuchAlgorithmException;
 
 import org.bouncycastle.openpgp.PGPException;
 import org.bouncycastle.openpgp.PGPSecretKey;
@@ -12,8 +17,10 @@ import org.bouncycastle.openpgp.PGPSecretKeyRing;
 import org.junit.jupiter.api.Test;
 import org.pgpainless.PGPainless;
 import org.pgpainless.exception.KeyIntegrityException;
+import org.pgpainless.key.generation.type.rsa.RsaLength;
 import org.pgpainless.key.protection.SecretKeyRingProtector;
 import org.pgpainless.key.protection.UnlockSecretKey;
+import org.pgpainless.key.util.KeyIdUtil;
 import org.pgpainless.util.Passphrase;
 
 public class ModifiedPublicKeysInvestigation {
@@ -123,35 +130,154 @@ public class ModifiedPublicKeysInvestigation {
             "=lI+G\n" +
             "-----END PGP PRIVATE KEY BLOCK-----\n";
 
-    @Test
+    // created with exploit code by cure53.de
-    public void investigate() throws IOException {
+    private static final String INJECTED_KEY = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" +
-        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("12345678"));
+            "Version: PGPainless\n" +
+            "Comment: F594 D7CC E7D0 1F15 1511  4395 C075 FD34 4B2A D41A\n" +
+            "Comment: Juliet <juliet@montague.lit>\n" +
+            "\n" +
+            "lQPGBGGx9LYBCACxNiT7XMd6WXuZFJQ1RaQXixA+rw/VRiDueNUAkNs0BkJ92qpe\n" +
+            "y5ljEiY2QY8O6hXxY7b2KF77jiuejGgz962+bIEhumtyPyN4oIML7tVWSyjN5pOd\n" +
+            "bySAuw25752vB8Z0sDkENhidBF5yeNqeayqZOHvn1eFZsvJ2yV7J/k/H+5v1L2rH\n" +
+            "CoO/lttbnnAH3cGqp9FkerejlE5HGld/LKhc2ViTayjJFWGAaAqpQJNYMRtJLTVG\n" +
+            "c2jfYGv0j6na8/K118b/wqfKNU9O2/lzu0EpaJu6TpWZ02TeVht9NRnan6cmXH/Q\n" +
+            "6yxJQLRn4a1MiAyYIoWs7BpBLAxh+Za/btAFABEBAAH+CQMCIJAO8LIQWNtgQSLB\n" +
+            "YNMXgafrSc+E871154/yYeMPtn739smaedfrnI5nxHnuWb0pXLrfkWUIMjUwxnZ+\n" +
+            "ktrVekc+RLhRWJqbO42/qdNzPlNwZFZXK6VRdT8DI23pOXkQiS7kXP3RwJzFw21q\n" +
+            "emkOv26ZCWjnB4p/R4zpEYEAMtPDpdrIzlLU5laT9ashWDMUBA3ZSI9tH+jTmR+h\n" +
+            "wvjVAXRNPf4Dhyh3sadorHHqgC8v6B5/Ou5fQoSIIn6FKB4lpvnXy/R/Gp8AhmRo\n" +
+            "99Fdk1/XxovBVnTExO9upe/JVvu4XIQM+OxTdMyzCoFvoOGb+xhC2h/HrByouY+k\n" +
+            "umSCq3XMQ4GVednXufWpFu3gapf2bnCzkgczwHrrXR6B3sNgXyKhS2yCPHfLsoFE\n" +
+            "pkfpBQSSh5vpz5s5JWxdYWr415g8sUyva0XSwJN8QvzcLNgUBuSTDuSxGJ3j+ojl\n" +
+            "E2isGyo0BhJkKp90bVnpBDJD2HAsHgFns5fA1xHuLzz86kEZwH852nhXQ64SzoJZ\n" +
+            "Uy2+sA8afEkLm+YwcVj+6O6Kki4fqeEHVGB4aRSYbVWSLAuBfZqdIHR25xp1mp1J\n" +
+            "lBFeo4F3/mXtY1fO5RrROmEsDz5O018jDbB7ZeLlROPV3s/F7Dvl6LC8omOHsinw\n" +
+            "cV+FBEIXodrHvX55Lo+bxMDTXkQlaZjUvoce+GY0Sy1b/p2BMccG3DCfH7JH3VcM\n" +
+            "cLpvLg+w0/o3mxEoSkCgohAcge1V4yUXDDGXNVs3UNvtzNemMjlUX6jlcC2WBeUu\n" +
+            "YW0U9MxqI7pPx8+kXp0hzakr1ESRL65nFQoVYk0t8Jp4fQN79wOxt7JMnwHJ/ftg\n" +
+            "y7czM2WdCHmxQgaBXX8SodKrwHfbm4armomgUCKBlcytQstMghfhNwUhu8F1l/Wt\n" +
+            "9x8vyuVWUdpUtBxKdWxpZXQgPGp1bGlldEBtb250YWd1ZS5saXQ+iQFNBBMBCgBB\n" +
+            "BQJhsfS2CZDAdf00SyrUGhahBPWU18zn0B8VFRFDlcB1/TRLKtQaAp4BApsDBZYC\n" +
+            "AwEABIsJCAcFlQoJCAsCmQEAADvLB/9x66wNnA0O5MTIFIYf3HkMceHHq1eVgx88\n" +
+            "NgItRXQh4Bg90C96SY6NWwoTkcZTGsmymNuuAzhCJwXFUr+mnoBODC6Qhoo4qr9D\n" +
+            "vip1ekIGZUVGGRLQK6LHYtvQVKTVV4yih3CtrnP7jpN7lBVaTLCvhXqG1Ebez99Y\n" +
+            "ne1DbHvIzHv6l9pf2rlUf8A6I8iBlPjWe3DLVKaMI3RjfMuRFI32UYnc+bBdcpVR\n" +
+            "XYhXNrwj2OSTyplSBDAJfrIG5Kp+YD9Vip70csR+hZviNvyv7b/I9qfTbZw/RWBR\n" +
+            "P6k/8mWU66NCnP4H4vqf5wak91T/KMI59rLRl8h4oIAXtSBHYGF1nKkEYbH0thII\n" +
+            "KoZIzj0DAQcCAwRrGeShPKqoZYAey4qDWnMmMK//UAfP83Sf+hryPzpVa+/ywD0+\n" +
+            "b+lU6W5AKoK6/9AySYE02XQdC8UawAhA9CtcAwEIB/4JAwIgkA7wshBY22DxMfI0\n" +
+            "y3FeCOMZhTmZRkB1UgFWXeYGyd6gKI5jYFQyRCeogVZDven2aGzWiyEey+j20NbZ\n" +
+            "KcS0S/YUvOIIDYN2wU+2yHG4iQEzBBgBCgAdBQJhsfS2Ap4BApsMBZYCAwEABIsJ\n" +
+            "CAcFlQoJCAsACgkQDG4T6nONbigLjwf9EjyKrAhdrznmC2+vVoJSq9DHqLtpiGid\n" +
+            "b3ImJ5REKzXs8JVyyRLj0dQMOx+D6lA5xmxMjMKAFu+QKXFv1khDQofz3x+GbHDu\n" +
+            "Q9jROzUpErcXmTHinRE3lA2ogd0uPbQcVvG8zBxy4GuEZgXoEgYHawijnXpTdNeh\n" +
+            "oeLWpnx/3UQlNbQR8oSj2InG96C8fHyOBEkGdY2KweI/BU+7ui3JfSoHuOiWnsa3\n" +
+            "d0bptkmD7d3grIuq8oHZBkOCPOYkZbYY4WFh5L01W95Hrzf1yEjqyzvVatpiSrMK\n" +
+            "JIsbPcS2yyN9uXP54vwsq7D/mx6CMV1XcpZGwsT8o35Txa00MoXRo5ypBGGx9LYS\n" +
+            "CCqGSM49AwEHAgME8pVmU/csKSjqhvuJ0siOaf1K91BWQ4/piZ3Fv3zrcrk2sl15\n" +
+            "ThOU0OyvPnUf77yDrW5NRv2gnFDQNpQq2x3spAMBCAf+CQMCIJAO8LIQWNtgb47o\n" +
+            "8lBl7RalDXipU01bB59q2wqHVKvF3+fPQ6+c0WdT6ZxZKmV4MXnaVpx4kDozYSf8\n" +
+            "E7Sj7PwFdUGcP2/i8Y/NYlRErIkBMwQYAQoAHQUCYbH0tgKeAQKbDAWWAgMBAASL\n" +
+            "CQgHBZUKCQgLAAoJEAxuE+pzjW4oKb0H/ictFOa+m1gbxuQ4WW81/Yxt4sprsa0p\n" +
+            "rf7KmUAKnChcFEswtmgfFltqgo6CZssWm9bp/bsqksONUHDF1ElU1kiwKQdEau38\n" +
+            "Ufj7tzdBmlZuFAokHnTG+pQpyJP0w/unFZD++QU7hjXN4if/q3q2kZ+JvYpCQ1yI\n" +
+            "mzUkYkTbP94PBsVO7SDWnFHsvGefXwacYvV/W+OvRLFuQVR53xqbn6wGtD61t8nD\n" +
+            "XIFyxOECyp+E22nkeeI3betGSq0LeExPbjEUpVWWhZ4Rt2UkkmaME9V717vl5x4s\n" +
+            "L24DZ9kR5ToqBF682oWOXe4H18WLeBQqCI7jpx/Mx95oC+Xsm7F/K4qcqQRhsfS2\n" +
+            "EggqhkjOPQMBBwIDBAc4vOQ08Z6IDj/7JSKomFsJtE++n1Bb22QdiQWnrQ/t0B2Y\n" +
+            "53woGsMh+KYDInE2XET7xpl5Ufscy2X3AMnFZlEDAQgH/gkDArgFuyIOfkRHYHi3\n" +
+            "iHCxic2RPt8FkLlQMjg66rKPv7sAye1tJG0QeEBOvDeTq0f64OddF7BuBe5t/wUg\n" +
+            "qABg8nj8tku9Tj8vUjnowraha5+JATMEGAEKAB0FAmGx9LYCngECmwwFlgIDAQAE\n" +
+            "iwkIBwWVCgkICwAKCRDAdf00SyrUGmfyB/9aVNKuDTH6yRZWPYoypA6UCChvJb38\n" +
+            "K4aW2DexljtmuA7i4lbomFkltbbEiZOw2+Q1uan2gVrhwPIh9aRFZH4H4djlVzEh\n" +
+            "Xg4G51N548Ye4xWHw7LLttoRghfUB4skgAxvuj5eBRfQJBM4/qm1V3QXGkbYPKuD\n" +
+            "QDgApRZNt5Cal4uD+dj7rxhq+RUC0KbFKMXQoGtgqeeKZ0AtLgjxDR/7NXo21YS2\n" +
+            "6hEQTtowHm3gFQPyC9LHZbnlp6lmz3gVTeR79kQkbwGjeFZtnbVboSwIYuurq4vc\n" +
+            "gHJaPa2iv3d1AmhtqLXIGfVPuW5+ldPDIeXGcVa1QWy+tPqf+d2V0O3SnKkEYbH0\n" +
+            "thIIKoZIzj0DAQcCAwSdwn1X0Iad1ljcVzuhCLfQgmfe6vslc/DzTrXK9zM/JeZ7\n" +
+            "pYQZybmkIqVr+azNDGR5A1queAf9Z6jgbPSR2uQlAwEIB/4JAwK4BbsiDn5ER2Aj\n" +
+            "XvTPCUUX6hL8kG3mybW/Y9B3GzMSAUjYm0waLsvmu8f/miOqZ/sprMQKhpFE76f7\n" +
+            "1NvDh2ZjMwVO3BOs2PRfnAOPE8KXiQEzBBgBCgAdBQJhsfS2Ap4BApsMBZYCAwEA\n" +
+            "BIsJCAcFlQoJCAsACgkQwHX9NEsq1BoVPQf/QY4yo51KqtpxxZ3DGc+A11kcQC2M\n" +
+            "GSJR7kMAlGY/wMhjVVVhdvU0d1tNUI//qil/xjdCHggGnIC0k6Gn64j2KDbwGn/n\n" +
+            "ptCO7X4w9r/dHjWe0s7OSVBKs8fF7/7gX3eejQ3IXV6CrwIZ1nP5Ugd5ywX2ciLE\n" +
+            "T1bWeJWPaNV+dz+ZzZqgd/vM8dmDUw3bfolJRdxdRIzJyq6TgdG/U8Ae2TkGEHyM\n" +
+            "a3ZBwq51y2Y6z9WuangJ00RFjnQOZvXsueJKepLPTioo4TJXaYkD7VOYNxFIJQPt\n" +
+            "7Yv10ZA5XaaMrWtWG6vei9Ji53/bYNRVqs5jcNS168zeMOYgwrEaDbpU3g==\n" +
+            "=WEYQ\n" +
+            "-----END PGP PRIVATE KEY BLOCK-----";
 
+    @Test
+    public void assertModifiedDSAKeyThrowsKeyIntegrityException() throws IOException {
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("12345678"));
         PGPSecretKeyRing dsa = PGPainless.readKeyRing().secretKeyRing(DSA);
+
+        assertThrows(KeyIntegrityException.class, () ->
+                UnlockSecretKey.unlockSecretKey(dsa.getSecretKey(KeyIdUtil.fromLongKeyId("b1bd1f049ec87f3d")), protector));
+        assertThrows(KeyIntegrityException.class, () ->
+                UnlockSecretKey.unlockSecretKey(dsa.getSecretKey(KeyIdUtil.fromLongKeyId("f5ffdf6d71dd5789")), protector));
+    }
+
+    @Test
+    public void assertModifiedElGamalKeyThrowsKeyIntegrityException() throws IOException {
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("12345678"));
         PGPSecretKeyRing elgamal = PGPainless.readKeyRing().secretKeyRing(ELGAMAL);
 
-        // CHECKSTYLE:OFF
+        assertThrows(KeyIntegrityException.class, () ->
-        for (PGPSecretKey secretKey : dsa) {
+                UnlockSecretKey.unlockSecretKey(elgamal.getSecretKey(KeyIdUtil.fromLongKeyId("f5ffdf6d71dd5789")), protector));
-            try {
+    }
-                UnlockSecretKey.unlockSecretKey(secretKey, protector);
+
-                System.out.println("No KeyIntegrityException for dsa key " + Long.toHexString(secretKey.getKeyID()));
+    @Test
-            } catch (KeyIntegrityException e) {
+    public void assertInjectedKeyRingFailsToUnlockPrimaryKey() throws IOException {
-                System.out.println("KeyIntegrityException for dsa key " + Long.toHexString(secretKey.getKeyID()));
+        PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(INJECTED_KEY);
-            } catch (PGPException e) {
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("pass"));
-                System.out.println("Cannot unlock dsa key: " + e.getMessage());
+
+        assertThrows(KeyIntegrityException.class, () ->
+                UnlockSecretKey.unlockSecretKey(secretKeys.getSecretKey(), protector));
+    }
+
+    @Test
+    public void assertCannotUnlockElGamalPrimaryKeyDueToDummyS2K() throws IOException {
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("12345678"));
+        PGPSecretKeyRing elgamal = PGPainless.readKeyRing().secretKeyRing(ELGAMAL);
+
+        assertThrows(PGPException.class, () ->
+                UnlockSecretKey.unlockSecretKey(elgamal.getSecretKey(KeyIdUtil.fromLongKeyId("5f04acf44fd822b1")), protector));
+    }
+
+    @Test
+    public void assertUnmodifiedRSAKeyDoesNotThrow() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException {
+        PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing()
+                .simpleRsaKeyRing("Unmodified", RsaLength._4096, "987654321");
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("987654321"));
+
+        for (PGPSecretKey secretKey : secretKeys) {
+            assertDoesNotThrow(() ->
+                    UnlockSecretKey.unlockSecretKey(secretKey, protector));
         }
     }
 
-        for (PGPSecretKey secretKey : elgamal) {
+    @Test
-            try {
+    public void assertUnmodifiedECKeyDoesNotThrow() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException {
-                UnlockSecretKey.unlockSecretKey(secretKey, protector);
+        PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing()
-                System.out.println("No KeyIntegrityException for elgamal key " + Long.toHexString(secretKey.getKeyID()));
+                .simpleEcKeyRing("Unmodified", "987654321");
-            } catch (KeyIntegrityException e) {
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("987654321"));
-                System.out.println("KeyIntegrityException for elgamal key " + Long.toHexString(secretKey.getKeyID()));
+
-            }catch (PGPException e) {
+        for (PGPSecretKey secretKey : secretKeys) {
-                System.out.println("Cannot unlock elgamal key: " + e.getMessage());
+            assertDoesNotThrow(() ->
+                    UnlockSecretKey.unlockSecretKey(secretKey, protector));
         }
     }
-        // CHECKSTYLE:ON
+
+    @Test
+    public void assertUnmodifiedModernKeyDoesNotThrow() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException {
+        PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing()
+                .modernKeyRing("Unmodified", "987654321");
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("987654321"));
+
+        for (PGPSecretKey secretKey : secretKeys) {
+            assertDoesNotThrow(() ->
+                    UnlockSecretKey.unlockSecretKey(secretKey, protector));
+        }
     }
 }