From bfbaa30e4cb5567078098a080055cf6cf3b376f7 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 15 Dec 2022 16:12:30 +0100 Subject: [PATCH] Make KO-countermeasures configurable (off by default) --- .../key/protection/UnlockSecretKey.java | 5 +++- .../java/org/pgpainless/policy/Policy.java | 29 +++++++++++++++++++ .../ModifiedPublicKeysInvestigation.java | 6 ++++ 3 files changed, 39 insertions(+), 1 deletion(-) 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 42a5b9b7..78c849ab 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 @@ -9,6 +9,7 @@ import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPrivateKey; import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; +import org.pgpainless.PGPainless; import org.pgpainless.exception.KeyIntegrityException; import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.key.info.KeyInfo; @@ -51,7 +52,9 @@ public final class UnlockSecretKey { throw new PGPException("Cannot decrypt secret key."); } - PublicKeyParameterValidationUtil.verifyPublicKeyParameterIntegrity(privateKey, secretKey.getPublicKey()); + if (PGPainless.getPolicy().isEnableKeyParameterValidation()) { + PublicKeyParameterValidationUtil.verifyPublicKeyParameterIntegrity(privateKey, secretKey.getPublicKey()); + } return privateKey; } diff --git a/pgpainless-core/src/main/java/org/pgpainless/policy/Policy.java b/pgpainless-core/src/main/java/org/pgpainless/policy/Policy.java index ad9f0635..3556f104 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/policy/Policy.java +++ b/pgpainless-core/src/main/java/org/pgpainless/policy/Policy.java @@ -49,6 +49,8 @@ public final class Policy { // Signers User-ID is soon to be deprecated. private SignerUserIdValidationLevel signerUserIdValidationLevel = SignerUserIdValidationLevel.DISABLED; + private boolean enableKeyParameterValidation = false; + public enum SignerUserIdValidationLevel { /** * PGPainless will verify {@link org.bouncycastle.bcpg.sig.SignerUserID} subpackets in signatures strictly. @@ -701,4 +703,31 @@ public final class Policy { this.signerUserIdValidationLevel = signerUserIdValidationLevel; return this; } + + /** + * Enable or disable validation of public key parameters when unlocking private keys. + * Disabled by default. + * When enabled, PGPainless will validate, whether public key parameters have been tampered with. + * This is a countermeasure against possible attacks described in the paper + * "Victory by KO: Attacking OpenPGP Using Key Overwriting" by Lara Bruseghini, Daniel Huigens, and Kenneth G. Paterson. + * Since these attacks are only possible in very special conditions (attacker has access to the encrypted private key), + * and the countermeasures are very costly, they are disabled by default, but can be enabled using this method. + * + * @see KOpenPGP.com + * @param enable boolean + * @return this + */ + public Policy setEnableKeyParameterValidation(boolean enable) { + this.enableKeyParameterValidation = enable; + return this; + } + + /** + * Return true, if countermeasures against the KOpenPGP attacks are enabled, false otherwise. + * + * @return true if countermeasures are enabled, false otherwise. + */ + public boolean isEnableKeyParameterValidation() { + return enableKeyParameterValidation; + } } diff --git a/pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java b/pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java index ba16e2d0..6930f78f 100644 --- a/pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java +++ b/pgpainless-core/src/test/java/investigations/ModifiedPublicKeysInvestigation.java @@ -212,10 +212,12 @@ public class ModifiedPublicKeysInvestigation { SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("12345678")); PGPSecretKeyRing dsa = PGPainless.readKeyRing().secretKeyRing(DSA); + PGPainless.getPolicy().setEnableKeyParameterValidation(true); assertThrows(KeyIntegrityException.class, () -> UnlockSecretKey.unlockSecretKey(dsa.getSecretKey(KeyIdUtil.fromLongKeyId("b1bd1f049ec87f3d")), protector)); assertThrows(KeyIntegrityException.class, () -> UnlockSecretKey.unlockSecretKey(dsa.getSecretKey(KeyIdUtil.fromLongKeyId("f5ffdf6d71dd5789")), protector)); + PGPainless.getPolicy().setEnableKeyParameterValidation(false); } @Test @@ -223,8 +225,10 @@ public class ModifiedPublicKeysInvestigation { SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("12345678")); PGPSecretKeyRing elgamal = PGPainless.readKeyRing().secretKeyRing(ELGAMAL); + PGPainless.getPolicy().setEnableKeyParameterValidation(true); assertThrows(KeyIntegrityException.class, () -> UnlockSecretKey.unlockSecretKey(elgamal.getSecretKey(KeyIdUtil.fromLongKeyId("f5ffdf6d71dd5789")), protector)); + PGPainless.getPolicy().setEnableKeyParameterValidation(false); } @Test @@ -232,8 +236,10 @@ public class ModifiedPublicKeysInvestigation { PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(INJECTED_KEY); SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAnyKeyWith(Passphrase.fromPassword("pass")); + PGPainless.getPolicy().setEnableKeyParameterValidation(true); assertThrows(KeyIntegrityException.class, () -> UnlockSecretKey.unlockSecretKey(secretKeys.getSecretKey(), protector)); + PGPainless.getPolicy().setEnableKeyParameterValidation(false); } @Test