From 710f96198446ecba70dc21fdacc0995d77b5bd47 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Mon, 20 Dec 2021 12:46:31 +0100 Subject: [PATCH] Rework key modification API. Fixes #225 --- .../org/pgpainless/key/info/KeyRingInfo.java | 125 +++++++-- .../secretkeyring/SecretKeyRingEditor.java | 237 ++++++++++-------- .../SecretKeyRingEditorInterface.java | 14 -- .../SubkeyBindingSignatureBuilder.java | 15 ++ .../signature/consumer/SignaturePicker.java | 4 - .../subpackets/SignatureSubpacketsUtil.java | 7 +- .../org/pgpainless/example/ModifyKeys.java | 29 --- .../pgpainless/key/info/KeyRingInfoTest.java | 7 +- .../modification/ChangeExpirationTest.java | 18 +- ...gePrimaryUserIdAndExpirationDatesTest.java | 224 +++++++++++++++++ ...gnatureSubpacketsArePreservedOnNewSig.java | 16 +- ...ithoutPreferredAlgorithmsOnPrimaryKey.java | 15 +- .../SignatureSubpacketsUtilTest.java | 3 +- 13 files changed, 510 insertions(+), 204 deletions(-) create mode 100644 pgpainless-core/src/test/java/org/pgpainless/key/modification/ChangePrimaryUserIdAndExpirationDatesTest.java 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 a75b5741..a0cf76c3 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 @@ -22,6 +22,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.bouncycastle.bcpg.sig.PrimaryUserID; +import org.bouncycastle.bcpg.sig.RevocationReason; import org.bouncycastle.openpgp.PGPKeyRing; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPPublicKeyRing; @@ -38,9 +39,10 @@ import org.pgpainless.algorithm.SymmetricKeyAlgorithm; import org.pgpainless.exception.KeyValidationError; import org.pgpainless.key.OpenPgpFingerprint; import org.pgpainless.key.SubkeyIdentifier; +import org.pgpainless.key.util.RevocationAttributes; import org.pgpainless.policy.Policy; -import org.pgpainless.signature.consumer.SignaturePicker; import org.pgpainless.signature.SignatureUtils; +import org.pgpainless.signature.consumer.SignaturePicker; import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil; /** @@ -52,6 +54,8 @@ public class KeyRingInfo { private final PGPKeyRing keys; private Signatures signatures; + private final Date evaluationDate; + private final String primaryUserId; /** * Evaluate the key ring at creation time of the given signature. @@ -82,6 +86,8 @@ public class KeyRingInfo { public KeyRingInfo(PGPKeyRing keys, Date validationDate) { this.keys = keys; this.signatures = new Signatures(keys, validationDate, PGPainless.getPolicy()); + this.evaluationDate = validationDate; + this.primaryUserId = findPrimaryUserId(); } /** @@ -251,45 +257,67 @@ public class KeyRingInfo { return OpenPgpFingerprint.of(getPublicKey()); } + public @Nullable String getPrimaryUserId() { + return primaryUserId; + } + /** * 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. + * this method returns the latest added user-id, otherwise null. * * @return primary user-id or null */ - public @Nullable String getPrimaryUserId() { + private String findPrimaryUserId() { + String nonPrimaryUserId = null; String primaryUserId = null; Date modificationDate = null; - List validUserIds = getValidUserIds(); - if (validUserIds.isEmpty()) { + List userIds = getUserIds(); + if (userIds.isEmpty()) { return null; } - for (String userId : validUserIds) { + if (userIds.size() == 1) { + return userIds.get(0); + } - PGPSignature signature = signatures.userIdCertifications.get(userId); - if (signature == null) { + for (String userId : userIds) { + PGPSignature certification = signatures.userIdCertifications.get(userId); + if (certification == null) { continue; } + Date creationTime = certification.getCreationTime(); - PrimaryUserID subpacket = SignatureSubpacketsUtil.getPrimaryUserId(signature); - if (subpacket != null && subpacket.isPrimaryUserID()) { - // if there are multiple primary userIDs, return most recently signed - if (modificationDate == null || !signature.getCreationTime().before(modificationDate)) { + if (certification.getHashedSubPackets().isPrimaryUserID()) { + if (nonPrimaryUserId != null) { + nonPrimaryUserId = null; + modificationDate = null; + } + + if (modificationDate == null || creationTime.after(modificationDate)) { primaryUserId = userId; - modificationDate = signature.getCreationTime(); + modificationDate = creationTime; + } + + } else { + if (primaryUserId != null) { + continue; + } + + if (modificationDate == null || creationTime.after(modificationDate)) { + nonPrimaryUserId = userId; + modificationDate = creationTime; } } } - // Workaround for keys with only one user-id but no primary user-id packet. - if (primaryUserId == null) { - return validUserIds.get(0); + + if (primaryUserId != null) { + return primaryUserId; } - return primaryUserId; + return nonPrimaryUserId; } /** @@ -314,7 +342,7 @@ public class KeyRingInfo { List valid = new ArrayList<>(); List userIds = getUserIds(); for (String userId : userIds) { - if (isUserIdValid(userId)) { + if (isUserIdBound(userId)) { valid.add(userId); } } @@ -328,6 +356,18 @@ public class KeyRingInfo { * @return true if user-id is valid */ public boolean isUserIdValid(String userId) { + if (!userId.equals(primaryUserId)) { + if (!isUserIdBound(primaryUserId)) { + // primary user-id not valid + return false; + } + } + return isUserIdBound(userId); + } + + + private boolean isUserIdBound(String userId) { + PGPSignature certification = signatures.userIdCertifications.get(userId); PGPSignature revocation = signatures.userIdRevocations.get(userId); @@ -338,6 +378,12 @@ public class KeyRingInfo { if (SignatureUtils.isSignatureExpired(certification)) { return false; } + if (certification.getHashedSubPackets().isPrimaryUserID()) { + Date keyExpiration = SignatureSubpacketsUtil.getKeyExpirationTimeAsDate(certification, keys.getPublicKey()); + if (keyExpiration != null && evaluationDate.after(keyExpiration)) { + return false; + } + } // Not revoked -> valid if (revocation == null) { return true; @@ -588,7 +634,7 @@ public class KeyRingInfo { } } - PGPSignature primaryUserIdCertification = getLatestUserIdCertification(getPrimaryUserId()); + PGPSignature primaryUserIdCertification = getLatestUserIdCertification(getPossiblyExpiredUserId()); if (primaryUserIdCertification != null) { return SignatureSubpacketsUtil.getKeyExpirationTimeAsDate(primaryUserIdCertification, getPublicKey()); } @@ -596,6 +642,38 @@ public class KeyRingInfo { throw new NoSuchElementException("No suitable signatures found on the key."); } + public String getPossiblyExpiredUserId() { + String validPrimaryUserId = getPrimaryUserId(); + if (validPrimaryUserId != null) { + return validPrimaryUserId; + } + + Date latestCreationTime = null; + String primaryUserId = null; + boolean foundPrimary = false; + for (String userId : getUserIds()) { + PGPSignature signature = getLatestUserIdCertification(userId); + if (signature == null) { + continue; + } + + boolean isPrimary = signature.getHashedSubPackets().isPrimaryUserID(); + if (foundPrimary && !isPrimary) { + continue; + } + + Date creationTime = signature.getCreationTime(); + if (latestCreationTime == null || creationTime.after(latestCreationTime) || isPrimary && !foundPrimary) { + latestCreationTime = creationTime; + primaryUserId = userId; + } + + foundPrimary |= isPrimary; + } + + return primaryUserId; + } + /** * Return the expiration date of the subkey with the provided fingerprint. * @@ -668,6 +746,15 @@ public class KeyRingInfo { return primaryExpiration; } + public boolean isHardRevoked(String userId) { + PGPSignature revocation = signatures.userIdRevocations.get(userId); + if (revocation == null) { + return false; + } + RevocationReason revocationReason = revocation.getHashedSubPackets().getRevocationReason(); + return revocationReason == null || RevocationAttributes.Reason.isHardRevocation(revocationReason.getRevocationReason()); + } + /** * Return true if the key ring is a {@link PGPSecretKeyRing}. * If it is a {@link PGPPublicKeyRing} return false and if it is neither, throw an {@link AssertionError}. 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 db657a31..8b703600 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 @@ -22,17 +22,14 @@ import javax.annotation.Nullable; import org.bouncycastle.bcpg.S2K; import org.bouncycastle.bcpg.SecretKeyPacket; +import org.bouncycastle.bcpg.sig.KeyExpirationTime; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPKeyPair; import org.bouncycastle.openpgp.PGPKeyRingGenerator; -import org.bouncycastle.openpgp.PGPPrivateKey; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.openpgp.PGPSignature; -import org.bouncycastle.openpgp.PGPSignatureGenerator; -import org.bouncycastle.openpgp.PGPSignatureSubpacketGenerator; -import org.bouncycastle.openpgp.PGPSignatureSubpacketVector; import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; import org.bouncycastle.openpgp.operator.PBESecretKeyEncryptor; import org.bouncycastle.openpgp.operator.PGPContentSignerBuilder; @@ -47,7 +44,6 @@ import org.pgpainless.algorithm.SignatureType; import org.pgpainless.algorithm.SymmetricKeyAlgorithm; import org.pgpainless.algorithm.negotiation.HashAlgorithmNegotiator; import org.pgpainless.implementation.ImplementationFactory; -import org.pgpainless.key.OpenPgpFingerprint; import org.pgpainless.key.generation.KeyRingBuilder; import org.pgpainless.key.generation.KeySpec; import org.pgpainless.key.info.KeyRingInfo; @@ -55,18 +51,16 @@ import org.pgpainless.key.protection.CachingSecretKeyRingProtector; import org.pgpainless.key.protection.KeyRingProtectionSettings; import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector; import org.pgpainless.key.protection.SecretKeyRingProtector; -import org.pgpainless.key.protection.UnlockSecretKey; import org.pgpainless.key.protection.UnprotectedKeysProtector; import org.pgpainless.key.protection.fixes.S2KUsageFix; import org.pgpainless.key.protection.passphrase_provider.SolitaryPassphraseProvider; import org.pgpainless.key.util.KeyRingUtils; import org.pgpainless.key.util.RevocationAttributes; -import org.pgpainless.signature.SignatureUtils; +import org.pgpainless.signature.builder.DirectKeySignatureBuilder; import org.pgpainless.signature.builder.RevocationSignatureBuilder; import org.pgpainless.signature.builder.SelfSignatureBuilder; import org.pgpainless.signature.subpackets.RevocationSignatureSubpackets; import org.pgpainless.signature.subpackets.SelfSignatureSubpackets; -import org.pgpainless.signature.subpackets.SignatureSubpacketGeneratorUtil; import org.pgpainless.signature.subpackets.SignatureSubpackets; import org.pgpainless.signature.subpackets.SignatureSubpacketsHelper; import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil; @@ -107,6 +101,9 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { // retain key flags from previous signature KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); + if (info.isHardRevoked(userId.toString())) { + throw new IllegalArgumentException("User-ID " + userId + " is hard revoked and cannot be re-certified."); + } List keyFlags = info.getKeyFlagsOf(info.getKeyId()); Set hashAlgorithmPreferences; @@ -146,15 +143,54 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { public SecretKeyRingEditorInterface addPrimaryUserId( @Nonnull CharSequence userId, @Nonnull SecretKeyRingProtector protector) throws PGPException { - return addUserId( + + // Determine previous key expiration date + PGPPublicKey primaryKey = secretKeyRing.getSecretKey().getPublicKey(); + /* + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); + String primaryUserId = info.getPrimaryUserId(); + PGPSignature signature = primaryUserId == null ? + info.getLatestDirectKeySelfSignature() : info.getLatestUserIdCertification(primaryUserId); + final Date previousKeyExpiration = signature == null ? null : + SignatureSubpacketsUtil.getKeyExpirationTimeAsDate(signature, primaryKey); + */ + final Date previousKeyExpiration = null; + + // Add new primary user-id signature + addUserId( userId, new SelfSignatureSubpackets.Callback() { @Override public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { hashedSubpackets.setPrimaryUserId(); + if (previousKeyExpiration != null) { + hashedSubpackets.setKeyExpirationTime(primaryKey, previousKeyExpiration); + } else { + hashedSubpackets.setKeyExpirationTime(null); + } } }, protector); + + // unmark previous primary user-ids to be non-primary + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); + for (String otherUserId : info.getValidUserIds()) { + if (userId.toString().equals(otherUserId)) { + continue; + } + + // We need to unmark this user-id as primary + if (info.getLatestUserIdCertification(otherUserId).getHashedSubPackets().isPrimaryUserID()) { + addUserId(otherUserId, new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + hashedSubpackets.setPrimaryUserId(null); + hashedSubpackets.setKeyExpirationTime(null); // non-primary + } + }, protector); + } + } + return this; } // TODO: Move to utility class? @@ -468,118 +504,119 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { @Nullable Date expiration, @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException { - return setExpirationDate(OpenPgpFingerprint.of(secretKeyRing), expiration, secretKeyRingProtector); - } - @Override - public SecretKeyRingEditorInterface setExpirationDate( - @Nonnull OpenPgpFingerprint fingerprint, - @Nullable Date expiration, - @Nonnull SecretKeyRingProtector secretKeyRingProtector) - throws PGPException { - - List secretKeyList = new ArrayList<>(); PGPSecretKey primaryKey = secretKeyRing.getSecretKey(); if (!primaryKey.isMasterKey()) { throw new IllegalArgumentException("Key Ring does not appear to contain a primary secret key."); } - boolean found = false; - for (PGPSecretKey secretKey : secretKeyRing) { - // Skip over unaffected subkeys - if (secretKey.getKeyID() != fingerprint.getKeyId()) { - secretKeyList.add(secretKey); + // reissue direct key sig + PGPSignature prevDirectKeySig = getPreviousDirectKeySignature(); + if (prevDirectKeySig != null) { + PGPSignature directKeySig = reissueDirectKeySignature(expiration, secretKeyRingProtector, prevDirectKeySig); + secretKeyRing = KeyRingUtils.injectCertification(secretKeyRing, primaryKey.getPublicKey(), directKeySig); + } + + // reissue primary user-id sig + String primaryUserId = PGPainless.inspectKeyRing(secretKeyRing).getPossiblyExpiredUserId(); + if (primaryUserId != null) { + PGPSignature prevUserIdSig = getPreviousUserIdSignatures(primaryUserId); + PGPSignature userIdSig = reissuePrimaryUserIdSig(expiration, secretKeyRingProtector, primaryUserId, prevUserIdSig); + secretKeyRing = KeyRingUtils.injectCertification(secretKeyRing, primaryUserId, userIdSig); + } + + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); + for (String userId : info.getValidUserIds()) { + if (userId.equals(primaryUserId)) { continue; } - // We found the target subkey - found = true; - secretKey = setExpirationDate(primaryKey, secretKey, expiration, secretKeyRingProtector); - secretKeyList.add(secretKey); - } - if (!found) { - throw new IllegalArgumentException("Key Ring does not contain secret key with fingerprint " + fingerprint); - } + PGPSignature prevUserIdSig = info.getLatestUserIdCertification(userId); + if (prevUserIdSig == null) { + throw new AssertionError("A valid user-id shall never have no user-id signature."); + } - secretKeyRing = new PGPSecretKeyRing(secretKeyList); + if (prevUserIdSig.getHashedSubPackets().isPrimaryUserID()) { + PGPSignature userIdSig = reissueNonPrimaryUserId(secretKeyRingProtector, userId, prevUserIdSig); + secretKeyRing = KeyRingUtils.injectCertification(secretKeyRing, primaryUserId, userIdSig); + } + } return this; } - private PGPSecretKey setExpirationDate(PGPSecretKey primaryKey, - PGPSecretKey subjectKey, - Date expiration, - SecretKeyRingProtector secretKeyRingProtector) + private PGPSignature reissueNonPrimaryUserId( + SecretKeyRingProtector secretKeyRingProtector, + String userId, + PGPSignature prevUserIdSig) throws PGPException { - - if (expiration != null && expiration.before(subjectKey.getPublicKey().getCreationTime())) { - throw new IllegalArgumentException("Expiration date cannot be before creation date."); - } - - PGPPrivateKey privateKey = UnlockSecretKey.unlockSecretKey(primaryKey, secretKeyRingProtector); - PGPPublicKey subjectPubKey = subjectKey.getPublicKey(); - - PGPSignature oldSignature = getPreviousSignature(primaryKey, subjectPubKey); - - PGPSignatureSubpacketVector oldSubpackets = oldSignature.getHashedSubPackets(); - PGPSignatureSubpacketGenerator subpacketGenerator = new PGPSignatureSubpacketGenerator(oldSubpackets); - SignatureSubpacketGeneratorUtil.setSignatureCreationTimeInSubpacketGenerator(new Date(), subpacketGenerator); - SignatureSubpacketGeneratorUtil.setKeyExpirationDateInSubpacketGenerator( - expiration, subjectPubKey.getCreationTime(), subpacketGenerator); - - PGPSignatureGenerator signatureGenerator = SignatureUtils.getSignatureGeneratorFor(primaryKey); - signatureGenerator.setHashedSubpackets(subpacketGenerator.generate()); - - if (primaryKey.getKeyID() == subjectKey.getKeyID()) { - signatureGenerator.init(PGPSignature.POSITIVE_CERTIFICATION, privateKey); - - for (Iterator it = subjectKey.getUserIDs(); it.hasNext(); ) { - String userId = it.next(); - PGPSignature signature = signatureGenerator.generateCertification(userId, subjectPubKey); - subjectPubKey = PGPPublicKey.addCertification(subjectPubKey, userId, signature); + SelfSignatureBuilder builder = new SelfSignatureBuilder(secretKeyRing.getSecretKey(), secretKeyRingProtector, prevUserIdSig); + builder.applyCallback(new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + // unmark as primary + hashedSubpackets.setPrimaryUserId(null); } - } else { - signatureGenerator.init(PGPSignature.SUBKEY_BINDING, privateKey); - - PGPSignature signature = signatureGenerator.generateCertification( - primaryKey.getPublicKey(), subjectPubKey); - subjectPubKey = PGPPublicKey.addCertification(subjectPubKey, signature); - } - - subjectKey = PGPSecretKey.replacePublicKey(subjectKey, subjectPubKey); - return subjectKey; + }); + return builder.build(secretKeyRing.getPublicKey(), userId); } - private PGPSignature getPreviousSignature(PGPSecretKey primaryKey, PGPPublicKey subjectPubKey) { - PGPSignature oldSignature = null; - if (primaryKey.getKeyID() == subjectPubKey.getKeyID()) { - Iterator keySignatures = subjectPubKey.getSignaturesForKeyID(primaryKey.getKeyID()); - while (keySignatures.hasNext()) { - PGPSignature next = keySignatures.next(); - SignatureType type = SignatureType.valueOf(next.getSignatureType()); - if (type == SignatureType.POSITIVE_CERTIFICATION || - type == SignatureType.CASUAL_CERTIFICATION || - type == SignatureType.GENERIC_CERTIFICATION) { - oldSignature = next; + private PGPSignature reissuePrimaryUserIdSig( + @Nullable Date expiration, + @Nonnull SecretKeyRingProtector secretKeyRingProtector, + @Nonnull String primaryUserId, + @Nonnull PGPSignature prevUserIdSig) + throws PGPException { + PGPSecretKey primaryKey = secretKeyRing.getSecretKey(); + PGPPublicKey publicKey = primaryKey.getPublicKey(); + + SelfSignatureBuilder builder = new SelfSignatureBuilder(primaryKey, secretKeyRingProtector, prevUserIdSig); + builder.applyCallback(new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + if (expiration != null) { + hashedSubpackets.setKeyExpirationTime(true, publicKey.getCreationTime(), expiration); + } else { + hashedSubpackets.setKeyExpirationTime(new KeyExpirationTime(true, 0)); + } + hashedSubpackets.setPrimaryUserId(); + } + }); + return builder.build(publicKey, primaryUserId); + } + + private PGPSignature reissueDirectKeySignature( + Date expiration, + SecretKeyRingProtector secretKeyRingProtector, + PGPSignature prevDirectKeySig) + throws PGPException { + PGPSecretKey primaryKey = secretKeyRing.getSecretKey(); + PGPPublicKey publicKey = primaryKey.getPublicKey(); + final Date keyCreationTime = publicKey.getCreationTime(); + + DirectKeySignatureBuilder builder = new DirectKeySignatureBuilder(primaryKey, secretKeyRingProtector, prevDirectKeySig); + builder.applyCallback(new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + if (expiration != null) { + hashedSubpackets.setKeyExpirationTime(keyCreationTime, expiration); + } else { + hashedSubpackets.setKeyExpirationTime(null); } } - if (oldSignature == null) { - throw new IllegalStateException("Key " + OpenPgpFingerprint.of(subjectPubKey) + - " does not have a previous positive/casual/generic certification signature."); - } - } else { - Iterator bindingSignatures = subjectPubKey.getSignaturesOfType( - SignatureType.SUBKEY_BINDING.getCode()); - while (bindingSignatures.hasNext()) { - oldSignature = bindingSignatures.next(); - } - } + }); - if (oldSignature == null) { - throw new IllegalStateException("Key " + OpenPgpFingerprint.of(subjectPubKey) + - " does not have a previous subkey binding signature."); - } - return oldSignature; + return builder.build(publicKey); + } + + private PGPSignature getPreviousDirectKeySignature() { + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); + return info.getLatestDirectKeySelfSignature(); + } + + private PGPSignature getPreviousUserIdSignatures(String userId) { + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); + return info.getLatestUserIdCertification(userId); } @Override 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 66fa58b5..b61dbbc1 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 @@ -366,20 +366,6 @@ public interface SecretKeyRingEditorInterface { @Nonnull SecretKeyRingProtector secretKeyRingProtector) throws PGPException; - /** - * Set key expiration time. - * - * @param fingerprint key that will have its expiration date adjusted - * @param expiration target expiration time or @{code null} for no expiration - * @param secretKeyRingProtector protector to unlock the priary key - * @return the builder - */ - 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 whole key. * diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SubkeyBindingSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SubkeyBindingSignatureBuilder.java index 6c84bab9..9c51955d 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SubkeyBindingSignatureBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/SubkeyBindingSignatureBuilder.java @@ -21,6 +21,21 @@ public class SubkeyBindingSignatureBuilder extends AbstractSignatureBuilder, 2021 Flowcrypt a.s. +// +// SPDX-License-Identifier: Apache-2.0 + +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.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.security.InvalidAlgorithmParameterException; +import java.security.NoSuchAlgorithmException; +import java.util.Date; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.bouncycastle.openpgp.PGPSignature; +import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.key.info.KeyRingInfo; +import org.pgpainless.key.protection.SecretKeyRingProtector; + +public class ChangePrimaryUserIdAndExpirationDatesTest { + + @Test + public void generateA_primaryB_revokeA_cantSecondaryA() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, InterruptedException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing() + .modernKeyRing("A", null); + SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys(); + + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + assertFalse(info.isHardRevoked("A")); + assertFalse(info.isHardRevoked("B")); + assertIsPrimaryUserId("A", info); + assertIsNotValid("B", info); + assertIsNotPrimaryUserId("B", info); + + Thread.sleep(1000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addPrimaryUserId("B", protector) + .done(); + info = PGPainless.inspectKeyRing(secretKeys); + + assertIsPrimaryUserId("B", info); + assertIsNotPrimaryUserId("A", info); + + Thread.sleep(1000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .revokeUserId("A", protector) // hard revoke A + .done(); + info = PGPainless.inspectKeyRing(secretKeys); + + assertTrue(info.isHardRevoked("A")); + assertFalse(info.isHardRevoked("B")); + assertIsPrimaryUserId("B", info); + assertIsNotValid("A", info); + + Thread.sleep(1000); + + PGPSecretKeyRing finalSecretKeys = secretKeys; + assertThrows(IllegalArgumentException.class, () -> + PGPainless.modifyKeyRing(finalSecretKeys).addUserId("A", protector)); + } + + @Test + public void generateA_primaryExpire_isExpired() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, InterruptedException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing() + .modernKeyRing("A", null); + SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys(); + + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + assertIsPrimaryUserId("A", info); + + Thread.sleep(1000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .setExpirationDate(new Date(), protector) // expire the whole key + .done(); + + Thread.sleep(1000); + + info = PGPainless.inspectKeyRing(secretKeys); + assertFalse(info.isUserIdValid("A")); // is expired by now + } + + @Test + public void generateA_primaryB_primaryExpire_bIsStillPrimary() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, InterruptedException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing() + .modernKeyRing("A", null); + SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys(); + + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + assertIsPrimaryUserId("A", info); + + Thread.sleep(1000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addPrimaryUserId("B", protector) + .done(); + info = PGPainless.inspectKeyRing(secretKeys); + + assertIsPrimaryUserId("B", info); + assertIsNotPrimaryUserId("A", info); + + Thread.sleep(1000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .setExpirationDate(new Date(new Date().getTime() + 1000), protector) // expire the whole key in 1 sec + .done(); + + info = PGPainless.inspectKeyRing(secretKeys); + assertIsValid("A", info); + assertIsValid("B", info); + assertIsPrimaryUserId("B", info); + assertIsNotPrimaryUserId("A", info); + + Thread.sleep(2000); + + info = PGPainless.inspectKeyRing(secretKeys); + assertIsPrimaryUserId("B", info); // B is still primary, even though + assertFalse(info.isUserIdValid("A")); // key is expired by now + assertFalse(info.isUserIdValid("B")); + } + + @Test + public void generateA_expire_certify() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, InterruptedException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("A", null); + SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys(); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .setExpirationDate(new Date(new Date().getTime() + 1000), protector) + .done(); + + Thread.sleep(2000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .setExpirationDate(new Date(new Date().getTime() + 2000), protector) + .done(); + + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + assertIsValid("A", info); + assertIsPrimaryUserId("A", info); + } + + @Test + public void generateA_expire_primaryB_expire_isPrimaryB() + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, InterruptedException, IOException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("A", null); + SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys(); + + Thread.sleep(1000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .setExpirationDate(new Date(), protector) + .done(); + + Thread.sleep(2000); + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + + assertIsPrimaryUserId("A", info); + assertIsNotValid("A", info); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addPrimaryUserId("B", protector) + .done(); + + info = PGPainless.inspectKeyRing(secretKeys); + + assertIsPrimaryUserId("B", info); + assertIsValid("B", info); + assertIsNotValid("A", info); // A is still expired + + Thread.sleep(1000); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .setExpirationDate(new Date(new Date().getTime() + 10000), protector) + .done(); + + Thread.sleep(1000); + info = PGPainless.inspectKeyRing(secretKeys); + + assertIsValid("B", info); + assertIsNotValid("A", info); // A was expired when the expiration date was changed, so it was not re-certified + assertIsPrimaryUserId("B", info); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addUserId("A", protector) // re-certify A as non-primary user-id + .done(); + info = PGPainless.inspectKeyRing(secretKeys); + + assertIsValid("B", info); + assertIsValid("A", info); + assertIsPrimaryUserId("B", info); + + } + + private static void assertIsPrimaryUserId(String userId, KeyRingInfo info) { + assertEquals(userId, info.getPrimaryUserId()); + } + + private static void assertIsNotPrimaryUserId(String userId, KeyRingInfo info) { + PGPSignature signature = info.getLatestUserIdCertification(userId); + if (signature == null) { + return; + } + + assertFalse(signature.getHashedSubPackets().isPrimaryUserID()); + } + + private static void assertIsValid(String userId, KeyRingInfo info) { + assertTrue(info.isUserIdValid(userId)); + } + + private static void assertIsNotValid(String userId, KeyRingInfo info) { + assertFalse(info.isUserIdValid(userId)); + } +} diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/modification/OldSignatureSubpacketsArePreservedOnNewSig.java b/pgpainless-core/src/test/java/org/pgpainless/key/modification/OldSignatureSubpacketsArePreservedOnNewSig.java index dbca56d4..f7d4d181 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/modification/OldSignatureSubpacketsArePreservedOnNewSig.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/modification/OldSignatureSubpacketsArePreservedOnNewSig.java @@ -10,6 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; +import java.util.Calendar; import java.util.Date; import org.bouncycastle.openpgp.PGPException; @@ -19,7 +20,6 @@ import org.bouncycastle.openpgp.PGPSignatureSubpacketVector; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; import org.pgpainless.PGPainless; -import org.pgpainless.key.OpenPgpV4Fingerprint; import org.pgpainless.key.protection.UnprotectedKeysProtector; import org.pgpainless.util.TestAllImplementations; @@ -32,18 +32,22 @@ public class OldSignatureSubpacketsArePreservedOnNewSig { PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing() .simpleEcKeyRing("Alice "); - OpenPgpV4Fingerprint subkeyFingerprint = new OpenPgpV4Fingerprint(PGPainless.inspectKeyRing(secretKeys).getPublicKeys().get(1)); - - PGPSignature oldSignature = PGPainless.inspectKeyRing(secretKeys).getCurrentSubkeyBindingSignature(subkeyFingerprint.getKeyId()); + PGPSignature oldSignature = PGPainless.inspectKeyRing(secretKeys).getLatestUserIdCertification("Alice "); PGPSignatureSubpacketVector oldPackets = oldSignature.getHashedSubPackets(); assertEquals(0, oldPackets.getKeyExpirationTime()); Thread.sleep(1000); + Date now = new Date(); + Calendar calendar = Calendar.getInstance(); + calendar.setTime(now); + calendar.add(Calendar.DATE, 5); + Date expiration = calendar.getTime(); // in 5 days + secretKeys = PGPainless.modifyKeyRing(secretKeys) - .setExpirationDate(subkeyFingerprint, new Date(), new UnprotectedKeysProtector()) + .setExpirationDate(expiration, new UnprotectedKeysProtector()) .done(); - PGPSignature newSignature = PGPainless.inspectKeyRing(secretKeys).getCurrentSubkeyBindingSignature(subkeyFingerprint.getKeyId()); + PGPSignature newSignature = PGPainless.inspectKeyRing(secretKeys).getLatestUserIdCertification("Alice "); PGPSignatureSubpacketVector newPackets = newSignature.getHashedSubPackets(); assertNotEquals(0, newPackets.getKeyExpirationTime()); diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeKeyWithoutPreferredAlgorithmsOnPrimaryKey.java b/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeKeyWithoutPreferredAlgorithmsOnPrimaryKey.java index 98d79556..7c6e0b10 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeKeyWithoutPreferredAlgorithmsOnPrimaryKey.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeKeyWithoutPreferredAlgorithmsOnPrimaryKey.java @@ -5,18 +5,14 @@ package org.pgpainless.key.modification; import java.io.IOException; -import java.util.ArrayList; import java.util.Date; -import java.util.List; import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPSecretKey; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.junit.JUtils; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtendWith; import org.pgpainless.PGPainless; -import org.pgpainless.key.OpenPgpV4Fingerprint; import org.pgpainless.key.info.KeyRingInfo; import org.pgpainless.key.modification.secretkeyring.SecretKeyRingEditorInterface; import org.pgpainless.key.protection.SecretKeyRingProtector; @@ -106,24 +102,15 @@ public class RevokeKeyWithoutPreferredAlgorithmsOnPrimaryKey { throws IOException, PGPException { Date expirationDate = DateUtil.parseUTCDate(DateUtil.formatUTCDate(new Date())); PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(KEY); - List fingerprintList = new ArrayList<>(); - for (PGPSecretKey secretKey : secretKeys) { - fingerprintList.add(new OpenPgpV4Fingerprint(secretKey)); - } + SecretKeyRingProtector protector = new UnprotectedKeysProtector(); SecretKeyRingEditorInterface modify = PGPainless.modifyKeyRing(secretKeys) .setExpirationDate(expirationDate, protector); - for (int i = 1; i < fingerprintList.size(); i++) { - modify.setExpirationDate(fingerprintList.get(i), expirationDate, protector); - } secretKeys = modify.done(); KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); JUtils.assertDateEquals(expirationDate, info.getPrimaryKeyExpirationDate()); - for (OpenPgpV4Fingerprint fingerprint : fingerprintList) { - JUtils.assertDateEquals(expirationDate, info.getSubkeyExpirationDate(fingerprint)); - } } } diff --git a/pgpainless-core/src/test/java/org/pgpainless/signature/SignatureSubpacketsUtilTest.java b/pgpainless-core/src/test/java/org/pgpainless/signature/SignatureSubpacketsUtilTest.java index ce5b1690..d990c4b1 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/signature/SignatureSubpacketsUtilTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/signature/SignatureSubpacketsUtilTest.java @@ -60,7 +60,8 @@ public class SignatureSubpacketsUtilTest { .setExpirationDate(expiration, SecretKeyRingProtector.unprotectedKeys()) .done(); - PGPSignature expirationSig = SignaturePicker.pickCurrentUserIdCertificationSignature(secretKeys, "Expire", Policy.getInstance(), new Date()); + PGPSignature expirationSig = SignaturePicker.pickCurrentUserIdCertificationSignature( + secretKeys, "Expire", Policy.getInstance(), new Date()); PGPPublicKey notTheRightKey = PGPainless.inspectKeyRing(secretKeys).getSigningSubkeys().get(0); assertThrows(IllegalArgumentException.class, () ->