Skip to content

Commit

Permalink
Merge 0acd6de into 2ac10e7
Browse files Browse the repository at this point in the history
  • Loading branch information
vanitasvitae committed Nov 12, 2021
2 parents 2ac10e7 + 0acd6de commit 8a2f459
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 116 deletions.
Expand Up @@ -4,8 +4,6 @@

package org.pgpainless.key.modification.secretkeyring;

import static org.pgpainless.util.CollectionUtils.iteratorToList;

import java.security.InvalidAlgorithmParameterException;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
Expand Down Expand Up @@ -56,7 +54,6 @@
import org.pgpainless.signature.SignatureUtils;
import org.pgpainless.signature.subpackets.SignatureSubpacketGeneratorUtil;
import org.pgpainless.util.Passphrase;
import org.pgpainless.util.selection.userid.SelectUserId;

public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {

Expand Down Expand Up @@ -122,34 +119,6 @@ private String sanitizeUserId(String userId) {
return userId;
}

@Override
public SecretKeyRingEditorInterface deleteUserId(String userId, SecretKeyRingProtector protector) {
return deleteUserIds(SelectUserId.exactMatch(userId), protector);
}

@Override
public SecretKeyRingEditorInterface deleteUserIds(SelectUserId selectionStrategy, SecretKeyRingProtector secretKeyRingProtector) {
List<PGPPublicKey> publicKeys = new ArrayList<>();
Iterator<PGPPublicKey> publicKeyIterator = secretKeyRing.getPublicKeys();
PGPPublicKey primaryKey = publicKeyIterator.next();
List<String> matchingUserIds = selectionStrategy.selectUserIds(iteratorToList(primaryKey.getUserIDs()));
if (matchingUserIds.isEmpty()) {
throw new NoSuchElementException("Key does not have a matching user-id attribute.");
}
for (String userId : matchingUserIds) {
primaryKey = PGPPublicKey.removeCertification(primaryKey, userId);
}
publicKeys.add(primaryKey);

while (publicKeyIterator.hasNext()) {
publicKeys.add(publicKeyIterator.next());
}

PGPPublicKeyRing publicKeyRing = new PGPPublicKeyRing(publicKeys);
secretKeyRing = PGPSecretKeyRing.replacePublicKeys(secretKeyRing, publicKeyRing);
return this;
}

@Override
public SecretKeyRingEditorInterface addSubKey(@Nonnull KeySpec keySpec,
@Nonnull Passphrase subKeyPassphrase,
Expand Down Expand Up @@ -213,29 +182,6 @@ private PGPSecretKey generateSubKey(@Nonnull KeySpec keySpec,
return secretKey;
}

@Override
public SecretKeyRingEditorInterface deleteSubKey(OpenPgpFingerprint fingerprint,
SecretKeyRingProtector protector) {
return deleteSubKey(fingerprint.getKeyId(), protector);
}

@Override
public SecretKeyRingEditorInterface deleteSubKey(long subKeyId,
SecretKeyRingProtector protector) {
if (secretKeyRing.getSecretKey().getKeyID() == subKeyId) {
throw new IllegalArgumentException("You cannot delete the primary key of this key ring.");
}

PGPSecretKey deleteMe = secretKeyRing.getSecretKey(subKeyId);
if (deleteMe == null) {
throw new NoSuchElementException("KeyRing does not contain a key with keyId " + Long.toHexString(subKeyId));
}

PGPSecretKeyRing newKeyRing = PGPSecretKeyRing.removeSecretKey(secretKeyRing, deleteMe);
secretKeyRing = newKeyRing;
return this;
}

@Override
public SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector,
RevocationAttributes revocationAttributes)
Expand Down
Expand Up @@ -22,7 +22,6 @@
import org.pgpainless.key.util.RevocationAttributes;
import org.pgpainless.key.util.UserId;
import org.pgpainless.util.Passphrase;
import org.pgpainless.util.selection.userid.SelectUserId;

public interface SecretKeyRingEditorInterface {

Expand All @@ -46,35 +45,6 @@ default SecretKeyRingEditorInterface addUserId(UserId userId, SecretKeyRingProte
*/
SecretKeyRingEditorInterface addUserId(String userId, SecretKeyRingProtector secretKeyRingProtector) throws PGPException;

/**
* Remove a user-id from the key ring.
*
* @param userId exact user-id to be removed
* @param secretKeyRingProtector protector to unlock the secret key
* @return the builder
*/
SecretKeyRingEditorInterface deleteUserId(String userId, SecretKeyRingProtector secretKeyRingProtector);

/**
* Remove a user-id from the key ring.
*
* @param userId exact user-id to be removed
* @param secretKeyRingProtector protector to unlock the secret key
* @return the builder
*/
default SecretKeyRingEditorInterface deleteUserId(UserId userId, SecretKeyRingProtector secretKeyRingProtector) {
return deleteUserId(userId.toString(), secretKeyRingProtector);
}

/**
* Delete all user-ids from the key, which match the provided {@link SelectUserId} strategy.
*
* @param selectionStrategy strategy to select user-ids
* @param secretKeyRingProtector protector to unlock the secret key
* @return the builder
*/
SecretKeyRingEditorInterface deleteUserIds(SelectUserId selectionStrategy, SecretKeyRingProtector secretKeyRingProtector);

/**
* Add a subkey to the key ring.
* The subkey will be generated from the provided {@link KeySpec}.
Expand All @@ -94,29 +64,6 @@ SecretKeyRingEditorInterface addSubKey(PGPSecretKey subKey,
PGPSignatureSubpacketVector unhashedSubpackets,
SecretKeyRingProtector subKeyProtector, SecretKeyRingProtector keyRingProtector)
throws PGPException;

/**
* Delete a subkey from the key ring.
* The subkey with the provided fingerprint will be remove from the key ring.
* If no suitable subkey is found, a {@link java.util.NoSuchElementException} will be thrown.
*
* @param fingerprint fingerprint of the subkey to be removed
* @param secretKeyRingProtector protector to unlock the secret key ring
* @return the builder
*/
SecretKeyRingEditorInterface deleteSubKey(OpenPgpFingerprint fingerprint, SecretKeyRingProtector secretKeyRingProtector);

/**
* Delete a subkey from the key ring.
* The subkey with the provided key-id will be removed from the key ring.
* If no suitable subkey is found, a {@link java.util.NoSuchElementException} will be thrown.
*
* @param subKeyId id of the subkey
* @param secretKeyRingProtector protector to unlock the secret key ring
* @return the builder
*/
SecretKeyRingEditorInterface deleteSubKey(long subKeyId, SecretKeyRingProtector secretKeyRingProtector);

/**
* Revoke the key ring.
* The revocation will be a hard revocation, rendering the whole key invalid for any past or future signatures.
Expand Down
Expand Up @@ -154,4 +154,50 @@ public static boolean keyRingContainsKeyWithId(@Nonnull PGPPublicKeyRing ring,
long keyId) {
return ring.getPublicKey(keyId) != null;
}

/**
* Delete the given user-id and its certification signatures from the given key.
*
* @deprecated Deleting user-ids is highly discouraged, since it might lead to all sorts of problems
* (e.g. lost key properties).
* Instead, user-ids should only be revoked.
*
* @param secretKeys secret keys
* @param userId user-id
* @return modified secret keys
*/
@Deprecated
public static PGPSecretKeyRing deleteUserIdFromSecretKeyRing(PGPSecretKeyRing secretKeys, String userId) {
PGPSecretKey secretKey = secretKeys.getSecretKey(); // user-ids are located on primary key only
PGPPublicKey publicKey = secretKey.getPublicKey(); // user-ids are placed on the public key part
publicKey = PGPPublicKey.removeCertification(publicKey, userId);
if (publicKey == null) {
throw new NoSuchElementException("User-ID " + userId + " not found on the key.");
}
secretKey = PGPSecretKey.replacePublicKey(secretKey, publicKey);
secretKeys = PGPSecretKeyRing.insertSecretKey(secretKeys, secretKey);
return secretKeys;
}

/**
* Delete the given user-id and its certification signatures from the given certificate.
*
* @deprecated Deleting user-ids is highly discouraged, since it might lead to all sorts of problems
* (e.g. lost key properties).
* Instead, user-ids should only be revoked.
*
* @param publicKeys certificate
* @param userId user-id
* @return modified secret keys
*/
@Deprecated
public static PGPPublicKeyRing deleteUserIdFromPublicKeyRing(PGPPublicKeyRing publicKeys, String userId) {
PGPPublicKey publicKey = publicKeys.getPublicKey(); // user-ids are located on primary key only
publicKey = PGPPublicKey.removeCertification(publicKey, userId);
if (publicKey == null) {
throw new NoSuchElementException("User-ID " + userId + " not found on the key.");
}
publicKeys = PGPPublicKeyRing.insertPublicKey(publicKeys, publicKey);
return publicKeys;
}
}
Expand Up @@ -21,6 +21,7 @@
import org.pgpainless.PGPainless;
import org.pgpainless.implementation.ImplementationFactory;
import org.pgpainless.key.TestKeys;
import org.pgpainless.key.info.KeyRingInfo;
import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector;
import org.pgpainless.key.protection.SecretKeyRingProtector;
import org.pgpainless.key.protection.UnprotectedKeysProtector;
Expand All @@ -30,11 +31,12 @@ public class AddUserIdTest {

@ParameterizedTest
@MethodSource("org.pgpainless.util.TestImplementationFactoryProvider#provideImplementationFactories")
public void addUserIdToExistingKeyRing(ImplementationFactory implementationFactory) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException {
public void addUserIdToExistingKeyRing(ImplementationFactory implementationFactory) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException, InterruptedException {
ImplementationFactory.setFactoryImplementation(implementationFactory);
PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().simpleEcKeyRing("alice@wonderland.lit", "rabb1th0le");

Iterator<String> userIds = secretKeys.getSecretKey().getPublicKey().getUserIDs();
KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys);
Iterator<String> userIds = info.getValidUserIds().iterator();
assertEquals("alice@wonderland.lit", userIds.next());
assertFalse(userIds.hasNext());

Expand All @@ -43,16 +45,18 @@ public void addUserIdToExistingKeyRing(ImplementationFactory implementationFacto
.addUserId("cheshirecat@wonderland.lit", protector)
.done();

userIds = secretKeys.getPublicKey().getUserIDs();
info = PGPainless.inspectKeyRing(secretKeys);
userIds = info.getValidUserIds().iterator();
assertEquals("alice@wonderland.lit", userIds.next());
assertEquals("cheshirecat@wonderland.lit", userIds.next());
assertFalse(userIds.hasNext());

secretKeys = PGPainless.modifyKeyRing(secretKeys)
.deleteUserId("cheshirecat@wonderland.lit", protector)
.revokeUserId("cheshirecat@wonderland.lit", protector)
.done();

userIds = secretKeys.getPublicKey().getUserIDs();
info = PGPainless.inspectKeyRing(secretKeys);
userIds = info.getValidUserIds().iterator();
assertEquals("alice@wonderland.lit", userIds.next());
assertFalse(userIds.hasNext());
}
Expand All @@ -64,7 +68,7 @@ public void deleteUserId_noSuchElementExceptionForMissingUserId(ImplementationFa

PGPSecretKeyRing secretKeys = TestKeys.getCryptieSecretKeyRing();
assertThrows(NoSuchElementException.class, () -> PGPainless.modifyKeyRing(secretKeys)
.deleteUserId("invalid@user.id", new UnprotectedKeysProtector()));
.revokeUserId("invalid@user.id", new UnprotectedKeysProtector()));
}

@ParameterizedTest
Expand All @@ -89,17 +93,19 @@ public void deleteExistingAndAddNewUserIdToExistingKeyRing(ImplementationFactory
"-----END PGP PRIVATE KEY BLOCK-----\r\n";

PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(ARMORED_PRIVATE_KEY);
Iterator<String> userIds = secretKeys.getSecretKey().getPublicKey().getUserIDs();
KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys);
Iterator<String> userIds = info.getValidUserIds().iterator();
assertEquals("<user@example.com>", userIds.next());
assertFalse(userIds.hasNext());

SecretKeyRingProtector protector = new UnprotectedKeysProtector();
secretKeys = PGPainless.modifyKeyRing(secretKeys)
.deleteUserId("<user@example.com>", protector)
.revokeUserId("<user@example.com>", protector)
.addUserId("cheshirecat@wonderland.lit", protector)
.done();

userIds = secretKeys.getSecretKey().getPublicKey().getUserIDs();
info = PGPainless.inspectKeyRing(secretKeys);
userIds = info.getValidUserIds().iterator();
assertEquals("cheshirecat@wonderland.lit", userIds.next());
assertFalse(userIds.hasNext());
}
Expand Down
@@ -0,0 +1,53 @@
// SPDX-FileCopyrightText: 2021 Paul Schaub <vanitasvitae@fsfe.org>
//
// SPDX-License-Identifier: Apache-2.0

package org.pgpainless.key.util;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.security.InvalidAlgorithmParameterException;
import java.security.NoSuchAlgorithmException;

import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSecretKeyRing;
import org.junit.jupiter.api.Test;
import org.pgpainless.PGPainless;
import org.pgpainless.key.protection.SecretKeyRingProtector;
import org.pgpainless.util.CollectionUtils;

public class KeyRingUtilTest {

@Test
public void testDeleteUserIdFromSecretKeyRing()
throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException {
PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing()
.modernKeyRing("Alice", null);

secretKeys = PGPainless.modifyKeyRing(secretKeys)
.addUserId("Bob", SecretKeyRingProtector.unprotectedKeys())
.done();
assertEquals(2, CollectionUtils.iteratorToList(secretKeys.getPublicKey().getUserIDs()).size());

secretKeys = KeyRingUtils.deleteUserIdFromSecretKeyRing(secretKeys, "Bob");

assertEquals(1, CollectionUtils.iteratorToList(secretKeys.getPublicKey().getUserIDs()).size());
}

@Test
public void testDeleteUserIdFromPublicKeyRing() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException {
PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing()
.modernKeyRing("Alice", null);

secretKeys = PGPainless.modifyKeyRing(secretKeys)
.addUserId("Bob", SecretKeyRingProtector.unprotectedKeys())
.done();
PGPPublicKeyRing publicKeys = PGPainless.extractCertificate(secretKeys);
assertEquals(2, CollectionUtils.iteratorToList(publicKeys.getPublicKey().getUserIDs()).size());

publicKeys = KeyRingUtils.deleteUserIdFromPublicKeyRing(publicKeys, "Alice");

assertEquals(1, CollectionUtils.iteratorToList(publicKeys.getPublicKey().getUserIDs()).size());
}
}

0 comments on commit 8a2f459

Please sign in to comment.