From 151d3c7b9657d39d9d05cdd79dde404efb964ebc Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Sat, 27 Nov 2021 14:50:04 +0100 Subject: [PATCH] SecretKeyRingEditor: Restructure arguments of modification methods --- .../org/pgpainless/key/info/KeyRingInfo.java | 6 +- .../secretkeyring/SecretKeyRingEditor.java | 140 ++++--- .../SecretKeyRingEditorInterface.java | 355 +++++++++++++----- .../KeyGenerationSubpacketsTest.java | 5 +- .../key/modification/AddUserIdTest.java | 18 + .../key/modification/RevokeUserIdsTest.java | 6 +- 6 files changed, 383 insertions(+), 147 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java index 37270b58..a75b5741 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java @@ -254,8 +254,8 @@ public class KeyRingInfo { /** * Return the primary user-id of the key ring. * - * Note: If no user-id is marked as primary key using a {@link PrimaryUserID} packet, this method returns the - * first valid user-id, otherwise null. + * Note: If no user-id is marked as primary key using a {@link PrimaryUserID} packet, + * this method returns the first valid user-id, otherwise null. * * @return primary user-id or null */ @@ -278,7 +278,7 @@ public class KeyRingInfo { PrimaryUserID subpacket = SignatureSubpacketsUtil.getPrimaryUserId(signature); if (subpacket != null && subpacket.isPrimaryUserID()) { // if there are multiple primary userIDs, return most recently signed - if (modificationDate == null || modificationDate.before(signature.getCreationTime())) { + if (modificationDate == null || !signature.getCreationTime().before(modificationDate)) { primaryUserId = userId; modificationDate = signature.getCreationTime(); } 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 25550dbf..0f2b0eee 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 @@ -87,18 +87,19 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Override public SecretKeyRingEditorInterface addUserId( - String userId, - SecretKeyRingProtector secretKeyRingProtector) + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { return addUserId(userId, null, secretKeyRingProtector); } @Override public SecretKeyRingEditorInterface addUserId( - String userId, + @Nonnull CharSequence userId, @Nullable SelfSignatureSubpackets.Callback signatureSubpacketCallback, - SecretKeyRingProtector protector) throws PGPException { - userId = sanitizeUserId(userId); + @Nonnull SecretKeyRingProtector protector) + throws PGPException { + String sanitizeUserId = sanitizeUserId(userId); // user-id certifications live on the primary key PGPSecretKey primaryKey = secretKeyRing.getSecretKey(); @@ -134,25 +135,39 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { builder.applyCallback(signatureSubpacketCallback); - PGPSignature signature = builder.build(primaryKey.getPublicKey(), userId); - secretKeyRing = KeyRingUtils.injectCertification(secretKeyRing, userId, signature); + PGPSignature signature = builder.build(primaryKey.getPublicKey(), sanitizeUserId); + secretKeyRing = KeyRingUtils.injectCertification(secretKeyRing, sanitizeUserId, signature); return this; } + @Override + public SecretKeyRingEditorInterface addPrimaryUserId( + @Nonnull CharSequence userId, @Nonnull SecretKeyRingProtector protector) + throws PGPException { + return addUserId( + userId, + new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + hashedSubpackets.setPrimaryUserId(); + } + }, + protector); + } + // TODO: Move to utility class? - private String sanitizeUserId(String userId) { - userId = userId.trim(); + private String sanitizeUserId(@Nonnull CharSequence userId) { // TODO: Further research how to sanitize user IDs. // eg. what about newlines? - return userId; + return userId.toString().trim(); } @Override public SecretKeyRingEditorInterface addSubKey( @Nonnull KeySpec keySpec, @Nonnull Passphrase subKeyPassphrase, - SecretKeyRingProtector secretKeyRingProtector) + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException, IOException { PGPKeyPair keyPair = KeyRingBuilder.generateKeyPair(keySpec); @@ -179,7 +194,7 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Nonnull KeySpec keySpec, @Nullable Passphrase subkeyPassphrase, @Nullable SelfSignatureSubpackets.Callback subpacketsCallback, - SecretKeyRingProtector secretKeyRingProtector) + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { PGPKeyPair keyPair = KeyRingBuilder.generateKeyPair(keySpec); @@ -195,11 +210,11 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Override public SecretKeyRingEditorInterface addSubKey( - PGPKeyPair subkey, + @Nonnull PGPKeyPair subkey, @Nullable SelfSignatureSubpackets.Callback bindingSignatureCallback, - SecretKeyRingProtector subkeyProtector, - SecretKeyRingProtector primaryKeyProtector, - KeyFlag keyFlag, + @Nonnull SecretKeyRingProtector subkeyProtector, + @Nonnull SecretKeyRingProtector primaryKeyProtector, + @Nonnull KeyFlag keyFlag, KeyFlag... additionalKeyFlags) throws PGPException, IOException { KeyFlag[] flags = concat(keyFlag, additionalKeyFlags); @@ -251,7 +266,7 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } @Override - public SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector, + public SecretKeyRingEditorInterface revoke(@Nonnull SecretKeyRingProtector secretKeyRingProtector, @Nullable RevocationAttributes revocationAttributes) throws PGPException { RevocationSignatureSubpackets.Callback callback = callbackFromRevocationAttributes(revocationAttributes); @@ -259,7 +274,7 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } @Override - public SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector, + public SecretKeyRingEditorInterface revoke(@Nonnull SecretKeyRingProtector secretKeyRingProtector, @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) throws PGPException { return revokeSubKey(secretKeyRing.getSecretKey().getKeyID(), secretKeyRingProtector, subpacketsCallback); @@ -276,7 +291,7 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Override public SecretKeyRingEditorInterface revokeSubKey(long keyID, - SecretKeyRingProtector secretKeyRingProtector, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) throws PGPException { // retrieve subkey to be revoked @@ -290,8 +305,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } @Override - public PGPSignature createRevocationCertificate(SecretKeyRingProtector secretKeyRingProtector, - RevocationAttributes revocationAttributes) + public PGPSignature createRevocationCertificate(@Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) throws PGPException { PGPPublicKey revokeeSubKey = secretKeyRing.getPublicKey(); PGPSignature revocationCertificate = generateRevocation( @@ -302,8 +317,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Override public PGPSignature createRevocationCertificate( long subkeyId, - SecretKeyRingProtector secretKeyRingProtector, - RevocationAttributes revocationAttributes) + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) throws PGPException { PGPPublicKey revokeeSubkey = KeyRingUtils.requirePublicKeyFrom(secretKeyRing, subkeyId); RevocationSignatureSubpackets.Callback callback = callbackFromRevocationAttributes(revocationAttributes); @@ -313,15 +328,15 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Override public PGPSignature createRevocationCertificate( long subkeyId, - SecretKeyRingProtector secretKeyRingProtector, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, @Nullable RevocationSignatureSubpackets.Callback certificateSubpacketsCallback) throws PGPException { PGPPublicKey revokeeSubkey = KeyRingUtils.requirePublicKeyFrom(secretKeyRing, subkeyId); return generateRevocation(secretKeyRingProtector, revokeeSubkey, certificateSubpacketsCallback); } - private PGPSignature generateRevocation(SecretKeyRingProtector protector, - PGPPublicKey revokeeSubKey, + private PGPSignature generateRevocation(@Nonnull SecretKeyRingProtector protector, + @Nonnull PGPPublicKey revokeeSubKey, @Nullable RevocationSignatureSubpackets.Callback callback) throws PGPException { PGPSecretKey primaryKey = secretKeyRing.getSecretKey(); @@ -336,7 +351,7 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } private static RevocationSignatureSubpackets.Callback callbackFromRevocationAttributes( - RevocationAttributes attributes) { + @Nullable RevocationAttributes attributes) { return new RevocationSignatureSubpackets.Callback() { @Override public void modifyHashedSubpackets(RevocationSignatureSubpackets hashedSubpackets) { @@ -348,9 +363,10 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } @Override - public SecretKeyRingEditorInterface revokeUserId(String userId, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationAttributes revocationAttributes) + public SecretKeyRingEditorInterface revokeUserId( + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) throws PGPException { if (revocationAttributes != null) { RevocationAttributes.Reason reason = revocationAttributes.getReason(); @@ -374,26 +390,41 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Override public SecretKeyRingEditorInterface revokeUserId( - String userId, - SecretKeyRingProtector secretKeyRingProtector, + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, @Nullable RevocationSignatureSubpackets.Callback subpacketCallback) throws PGPException { - Iterator userIds = secretKeyRing.getPublicKey().getUserIDs(); - boolean found = false; - while (userIds.hasNext()) { - if (userId.equals(userIds.next())) { - found = true; - break; - } - } - if (!found) { - throw new NoSuchElementException("No user-id '" + userId + "' found on the key."); - } - return doRevokeUserId(userId, secretKeyRingProtector, subpacketCallback); + String sanitized = sanitizeUserId(userId); + return revokeUserIds( + SelectUserId.exactMatch(sanitized), + secretKeyRingProtector, + subpacketCallback); } @Override - public SecretKeyRingEditorInterface revokeUserIds(SelectUserId userIdSelector, SecretKeyRingProtector secretKeyRingProtector, @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) throws PGPException { + public SecretKeyRingEditorInterface revokeUserIds( + @Nonnull SelectUserId userIdSelector, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) + throws PGPException { + + return revokeUserIds( + userIdSelector, + secretKeyRingProtector, + new RevocationSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(RevocationSignatureSubpackets hashedSubpackets) { + hashedSubpackets.setRevocationReason(revocationAttributes); + } + }); + } + + @Override + public SecretKeyRingEditorInterface revokeUserIds( + @Nonnull SelectUserId userIdSelector, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) + throws PGPException { List selected = userIdSelector.selectUserIds(secretKeyRing); if (selected.isEmpty()) { throw new NoSuchElementException("No matching user-ids found on the key."); @@ -406,9 +437,10 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { return this; } - private SecretKeyRingEditorInterface doRevokeUserId(String userId, - SecretKeyRingProtector protector, - @Nullable RevocationSignatureSubpackets.Callback callback) + private SecretKeyRingEditorInterface doRevokeUserId( + @Nonnull String userId, + @Nonnull SecretKeyRingProtector protector, + @Nullable RevocationSignatureSubpackets.Callback callback) throws PGPException { PGPSecretKey primarySecretKey = secretKeyRing.getSecretKey(); RevocationSignatureBuilder signatureBuilder = new RevocationSignatureBuilder( @@ -424,16 +456,18 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } @Override - public SecretKeyRingEditorInterface setExpirationDate(Date expiration, - SecretKeyRingProtector secretKeyRingProtector) + public SecretKeyRingEditorInterface setExpirationDate( + @Nullable Date expiration, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { return setExpirationDate(OpenPgpFingerprint.of(secretKeyRing), expiration, secretKeyRingProtector); } @Override - public SecretKeyRingEditorInterface setExpirationDate(OpenPgpFingerprint fingerprint, - Date expiration, - SecretKeyRingProtector secretKeyRingProtector) + public SecretKeyRingEditorInterface setExpirationDate( + @Nonnull OpenPgpFingerprint fingerprint, + @Nullable Date expiration, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { List secretKeyList = new ArrayList<>(); 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 0e662a50..87c53acd 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 @@ -21,7 +21,6 @@ import org.pgpainless.key.generation.KeySpec; import org.pgpainless.key.protection.KeyRingProtectionSettings; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.key.util.RevocationAttributes; -import org.pgpainless.key.util.UserId; import org.pgpainless.signature.subpackets.RevocationSignatureSubpackets; import org.pgpainless.signature.subpackets.SelfSignatureSubpackets; import org.pgpainless.util.Passphrase; @@ -36,23 +35,39 @@ public interface SecretKeyRingEditorInterface { * @param secretKeyRingProtector protector to unlock the secret key * @return the builder */ - default SecretKeyRingEditorInterface addUserId(UserId userId, SecretKeyRingProtector secretKeyRingProtector) throws PGPException { - return addUserId(userId.toString(), secretKeyRingProtector); - } + SecretKeyRingEditorInterface addUserId( + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) + throws PGPException; /** * Add a user-id to the key ring. * * @param userId user-id - * @param secretKeyRingProtector protector to unlock the secret key + * @param signatureSubpacketCallback callback that can be used to modify signature subpackets of the + * certification signature. + * @param protector protector to unlock the primary secret key + * @return the builder + * @throws PGPException + */ + SecretKeyRingEditorInterface addUserId( + @Nonnull CharSequence userId, + @Nullable SelfSignatureSubpackets.Callback signatureSubpacketCallback, + @Nonnull SecretKeyRingProtector protector) + throws PGPException; + + /** + * Add a user-id to the key ring and mark it as primary. + * If the user-id is already present, a new certification signature will be created. + * + * @param userId user id + * @param protector protector to unlock the secret key * @return the builder */ - SecretKeyRingEditorInterface addUserId(String userId, SecretKeyRingProtector secretKeyRingProtector) throws PGPException; - - SecretKeyRingEditorInterface addUserId( - String userId, - @Nullable SelfSignatureSubpackets.Callback signatureSubpacketCallback, - SecretKeyRingProtector protector) throws PGPException; + SecretKeyRingEditorInterface addPrimaryUserId( + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector protector) + throws PGPException; /** * Add a subkey to the key ring. @@ -63,22 +78,48 @@ public interface SecretKeyRingEditorInterface { * @param secretKeyRingProtector protector to unlock the secret key of the key ring * @return the builder */ - SecretKeyRingEditorInterface addSubKey(@Nonnull KeySpec keySpec, - @Nullable Passphrase subKeyPassphrase, - SecretKeyRingProtector secretKeyRingProtector) + SecretKeyRingEditorInterface addSubKey( + @Nonnull KeySpec keySpec, + @Nonnull Passphrase subKeyPassphrase, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException, IOException; - SecretKeyRingEditorInterface addSubKey(@Nonnull KeySpec keySpec, - @Nullable Passphrase subkeyPassphrase, - @Nullable SelfSignatureSubpackets.Callback subpacketsCallback, - SecretKeyRingProtector secretKeyRingProtector) throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException; + /** + * Add a subkey to the key ring. + * The subkey will be generated from the provided {@link KeySpec}. + * + * @param keySpec key spec of the subkey + * @param subkeyPassphrase passphrase to encrypt the subkey + * @param subpacketsCallback callback to modify the subpackets of the subkey binding signature + * @param secretKeyRingProtector protector to unlock the primary key + * @return builder + */ + SecretKeyRingEditorInterface addSubKey( + @Nonnull KeySpec keySpec, + @Nonnull Passphrase subkeyPassphrase, + @Nullable SelfSignatureSubpackets.Callback subpacketsCallback, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException; - SecretKeyRingEditorInterface addSubKey(PGPKeyPair subkey, - @Nullable SelfSignatureSubpackets.Callback bindingSignatureCallback, - SecretKeyRingProtector subkeyProtector, - SecretKeyRingProtector primaryKeyProtector, - KeyFlag keyFlag, - KeyFlag... additionalKeyFlags) throws PGPException, IOException; + /** + * Add a subkey to the key ring. + * + * @param subkey subkey key pair + * @param bindingSignatureCallback callback to modify the subpackets of the subkey binding signature + * @param subkeyProtector protector to unlock and encrypt the subkey + * @param primaryKeyProtector protector to unlock the primary key + * @param keyFlag first key flag for the subkey + * @param additionalKeyFlags optional additional key flags + * @return builder + */ + SecretKeyRingEditorInterface addSubKey( + @Nonnull PGPKeyPair subkey, + @Nullable SelfSignatureSubpackets.Callback bindingSignatureCallback, + @Nonnull SecretKeyRingProtector subkeyProtector, + @Nonnull SecretKeyRingProtector primaryKeyProtector, + @Nonnull KeyFlag keyFlag, + KeyFlag... additionalKeyFlags) + throws PGPException, IOException; /** * Revoke the key ring. @@ -87,37 +128,55 @@ public interface SecretKeyRingEditorInterface { * @param secretKeyRingProtector protector of the primary key * @return the builder */ - default SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector) + default SecretKeyRingEditorInterface revoke( + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { return revoke(secretKeyRingProtector, (RevocationAttributes) null); } /** * Revoke the key ring using the provided revocation attributes. - * The attributes define, whether or not the revocation was a hard revocation or not. + * The attributes define, whether the revocation was a hard revocation or not. * * @param secretKeyRingProtector protector of the primary key * @param revocationAttributes reason for the revocation * @return the builder */ - SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationAttributes revocationAttributes) + SecretKeyRingEditorInterface revoke( + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) throws PGPException; - SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) throws PGPException; + /** + * Revoke the key ring. + * You can use the {@link RevocationSignatureSubpackets.Callback} to modify the revocation signatures + * subpackets, eg. in order to define whether this is a hard or soft revocation. + * + * @param secretKeyRingProtector protector to unlock the primary secret key + * @param subpacketsCallback callback to modify the revocations subpackets + * @return builder + */ + SecretKeyRingEditorInterface revoke( + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) throws PGPException; /** * Revoke the subkey binding signature of a subkey. * The subkey with the provided fingerprint will be revoked. * If no suitable subkey is found, a {@link java.util.NoSuchElementException} will be thrown. * + * Note: This method will hard-revoke the provided subkey, meaning it cannot be re-certified at a later point. + * If you instead want to temporarily "deactivate" the subkey, provide a soft revocation reason, + * eg. by calling {@link #revokeSubKey(OpenPgpFingerprint, SecretKeyRingProtector, RevocationAttributes)} + * and provide a suitable {@link RevocationAttributes} object. + * * @param fingerprint fingerprint of the subkey to be revoked * @param secretKeyRingProtector protector to unlock the secret key ring * @return the builder */ - default SecretKeyRingEditorInterface revokeSubKey(OpenPgpFingerprint fingerprint, - SecretKeyRingProtector secretKeyRingProtector) + default SecretKeyRingEditorInterface revokeSubKey( + @Nonnull OpenPgpFingerprint fingerprint, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { return revokeSubKey(fingerprint, secretKeyRingProtector, null); } @@ -132,9 +191,10 @@ public interface SecretKeyRingEditorInterface { * @param revocationAttributes reason for the revocation * @return the builder */ - default SecretKeyRingEditorInterface revokeSubKey(OpenPgpFingerprint fingerprint, - SecretKeyRingProtector secretKeyRingProtector, - RevocationAttributes revocationAttributes) + default SecretKeyRingEditorInterface revokeSubKey( + OpenPgpFingerprint fingerprint, + SecretKeyRingProtector secretKeyRingProtector, + RevocationAttributes revocationAttributes) throws PGPException { return revokeSubKey(fingerprint.getKeyId(), secretKeyRingProtector, @@ -144,16 +204,17 @@ public interface SecretKeyRingEditorInterface { /** * 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. + * 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 primary key * @param revocationAttributes reason for the revocation * @return the builder */ - SecretKeyRingEditorInterface revokeSubKey(long subKeyId, - SecretKeyRingProtector secretKeyRingProtector, - RevocationAttributes revocationAttributes) + SecretKeyRingEditorInterface revokeSubKey( + long subKeyId, + SecretKeyRingProtector secretKeyRingProtector, + RevocationAttributes revocationAttributes) throws PGPException; /** @@ -161,31 +222,59 @@ public interface SecretKeyRingEditorInterface { * The subkey with the provided key-id will be revoked. * If no suitable subkey is found, q {@link java.util.NoSuchElementException} will be thrown. * + * Note: This method will hard-revoke the subkey, meaning it cannot be re-bound at a later point. + * If you intend to re-bind the subkey in order to make it usable again at a later point in time, + * consider using {@link #revokeSubKey(long, SecretKeyRingProtector, RevocationAttributes)} + * and provide a soft revocation reason. + * * @param subKeyId id of the subkey * @param secretKeyRingProtector protector to unlock the secret key ring * @return the builder */ - default SecretKeyRingEditorInterface revokeSubKey(long subKeyId, - SecretKeyRingProtector secretKeyRingProtector) + default SecretKeyRingEditorInterface revokeSubKey( + long subKeyId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { - return revokeSubKey(subKeyId, secretKeyRingProtector, (RevocationSignatureSubpackets.Callback) null); + + return revokeSubKey( + subKeyId, + secretKeyRingProtector, + (RevocationSignatureSubpackets.Callback) null); } - SecretKeyRingEditorInterface revokeSubKey(long keyID, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) + /** + * 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. + * + * The provided subpackets callback is used to modify the revocation signatures subpackets. + * + * @param keyID id of the subkey + * @param secretKeyRingProtector protector to unlock the secret key ring + * @param subpacketsCallback callback which can be used to modify the subpackets of the revocation + * signature + * @return the builder + */ + SecretKeyRingEditorInterface revokeSubKey( + long keyID, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) throws PGPException; /** * Revoke the given userID. * The revocation will be a hard revocation, rendering the user-id invalid for any past or future signatures. + * If you intend to re-certify the user-id at a later point in time, consider using + * {@link #revokeUserId(CharSequence, SecretKeyRingProtector, RevocationAttributes)} instead and provide + * a soft revocation reason. * * @param userId userId to revoke * @param secretKeyRingProtector protector to unlock the primary key * @return the builder */ - default SecretKeyRingEditorInterface revokeUserId(String userId, - SecretKeyRingProtector secretKeyRingProtector) + default SecretKeyRingEditorInterface revokeUserId( + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { return revokeUserId(userId, secretKeyRingProtector, (RevocationAttributes) null); } @@ -198,20 +287,71 @@ public interface SecretKeyRingEditorInterface { * @param revocationAttributes reason for the revocation * @return the builder */ - SecretKeyRingEditorInterface revokeUserId(String userId, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationAttributes revocationAttributes) + SecretKeyRingEditorInterface revokeUserId( + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) throws PGPException; - SecretKeyRingEditorInterface revokeUserId(String userId, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationSignatureSubpackets.Callback subpacketCallback) - throws PGPException; + /** + * Revoke the provided user-id. + * Note: If you don't provide a {@link RevocationSignatureSubpackets.Callback} which + * sets a revocation reason ({@link RevocationAttributes}), the revocation might be considered hard. + * So if you intend to re-certify the user-id at a later point to make it valid again, + * make sure to set a soft revocation reason in the signatures hashed area using the subpacket callback. + * + * @param userId userid to be revoked + * @param secretKeyRingProtector protector to unlock the primary secret key + * @param subpacketCallback callback to modify the revocations subpackets + * @return builder + */ + SecretKeyRingEditorInterface revokeUserId( + @Nonnull CharSequence userId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationSignatureSubpackets.Callback subpacketCallback) + throws PGPException; - SecretKeyRingEditorInterface revokeUserIds(SelectUserId userIdSelector, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) - throws PGPException; + /** + * Revoke all user-ids that match the provided {@link SelectUserId} filter. + * The provided {@link RevocationAttributes} will be set as reason for revocation in each + * revocation signature. + * + * Note: If you intend to re-certify these user-ids at a later point, make sure to choose + * a soft revocation reason. See {@link RevocationAttributes.Reason} for more information. + * + * @param userIdSelector user-id selector + * @param secretKeyRingProtector protector to unlock the primary secret key + * @param revocationAttributes revocation attributes + * @return builder + * @throws PGPException + */ + SecretKeyRingEditorInterface revokeUserIds( + @Nonnull SelectUserId userIdSelector, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) + throws PGPException; + + /** + * Revoke all user-ids that match the provided {@link SelectUserId} filter. + * The provided {@link RevocationSignatureSubpackets.Callback} will be used to modify the + * revocation signatures subpackets. + * + * Note: If you intend to re-certify these user-ids at a later point, make sure to set + * a soft revocation reason in the revocation signatures hashed subpacket area using the callback. + * + * See {@link RevocationAttributes.Reason} for more information. + * + * @param userIdSelector user-id selector + * @param secretKeyRingProtector protector to unlock the primary secret key + * @param subpacketsCallback callback to modify the revocations subpackets + * @return builder + * @throws PGPException + */ + SecretKeyRingEditorInterface revokeUserIds( + @Nonnull SelectUserId userIdSelector, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback) + throws PGPException; /** * Set the expiration date for the primary key of the key ring. @@ -221,8 +361,9 @@ public interface SecretKeyRingEditorInterface { * @param secretKeyRingProtector to unlock the secret key * @return the builder */ - SecretKeyRingEditorInterface setExpirationDate(Date expiration, - SecretKeyRingProtector secretKeyRingProtector) + SecretKeyRingEditorInterface setExpirationDate( + @Nullable Date expiration, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException; /** @@ -233,37 +374,70 @@ public interface SecretKeyRingEditorInterface { * @param secretKeyRingProtector protector to unlock the priary key * @return the builder */ - SecretKeyRingEditorInterface setExpirationDate(OpenPgpFingerprint fingerprint, - Date expiration, - SecretKeyRingProtector secretKeyRingProtector) + SecretKeyRingEditorInterface setExpirationDate( + @Nonnull OpenPgpFingerprint fingerprint, + @Nullable Date expiration, + @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException; /** - * Create a detached revocation certificate, which can be used to revoke the specified key. + * Create a detached revocation certificate, which can be used to revoke the whole key. * * @param secretKeyRingProtector protector to unlock the primary key. * @param revocationAttributes reason for the revocation * @return revocation certificate */ - PGPSignature createRevocationCertificate(SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationAttributes revocationAttributes) + PGPSignature createRevocationCertificate( + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) throws PGPException; - PGPSignature createRevocationCertificate(long subkeyId, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationAttributes revocationAttributes) - throws PGPException; + /** + * Create a detached revocation certificate, which can be used to revoke the specified subkey. + * + * @param subkeyId id of the subkey to be revoked + * @param secretKeyRingProtector protector to unlock the primary key. + * @param revocationAttributes reason for the revocation + * @return revocation certificate + */ + PGPSignature createRevocationCertificate( + long subkeyId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) + throws PGPException; - PGPSignature createRevocationCertificate(long subkeyId, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationSignatureSubpackets.Callback certificateSubpacketsCallback) - throws PGPException; + /** + * Create a detached revocation certificate, which can be used to revoke the specified subkey. + * + * @param subkeyId id of the subkey to be revoked + * @param secretKeyRingProtector protector to unlock the primary key. + * @param certificateSubpacketsCallback callback to modify the subpackets of the revocation certificate. + * @return revocation certificate + */ + PGPSignature createRevocationCertificate( + long subkeyId, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationSignatureSubpackets.Callback certificateSubpacketsCallback) + throws PGPException; - default PGPSignature createRevocationCertificate(OpenPgpFingerprint subkeyFingerprint, - SecretKeyRingProtector secretKeyRingProtector, - @Nullable RevocationAttributes revocationAttributes) - throws PGPException { - return createRevocationCertificate(subkeyFingerprint.getKeyId(), secretKeyRingProtector, revocationAttributes); + /** + * Create a detached revocation certificate, which can be used to revoke the specified subkey. + * + * @param subkeyFingerprint fingerprint of the subkey to be revoked + * @param secretKeyRingProtector protector to unlock the primary key. + * @param revocationAttributes reason for the revocation + * @return revocation certificate + */ + default PGPSignature createRevocationCertificate( + OpenPgpFingerprint subkeyFingerprint, + SecretKeyRingProtector secretKeyRingProtector, + @Nullable RevocationAttributes revocationAttributes) + throws PGPException { + + return createRevocationCertificate( + subkeyFingerprint.getKeyId(), + secretKeyRingProtector, + revocationAttributes); } /** @@ -272,7 +446,8 @@ public interface SecretKeyRingEditorInterface { * @param oldPassphrase old passphrase or null, if the key was unprotected * @return next builder step */ - default WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase(@Nullable Passphrase oldPassphrase) { + default WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase( + @Nullable Passphrase oldPassphrase) { return changePassphraseFromOldPassphrase(oldPassphrase, KeyRingProtectionSettings.secureDefaultSettings()); } @@ -283,8 +458,9 @@ public interface SecretKeyRingEditorInterface { * @param oldProtectionSettings custom settings for the old passphrase * @return next builder step */ - WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase(@Nullable Passphrase oldPassphrase, - @Nonnull KeyRingProtectionSettings oldProtectionSettings); + WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase( + @Nullable Passphrase oldPassphrase, + @Nonnull KeyRingProtectionSettings oldProtectionSettings); /** * Change the passphrase of a single subkey in the key ring. @@ -296,14 +472,16 @@ public interface SecretKeyRingEditorInterface { * @param oldPassphrase old passphrase * @return next builder step */ - default WithKeyRingEncryptionSettings changeSubKeyPassphraseFromOldPassphrase(@Nonnull Long keyId, - @Nullable Passphrase oldPassphrase) { + 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); + WithKeyRingEncryptionSettings changeSubKeyPassphraseFromOldPassphrase( + @Nonnull Long keyId, + @Nullable Passphrase oldPassphrase, + @Nonnull KeyRingProtectionSettings oldProtectionSettings); interface WithKeyRingEncryptionSettings { @@ -333,7 +511,8 @@ public interface SecretKeyRingEditorInterface { * @param passphrase passphrase * @return editor builder */ - SecretKeyRingEditorInterface toNewPassphrase(Passphrase passphrase) throws PGPException; + SecretKeyRingEditorInterface toNewPassphrase(Passphrase passphrase) + throws PGPException; /** * Leave the key unprotected. diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java index 5451a4dc..4d6458ef 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java @@ -39,7 +39,7 @@ public class KeyGenerationSubpacketsTest { @Test public void verifyDefaultSubpacketsForUserIdSignatures() - throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException { + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, InterruptedException { PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("Alice", null); KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); @@ -87,6 +87,9 @@ public class KeyGenerationSubpacketsTest { assertEquals("Bob", info.getPrimaryUserId()); + // wait one sec so that it is clear that the new certification for alice is the most recent one + Thread.sleep(1000); + secretKeys = PGPainless.modifyKeyRing(secretKeys) .addUserId("Alice", new SelfSignatureSubpackets.Callback() { @Override diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/modification/AddUserIdTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/modification/AddUserIdTest.java index f8998c2b..705e7d48 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/modification/AddUserIdTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/modification/AddUserIdTest.java @@ -6,6 +6,7 @@ package org.pgpainless.key.modification; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; @@ -16,6 +17,7 @@ import java.util.NoSuchElementException; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.pgpainless.PGPainless; @@ -25,6 +27,7 @@ import org.pgpainless.key.info.KeyRingInfo; import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.key.protection.UnprotectedKeysProtector; +import org.pgpainless.key.util.UserId; import org.pgpainless.util.Passphrase; public class AddUserIdTest { @@ -109,4 +112,19 @@ public class AddUserIdTest { assertEquals("cheshirecat@wonderland.lit", userIds.next()); assertFalse(userIds.hasNext()); } + + @Test + public void addNewPrimaryUserIdTest() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing() + .modernKeyRing("Alice", null); + UserId bob = UserId.newBuilder().withName("Bob").noEmail().noComment().build(); + + assertNotEquals("Bob", PGPainless.inspectKeyRing(secretKeys).getPrimaryUserId()); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addPrimaryUserId(bob, SecretKeyRingProtector.unprotectedKeys()) + .done(); + + assertEquals("Bob", PGPainless.inspectKeyRing(secretKeys).getPrimaryUserId()); + } } diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeUserIdsTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeUserIdsTest.java index f5d070d7..4be6c245 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeUserIdsTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeUserIdsTest.java @@ -10,6 +10,8 @@ import org.junit.jupiter.api.Test; import org.pgpainless.PGPainless; import org.pgpainless.key.info.KeyRingInfo; import org.pgpainless.key.protection.SecretKeyRingProtector; +import org.pgpainless.key.util.RevocationAttributes; +import org.pgpainless.signature.subpackets.RevocationSignatureSubpackets; import org.pgpainless.util.selection.userid.SelectUserId; import java.security.InvalidAlgorithmParameterException; @@ -39,7 +41,7 @@ public class RevokeUserIdsTest { assertTrue(info.isUserIdValid("Alice ")); secretKeys = PGPainless.modifyKeyRing(secretKeys) - .revokeUserIds(SelectUserId.containsEmailAddress("alice@example.org"), protector, null) + .revokeUserIds(SelectUserId.containsEmailAddress("alice@example.org"), protector, (RevocationSignatureSubpackets.Callback) null) .done(); info = PGPainless.inspectKeyRing(secretKeys); @@ -57,6 +59,6 @@ public class RevokeUserIdsTest { PGPainless.modifyKeyRing(secretKeys).revokeUserIds( SelectUserId.containsEmailAddress("alice@example.org"), SecretKeyRingProtector.unprotectedKeys(), - null)); + (RevocationAttributes) null)); } }