diff --git a/pgpainless-core/src/main/java/org/pgpainless/exception/WrongPassphraseException.java b/pgpainless-core/src/main/java/org/pgpainless/exception/WrongPassphraseException.java index f5207c06..bd0c051c 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/exception/WrongPassphraseException.java +++ b/pgpainless-core/src/main/java/org/pgpainless/exception/WrongPassphraseException.java @@ -19,6 +19,10 @@ import org.bouncycastle.openpgp.PGPException; public class WrongPassphraseException extends PGPException { + public WrongPassphraseException(String message) { + super(message); + } + public WrongPassphraseException(long keyId, PGPException cause) { this("Wrong passphrase provided for key " + Long.toHexString(keyId), cause); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/fixes/S2KUsageFix.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/fixes/S2KUsageFix.java new file mode 100644 index 00000000..c4a43fe4 --- /dev/null +++ b/pgpainless-core/src/main/java/org/pgpainless/key/protection/fixes/S2KUsageFix.java @@ -0,0 +1,103 @@ +/* + * Copyright 2021 Paul Schaub. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.pgpainless.key.protection.fixes; + +import org.bouncycastle.bcpg.SecretKeyPacket; +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPPrivateKey; +import org.bouncycastle.openpgp.PGPSecretKey; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.bouncycastle.openpgp.operator.PBESecretKeyEncryptor; +import org.bouncycastle.openpgp.operator.PGPDigestCalculator; +import org.pgpainless.algorithm.HashAlgorithm; +import org.pgpainless.exception.WrongPassphraseException; +import org.pgpainless.implementation.ImplementationFactory; +import org.pgpainless.key.protection.SecretKeyRingProtector; +import org.pgpainless.key.protection.UnlockSecretKey; + +/** + * Repair class to fix keys which use S2K usage of value {@link SecretKeyPacket#USAGE_CHECKSUM}. + * The method {@link #replaceUsageChecksumWithUsageSha1(PGPSecretKeyRing, SecretKeyRingProtector)} ensures + * that such keys are encrypted using S2K usage {@link SecretKeyPacket#USAGE_SHA1} instead. + * + * @see Related PGPainless Bug Report + * @see Related PGPainless Feature Request + * @see Related upstream BC bug report + */ +public final class S2KUsageFix { + + private S2KUsageFix() { + + } + + /** + * Repair method for keys which use S2K usage
USAGE_CHECKSUM
which is deemed insecure. + * This method fixes the private keys by changing them to
USAGE_SHA1
instead. + * + * @param keys keys + * @param protector protector to unlock and re-lock affected private keys + * @return fixed key ring + * @throws PGPException in case of a PGP error. + */ + public static PGPSecretKeyRing replaceUsageChecksumWithUsageSha1(PGPSecretKeyRing keys, SecretKeyRingProtector protector) throws PGPException { + return replaceUsageChecksumWithUsageSha1(keys, protector, false); + } + + /** + * Repair method for keys which use S2K usage
USAGE_CHECKSUM
which is deemed insecure. + * This method fixes the private keys by changing them to
USAGE_SHA1
instead. + * + * @param keys keys + * @param protector protector to unlock and re-lock affected private keys + * @param skipKeysWithMissingPassphrase if set to true, missing subkey passphrases will cause the subkey to stay unaffected. + * @return fixed key ring + * @throws PGPException in case of a PGP error. + */ + public static PGPSecretKeyRing replaceUsageChecksumWithUsageSha1(PGPSecretKeyRing keys, + SecretKeyRingProtector protector, + boolean skipKeysWithMissingPassphrase) throws PGPException { + PGPDigestCalculator digestCalculator = ImplementationFactory.getInstance().getPGPDigestCalculator(HashAlgorithm.SHA1); + for (PGPSecretKey key : keys) { + // CHECKSUM is not recommended + if (key.getS2KUsage() != SecretKeyPacket.USAGE_CHECKSUM) { + continue; + } + + long keyId = key.getKeyID(); + PBESecretKeyEncryptor encryptor = protector.getEncryptor(keyId); + if (encryptor == null) { + if (skipKeysWithMissingPassphrase) { + continue; + } + throw new WrongPassphraseException("Missing passphrase for key with ID " + Long.toHexString(keyId)); + } + + PGPPrivateKey privateKey = UnlockSecretKey.unlockSecretKey(key, protector); + // This constructor makes use of USAGE_SHA1 by default + PGPSecretKey fixedKey = new PGPSecretKey( + privateKey, + key.getPublicKey(), + digestCalculator, + key.isMasterKey(), + protector.getEncryptor(keyId) + ); + + // replace the original key with the fixed one + keys = PGPSecretKeyRing.insertSecretKey(keys, fixedKey); + } + return keys; + } +} diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/fixes/package-info.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/fixes/package-info.java new file mode 100644 index 00000000..69759cbd --- /dev/null +++ b/pgpainless-core/src/main/java/org/pgpainless/key/protection/fixes/package-info.java @@ -0,0 +1,19 @@ +/* + * Copyright 2021 Paul Schaub. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/** + * Secret Key Protection Fixes. + */ +package org.pgpainless.key.protection.fixes; diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/protection/fixes/EnsureSecureS2KUsageTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/protection/fixes/EnsureSecureS2KUsageTest.java new file mode 100644 index 00000000..9679c61e --- /dev/null +++ b/pgpainless-core/src/test/java/org/pgpainless/key/protection/fixes/EnsureSecureS2KUsageTest.java @@ -0,0 +1,135 @@ +/* + * Copyright 2021 Paul Schaub. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.pgpainless.key.protection.fixes; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.security.InvalidAlgorithmParameterException; +import java.security.NoSuchAlgorithmException; + +import org.bouncycastle.bcpg.SecretKeyPacket; +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPSecretKey; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.bouncycastle.util.io.Streams; +import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.decryption_verification.ConsumerOptions; +import org.pgpainless.decryption_verification.DecryptionStream; +import org.pgpainless.key.protection.SecretKeyRingProtector; +import org.pgpainless.util.Passphrase; + +public class EnsureSecureS2KUsageTest { + + private static final String KEY_WITH_USAGE_CHECKSUM = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: PGPainless\n" + + "Comment: 23BF 0F50 8FE4 2304 6AC6 6EE3 7616 09ED DAF2 A7EE\n" + + "Comment: Alice\n" + + "\n" + + "lHQEYTpM0xYJKwYBBAHaRw8BAQdA77HCCgjnEWBk9O9guvePTA7235v6yoTGUmUo\n" + + "91V2OVr/CQMC5+tUnE2l4Ypg4dv9+qzBXjlQKAU5fktbQpIvYV9+pPXY6O+Wa7qJ\n" + + "mIJsX2GsMR4uSLeAeQtFuPP0ZydL8rQFQWxpY2WIeAQTFgoAIAUCYTpM1AIbAQUW\n" + + "AgMBAAQLCQgHBRUKCQgLAh4BAhkBAAoJEHYWCe3a8qfu9uYBAJGoTlRUEPCmzDBY\n" + + "iqAXI8q1ZJDpYAF/AgdyfHgPZBLZAQDFagfWe/YyZV36EQ8P978gycUn4psQZyXE\n" + + "QoNByDZDAJx5BGE6TNQSCisGAQQBl1UBBQEBB0BM9uoewIA3wDjChe7qdpp9B/uD\n" + + "fQwxFj8AFcgR0qmDNgMBCAf/CQMC4B9NmOhlt+BgyZTk4BqqudczkJsRhoKPPC3e\n" + + "TEqvyeBavp1fPgvAUfsZdL7Z8RoVQNd0LFptsj2zN2VcmIh1BBgWCgAdBQJhOkzU\n" + + "AhsMBRYCAwEABAsJCAcFFQoJCAsCHgEACgkQdhYJ7dryp+4FxgEA29VePCidazQt\n" + + "F6DfQCyNPW/d0Y+rm64KaMfBHJGCorQA/Rdg/gGVH7RoMiIJ8+kDxWOC92tn8JBJ\n" + + "nIekiMcU45QJnHQEYTpM1BYJKwYBBAHaRw8BAQdAv2fItqEBrRsnrtWOU0Rc/S62\n" + + "tafcr2huX3W5Nu6R1On/CQMCR9EMBma4cl9gE854C43bgYY2o53XSnBS/OMzo1rt\n" + + "h+ixzZ6RZNafiAcXRUldVa55kc5KUvpc3y8lMlkDjYjVBBgWCgB9BQJhOkzUAhsC\n" + + "BRYCAwEABAsJCAcFFQoJCAsCHgFfIAQZFgoABgUCYTpM1AAKCRB7qwD3I/Rg7Iic\n" + + "AP4gOcJFEkRcNJMXVyXWWMaHGzvH7SJSn7/NDxjlbo+IhAD9HiPJoOm/88mHWxSr\n" + + "udMemY82nrspaOyxcwqgkJbT8wIACgkQdhYJ7dryp+6yUQD+IufwT/XdopWA+GPD\n" + + "IgT1CHRecJCkeYYmr7sZdPCLs3YA/jeVaFw4Z0drFnys4rh6aSoG6uf+YSba56V3\n" + + "VBL8P8MC\n" + + "=m6iF\n" + + "-----END PGP PRIVATE KEY BLOCK-----"; + // message encrypted to above key + private static final String MESSAGE = "-----BEGIN PGP MESSAGE-----\n" + + "Version: PGPainless\n" + + "\n" + + "hF4DnMY7X1OYFewSAQdAxAxbWTZordzH+fN5s32ZU4PjYM/Og8z6DG7mrjOy+2Mw\n" + + "BCiqa3G9GNZrRQyXihd1sFaxlgqiHrhhmFyCCMLgj2RxjZ7DJ4E0tA7RbF0lkqx4\n" + + "0kAB1XXqAOJ50mEEQLYRN94xojDoJrlz2ZdmV1zqC2ZFd6YITEPIqSdwBuEG61rd\n" + + "BLkPg8RuGdPMKZHZzOxIALtv\n" + + "=//m8\n" + + "-----END PGP MESSAGE-----"; + // same message, but unencrypted + private static final String MESSAGE_PLAIN = "Hello, World!\n"; + + @Test + public void verifyBouncycastleChangesUnprotectedKeysTo_USAGE_CHECKSUM() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + // Bouncycastle unfortunately uses USAGE_CHECKSUM as default S2K usage when setting a passphrase + // on a previously unprotected key. + // This test verifies this hypothesis by creating a fresh, protected key (which uses the recommended USAGE_SHA1), + // unprotecting the key and then again setting a passphrase on it. + PGPSecretKeyRing before = PGPainless.generateKeyRing().modernKeyRing("Alice", "before"); + for (PGPSecretKey key : before) { + assertEquals(SecretKeyPacket.USAGE_SHA1, key.getS2KUsage()); + } + + PGPSecretKeyRing unprotected = PGPainless.modifyKeyRing(before) + .changePassphraseFromOldPassphrase(Passphrase.fromPassword("before")) + .withSecureDefaultSettings() + .toNoPassphrase() + .done(); + for (PGPSecretKey key : unprotected) { + assertEquals(SecretKeyPacket.USAGE_NONE, key.getS2KUsage()); + } + + PGPSecretKeyRing after = PGPainless.modifyKeyRing(unprotected) + .changePassphraseFromOldPassphrase(null) + .withSecureDefaultSettings() + .toNewPassphrase(Passphrase.fromPassword("after")) + .done(); + for (PGPSecretKey key : after) { + assertEquals(SecretKeyPacket.USAGE_CHECKSUM, key.getS2KUsage(), "Looks like BC fixed the default S2K usage. Yay!"); + } + } + + @Test + public void testFixS2KUsageFrom_USAGE_CHECKSUM_to_USAGE_SHA1() throws IOException, PGPException { + PGPSecretKeyRing keys = PGPainless.readKeyRing().secretKeyRing(KEY_WITH_USAGE_CHECKSUM); + SecretKeyRingProtector protector = SecretKeyRingProtector.unlockAllKeysWith(Passphrase.fromPassword("after"), keys); + + PGPSecretKeyRing fixed = S2KUsageFix.replaceUsageChecksumWithUsageSha1(keys, protector); + for (PGPSecretKey key : fixed) { + assertEquals(SecretKeyPacket.USAGE_SHA1, key.getS2KUsage()); + } + + testCanStillDecrypt(keys, protector); + } + + private void testCanStillDecrypt(PGPSecretKeyRing keys, SecretKeyRingProtector protector) throws PGPException, IOException { + ByteArrayInputStream in = new ByteArrayInputStream(MESSAGE.getBytes(StandardCharsets.UTF_8)); + DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify() + .onInputStream(in) + .withOptions(new ConsumerOptions() + .addDecryptionKey(keys, protector)); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + Streams.pipeAll(decryptionStream, out); + decryptionStream.close(); + + assertArrayEquals(MESSAGE_PLAIN.getBytes(StandardCharsets.UTF_8), out.toByteArray()); + } +}