Rework subkey-revocation using new signature subpackets api

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

@@ -58,9 +58,11 @@ import org.pgpainless.key.util.KeyRingUtils;
 import org.pgpainless.key.util.RevocationAttributes;
 import org.pgpainless.signature.SignatureUtils;
 import org.pgpainless.signature.builder.PrimaryKeyBindingSignatureBuilder;
+import org.pgpainless.signature.builder.RevocationSignatureBuilder;
 import org.pgpainless.signature.builder.SelfSignatureBuilder;
 import org.pgpainless.signature.builder.SignatureFactory;
 import org.pgpainless.signature.builder.SubkeyBindingSignatureBuilder;
+import org.pgpainless.signature.subpackets.RevocationSignatureSubpackets;
 import org.pgpainless.signature.subpackets.SelfSignatureSubpackets;
 import org.pgpainless.signature.subpackets.SignatureSubpacketGeneratorUtil;
 import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil;
@@ -79,10 +81,14 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
     }
 
     @Override
-    public SecretKeyRingEditorInterface addUserId(String userId, SecretKeyRingProtector secretKeyRingProtector) throws PGPException {
+    public SecretKeyRingEditorInterface addUserId(
+            String userId,
+            SecretKeyRingProtector secretKeyRingProtector)
+            throws PGPException {
         return addUserId(userId, null, secretKeyRingProtector);
     }
 
+    @Override
     public SecretKeyRingEditorInterface addUserId(
             String userId,
             @Nullable SelfSignatureSubpackets.Callback signatureSubpacketCallback,
@@ -97,7 +103,11 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         PGPPublicKey publicKey = primaryKey.getPublicKey();
         KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing);
         List<KeyFlag> keyFlags = info.getKeyFlagsOf(info.getKeyId());
-        SelfSignatureBuilder builder = SignatureFactory.selfCertifyUserId(primaryKey, protector, signatureSubpacketCallback, keyFlags.toArray(new KeyFlag[0]));
+        SelfSignatureBuilder builder = SignatureFactory.selfCertifyUserId(
+                primaryKey,
+                protector,
+                signatureSubpacketCallback,
+                keyFlags.toArray(new KeyFlag[0]));
         builder.setSignatureType(SignatureType.POSITIVE_CERTIFICATION);
         builder.applyCallback(signatureSubpacketCallback);
         PGPSignature signature = builder.build(publicKey, userId);
@@ -176,7 +186,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
                                                   SecretKeyRingProtector subkeyProtector,
                                                   SecretKeyRingProtector primaryKeyProtector,
                                                   KeyFlag keyFlag,
-                                                  KeyFlag... additionalKeyFlags) throws PGPException, IOException {
+                                                  KeyFlag... additionalKeyFlags)
+            throws PGPException, IOException {
         KeyFlag[] flags = concat(keyFlag, additionalKeyFlags);
         PublicKeyAlgorithm subkeyAlgorithm = PublicKeyAlgorithm.fromId(subkey.getPublicKey().getAlgorithm());
         SignatureSubpacketsUtil.assureKeyCanCarryFlags(subkeyAlgorithm);
@@ -214,17 +225,17 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
 
     @Override
     public SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector,
-                                               RevocationAttributes revocationAttributes)
+                                               @Nullable RevocationAttributes revocationAttributes)
             throws PGPException {
-        return revokeSubKey(secretKeyRing.getSecretKey().getKeyID(), secretKeyRingProtector, revocationAttributes);
+        RevocationSignatureSubpackets.Callback callback = callbackFromRevocationAttributes(revocationAttributes);
+        return revoke(secretKeyRingProtector, callback);
     }
 
     @Override
-    public SecretKeyRingEditorInterface revokeSubKey(OpenPgpFingerprint fingerprint,
+    public SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector,
-                                                     SecretKeyRingProtector protector,
+                                               @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback)
-                                                     RevocationAttributes revocationAttributes)
             throws PGPException {
-        return revokeSubKey(fingerprint.getKeyId(), protector, revocationAttributes);
+        return revokeSubKey(secretKeyRing.getSecretKey().getKeyID(), secretKeyRingProtector, subpacketsCallback);
     }
 
     @Override
@@ -232,15 +243,75 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
                                                      SecretKeyRingProtector protector,
                                                      RevocationAttributes revocationAttributes)
             throws PGPException {
-        PGPPublicKey revokeeSubKey = secretKeyRing.getPublicKey(subKeyId);
+        RevocationSignatureSubpackets.Callback callback = callbackFromRevocationAttributes(revocationAttributes);
-        if (revokeeSubKey == null) {
+        return revokeSubKey(subKeyId, protector, callback);
-            throw new NoSuchElementException("No subkey with id " + Long.toHexString(subKeyId) + " found.");
     }
 
-        secretKeyRing = revokeSubKey(protector, revokeeSubKey, revocationAttributes);
+    @Override
+    public SecretKeyRingEditorInterface revokeSubKey(long keyID,
+                                                     SecretKeyRingProtector secretKeyRingProtector,
+                                                     @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback)
+            throws PGPException {
+        PGPPublicKey revokeeSubKey = KeyRingUtils.requirePublicKeyFrom(secretKeyRing, keyID);
+        PGPSignature subKeyRevocation = generateRevocation(secretKeyRingProtector, revokeeSubKey,
+                subpacketsCallback);
+        revokeeSubKey = PGPPublicKey.addCertification(revokeeSubKey, subKeyRevocation);
+
+        // Inject revoked public key into key ring
+        PGPPublicKeyRing publicKeyRing = KeyRingUtils.publicKeyRingFrom(secretKeyRing);
+        publicKeyRing = PGPPublicKeyRing.insertPublicKey(publicKeyRing, revokeeSubKey);
+        secretKeyRing = PGPSecretKeyRing.replacePublicKeys(secretKeyRing, publicKeyRing);
         return this;
     }
 
+    @Override
+    public PGPSignature createRevocationCertificate(SecretKeyRingProtector secretKeyRingProtector,
+                                                    RevocationAttributes revocationAttributes)
+            throws PGPException {
+        PGPPublicKey revokeeSubKey = secretKeyRing.getPublicKey();
+        PGPSignature revocationCertificate = generateRevocation(
+                secretKeyRingProtector, revokeeSubKey, callbackFromRevocationAttributes(revocationAttributes));
+        return revocationCertificate;
+    }
+
+    @Override
+    public PGPSignature createRevocationCertificate(
+            long subkeyId,
+            SecretKeyRingProtector secretKeyRingProtector,
+            RevocationAttributes revocationAttributes)
+            throws PGPException {
+        PGPPublicKey revokeeSubkey = KeyRingUtils.requirePublicKeyFrom(secretKeyRing, subkeyId);
+        RevocationSignatureSubpackets.Callback callback = callbackFromRevocationAttributes(revocationAttributes);
+        return generateRevocation(secretKeyRingProtector, revokeeSubkey, callback);
+    }
+
+    private PGPSignature generateRevocation(SecretKeyRingProtector protector,
+                                            PGPPublicKey revokeeSubKey,
+                                            @Nullable RevocationSignatureSubpackets.Callback callback)
+            throws PGPException {
+        PGPSecretKey primaryKey = secretKeyRing.getSecretKey();
+        SignatureType signatureType = revokeeSubKey.isMasterKey() ?
+                SignatureType.KEY_REVOCATION : SignatureType.SUBKEY_REVOCATION;
+
+        RevocationSignatureBuilder signatureBuilder =
+                new RevocationSignatureBuilder(signatureType, primaryKey, protector);
+        signatureBuilder.applyCallback(callback);
+        PGPSignature revocation = signatureBuilder.build(revokeeSubKey);
+        return revocation;
+    }
+
+    private static RevocationSignatureSubpackets.Callback callbackFromRevocationAttributes(
+            RevocationAttributes attributes) {
+        return new RevocationSignatureSubpackets.Callback() {
+            @Override
+            public void modifyHashedSubpackets(RevocationSignatureSubpackets hashedSubpackets) {
+                if (attributes != null) {
+                    hashedSubpackets.setRevocationReason(attributes);
+                }
+            }
+        };
+    }
+
     @Override
     public SecretKeyRingEditorInterface revokeUserId(String userId,
                                                      SecretKeyRingProtector secretKeyRingProtector,
@@ -262,7 +333,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
 
     private SecretKeyRingEditorInterface doRevokeUserId(String userId,
                                                         SecretKeyRingProtector protector,
-                                                        RevocationAttributes revocationAttributes) throws PGPException {
+                                                        RevocationAttributes revocationAttributes)
+            throws PGPException {
         PGPSecretKey primarySecretKey = secretKeyRing.getSecretKey();
         PGPPublicKey primaryPublicKey = primarySecretKey.getPublicKey();
         PGPPrivateKey privateKey = UnlockSecretKey.unlockSecretKey(primarySecretKey, protector);
@@ -277,7 +349,10 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
                     && reason != RevocationAttributes.Reason.USER_ID_NO_LONGER_VALID) {
                 throw new IllegalArgumentException("Revocation reason must either be NO_REASON or USER_ID_NO_LONGER_VALID");
             }
-            subpacketGenerator.setRevocationReason(false, revocationAttributes.getReason().code(), revocationAttributes.getDescription());
+            subpacketGenerator.setRevocationReason(
+                    false,
+                    revocationAttributes.getReason().code(),
+                    revocationAttributes.getDescription());
         }
 
         PGPSignatureGenerator signatureGenerator = SignatureUtils.getSignatureGeneratorFor(primarySecretKey);
@@ -353,7 +428,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         PGPSignatureSubpacketVector oldSubpackets = oldSignature.getHashedSubPackets();
         PGPSignatureSubpacketGenerator subpacketGenerator = new PGPSignatureSubpacketGenerator(oldSubpackets);
         SignatureSubpacketGeneratorUtil.setSignatureCreationTimeInSubpacketGenerator(new Date(), subpacketGenerator);
-        SignatureSubpacketGeneratorUtil.setKeyExpirationDateInSubpacketGenerator(expiration, subjectPubKey.getCreationTime(), subpacketGenerator);
+        SignatureSubpacketGeneratorUtil.setKeyExpirationDateInSubpacketGenerator(
+                expiration, subjectPubKey.getCreationTime(), subpacketGenerator);
 
         PGPSignatureGenerator signatureGenerator = SignatureUtils.getSignatureGeneratorFor(primaryKey);
         signatureGenerator.setHashedSubpackets(subpacketGenerator.generate());
@@ -369,7 +445,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         } else {
             signatureGenerator.init(PGPSignature.SUBKEY_BINDING, privateKey);
 
-            PGPSignature signature = signatureGenerator.generateCertification(primaryKey.getPublicKey(), subjectPubKey);
+            PGPSignature signature = signatureGenerator.generateCertification(
+                    primaryKey.getPublicKey(), subjectPubKey);
             subjectPubKey = PGPPublicKey.addCertification(subjectPubKey, signature);
         }
 
@@ -391,81 +468,27 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
                 }
             }
             if (oldSignature == null) {
-                throw new IllegalStateException("Key " + OpenPgpFingerprint.of(subjectPubKey) + " does not have a previous positive/casual/generic certification signature.");
+                throw new IllegalStateException("Key " + OpenPgpFingerprint.of(subjectPubKey) +
+                        " does not have a previous positive/casual/generic certification signature.");
             }
         } else {
-            Iterator<PGPSignature> bindingSignatures = subjectPubKey.getSignaturesOfType(SignatureType.SUBKEY_BINDING.getCode());
+            Iterator<PGPSignature> 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.");
+            throw new IllegalStateException("Key " + OpenPgpFingerprint.of(subjectPubKey) +
+                    " does not have a previous subkey binding signature.");
         }
         return oldSignature;
     }
 
     @Override
-    public PGPSignature createRevocationCertificate(SecretKeyRingProtector secretKeyRingProtector,
+    public WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase(
-                                                    RevocationAttributes revocationAttributes)
+            @Nullable Passphrase oldPassphrase,
-            throws PGPException {
-        PGPPublicKey revokeeSubKey = secretKeyRing.getPublicKey();
-        PGPSignature revocationCertificate = generateRevocation(secretKeyRingProtector, revokeeSubKey, revocationAttributes);
-        return revocationCertificate;
-    }
-
-    @Override
-    public PGPSignature createRevocationCertificate(long subkeyId, SecretKeyRingProtector secretKeyRingProtector, RevocationAttributes revocationAttributes) throws PGPException {
-        PGPPublicKey revokeeSubKey = KeyRingUtils.requirePublicKeyFrom(secretKeyRing, subkeyId);
-        PGPSignature revocationCertificate = generateRevocation(secretKeyRingProtector, revokeeSubKey, revocationAttributes);
-        return revocationCertificate;
-    }
-
-    private PGPSecretKeyRing revokeSubKey(SecretKeyRingProtector protector,
-                                          PGPPublicKey revokeeSubKey,
-                                          RevocationAttributes revocationAttributes)
-            throws PGPException {
-        PGPSignature subKeyRevocation = generateRevocation(protector, revokeeSubKey, revocationAttributes);
-        revokeeSubKey = PGPPublicKey.addCertification(revokeeSubKey, subKeyRevocation);
-
-        // Inject revoked public key into key ring
-        PGPPublicKeyRing publicKeyRing = KeyRingUtils.publicKeyRingFrom(secretKeyRing);
-        publicKeyRing = PGPPublicKeyRing.insertPublicKey(publicKeyRing, revokeeSubKey);
-        return PGPSecretKeyRing.replacePublicKeys(secretKeyRing, publicKeyRing);
-    }
-
-    private PGPSignature generateRevocation(SecretKeyRingProtector protector,
-                                            PGPPublicKey revokeeSubKey,
-                                            RevocationAttributes revocationAttributes)
-            throws PGPException {
-        PGPSecretKey primaryKey = secretKeyRing.getSecretKey();
-        PGPSignatureGenerator signatureGenerator = SignatureUtils.getSignatureGeneratorFor(primaryKey);
-        PGPSignatureSubpacketGenerator subpacketGenerator = new PGPSignatureSubpacketGenerator();
-        subpacketGenerator.setIssuerFingerprint(false, primaryKey);
-
-        if (revocationAttributes != null) {
-            subpacketGenerator.setRevocationReason(false, revocationAttributes.getReason().code(), revocationAttributes.getDescription());
-        }
-
-        PGPSignatureSubpacketVector subPackets = subpacketGenerator.generate();
-        signatureGenerator.setHashedSubpackets(subPackets);
-
-        PGPPrivateKey privateKey = UnlockSecretKey.unlockSecretKey(primaryKey, protector);
-
-        PGPSignature revocation;
-        if (revokeeSubKey.isMasterKey()) {
-            signatureGenerator.init(SignatureType.KEY_REVOCATION.getCode(), privateKey);
-            revocation = signatureGenerator.generateCertification(revokeeSubKey);
-        } else {
-            signatureGenerator.init(SignatureType.SUBKEY_REVOCATION.getCode(), privateKey);
-            revocation = signatureGenerator.generateCertification(primaryKey.getPublicKey(), revokeeSubKey);
-        }
-        return revocation;
-    }
-
-    @Override
-    public WithKeyRingEncryptionSettings changePassphraseFromOldPassphrase(@Nullable Passphrase oldPassphrase,
             @Nonnull KeyRingProtectionSettings oldProtectionSettings) {
         SecretKeyRingProtector protector = new PasswordBasedSecretKeyRingProtector(
                 oldProtectionSettings,
@@ -475,7 +498,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
     }
 
     @Override
-    public WithKeyRingEncryptionSettings changeSubKeyPassphraseFromOldPassphrase(@Nonnull Long keyId,
+    public WithKeyRingEncryptionSettings changeSubKeyPassphraseFromOldPassphrase(
+            @Nonnull Long keyId,
             @Nullable Passphrase oldPassphrase,
             @Nonnull KeyRingProtectionSettings oldProtectionSettings) {
         Map<Long, Passphrase> passphraseMap = Collections.singletonMap(keyId, oldPassphrase);
@@ -526,28 +550,35 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         private final KeyRingProtectionSettings newProtectionSettings;
         private final Long keyId;
 
-        private WithPassphraseImpl(Long keyId, SecretKeyRingProtector oldProtector, KeyRingProtectionSettings newProtectionSettings) {
+        private WithPassphraseImpl(
+                Long keyId,
+                SecretKeyRingProtector oldProtector,
+                KeyRingProtectionSettings newProtectionSettings) {
             this.keyId = keyId;
             this.oldProtector = oldProtector;
             this.newProtectionSettings = newProtectionSettings;
         }
 
         @Override
-        public SecretKeyRingEditorInterface toNewPassphrase(Passphrase passphrase) throws PGPException {
+        public SecretKeyRingEditorInterface toNewPassphrase(Passphrase passphrase)
+                throws PGPException {
             SecretKeyRingProtector newProtector = new PasswordBasedSecretKeyRingProtector(
                     newProtectionSettings, new SolitaryPassphraseProvider(passphrase));
 
-            PGPSecretKeyRing secretKeys = changePassphrase(keyId, SecretKeyRingEditor.this.secretKeyRing, oldProtector, newProtector);
+            PGPSecretKeyRing secretKeys = changePassphrase(
+                    keyId, SecretKeyRingEditor.this.secretKeyRing, oldProtector, newProtector);
             SecretKeyRingEditor.this.secretKeyRing = secretKeys;
 
             return SecretKeyRingEditor.this;
         }
 
         @Override
-        public SecretKeyRingEditorInterface toNoPassphrase() throws PGPException {
+        public SecretKeyRingEditorInterface toNoPassphrase()
+                throws PGPException {
             SecretKeyRingProtector newProtector = new UnprotectedKeysProtector();
 
-            PGPSecretKeyRing secretKeys = changePassphrase(keyId, SecretKeyRingEditor.this.secretKeyRing, oldProtector, newProtector);
+            PGPSecretKeyRing secretKeys = changePassphrase(
+                    keyId, SecretKeyRingEditor.this.secretKeyRing, oldProtector, newProtector);
             SecretKeyRingEditor.this.secretKeyRing = secretKeys;
 
             return SecretKeyRingEditor.this;
@@ -557,7 +588,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
     private PGPSecretKeyRing changePassphrase(Long keyId,
                                               PGPSecretKeyRing secretKeys,
                                               SecretKeyRingProtector oldProtector,
-                                              SecretKeyRingProtector newProtector) throws PGPException {
+                                              SecretKeyRingProtector newProtector)
+            throws PGPException {
         List<PGPSecretKey> secretKeyList = new ArrayList<>();
         if (keyId == null) {
             // change passphrase of whole key ring
@@ -585,7 +617,8 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         return newRing;
     }
 
-    private PGPSecretKeyRing s2kUsageFixIfNecessary(PGPSecretKeyRing secretKeys, SecretKeyRingProtector protector) throws PGPException {
+    private PGPSecretKeyRing s2kUsageFixIfNecessary(PGPSecretKeyRing secretKeys, SecretKeyRingProtector protector)
+            throws PGPException {
         boolean hasS2KUsageChecksum = false;
         for (PGPSecretKey secKey : secretKeys) {
             if (secKey.getS2KUsage() == SecretKeyPacket.USAGE_CHECKSUM) {
@@ -594,12 +627,17 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
             }
         }
         if (hasS2KUsageChecksum) {
-            secretKeys = S2KUsageFix.replaceUsageChecksumWithUsageSha1(secretKeys, protector, true);
+            secretKeys = S2KUsageFix.replaceUsageChecksumWithUsageSha1(
+                    secretKeys, protector, true);
         }
         return secretKeys;
     }
 
-    private static PGPSecretKey reencryptPrivateKey(PGPSecretKey secretKey, SecretKeyRingProtector oldProtector, SecretKeyRingProtector newProtector) throws PGPException {
+    private static PGPSecretKey reencryptPrivateKey(
+            PGPSecretKey secretKey,
+            SecretKeyRingProtector oldProtector,
+            SecretKeyRingProtector newProtector)
+            throws PGPException {
         S2K s2k = secretKey.getS2K();
         // If the key uses GNU_DUMMY_S2K, we leave it as is and skip this block
         if (s2k == null || s2k.getType() != S2K.GNU_DUMMY_S2K) {

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

@@ -23,6 +23,7 @@ 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;
 
@@ -48,6 +49,11 @@ public interface SecretKeyRingEditorInterface {
      */
     SecretKeyRingEditorInterface addUserId(String userId, SecretKeyRingProtector secretKeyRingProtector) throws PGPException;
 
+    SecretKeyRingEditorInterface addUserId(
+            String userId,
+            @Nullable SelfSignatureSubpackets.Callback signatureSubpacketCallback,
+            SecretKeyRingProtector protector) throws PGPException;
+
     /**
      * Add a subkey to the key ring.
      * The subkey will be generated from the provided {@link KeySpec}.
@@ -86,7 +92,7 @@ public interface SecretKeyRingEditorInterface {
      */
     default SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector)
             throws PGPException {
-        return revoke(secretKeyRingProtector, null);
+        return revoke(secretKeyRingProtector, (RevocationAttributes) null);
     }
 
     /**
@@ -98,9 +104,12 @@ public interface SecretKeyRingEditorInterface {
      * @return the builder
      */
     SecretKeyRingEditorInterface revoke(SecretKeyRingProtector secretKeyRingProtector,
-                                        RevocationAttributes revocationAttributes)
+                                        @Nullable RevocationAttributes revocationAttributes)
             throws PGPException;
 
+    SecretKeyRingEditorInterface revoke(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.
@@ -126,24 +135,13 @@ public interface SecretKeyRingEditorInterface {
      * @param revocationAttributes reason for the revocation
      * @return the builder
      */
-    SecretKeyRingEditorInterface revokeSubKey(OpenPgpFingerprint fingerprint,
+    default SecretKeyRingEditorInterface revokeSubKey(OpenPgpFingerprint fingerprint,
                                               SecretKeyRingProtector secretKeyRingProtector,
                                               RevocationAttributes revocationAttributes)
-            throws PGPException;
-
-    /**
-     * 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.
-     *
-     * @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)
             throws PGPException {
-        return revokeSubKey(subKeyId, secretKeyRingProtector, null);
+        return revokeSubKey(fingerprint.getKeyId(),
+                secretKeyRingProtector,
+                revocationAttributes);
     }
 
     /**
@@ -161,6 +159,25 @@ public interface SecretKeyRingEditorInterface {
                                               RevocationAttributes revocationAttributes)
             throws PGPException;
 
+    /**
+     * 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.
+     *
+     * @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)
+            throws PGPException {
+        return revokeSubKey(subKeyId, secretKeyRingProtector, (RevocationSignatureSubpackets.Callback) null);
+    }
+
+    SecretKeyRingEditorInterface revokeSubKey(long keyID,
+                                              SecretKeyRingProtector secretKeyRingProtector,
+                                              @Nullable RevocationSignatureSubpackets.Callback subpacketsCallback)
+            throws PGPException;
 
     /**
      * Revoke the given userID.

pgpainless-core/src/main/java/org/pgpainless/key/util/KeyRingUtils.java

@@ -97,7 +97,7 @@ public final class KeyRingUtils {
     public static PGPPublicKey requirePublicKeyFrom(PGPKeyRing keyRing, long subKeyId) {
         PGPPublicKey publicKey = getPublicKeyFrom(keyRing, subKeyId);
         if (publicKey == null) {
-            throw new IllegalArgumentException("KeyRing does not contain public key with keyID " + Long.toHexString(subKeyId));
+            throw new NoSuchElementException("KeyRing does not contain public key with keyID " + Long.toHexString(subKeyId));
         }
         return publicKey;
     }
@@ -105,7 +105,7 @@ public final class KeyRingUtils {
     public static PGPSecretKey requireSecretKeyFrom(PGPSecretKeyRing keyRing, long subKeyId) {
         PGPSecretKey secretKey = keyRing.getSecretKey(subKeyId);
         if (secretKey == null) {
-            throw new IllegalArgumentException("KeyRing does not contain secret key with keyID " + Long.toHexString(subKeyId));
+            throw new NoSuchElementException("KeyRing does not contain secret key with keyID " + Long.toHexString(subKeyId));
         }
         return secretKey;
     }

pgpainless-core/src/main/java/org/pgpainless/signature/builder/RevocationSignatureBuilder.java

@@ -6,8 +6,11 @@ package org.pgpainless.signature.builder;
 
 import javax.annotation.Nullable;
 
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPPublicKey;
 import org.bouncycastle.openpgp.PGPSecretKey;
 import org.bouncycastle.openpgp.PGPSignature;
+import org.bouncycastle.openpgp.PGPSignatureGenerator;
 import org.pgpainless.algorithm.SignatureType;
 import org.pgpainless.exception.WrongPassphraseException;
 import org.pgpainless.key.protection.SecretKeyRingProtector;
@@ -46,7 +49,15 @@ public class RevocationSignatureBuilder extends AbstractSignatureBuilder<Revocat
         }
     }
 
-    public PGPSignature build() {
+    public PGPSignature build(PGPPublicKey revokeeSubkey) throws PGPException {
-        return null;
+        PGPSignatureGenerator signatureGenerator = buildAndInitSignatureGenerator();
+        if (signatureType == SignatureType.KEY_REVOCATION) {
+            if (revokeeSubkey.getKeyID() != publicSigningKey.getKeyID()) {
+                throw new IllegalArgumentException("Signature type is KEY_REVOCATION, but provided revokeeSubkey is != signingPublicKey.");
+            }
+            return signatureGenerator.generateCertification(publicSigningKey);
+        } else {
+            return signatureGenerator.generateCertification(publicSigningKey, revokeeSubkey);
+        }
     }
 }

pgpainless-core/src/main/java/org/pgpainless/signature/subpackets/SignatureSubpacketsHelper.java

@@ -177,4 +177,10 @@ public class SignatureSubpacketsHelper {
         applyTo(subpackets, generator);
         return generator.generate();
     }
+
+    public static PGPSignatureSubpacketVector toVector(RevocationSignatureSubpackets subpackets) {
+        PGPSignatureSubpacketGenerator generator = new PGPSignatureSubpacketGenerator();
+        applyTo((SignatureSubpackets) subpackets, generator);
+        return generator.generate();
+    }
 }

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

@@ -4,14 +4,20 @@
 
 package org.pgpainless.key.modification;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 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.Iterator;
 
+import org.bouncycastle.bcpg.sig.IssuerFingerprint;
 import org.bouncycastle.openpgp.PGPException;
 import org.bouncycastle.openpgp.PGPPublicKey;
 import org.bouncycastle.openpgp.PGPSecretKey;
@@ -21,6 +27,7 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.pgpainless.PGPainless;
+import org.pgpainless.algorithm.EncryptionPurpose;
 import org.pgpainless.algorithm.SignatureType;
 import org.pgpainless.implementation.ImplementationFactory;
 import org.pgpainless.key.OpenPgpV4Fingerprint;
@@ -29,6 +36,9 @@ import org.pgpainless.key.modification.secretkeyring.SecretKeyRingEditorInterfac
 import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector;
 import org.pgpainless.key.protection.SecretKeyRingProtector;
 import org.pgpainless.key.util.RevocationAttributes;
+import org.pgpainless.signature.SignatureUtils;
+import org.pgpainless.signature.subpackets.RevocationSignatureSubpackets;
+import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil;
 import org.pgpainless.util.ArmorUtils;
 import org.pgpainless.util.Passphrase;
 
@@ -123,4 +133,62 @@ public class RevokeSubKeyTest {
         RevocationAttributes.Reason reason = RevocationAttributes.Reason.KEY_COMPROMISED;
         assertEquals("2 - KEY_COMPROMISED", reason.toString());
     }
+
+    @Test
+    public void inspectSubpacketsOnDefaultRevocationSignature()
+            throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException {
+        PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("Alice", null);
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys();
+        PGPPublicKey encryptionSubkey = PGPainless.inspectKeyRing(secretKeys)
+                .getEncryptionSubkeys(EncryptionPurpose.ANY).get(0);
+
+        secretKeys = PGPainless.modifyKeyRing(secretKeys)
+                .revokeSubKey(encryptionSubkey.getKeyID(), protector)
+                .done();
+
+        encryptionSubkey = secretKeys.getPublicKey(encryptionSubkey.getKeyID());
+        PGPSignature revocation = encryptionSubkey.getSignaturesOfType(SignatureType.SUBKEY_REVOCATION.getCode()).next();
+        assertNotNull(revocation);
+
+        assertArrayEquals(
+                secretKeys.getPublicKey().getFingerprint(),
+                revocation.getHashedSubPackets().getIssuerFingerprint().getFingerprint());
+        assertEquals(secretKeys.getPublicKey().getKeyID(),
+                revocation.getHashedSubPackets().getIssuerKeyID());
+        assertNull(SignatureSubpacketsUtil.getRevocationReason(revocation));
+        assertTrue(SignatureUtils.isHardRevocation(revocation));
+    }
+
+    @Test
+    public void inspectSubpacketsOnModifiedRevocationSignature()
+            throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException {
+        PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("Alice", null);
+        SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys();
+        PGPPublicKey encryptionSubkey = PGPainless.inspectKeyRing(secretKeys)
+                .getEncryptionSubkeys(EncryptionPurpose.ANY).get(0);
+
+        secretKeys = PGPainless.modifyKeyRing(secretKeys)
+                .revokeSubKey(encryptionSubkey.getKeyID(), protector, new RevocationSignatureSubpackets.Callback() {
+                    @Override
+                    public void modifyHashedSubpackets(RevocationSignatureSubpackets hashedSubpackets) {
+                        hashedSubpackets.setRevocationReason(
+                                RevocationAttributes.createKeyRevocation()
+                                        .withReason(RevocationAttributes.Reason.KEY_RETIRED)
+                                        .withDescription("I have a new Key."));
+                        // override issuer-fingerprint with null to test nulling of subpackets
+                        hashedSubpackets.setIssuerFingerprint((IssuerFingerprint) null);
+                    }
+                })
+                .done();
+
+        encryptionSubkey = secretKeys.getPublicKey(encryptionSubkey.getKeyID());
+        PGPSignature revocation = encryptionSubkey.getSignaturesOfType(SignatureType.SUBKEY_REVOCATION.getCode()).next();
+        assertNotNull(revocation);
+
+        assertNull(revocation.getHashedSubPackets().getIssuerFingerprint());
+        assertEquals(secretKeys.getPublicKey().getKeyID(),
+                revocation.getHashedSubPackets().getIssuerKeyID());
+        assertNotNull(SignatureSubpacketsUtil.getRevocationReason(revocation));
+        assertFalse(SignatureUtils.isHardRevocation(revocation));
+    }
 }