diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java index 7418401f..e1b1b23e 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java @@ -205,12 +205,61 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Override public SecretKeyRingEditorInterface removeUserId( CharSequence userId, - SecretKeyRingProtector protector) throws PGPException { + SecretKeyRingProtector protector) + throws PGPException { return removeUserId( SelectUserId.exactMatch(userId.toString()), protector); } + @Override + public SecretKeyRingEditorInterface replaceUserId(@Nonnull CharSequence oldUserId, + @Nonnull CharSequence newUserId, + @Nonnull SecretKeyRingProtector protector) + throws PGPException { + String oldUID = oldUserId.toString().trim(); + String newUID = newUserId.toString().trim(); + if (oldUID.isEmpty()) { + throw new IllegalArgumentException("Old user-id cannot be empty."); + } + + if (newUID.isEmpty()) { + throw new IllegalArgumentException("New user-id cannot be empty."); + } + + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); + if (!info.isUserIdValid(oldUID)) { + throw new NoSuchElementException("Key does not carry user-id '" + oldUID + "', or it is not valid."); + } + + PGPSignature oldCertification = info.getLatestUserIdCertification(oldUID); + if (oldCertification == null) { + throw new AssertionError("Certification for old user-id MUST NOT be null."); + } + + // Bind new user-id + addUserId(newUserId, new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + SignatureSubpacketsHelper.applyFrom(oldCertification.getHashedSubPackets(), (SignatureSubpackets) hashedSubpackets); + // Primary user-id + if (oldUID.equals(info.getPrimaryUserId())) { + // Implicit primary user-id + if (!oldCertification.getHashedSubPackets().isPrimaryUserID()) { + hashedSubpackets.setPrimaryUserId(); + } + } + } + + @Override + public void modifyUnhashedSubpackets(SelfSignatureSubpackets unhashedSubpackets) { + SignatureSubpacketsHelper.applyFrom(oldCertification.getUnhashedSubPackets(), (SignatureSubpackets) unhashedSubpackets); + } + }, protector); + + return revokeUserId(oldUID, protector); + } + // TODO: Move to utility class? private String sanitizeUserId(@Nonnull CharSequence userId) { // TODO: Further research how to sanitize user IDs. diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java index 6f4d34b8..7014d518 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java @@ -104,6 +104,26 @@ public interface SecretKeyRingEditorInterface { SecretKeyRingProtector protector) throws PGPException; + /** + * Replace a user-id on the key with a new one. + * The old user-id gets soft revoked and the new user-id gets bound with the same signature subpackets as the + * old one, with one exception: + * If the old user-id was implicitly primary (did not carry a {@link org.bouncycastle.bcpg.sig.PrimaryUserID} packet, + * but effectively was primary, then the new user-id will be explicitly marked as primary. + * + * @param oldUserId old user-id + * @param newUserId new user-id + * @param protector protector to unlock the secret key + * @return the builder + * @throws PGPException in case we cannot generate a revocation and certification signature + * @throws java.util.NoSuchElementException if the old user-id was not found on the key; or if the oldUserId + * was already invalid + */ + SecretKeyRingEditorInterface replaceUserId(CharSequence oldUserId, + CharSequence newUserId, + SecretKeyRingProtector protector) + throws PGPException; + /** * Add a subkey to the key ring. * The subkey will be generated from the provided {@link KeySpec}. diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/modification/FixUserIdDoesNotBreakEncryptionCapabilityTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/modification/FixUserIdDoesNotBreakEncryptionCapabilityTest.java new file mode 100644 index 00000000..e0429287 --- /dev/null +++ b/pgpainless-core/src/test/java/org/pgpainless/key/modification/FixUserIdDoesNotBreakEncryptionCapabilityTest.java @@ -0,0 +1,162 @@ +// SPDX-FileCopyrightText: 2022 Paul Schaub +// +// SPDX-License-Identifier: Apache-2.0 + +package org.pgpainless.key.modification; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPPublicKeyRing; +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.decryption_verification.OpenPgpMetadata; +import org.pgpainless.encryption_signing.EncryptionOptions; +import org.pgpainless.encryption_signing.EncryptionResult; +import org.pgpainless.encryption_signing.EncryptionStream; +import org.pgpainless.encryption_signing.ProducerOptions; +import org.pgpainless.key.info.KeyRingInfo; +import org.pgpainless.key.protection.SecretKeyRingProtector; +import org.pgpainless.signature.subpackets.SelfSignatureSubpackets; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.NoSuchElementException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test for #298. + */ +public class FixUserIdDoesNotBreakEncryptionCapabilityTest { + + private static final String SECRET_KEY = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "\n" + + "lFgEYsyc4hYJKwYBBAHaRw8BAQdAjm3bQ61H2E6/xzjjHjl6G+mNl72r7fwdux9f\n" + + "CXQrCpoAAQDwY5Vblm+7Dq8NfP5gqThyv+23aMBYLr3UgJAZyAgu/RDBtCQoQilv\n" + + "YiAoSilvaG5zb24gPGJqQGV2YWx1YXRpb24udGVzdD6IkAQTFggAOBYhBI70BlHo\n" + + "XvYV3ufIc8MDl+w8xmx4BQJizJziAhsjBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheA\n" + + "AAoJEMMDl+w8xmx4ZAMBAIZsBqoClMlwymvNWIENCAZMQSy9NpBABk3jDyEjbhgs\n" + + "AP9sGI7URQNUDXiV+sIzvastNX/nOZ7fkwp6Xrx+74WxC5xdBGLMnOISCisGAQQB\n" + + "l1UBBQEBB0CGU2EGdS4mvy0apuPukStWSqEDH16AFSGEeTt2GyN1IQMBCAcAAP9J\n" + + "nrIGndqzxxIUHVsoImYIu9SFl9Z1tCSia6mADTtbsA88iHgEGBYIACAWIQSO9AZR\n" + + "6F72Fd7nyHPDA5fsPMZseAUCYsyc4gIbDAAKCRDDA5fsPMZseACnAQDIR7QwBTIs\n" + + "Hfu4XIpZTyipOy6ZOEKlY3akyb9TtOq1wAD8Da+0Insssuf0J5WPqShJ/wMX3+xk\n" + + "gqeRV2HyogQ7aAE=\n" + + "=6zZo\n" + + "-----END PGP PRIVATE KEY BLOCK-----\n"; + private static final String CERTIFICATE = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" + + "\n" + + "mDMEYsyc4hYJKwYBBAHaRw8BAQdAjm3bQ61H2E6/xzjjHjl6G+mNl72r7fwdux9f\n" + + "CXQrCpq0JChCKW9iIChKKW9obnNvbiA8YmpAZXZhbHVhdGlvbi50ZXN0PoiQBBMW\n" + + "CAA4FiEEjvQGUehe9hXe58hzwwOX7DzGbHgFAmLMnOICGyMFCwkIBwIGFQoJCAsC\n" + + "BBYCAwECHgECF4AACgkQwwOX7DzGbHhkAwEAhmwGqgKUyXDKa81YgQ0IBkxBLL02\n" + + "kEAGTeMPISNuGCwA/2wYjtRFA1QNeJX6wjO9qy01f+c5nt+TCnpevH7vhbELuDgE\n" + + "Ysyc4hIKKwYBBAGXVQEFAQEHQIZTYQZ1Lia/LRqm4+6RK1ZKoQMfXoAVIYR5O3Yb\n" + + "I3UhAwEIB4h4BBgWCAAgFiEEjvQGUehe9hXe58hzwwOX7DzGbHgFAmLMnOICGwwA\n" + + "CgkQwwOX7DzGbHgApwEAyEe0MAUyLB37uFyKWU8oqTsumThCpWN2pMm/U7TqtcAA\n" + + "/A2vtCJ7LLLn9CeVj6koSf8DF9/sZIKnkVdh8qIEO2gB\n" + + "=3sNT\n" + + "-----END PGP PUBLIC KEY BLOCK-----"; + + private static final String userIdBefore = "(B)ob (J)ohnson "; + private static final String userIdAfter = "\"(B)ob (J)ohnson\" "; + + @Test + public void manualReplaceUserIdWithFixedVersionDoesNotHinderEncryptionCapability() throws IOException, PGPException { + PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(SECRET_KEY); + SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys(); + PGPSecretKeyRing modified = PGPainless.modifyKeyRing(secretKeys) + .addUserId(userIdAfter, new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + hashedSubpackets.setPrimaryUserId(); + } + }, protector) + .removeUserId(userIdBefore, protector) + .done(); + + KeyRingInfo before = PGPainless.inspectKeyRing(secretKeys); + KeyRingInfo after = PGPainless.inspectKeyRing(modified); + + assertTrue(before.isUsableForEncryption()); + assertTrue(before.isUsableForSigning()); + assertTrue(before.isUserIdValid(userIdBefore)); + assertFalse(before.isUserIdValid(userIdAfter)); + + assertTrue(after.isUsableForEncryption()); + assertTrue(after.isUsableForSigning()); + assertFalse(after.isUserIdValid(userIdBefore)); + assertTrue(after.isUserIdValid(userIdAfter)); + } + + @Test + public void testReplaceUserId_missingOldUserIdThrows() throws IOException { + PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(SECRET_KEY); + assertThrows(NoSuchElementException.class, () -> PGPainless.modifyKeyRing(secretKeys) + .replaceUserId("missing", userIdAfter, SecretKeyRingProtector.unprotectedKeys())); + } + + @Test + public void testReplaceUserId_emptyOldUserIdThrows() throws IOException { + PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(SECRET_KEY); + assertThrows(IllegalArgumentException.class, () -> PGPainless.modifyKeyRing(secretKeys) + .replaceUserId(" ", userIdAfter, SecretKeyRingProtector.unprotectedKeys())); + } + + @Test + public void testReplaceUserId_emptyNewUserIdThrows() throws IOException { + PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(SECRET_KEY); + assertThrows(IllegalArgumentException.class, () -> PGPainless.modifyKeyRing(secretKeys) + .replaceUserId(userIdBefore, " ", SecretKeyRingProtector.unprotectedKeys())); + } + + @Test + public void testReplaceImplicitUserIdDoesNotBreakStuff() throws IOException, PGPException { + PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(SECRET_KEY); + + PGPSecretKeyRing edited = PGPainless.modifyKeyRing(secretKeys) + .replaceUserId(userIdBefore, userIdAfter, SecretKeyRingProtector.unprotectedKeys()) + .done(); + + KeyRingInfo info = PGPainless.inspectKeyRing(edited); + assertTrue(info.isUserIdValid(userIdAfter)); + assertEquals(userIdAfter, info.getPrimaryUserId()); + + assertTrue(info.getLatestUserIdCertification(userIdAfter).getHashedSubPackets().isPrimaryUserID()); + + PGPPublicKeyRing cert = PGPainless.extractCertificate(edited); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + EncryptionStream encryptionStream = PGPainless.encryptAndOrSign() + .onOutputStream(out) + .withOptions(ProducerOptions.encrypt(new EncryptionOptions() + .addRecipient(cert))); + + encryptionStream.write("Hello".getBytes(StandardCharsets.UTF_8)); + encryptionStream.close(); + + EncryptionResult result = encryptionStream.getResult(); + assertTrue(result.isEncryptedFor(cert)); + + ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray()); + ByteArrayOutputStream plain = new ByteArrayOutputStream(); + DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify() + .onInputStream(in) + .withOptions(new ConsumerOptions() + .addDecryptionKey(edited)); + + Streams.pipeAll(decryptionStream, plain); + decryptionStream.close(); + + OpenPgpMetadata metadata = decryptionStream.getResult(); + assertTrue(metadata.isEncrypted()); + } +}