From 651a69c118b47a45ab5719a6980c1cba7b2a36ad Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Fri, 23 Oct 2020 16:44:21 +0200 Subject: [PATCH] Work on the editing api --- .../main/java/org/pgpainless/PGPainless.java | 9 ++ .../key/modification/KeyRingEditor.java | 108 +++++++++++++++++- .../modification/KeyRingEditorInterface.java | 94 +++++++++++++-- .../PassphraseMapKeyRingProtector.java | 1 + .../PasswordBasedSecretKeyRingProtector.java | 1 + .../MapBasedPassphraseProvider.java | 26 +++++ .../SecretKeyPassphraseProvider.java | 2 +- .../SolitaryPassphraseProvider.java | 20 ++++ .../ChangeSecretKeyRingPassphraseTest.java | 59 ++++++++++ .../PassphraseProtectedKeyTest.java | 1 + 10 files changed, 307 insertions(+), 14 deletions(-) create mode 100644 pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/MapBasedPassphraseProvider.java rename pgpainless-core/src/main/java/org/pgpainless/key/protection/{ => passphrase_provider}/SecretKeyPassphraseProvider.java (95%) create mode 100644 pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/SolitaryPassphraseProvider.java create mode 100644 pgpainless-core/src/test/java/org/pgpainless/key/modification/ChangeSecretKeyRingPassphraseTest.java diff --git a/pgpainless-core/src/main/java/org/pgpainless/PGPainless.java b/pgpainless-core/src/main/java/org/pgpainless/PGPainless.java index a3c4d24c..9997bfba 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/PGPainless.java +++ b/pgpainless-core/src/main/java/org/pgpainless/PGPainless.java @@ -19,6 +19,7 @@ import javax.annotation.Nonnull; import java.io.IOException; import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.pgpainless.algorithm.CompressionAlgorithm; import org.pgpainless.algorithm.SymmetricKeyAlgorithm; import org.pgpainless.decryption_verification.DecryptionBuilder; @@ -26,7 +27,11 @@ import org.pgpainless.decryption_verification.DecryptionStream; import org.pgpainless.encryption_signing.EncryptionBuilder; import org.pgpainless.encryption_signing.EncryptionStream; import org.pgpainless.key.generation.KeyRingBuilder; +import org.pgpainless.key.modification.KeyRingEditor; +import org.pgpainless.key.modification.KeyRingEditorInterface; import org.pgpainless.key.parsing.KeyRingReader; +import org.pgpainless.key.protection.passphrase_provider.SecretKeyPassphraseProvider; +import org.pgpainless.key.protection.passphrase_provider.SolitaryPassphraseProvider; import org.pgpainless.symmetric_encryption.SymmetricEncryptorDecryptor; import org.pgpainless.util.Passphrase; @@ -64,6 +69,10 @@ public class PGPainless { return new DecryptionBuilder(); } + public static KeyRingEditorInterface modifyKeyRing(PGPSecretKeyRing secretKeys) { + return new KeyRingEditor(secretKeys); + } + /** * Encrypt some data symmetrically using OpenPGP and a password. * The resulting data will be uncompressed and integrity protected. diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditor.java b/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditor.java index 27dcc9b6..a8c2f503 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditor.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditor.java @@ -1,10 +1,30 @@ package org.pgpainless.key.modification; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPPrivateKey; +import org.bouncycastle.openpgp.PGPPublicKey; +import org.bouncycastle.openpgp.PGPPublicKeyRing; +import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.bouncycastle.openpgp.PGPSignature; +import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; +import org.pgpainless.algorithm.SignatureType; import org.pgpainless.key.OpenPgpV4Fingerprint; +import org.pgpainless.key.collection.PGPKeyRing; import org.pgpainless.key.generation.KeySpec; +import org.pgpainless.key.protection.KeyRingProtectionSettings; +import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector; +import org.pgpainless.key.protection.passphrase_provider.SecretKeyPassphraseProvider; +import org.pgpainless.key.protection.SecretKeyRingProtector; +import org.pgpainless.key.protection.UnprotectedKeysProtector; +import org.pgpainless.key.protection.passphrase_provider.SolitaryPassphraseProvider; import org.pgpainless.util.Passphrase; public class KeyRingEditor implements KeyRingEditorInterface { @@ -12,10 +32,6 @@ public class KeyRingEditor implements KeyRingEditorInterface { private PGPSecretKeyRing secretKeyRing; public KeyRingEditor(PGPSecretKeyRing secretKeyRing) { - this(secretKeyRing, null); - } - - public KeyRingEditor(PGPSecretKeyRing secretKeyRing, @Nullable Passphrase passphrase) { if (secretKeyRing == null) { throw new NullPointerException("SecretKeyRing MUST NOT be null."); } @@ -23,10 +39,53 @@ public class KeyRingEditor implements KeyRingEditorInterface { } @Override - public KeyRingEditorInterface addUserId(String userId) { + public KeyRingEditorInterface addUserId(String userId, SecretKeyRingProtector secretKeyRingProtector) throws PGPException { + userId = sanitizeUserId(userId); + + PGPPublicKey primaryPubKey = secretKeyRing.getPublicKey(); + PGPPrivateKey privateKey = unlockSecretKey(primaryPubKey.getKeyID(), secretKeyRingProtector); + + signatureGenerator.init(SignatureType.POSITIVE_CERTIFICATION.getCode(), privateKey); + PGPSignature userIdSignature = signatureGenerator.generateCertification(userId, primaryPubKey); + primaryPubKey = PGPPublicKey.addCertification(primaryPubKey, + userId, userIdSignature); + + // "reassemble" secret key ring with modified primary key + PGPSecretKey primarySecKey = new PGPSecretKey( + privateKey, + primaryPubKey, digestCalculator, true, secretKeyRingProtector); + List secretKeyList = new ArrayList<>(); + secretKeyList.add(primarySecKey); + while (secretKeys.hasNext()) { + secretKeyList.add(secretKeys.next()); + } + secretKeyRing = new PGPSecretKeyRing(secretKeyList); + + // extract public key ring from secret keys + List publicKeyList = new ArrayList<>(); + Iterator publicKeys = secretKeyRing.getPublicKeys(); + while (publicKeys.hasNext()) { + publicKeyList.add(publicKeys.next()); + } + PGPPublicKeyRing publicKeyRing = new PGPPublicKeyRing(publicKeyList); + return this; } + private PGPPrivateKey unlockSecretKey(long keyId, SecretKeyRingProtector protector) throws PGPException { + PGPSecretKey secretKey = secretKeyRing.getSecretKey(keyId); + PBESecretKeyDecryptor secretKeyDecryptor = protector.getDecryptor(keyId); + PGPPrivateKey privateKey = secretKey.extractPrivateKey(secretKeyDecryptor); + return privateKey; + } + + private String sanitizeUserId(String userId) { + userId = userId.trim(); + // TODO: Further research how to sanitize user IDs. + // eg. what about newlines? + return userId; + } + @Override public KeyRingEditorInterface deleteUserId(String userId) { return this; @@ -57,8 +116,47 @@ public class KeyRingEditor implements KeyRingEditorInterface { return this; } + @Override + public WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase(@Nullable Passphrase oldPassphrase, + @Nonnull KeyRingProtectionSettings oldProtectionSettings) { + return new WithKeyRingEncryptionSettingsImpl(); + } + + @Override + public WithKeyRingEncryptionSettings changeSubKeyPassphraseFromOldPassphrase(@Nonnull Long keyId, + @Nullable Passphrase oldPassphrase, + @Nonnull KeyRingProtectionSettings oldProtectionSettings) { + return new WithKeyRingEncryptionSettingsImpl(); + } + @Override public PGPSecretKeyRing done() { return secretKeyRing; } + + private class WithKeyRingEncryptionSettingsImpl implements WithKeyRingEncryptionSettings { + + @Override + public WithPassphrase withSecureDefaultSettings() { + return withCustomSettings(KeyRingProtectionSettings.secureDefaultSettings()); + } + + @Override + public WithPassphrase withCustomSettings(KeyRingProtectionSettings settings) { + return new WithPassphraseImpl(); + } + } + + private class WithPassphraseImpl implements WithPassphrase { + + @Override + public KeyRingEditorInterface toNewPassphrase(Passphrase passphrase) { + return KeyRingEditor.this; + } + + @Override + public KeyRingEditorInterface noPassphrase() { + return KeyRingEditor.this; + } + } } diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditorInterface.java b/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditorInterface.java index ac02c491..df9d45b8 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditorInterface.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditorInterface.java @@ -15,9 +15,16 @@ */ package org.pgpainless.key.modification; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.pgpainless.key.OpenPgpV4Fingerprint; import org.pgpainless.key.generation.KeySpec; +import org.pgpainless.key.protection.KeyRingProtectionSettings; +import org.pgpainless.key.protection.SecretKeyRingProtector; +import org.pgpainless.util.Passphrase; public interface KeyRingEditorInterface { @@ -27,7 +34,7 @@ public interface KeyRingEditorInterface { * @param userId user-id * @return the builder */ - KeyRingEditorInterface addUserId(String userId); + KeyRingEditorInterface addUserId(String userId, SecretKeyRingProtector secretKeyRingProtector) throws PGPException; /** * Remove a user-id from the primary key of the key ring. @@ -35,7 +42,7 @@ public interface KeyRingEditorInterface { * @param userId exact user-id to be removed * @return the builder */ - KeyRingEditorInterface deleteUserId(String userId); + KeyRingEditorInterface deleteUserId(String userId, SecretKeyRingProtector secretKeyRingProtector); /** * Add a subkey to the key ring. @@ -44,7 +51,7 @@ public interface KeyRingEditorInterface { * @param keySpec key specification * @return the builder */ - KeyRingEditorInterface addSubKey(KeySpec keySpec); + KeyRingEditorInterface addSubKey(KeySpec keySpec, SecretKeyRingProtector secretKeyRingProtector); /** * Delete a subkey from the key ring. @@ -54,7 +61,7 @@ public interface KeyRingEditorInterface { * @param fingerprint fingerprint of the subkey to be removed * @return the builder */ - KeyRingEditorInterface deleteSubKey(OpenPgpV4Fingerprint fingerprint); + KeyRingEditorInterface deleteSubKey(OpenPgpV4Fingerprint fingerprint, SecretKeyRingProtector secretKeyRingProtector); /** * Delete a subkey from the key ring. @@ -64,7 +71,7 @@ public interface KeyRingEditorInterface { * @param subKeyId id of the subkey * @return the builder */ - KeyRingEditorInterface deleteSubKey(long subKeyId); + KeyRingEditorInterface deleteSubKey(long subKeyId, SecretKeyRingProtector secretKeyRingProtector); /** * Revoke the subkey binding signature of a subkey. @@ -74,21 +81,92 @@ public interface KeyRingEditorInterface { * @param fingerprint fingerprint of the subkey to be revoked * @return the builder */ - KeyRingEditorInterface revokeSubKey(OpenPgpV4Fingerprint fingerprint); + KeyRingEditorInterface revokeSubKey(OpenPgpV4Fingerprint fingerprint, SecretKeyRingProtector secretKeyRingProtector); /** - * Revoke the subkey binding sugnature of a subkey. + * Revoke the subkey binding signature of a subkey. * The subkey with the provided key-id will be revoked. * If no suitable subkey is found, q {@link java.util.NoSuchElementException} will be thrown. * * @param subKeyId id of the subkey * @return the builder */ - KeyRingEditorInterface revokeSubKey(long subKeyId); + KeyRingEditorInterface revokeSubKey(long subKeyId, SecretKeyRingProtector secretKeyRingProtector); + + /** + * Change the passphrase of the whole key ring. + * + * @param oldPassphrase old passphrase or null, if the key was unprotected + * @return next builder step + */ + default WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase(@Nullable Passphrase oldPassphrase) { + return changePassphraseFromOldPassphrase(oldPassphrase, KeyRingProtectionSettings.secureDefaultSettings()); + } + + WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase(@Nullable Passphrase oldPassphrase, + @Nonnull KeyRingProtectionSettings oldProtectionSettings); + + /** + * Change the passphrase of a single subkey in the key ring. + * + * Note: While it is a valid use-case to have different passphrases per subKey, + * this is one of the reasons why OpenPGP sucks in practice. + * + * @param keyId id of the subkey + * @param oldPassphrase old passphrase + * @return next builder step + */ + default WithKeyRingEncryptionSettings changeSubKeyPassphraseFromOldPassphrase(@Nonnull Long keyId, + @Nullable Passphrase oldPassphrase) { + return changeSubKeyPassphraseFromOldPassphrase(keyId, oldPassphrase, KeyRingProtectionSettings.secureDefaultSettings()); + } + + WithKeyRingEncryptionSettings changeSubKeyPassphraseFromOldPassphrase(@Nonnull Long keyId, + @Nullable Passphrase oldPassphrase, + @Nonnull KeyRingProtectionSettings oldProtectionSettings); + + interface WithKeyRingEncryptionSettings { + + /** + * Set secure default settings for the symmetric passphrase encryption. + * Note that this obviously has no effect if you decide to set {@link WithPassphrase#noPassphrase()}. + * + * @return next builder step + */ + WithPassphrase withSecureDefaultSettings(); + + /** + * Set custom settings for the symmetric passphrase encryption. + * + * @param settings custom settings + * @return next builder step + */ + WithPassphrase withCustomSettings(KeyRingProtectionSettings settings); + + } + + interface WithPassphrase { + + /** + * Set the passphrase. + * + * @param passphrase passphrase + * @return editor builder + */ + KeyRingEditorInterface toNewPassphrase(Passphrase passphrase); + + /** + * Leave the key unprotected. + * + * @return editor builder + */ + KeyRingEditorInterface noPassphrase(); + } /** * Return the {@link PGPSecretKeyRing} * @return the key */ PGPSecretKeyRing done(); + } diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/PassphraseMapKeyRingProtector.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/PassphraseMapKeyRingProtector.java index 481f30c4..385e82fb 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/protection/PassphraseMapKeyRingProtector.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/protection/PassphraseMapKeyRingProtector.java @@ -23,6 +23,7 @@ import java.util.Map; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; import org.bouncycastle.openpgp.operator.PBESecretKeyEncryptor; +import org.pgpainless.key.protection.passphrase_provider.SecretKeyPassphraseProvider; import org.pgpainless.util.Passphrase; /** diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/PasswordBasedSecretKeyRingProtector.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/PasswordBasedSecretKeyRingProtector.java index fabd222b..cb8bfd91 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/protection/PasswordBasedSecretKeyRingProtector.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/protection/PasswordBasedSecretKeyRingProtector.java @@ -26,6 +26,7 @@ import org.bouncycastle.openpgp.operator.PGPDigestCalculatorProvider; import org.bouncycastle.openpgp.operator.bc.BcPBESecretKeyDecryptorBuilder; import org.bouncycastle.openpgp.operator.bc.BcPBESecretKeyEncryptorBuilder; import org.bouncycastle.openpgp.operator.bc.BcPGPDigestCalculatorProvider; +import org.pgpainless.key.protection.passphrase_provider.SecretKeyPassphraseProvider; import org.pgpainless.util.Passphrase; /** diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/MapBasedPassphraseProvider.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/MapBasedPassphraseProvider.java new file mode 100644 index 00000000..a29ea423 --- /dev/null +++ b/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/MapBasedPassphraseProvider.java @@ -0,0 +1,26 @@ +package org.pgpainless.key.protection.passphrase_provider; + +import java.util.Map; +import javax.annotation.Nullable; + +import org.pgpainless.util.Passphrase; + +public class MapBasedPassphraseProvider implements SecretKeyPassphraseProvider { + + private final Map map; + + /** + * Create a new map based passphrase provider. + * + * @param passphraseMap map of key-ids and passphrases + */ + public MapBasedPassphraseProvider(Map passphraseMap) { + this.map = passphraseMap; + } + + @Nullable + @Override + public Passphrase getPassphraseFor(Long keyId) { + return map.get(keyId); + } +} diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/SecretKeyPassphraseProvider.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/SecretKeyPassphraseProvider.java similarity index 95% rename from pgpainless-core/src/main/java/org/pgpainless/key/protection/SecretKeyPassphraseProvider.java rename to pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/SecretKeyPassphraseProvider.java index 4c878024..3fb1e2e2 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/protection/SecretKeyPassphraseProvider.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/SecretKeyPassphraseProvider.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.pgpainless.key.protection; +package org.pgpainless.key.protection.passphrase_provider; import javax.annotation.Nullable; diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/SolitaryPassphraseProvider.java b/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/SolitaryPassphraseProvider.java new file mode 100644 index 00000000..b3acfbf4 --- /dev/null +++ b/pgpainless-core/src/main/java/org/pgpainless/key/protection/passphrase_provider/SolitaryPassphraseProvider.java @@ -0,0 +1,20 @@ +package org.pgpainless.key.protection.passphrase_provider; + +import javax.annotation.Nullable; + +import org.pgpainless.util.Passphrase; + +public class SolitaryPassphraseProvider implements SecretKeyPassphraseProvider { + + private final Passphrase passphrase; + + public SolitaryPassphraseProvider(Passphrase passphrase) { + this.passphrase = passphrase; + } + + @Nullable + @Override + public Passphrase getPassphraseFor(Long keyId) { + return passphrase; + } +} diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/modification/ChangeSecretKeyRingPassphraseTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/modification/ChangeSecretKeyRingPassphraseTest.java new file mode 100644 index 00000000..a00612a7 --- /dev/null +++ b/pgpainless-core/src/test/java/org/pgpainless/key/modification/ChangeSecretKeyRingPassphraseTest.java @@ -0,0 +1,59 @@ +package org.pgpainless.key.modification; + +import static org.junit.Assert.fail; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.security.InvalidAlgorithmParameterException; +import java.security.NoSuchAlgorithmException; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.bouncycastle.util.io.Streams; +import org.junit.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.encryption_signing.EncryptionStream; +import org.pgpainless.key.collection.PGPKeyRing; +import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector; +import org.pgpainless.util.Passphrase; + +public class ChangeSecretKeyRingPassphraseTest { + + @Test + public void changePassphraseTest() throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException, IOException { + PGPKeyRing keyRing = PGPainless.generateKeyRing().simpleEcKeyRing("password@encryp.ted", "weakPassphrase"); + PGPSecretKeyRing secretKeys = PGPainless.modifyKeyRing(keyRing.getSecretKeys()) + .changePassphraseFromOldPassphrase(Passphrase.fromPassword("weakPassphrase")) + .withSecureDefaultSettings() + .toNewPassphrase(Passphrase.fromPassword("1337p455phr453")) + .done(); + + keyRing = new PGPKeyRing(secretKeys); + + try { + signDummyMessageWithKeysAndPassphrase(keyRing, Passphrase.fromPassword("weakPassphrase")); + fail("Unlocking secret key ring with old passphrase MUST fail."); + } catch (PGPException e) { + // yay + } + + try { + signDummyMessageWithKeysAndPassphrase(keyRing, Passphrase.fromPassword("1337p455phr453")); + } catch (PGPException e) { + fail("Unlocking the secret key ring with the new passphrase MUST succeed."); + } + } + + private void signDummyMessageWithKeysAndPassphrase(PGPKeyRing keyRing, Passphrase passphrase) throws IOException, PGPException { + String dummyMessage = "dummy"; + ByteArrayOutputStream dummy = new ByteArrayOutputStream(); + EncryptionStream stream = PGPainless.createEncryptor().onOutputStream(dummy) + .doNotEncrypt() + .signWith(PasswordBasedSecretKeyRingProtector.forKey(keyRing.getSecretKeys(), passphrase), keyRing.getSecretKeys()) + .noArmor(); + + Streams.pipeAll(new ByteArrayInputStream(dummyMessage.getBytes()), stream); + stream.close(); + } +} diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/protection/PassphraseProtectedKeyTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/protection/PassphraseProtectedKeyTest.java index ea374cd7..f6a2a6bf 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/protection/PassphraseProtectedKeyTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/protection/PassphraseProtectedKeyTest.java @@ -23,6 +23,7 @@ import javax.annotation.Nullable; import org.bouncycastle.openpgp.PGPException; import org.junit.Test; import org.pgpainless.key.TestKeys; +import org.pgpainless.key.protection.passphrase_provider.SecretKeyPassphraseProvider; import org.pgpainless.util.Passphrase; public class PassphraseProtectedKeyTest {