From 073cf870d25db933460634ff66c5080b1704822c Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Mon, 6 Dec 2021 15:01:37 +0100 Subject: [PATCH] Fix NPE when attempting to decrypt GNU_DUMMY_S2K keys --- .../key/protection/UnlockSecretKey.java | 45 ++++++++++++------- .../builder/AbstractSignatureBuilder.java | 10 ++--- .../builder/DirectKeySignatureBuilder.java | 6 +-- .../PrimaryKeyBindingSignatureBuilder.java | 3 +- .../builder/RevocationSignatureBuilder.java | 3 +- .../builder/SelfSignatureBuilder.java | 7 ++- .../SubkeyBindingSignatureBuilder.java | 3 +- ...irdPartyCertificationSignatureBuilder.java | 6 +-- .../builder/UniversalSignatureBuilder.java | 5 +-- .../pgpainless/example/UnlockSecretKeys.java | 3 +- 10 files changed, 48 insertions(+), 43 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 c68e4914..36ceac44 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 @@ -20,27 +20,40 @@ public final class UnlockSecretKey { } public static PGPPrivateKey unlockSecretKey(PGPSecretKey secretKey, SecretKeyRingProtector protector) - throws WrongPassphraseException { + throws PGPException { + + PBESecretKeyDecryptor decryptor = null; + if (KeyInfo.isEncrypted(secretKey)) { + decryptor = protector.getDecryptor(secretKey.getKeyID()); + } + PGPPrivateKey privateKey = unlockSecretKey(secretKey, decryptor); + return privateKey; + } + + public static PGPPrivateKey unlockSecretKey(PGPSecretKey secretKey, PBESecretKeyDecryptor decryptor) + throws PGPException { + PGPPrivateKey privateKey; try { - PBESecretKeyDecryptor decryptor = null; - if (KeyInfo.isEncrypted(secretKey)) { - decryptor = protector.getDecryptor(secretKey.getKeyID()); + privateKey = secretKey.extractPrivateKey(decryptor); + } catch (PGPException e) { + throw new WrongPassphraseException(secretKey.getKeyID(), e); + } + + if (privateKey == null) { + int s2kType = secretKey.getS2K().getType(); + if (s2kType >= 100 && s2kType <= 110) { + throw new PGPException("Cannot decrypt secret key" + Long.toHexString(secretKey.getKeyID()) + ": " + + "Unsupported private S2K usage type " + s2kType); } - return secretKey.extractPrivateKey(decryptor); - } catch (PGPException e) { - throw new WrongPassphraseException(secretKey.getKeyID(), e); + + throw new PGPException("Cannot decrypt secret key."); } + + return privateKey; } - public static PGPPrivateKey unlockSecretKey(PGPSecretKey secretKey, PBESecretKeyDecryptor decryptor) throws WrongPassphraseException { - try { - return secretKey.extractPrivateKey(decryptor); - } catch (PGPException e) { - throw new WrongPassphraseException(secretKey.getKeyID(), e); - } - } - - public static PGPPrivateKey unlockSecretKey(PGPSecretKey secretKey, Passphrase passphrase) throws WrongPassphraseException { + public static PGPPrivateKey unlockSecretKey(PGPSecretKey secretKey, Passphrase passphrase) + throws PGPException { return unlockSecretKey(secretKey, SecretKeyRingProtector.unlockSingleKeyWith(passphrase, secretKey)); } } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java index 7b92ab02..9eaa70b4 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java @@ -5,6 +5,7 @@ package org.pgpainless.signature.builder; import java.util.Set; +import javax.annotation.Nonnull; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPrivateKey; @@ -16,7 +17,6 @@ import org.pgpainless.PGPainless; import org.pgpainless.algorithm.HashAlgorithm; import org.pgpainless.algorithm.SignatureType; import org.pgpainless.algorithm.negotiation.HashAlgorithmNegotiator; -import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.implementation.ImplementationFactory; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.key.protection.UnlockSecretKey; @@ -24,8 +24,6 @@ import org.pgpainless.key.util.OpenPgpKeyAttributeUtil; import org.pgpainless.signature.subpackets.SignatureSubpackets; import org.pgpainless.signature.subpackets.SignatureSubpacketsHelper; -import javax.annotation.Nonnull; - public abstract class AbstractSignatureBuilder> { protected final PGPPrivateKey privateSigningKey; protected final PGPPublicKey publicSigningKey; @@ -42,7 +40,7 @@ public abstract class AbstractSignatureBuilder { - public DirectKeySignatureBuilder(PGPSecretKey certificationKey, SecretKeyRingProtector protector, PGPSignature archetypeSignature) throws WrongPassphraseException { + public DirectKeySignatureBuilder(PGPSecretKey certificationKey, SecretKeyRingProtector protector, PGPSignature archetypeSignature) + throws PGPException { super(certificationKey, protector, archetypeSignature); } - public DirectKeySignatureBuilder(PGPSecretKey signingKey, SecretKeyRingProtector protector) throws WrongPassphraseException { + public DirectKeySignatureBuilder(PGPSecretKey signingKey, SecretKeyRingProtector protector) throws PGPException { super(SignatureType.DIRECT_KEY, signingKey, protector); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/PrimaryKeyBindingSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/PrimaryKeyBindingSignatureBuilder.java index 0f88dec4..93339f86 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/PrimaryKeyBindingSignatureBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/PrimaryKeyBindingSignatureBuilder.java @@ -11,14 +11,13 @@ import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSignature; import org.pgpainless.algorithm.SignatureType; -import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.signature.subpackets.SelfSignatureSubpackets; public class PrimaryKeyBindingSignatureBuilder extends AbstractSignatureBuilder { public PrimaryKeyBindingSignatureBuilder(PGPSecretKey subkey, SecretKeyRingProtector subkeyProtector) - throws WrongPassphraseException { + throws PGPException { super(SignatureType.PRIMARYKEY_BINDING, subkey, subkeyProtector); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/RevocationSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/RevocationSignatureBuilder.java index 3449a5dd..ebae151d 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/RevocationSignatureBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/RevocationSignatureBuilder.java @@ -12,13 +12,12 @@ import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.openpgp.PGPSignatureGenerator; import org.pgpainless.algorithm.SignatureType; -import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.signature.subpackets.RevocationSignatureSubpackets; public class RevocationSignatureBuilder extends AbstractSignatureBuilder { - public RevocationSignatureBuilder(SignatureType signatureType, PGPSecretKey signingKey, SecretKeyRingProtector protector) throws WrongPassphraseException { + public RevocationSignatureBuilder(SignatureType signatureType, PGPSecretKey signingKey, SecretKeyRingProtector protector) throws PGPException { super(signatureType, signingKey, protector); getHashedSubpackets().setRevocable(true, false); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SelfSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SelfSignatureBuilder.java index a7ffd488..e6bf94c3 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SelfSignatureBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SelfSignatureBuilder.java @@ -12,18 +12,17 @@ import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.openpgp.PGPUserAttributeSubpacketVector; import org.pgpainless.algorithm.SignatureType; -import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.signature.subpackets.SelfSignatureSubpackets; public class SelfSignatureBuilder extends AbstractSignatureBuilder { - public SelfSignatureBuilder(PGPSecretKey signingKey, SecretKeyRingProtector protector) throws WrongPassphraseException { + public SelfSignatureBuilder(PGPSecretKey signingKey, SecretKeyRingProtector protector) throws PGPException { this(SignatureType.GENERIC_CERTIFICATION, signingKey, protector); } public SelfSignatureBuilder(SignatureType signatureType, PGPSecretKey signingKey, SecretKeyRingProtector protector) - throws WrongPassphraseException { + throws PGPException { super(signatureType, signingKey, protector); } @@ -31,7 +30,7 @@ public class SelfSignatureBuilder extends AbstractSignatureBuilder { public SubkeyBindingSignatureBuilder(PGPSecretKey signingKey, SecretKeyRingProtector protector) - throws WrongPassphraseException { + throws PGPException { super(SignatureType.SUBKEY_BINDING, signingKey, protector); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/ThirdPartyCertificationSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/ThirdPartyCertificationSignatureBuilder.java index 5d19fa83..9128e5da 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/ThirdPartyCertificationSignatureBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/ThirdPartyCertificationSignatureBuilder.java @@ -31,7 +31,7 @@ public class ThirdPartyCertificationSignatureBuilder extends AbstractSignatureBu * @throws WrongPassphraseException in case of a wrong passphrase */ public ThirdPartyCertificationSignatureBuilder(PGPSecretKey signingKey, SecretKeyRingProtector protector) - throws WrongPassphraseException { + throws PGPException { this(SignatureType.GENERIC_CERTIFICATION, signingKey, protector); } @@ -44,7 +44,7 @@ public class ThirdPartyCertificationSignatureBuilder extends AbstractSignatureBu * @throws WrongPassphraseException in case of a wrong passphrase */ public ThirdPartyCertificationSignatureBuilder(SignatureType signatureType, PGPSecretKey signingKey, SecretKeyRingProtector protector) - throws WrongPassphraseException { + throws PGPException { super(signatureType, signingKey, protector); } @@ -60,7 +60,7 @@ public class ThirdPartyCertificationSignatureBuilder extends AbstractSignatureBu PGPSecretKey signingKey, SecretKeyRingProtector protector, PGPSignature archetypeSignature) - throws WrongPassphraseException { + throws PGPException { super(signingKey, protector, archetypeSignature); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/UniversalSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/UniversalSignatureBuilder.java index 3b63a032..b3ed5bf0 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/UniversalSignatureBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/UniversalSignatureBuilder.java @@ -11,7 +11,6 @@ import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.openpgp.PGPSignatureGenerator; import org.pgpainless.algorithm.SignatureType; -import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.signature.subpackets.BaseSignatureSubpackets; import org.pgpainless.signature.subpackets.SignatureSubpackets; @@ -22,12 +21,12 @@ import org.pgpainless.signature.subpackets.SignatureSubpackets; public class UniversalSignatureBuilder extends AbstractSignatureBuilder { public UniversalSignatureBuilder(SignatureType signatureType, PGPSecretKey signingKey, SecretKeyRingProtector protector) - throws WrongPassphraseException { + throws PGPException { super(signatureType, signingKey, protector); } public UniversalSignatureBuilder(PGPSecretKey certificationKey, SecretKeyRingProtector protector, PGPSignature archetypeSignature) - throws WrongPassphraseException { + throws PGPException { super(certificationKey, protector, archetypeSignature); } diff --git a/pgpainless-core/src/test/java/org/pgpainless/example/UnlockSecretKeys.java b/pgpainless-core/src/test/java/org/pgpainless/example/UnlockSecretKeys.java index a7b056b7..f4716211 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/example/UnlockSecretKeys.java +++ b/pgpainless-core/src/test/java/org/pgpainless/example/UnlockSecretKeys.java @@ -11,7 +11,6 @@ import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.junit.jupiter.api.Test; import org.pgpainless.PGPainless; -import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.key.OpenPgpV4Fingerprint; import org.pgpainless.key.TestKeys; import org.pgpainless.key.protection.CachingSecretKeyRingProtector; @@ -120,7 +119,7 @@ public class UnlockSecretKeys { } private void assertProtectorUnlocksAllSecretKeys(PGPSecretKeyRing secretKey, SecretKeyRingProtector protector) - throws WrongPassphraseException { + throws PGPException { for (PGPSecretKey key : secretKey) { UnlockSecretKey.unlockSecretKey(key, protector); }