Fix changing of expiration dates for keys and subkeys

pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java

@@ -62,6 +62,7 @@ import org.pgpainless.key.util.KeyRingUtils;
 import org.pgpainless.key.util.RevocationAttributes;
 import org.pgpainless.key.util.SignatureUtils;
 import org.pgpainless.util.Passphrase;
+import org.pgpainless.util.SignatureSubpacketGeneratorUtil;
 
 public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
 
@@ -287,57 +288,120 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         return this;
     }
 
+    @Override
+    public SecretKeyRingEditorInterface setExpirationDate(Date expiration,
+                                                          SecretKeyRingProtector secretKeyRingProtector)
+            throws PGPException {
+        return setExpirationDate(new OpenPgpV4Fingerprint(secretKeyRing), expiration, secretKeyRingProtector);
+    }
+
     @Override
     public SecretKeyRingEditorInterface setExpirationDate(OpenPgpV4Fingerprint fingerprint,
                                                           Date expiration,
                                                           SecretKeyRingProtector secretKeyRingProtector)
             throws PGPException {
-        Iterator<PGPSecretKey> secretKeyIterator = secretKeyRing.getSecretKeys();
-
-        if (!secretKeyIterator.hasNext()) {
-            throw new NoSuchElementException("No secret keys in the ring.");
-        }
-
-        PGPSecretKey secretKey = secretKeyIterator.next();
-        PGPPublicKey publicKey = secretKey.getPublicKey();
-
-        if (!new OpenPgpV4Fingerprint(publicKey).equals(fingerprint)) {
-            throw new IllegalArgumentException("Currently it is possible to adjust expiration date for primary key only.");
-        }
 
         List<PGPSecretKey> secretKeyList = new ArrayList<>();
-        PGPPrivateKey privateKey = unlockSecretKey(secretKey, secretKeyRingProtector);
-
         PGPSecretKey primaryKey = secretKeyRing.getSecretKey();
-        PGPSignatureGenerator signatureGenerator = SignatureUtils.getSignatureGeneratorFor(primaryKey);
+        if (!primaryKey.isMasterKey()) {
-        PGPSignatureSubpacketGenerator subpacketGenerator = new PGPSignatureSubpacketGenerator();
+            throw new IllegalArgumentException("Key Ring does not appear to contain a primary secret key.");
-
-        long secondsToExpire = 0; // 0 means "no expiration"
-        if (expiration != null) {
-            secondsToExpire = (expiration.getTime() - primaryKey.getPublicKey().getCreationTime().getTime()) / 1000;
-        }
-        subpacketGenerator.setKeyExpirationTime(false, secondsToExpire);
-
-        PGPSignatureSubpacketVector subPackets = subpacketGenerator.generate();
-        signatureGenerator.setHashedSubpackets(subPackets);
-
-        signatureGenerator.init(PGPSignature.POSITIVE_CERTIFICATION, privateKey);
-
-        Iterator<String> users = publicKey.getUserIDs();
-        while (users.hasNext()) {
-            String user = users.next();
-            PGPSignature signature = signatureGenerator.generateCertification(user, primaryKey.getPublicKey());
-            publicKey = PGPPublicKey.addCertification(publicKey, user, signature);
         }
 
-        secretKey = PGPSecretKey.replacePublicKey(secretKey, publicKey);
+        boolean found = false;
+        Iterator<PGPSecretKey> iterator = secretKeyRing.iterator();
+        while (iterator.hasNext()) {
+            PGPSecretKey secretKey = iterator.next();
+
+            // Skip over unaffected subkeys
+            if (secretKey.getKeyID() != fingerprint.getKeyId()) {
                 secretKeyList.add(secretKey);
+                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);
+        }
 
         secretKeyRing = new PGPSecretKeyRing(secretKeyList);
 
         return this;
     }
 
+    private PGPSecretKey setExpirationDate(PGPSecretKey primaryKey,
+                                           PGPSecretKey subjectKey,
+                                           Date expiration,
+                                           SecretKeyRingProtector secretKeyRingProtector)
+            throws PGPException {
+
+        if (expiration != null && expiration.before(subjectKey.getPublicKey().getCreationTime())) {
+            throw new IllegalArgumentException("Expiration date cannot be before creation date.");
+        }
+
+        PGPPrivateKey privateKey = KeyRingUtils.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.setExpirationDateInSubpacketGenerator(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<String> it = subjectKey.getUserIDs(); it.hasNext(); ) {
+                String userId = it.next();
+                PGPSignature signature = signatureGenerator.generateCertification(userId, subjectPubKey);
+                subjectPubKey = PGPPublicKey.addCertification(subjectPubKey, userId, signature);
+            }
+        } 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;
+    }
+
+    private PGPSignature getPreviousSignature(PGPSecretKey primaryKey, PGPPublicKey subjectPubKey) {
+        PGPSignature oldSignature = null;
+        if (primaryKey.getKeyID() == subjectPubKey.getKeyID()) {
+            Iterator<PGPSignature> keySignatures = subjectPubKey.getSignaturesForKeyID(primaryKey.getKeyID());
+            while (keySignatures.hasNext()) {
+                PGPSignature next = keySignatures.next();
+                if (next.getSignatureType() == PGPSignature.POSITIVE_CERTIFICATION) {
+                    oldSignature = next;
+                }
+            }
+            if (oldSignature == null) {
+                throw new IllegalStateException("Key " + new OpenPgpV4Fingerprint(subjectPubKey) + " does not have a previous positive signature.");
+            }
+        } else {
+            Iterator bindingSignatures = subjectPubKey.getSignaturesOfType(SignatureType.SUBKEY_BINDING.getCode());
+            while (bindingSignatures.hasNext()) {
+                oldSignature = (PGPSignature) bindingSignatures.next();
+            }
+        }
+
+        if (oldSignature == null) {
+            throw new IllegalStateException("Key " + new OpenPgpV4Fingerprint(subjectPubKey) + " does not have a previous subkey binding signature.");
+        }
+        return oldSignature;
+    }
+
+
+
     @Override
     public PGPSignature createRevocationCertificate(OpenPgpV4Fingerprint fingerprint,
                                                     SecretKeyRingProtector secretKeyRingProtector,

pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java

@@ -15,9 +15,9 @@
  */
 package org.pgpainless.key.modification.secretkeyring;
 
-import java.util.Date;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.NoSuchAlgorithmException;
+import java.util.Date;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -198,6 +198,10 @@ public interface SecretKeyRingEditorInterface {
                                               RevocationAttributes revocationAttributes)
             throws PGPException;
 
+    SecretKeyRingEditorInterface setExpirationDate(Date expiration,
+                                                   SecretKeyRingProtector secretKeyRingProtector)
+            throws PGPException;
+
     /**
      * Set key expiration time.
      *

pgpainless-core/src/test/java/org/pgpainless/key/modification/ChangeExpirationTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020 Paul Schaub.
+ * Copyright 2021 Paul Schaub.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,28 +33,64 @@ import java.util.Date;
 
 public class ChangeExpirationTest {
 
-    @Test
+    private final OpenPgpV4Fingerprint subKeyFingerprint = new OpenPgpV4Fingerprint("F73FDE6439ABE210B1AF4EDD273EF7A0C749807B");
-    public void setExpirationDateAndThenUnsetIt() throws PGPException, IOException, InterruptedException {
-        PGPSecretKeyRing secretKeys = TestKeys.getEmilSecretKeyRing();
 
+    @Test
+    public void setExpirationDateAndThenUnsetIt_OnPrimaryKey() throws PGPException, IOException, InterruptedException {
+        PGPSecretKeyRing secretKeys = TestKeys.getEmilSecretKeyRing();
         KeyRingInfo sInfo = PGPainless.inspectKeyRing(secretKeys);
-        OpenPgpV4Fingerprint fingerprint = sInfo.getFingerprint();
 
         assertNull(sInfo.getExpirationDate());
+        assertNull(sInfo.getExpirationDate(subKeyFingerprint));
 
         Date date = new Date(1606493432000L);
-        secretKeys = PGPainless.modifyKeyRing(secretKeys).setExpirationDate(fingerprint, date, new UnprotectedKeysProtector()).done();
+        secretKeys = PGPainless.modifyKeyRing(secretKeys)
+                .setExpirationDate(date, new UnprotectedKeysProtector()).done();
         sInfo = PGPainless.inspectKeyRing(secretKeys);
         assertNotNull(sInfo.getExpirationDate());
         assertEquals(date.getTime(), sInfo.getExpirationDate().getTime());
+        // subkey unchanged
+        assertNull(sInfo.getExpirationDate(subKeyFingerprint));
 
         // We need to wait for one second as OpenPGP signatures have coarse-grained (up to a second)
         // accuracy. Creating two signatures within a short amount of time will make the second one
         // "invisible"
-        Thread.sleep(1000);
+        Thread.sleep(1100);
+
+        secretKeys = PGPainless.modifyKeyRing(secretKeys)
+                .setExpirationDate(null, new UnprotectedKeysProtector()).done();
 
-        secretKeys = PGPainless.modifyKeyRing(secretKeys).setExpirationDate(fingerprint, null, new UnprotectedKeysProtector()).done();
         sInfo = PGPainless.inspectKeyRing(secretKeys);
         assertNull(sInfo.getExpirationDate());
+        assertNull(sInfo.getExpirationDate(subKeyFingerprint));
+    }
+
+    @Test
+    public void setExpirationDateAndThenUnsetIt_OnSubkey() throws PGPException, IOException, InterruptedException {
+        PGPSecretKeyRing secretKeys = TestKeys.getEmilSecretKeyRing();
+        KeyRingInfo sInfo = PGPainless.inspectKeyRing(secretKeys);
+
+        assertNull(sInfo.getExpirationDate(subKeyFingerprint));
+        assertNull(sInfo.getExpirationDate());
+
+        Date date = new Date(1606493432000L);
+        secretKeys = PGPainless.modifyKeyRing(secretKeys)
+                .setExpirationDate(subKeyFingerprint, date, new UnprotectedKeysProtector()).done();
+        sInfo = PGPainless.inspectKeyRing(secretKeys);
+        assertNotNull(sInfo.getExpirationDate(subKeyFingerprint));
+        assertEquals(date.getTime(), sInfo.getExpirationDate(subKeyFingerprint).getTime());
+        assertNull(sInfo.getExpirationDate());
+
+        // We need to wait for one second as OpenPGP signatures have coarse-grained (up to a second)
+        // accuracy. Creating two signatures within a short amount of time will make the second one
+        // "invisible"
+        Thread.sleep(1100);
+
+        secretKeys = PGPainless.modifyKeyRing(secretKeys)
+                .setExpirationDate(subKeyFingerprint, null, new UnprotectedKeysProtector()).done();
+
+        sInfo = PGPainless.inspectKeyRing(secretKeys);
+        assertNull(sInfo.getExpirationDate(subKeyFingerprint));
+        assertNull(sInfo.getExpirationDate());
     }
 }