Add S2KUsageFix class and tests to switch secret keys encrypted with USAGE_CHECKSUM over to USAGE_SHA1

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);
     }

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 <a href="https://github.com/pgpainless/pgpainless/issues/176">Related PGPainless Bug Report</a>
+ * @see <a href="https://github.com/pgpainless/pgpainless/issues/178">Related PGPainless Feature Request</a>
+ * @see <a href="https://github.com/bcgit/bc-java/issues/1020">Related upstream BC bug report</a>
+ */
+public final class S2KUsageFix {
+
+    private S2KUsageFix() {
+
+    }
+
+    /**
+     * Repair method for keys which use S2K usage <pre>USAGE_CHECKSUM</pre> which is deemed insecure.
+     * This method fixes the private keys by changing them to <pre>USAGE_SHA1</pre> 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 <pre>USAGE_CHECKSUM</pre> which is deemed insecure.
+     * This method fixes the private keys by changing them to <pre>USAGE_SHA1</pre> 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;
+    }
+}

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;

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());
+    }
+}