From 35462ab539d14c783f7cfe8ff231e08f857892ea Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 9 Dec 2021 13:25:23 +0100 Subject: [PATCH] Add tests for PublicKeyParameterValidation --- .../key/protection/UnlockSecretKey.java | 3 + .../ModifiedPublicKeysInvestigation.java | 174 +++++++++++++++--- 2 files changed, 153 insertions(+), 24 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/UnlockSecretKey.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/UnlockSecretKey.java index 71b9265b..42a5b9b7 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/protection/UnlockSecretKey.java +++ b/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; } diff --git a/pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java b/pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java index f418f577..ba16e2d0 100644 --- a/pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java +++ b/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 - public void investigate() throws IOException { - SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("12345678")); + // created with exploit code by cure53.de + private static final String INJECTED_KEY = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: PGPainless\n" + + "Comment: F594 D7CC E7D0 1F15 1511 4395 C075 FD34 4B2A D41A\n" + + "Comment: Juliet \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 - for (PGPSecretKey secretKey : dsa) { - try { - UnlockSecretKey.unlockSecretKey(secretKey, protector); - System.out.println("No KeyIntegrityException for dsa key " + Long.toHexString(secretKey.getKeyID())); - } catch (KeyIntegrityException e) { - System.out.println("KeyIntegrityException for dsa key " + Long.toHexString(secretKey.getKeyID())); - } catch (PGPException e) { - System.out.println("Cannot unlock dsa key: " + e.getMessage()); - } - } + assertThrows(KeyIntegrityException.class, () -> + UnlockSecretKey.unlockSecretKey(elgamal.getSecretKey(KeyIdUtil.fromLongKeyId("f5ffdf6d71dd5789")), protector)); + } - for (PGPSecretKey secretKey : elgamal) { - try { - UnlockSecretKey.unlockSecretKey(secretKey, protector); - System.out.println("No KeyIntegrityException for elgamal key " + Long.toHexString(secretKey.getKeyID())); - } catch (KeyIntegrityException e) { - System.out.println("KeyIntegrityException for elgamal key " + Long.toHexString(secretKey.getKeyID())); - }catch (PGPException e) { - System.out.println("Cannot unlock elgamal key: " + e.getMessage()); - } + @Test + public void assertInjectedKeyRingFailsToUnlockPrimaryKey() throws IOException { + PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(INJECTED_KEY); + SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("pass")); + + 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)); + } + } + + @Test + public void assertUnmodifiedECKeyDoesNotThrow() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing() + .simpleEcKeyRing("Unmodified", "987654321"); + SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("987654321")); + + for (PGPSecretKey secretKey : secretKeys) { + assertDoesNotThrow(() -> + UnlockSecretKey.unlockSecretKey(secretKey, protector)); + } + } + + @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)); } - // CHECKSTYLE:ON } }