WIP: Port SecretKeyRingEditor to use OpenPGPKeyEditor

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

@@ -22,12 +22,10 @@ import org.bouncycastle.openpgp.api.OpenPGPSignature
 import org.bouncycastle.openpgp.api.SignatureParameters
 import org.pgpainless.PGPainless
 import org.pgpainless.PGPainless.Companion.inspectKeyRing
-import org.pgpainless.algorithm.AlgorithmSuite
 import org.pgpainless.algorithm.KeyFlag
 import org.pgpainless.algorithm.OpenPGPKeyVersion
 import org.pgpainless.algorithm.SignatureType
 import org.pgpainless.algorithm.negotiation.HashAlgorithmNegotiator
-import org.pgpainless.bouncycastle.extensions.getKeyExpirationDate
 import org.pgpainless.bouncycastle.extensions.publicKeyAlgorithm
 import org.pgpainless.bouncycastle.extensions.requirePublicKey
 import org.pgpainless.key.OpenPgpFingerprint
@@ -66,46 +64,11 @@ class SecretKeyRingEditor(var key: OpenPGPKey, override val referenceTime: Date
             "User-ID $userId is hard revoked and cannot be re-certified."
         }
 
-        val hashAlgorithmPreferences =
-            info.preferredHashAlgorithms ?: AlgorithmSuite.defaultHashAlgorithms
-        val symmetricAlgorithmPreferences =
-            info.preferredSymmetricKeyAlgorithms ?: AlgorithmSuite.defaultSymmetricKeyAlgorithms
-        val compressionAlgorithmPreferences =
-            info.preferredCompressionAlgorithms ?: AlgorithmSuite.defaultCompressionAlgorithms
-        val aeadAlgorithmPreferences =
-            info.preferredAEADCipherSuites ?: AlgorithmSuite.defaultAEADAlgorithmSuites
-
         key =
             OpenPGPKeyEditor(key, protector)
                 .addUserId(
                     sanitizeUserId(userId).toString(),
-                    object : SignatureParameters.Callback {
+                    toSignatureParametersCallback(callback, referenceTime))
-                        override fun apply(parameters: SignatureParameters): SignatureParameters {
-                            return parameters
-                                .setSignatureCreationTime(referenceTime)
-                                .setHashedSubpacketsFunction { subpacketGenerator ->
-                                    val subpackets = SignatureSubpackets(subpacketGenerator)
-                                    subpackets.setAppropriateIssuerInfo(secretKeyRing.publicKey)
-
-                                    subpackets.setKeyFlags(info.getKeyFlagsOf(key.keyIdentifier))
-                                    subpackets.setPreferredHashAlgorithms(hashAlgorithmPreferences)
-                                    subpackets.setPreferredSymmetricKeyAlgorithms(
-                                        symmetricAlgorithmPreferences)
-                                    subpackets.setPreferredCompressionAlgorithms(
-                                        compressionAlgorithmPreferences)
-                                    subpackets.setPreferredAEADCiphersuites(
-                                        aeadAlgorithmPreferences)
-
-                                    callback?.modifyHashedSubpackets(subpackets)
-                                    subpacketGenerator
-                                }
-                                .setUnhashedSubpacketsFunction { subpacketGenerator ->
-                                    callback?.modifyUnhashedSubpackets(
-                                        SignatureSubpackets(subpacketGenerator))
-                                    subpacketGenerator
-                                }
-                        }
-                    })
                 .done()
         secretKeyRing = key.pgpSecretKeyRing
         return this
@@ -115,71 +78,14 @@ class SecretKeyRingEditor(var key: OpenPGPKey, override val referenceTime: Date
         userId: CharSequence,
         protector: SecretKeyRingProtector
     ): SecretKeyRingEditorInterface {
-        val uid = sanitizeUserId(userId)
+        return addUserId(
-        val primaryKey = secretKeyRing.publicKey
+            userId,
-        var info = inspectKeyRing(secretKeyRing, referenceTime)
-        val primaryUserId = info.primaryUserId
-        val signature =
-            if (primaryUserId == null) info.latestDirectKeySelfSignature
-            else info.getLatestUserIdCertification(primaryUserId)
-        val previousKeyExpiration = signature?.getKeyExpirationDate(primaryKey.creationTime)
-
-        // Add new primary user-id signature
-        addUserId(
-            uid,
             object : SelfSignatureSubpackets.Callback {
                 override fun modifyHashedSubpackets(hashedSubpackets: SelfSignatureSubpackets) {
-                    hashedSubpackets.apply {
+                    hashedSubpackets.setPrimaryUserId()
-                        setPrimaryUserId()
-                        if (previousKeyExpiration != null)
-                            setKeyExpirationTime(primaryKey, previousKeyExpiration)
-                        else setKeyExpirationTime(null)
-                    }
                 }
             },
             protector)
-
-        // unmark previous primary user-ids to be non-primary
-        info = inspectKeyRing(secretKeyRing, referenceTime)
-        info.validAndExpiredUserIds
-            .filterNot { it == uid }
-            .forEach { otherUserId ->
-                if (info
-                    .getLatestUserIdCertification(otherUserId)!!
-                    .hashedSubPackets
-                    .isPrimaryUserID) {
-                    // We need to unmark this user-id as primary
-                    addUserId(
-                        otherUserId,
-                        object : SelfSignatureSubpackets.Callback {
-                            override fun modifyHashedSubpackets(
-                                hashedSubpackets: SelfSignatureSubpackets
-                            ) {
-                                hashedSubpackets.apply {
-                                    setPrimaryUserId(null)
-                                    setKeyExpirationTime(null) // non-primary
-                                }
-                            }
-                        },
-                        protector)
-                }
-            }
-        return this
-    }
-
-    @Deprecated(
-        "Use of SelectUserId class is deprecated.",
-        replaceWith = ReplaceWith("removeUserId(protector, predicate)"))
-    override fun removeUserId(
-        selector: SelectUserId,
-        protector: SecretKeyRingProtector
-    ): SecretKeyRingEditorInterface {
-        return revokeUserIds(
-            selector,
-            protector,
-            RevocationAttributes.createCertificateRevocation()
-                .withReason(RevocationAttributes.Reason.USER_ID_NO_LONGER_VALID)
-                .withoutDescription())
     }
 
     override fun removeUserId(
@@ -266,11 +172,46 @@ class SecretKeyRingEditor(var key: OpenPGPKey, override val referenceTime: Date
         callback: SelfSignatureSubpackets.Callback?,
         protector: SecretKeyRingProtector
     ): SecretKeyRingEditorInterface {
-        val version = OpenPGPKeyVersion.from(secretKeyRing.getPublicKey().version)
+        val version = OpenPGPKeyVersion.from(secretKeyRing.publicKey.version)
-        val keyPair = KeyRingBuilder.generateKeyPair(keySpec, OpenPGPKeyVersion.v4, referenceTime)
+        val keyPair = KeyRingBuilder.generateKeyPair(keySpec, version, referenceTime)
         val subkeyProtector =
             PasswordBasedSecretKeyRingProtector.forKeyId(keyPair.keyIdentifier, subkeyPassphrase)
         val keyFlags = KeyFlag.fromBitmask(keySpec.subpackets.keyFlags).toMutableList()
+        keySpec.subpacketGenerator
+        val backSigParameters =
+            if (!keyFlags.contains(KeyFlag.SIGN_DATA)) null
+            else
+                object : SignatureParameters.Callback {
+                    override fun apply(parameters: SignatureParameters): SignatureParameters {
+                        return parameters.setHashedSubpacketsFunction {
+                            it.apply {
+                                SignatureSubpackets(it)
+                                    .setSignatureCreationTime(referenceTime)
+                                    .setAppropriateIssuerInfo(key.primaryKey)
+                            }
+                        }
+                    }
+                }
+
+        key =
+            OpenPGPKeyEditor(key, protector)
+                .addSubkey(
+                    keyPair,
+                    object : SignatureParameters.Callback {
+                        override fun apply(parameters: SignatureParameters): SignatureParameters {
+                            return parameters.setHashedSubpacketsFunction {
+                                it.apply {
+                                    SignatureSubpackets(it)
+                                        .setKeyFlags(keyFlags)
+                                        .setAppropriateIssuerInfo(key.primaryKey)
+                                }
+                            }
+                        }
+                    },
+                    backSigParameters)
+                .changePassphrase(keyPair.keyIdentifier, null, subkeyPassphrase.getChars(), false)
+                .done()
+
         return addSubKey(
             keyPair,
             callback,
@@ -352,30 +293,50 @@ class SecretKeyRingEditor(var key: OpenPGPKey, override val referenceTime: Date
     override fun revoke(
         protector: SecretKeyRingProtector,
         callback: RevocationSignatureSubpackets.Callback?
-    ): SecretKeyRingEditorInterface {
+    ): SecretKeyRingEditorInterface = apply {
-        return revokeSubKey(secretKeyRing.secretKey.keyID, protector, callback)
+        key =
+            OpenPGPKeyEditor(key, protector)
+                .revokeKey(toSignatureParametersCallback(callback, referenceTime))
+                .done()
     }
 
-    override fun revokeSubKey(
+    override fun revokeSubkey(
-        subkeyId: Long,
+        keyIdentifier: KeyIdentifier,
         protector: SecretKeyRingProtector,
-        revocationAttributes: RevocationAttributes?
+        revocationAttributes: RevocationAttributes
-    ): SecretKeyRingEditorInterface {
+    ): SecretKeyRingEditor {
-        return revokeSubKey(
+        return revokeSubkey(
-            subkeyId, protector, callbackFromRevocationAttributes(revocationAttributes))
+            keyIdentifier,
+            protector,
+            object : RevocationSignatureSubpackets.Callback {
+                override fun modifyHashedSubpackets(
+                    hashedSubpackets: RevocationSignatureSubpackets
+                ) {
+                    hashedSubpackets.setRevocationReason(revocationAttributes)
+                }
+            })
     }
 
-    override fun revokeSubKey(
+    override fun revokeSubkey(
-        subkeyId: Long,
+        keyIdentifier: KeyIdentifier,
         protector: SecretKeyRingProtector,
-        callback: RevocationSignatureSubpackets.Callback?
+        revocationSignatureCallback: RevocationSignatureSubpackets.Callback?
-    ): SecretKeyRingEditorInterface {
+    ): SecretKeyRingEditor = apply {
-        val revokeeSubKey = secretKeyRing.requirePublicKey(subkeyId)
+        key =
-        val subkeyRevocation = generateRevocation(protector, revokeeSubKey, callback)
+            OpenPGPKeyEditor(key, protector)
-        secretKeyRing = injectCertification(secretKeyRing, revokeeSubKey, subkeyRevocation)
+                .revokeComponentKey(
-        return this
+                    key.getKey(keyIdentifier)
+                        ?: throw NoSuchElementException(
+                            "Missing component key $keyIdentifier on key ${key.keyIdentifier}"),
+                    toSignatureParametersCallback(revocationSignatureCallback, referenceTime))
+                .done()
     }
 
+    override fun revokeSubkey(
+        keyIdentifier: KeyIdentifier,
+        protector: SecretKeyRingProtector
+    ): SecretKeyRingEditor = revokeSubkey(keyIdentifier, protector, null)
+
     override fun revokeUserId(
         userId: CharSequence,
         protector: SecretKeyRingProtector,
@@ -603,7 +564,7 @@ class SecretKeyRingEditor(var key: OpenPGPKey, override val referenceTime: Date
     private fun sanitizeUserId(userId: CharSequence): CharSequence =
         // TODO: Further research how to sanitize user IDs.
         //  e.g. what about newlines?
-        userId.toString().trim()
+        userId.toString().trim().also { require(it.isNotBlank()) { "User-ID cannot be blank." } }
 
     private fun callbackFromRevocationAttributes(attributes: RevocationAttributes?) =
         object : RevocationSignatureSubpackets.Callback {
@@ -633,19 +594,38 @@ class SecretKeyRingEditor(var key: OpenPGPKey, override val referenceTime: Date
         protector: SecretKeyRingProtector,
         callback: RevocationSignatureSubpackets.Callback?
     ): SecretKeyRingEditorInterface {
-        RevocationSignatureBuilder(
+        val uid = key.getUserId(userId.toString())!!
-                SignatureType.CERTIFICATION_REVOCATION, key.primarySecretKey, protector)
+        key =
-            .apply {
+            OpenPGPKeyEditor(key, protector)
-                hashedSubpackets.setSignatureCreationTime(referenceTime)
+                .revokeIdentity(uid, toSignatureParametersCallback(callback, referenceTime))
-                applyCallback(callback)
+                .done()
-            }
-            .let {
-                secretKeyRing =
-                    injectCertification(secretKeyRing, userId, it.build(userId.toString()))
-            }
         return this
     }
 
+    private fun <S : BaseSignatureSubpackets> toSignatureParametersCallback(
+        callback: SignatureSubpacketCallback<S>?,
+        referenceTime: Date
+    ): SignatureParameters.Callback {
+        return object : SignatureParameters.Callback {
+            override fun apply(parameters: SignatureParameters): SignatureParameters {
+                return parameters
+                    .setSignatureCreationTime(referenceTime)
+                    .setHashedSubpacketsFunction {
+                        it.apply {
+                            val subpackets = SignatureSubpackets(it) as S
+                            subpackets.setAppropriateIssuerInfo(key.primaryKey)
+                            callback?.modifyHashedSubpackets(subpackets)
+                        }
+                    }
+                    .setUnhashedSubpacketsFunction {
+                        it.apply {
+                            callback?.modifyUnhashedSubpackets(SignatureSubpackets(it) as S)
+                        }
+                    }
+            }
+        }
+    }
+
     private fun getPreviousDirectKeySignature(): PGPSignature? {
         val info = inspectKeyRing(secretKeyRing, referenceTime)
         return info.latestDirectKeySelfSignature

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

@@ -233,6 +233,26 @@ interface SecretKeyRingEditorInterface {
         callback: RevocationSignatureSubpackets.Callback?
     ): SecretKeyRingEditorInterface
 
+    @Throws(PGPException::class)
+    fun revokeSubkey(
+        keyIdentifier: KeyIdentifier,
+        protector: SecretKeyRingProtector,
+        revocationAttributes: RevocationAttributes
+    ): SecretKeyRingEditorInterface
+
+    @Throws(PGPException::class)
+    fun revokeSubkey(
+        keyIdentifier: KeyIdentifier,
+        protector: SecretKeyRingProtector,
+        revocationSignatureCallback: RevocationSignatureSubpackets.Callback?
+    ): SecretKeyRingEditorInterface
+
+    @Throws(PGPException::class)
+    fun revokeSubkey(
+        keyIdentifier: KeyIdentifier,
+        protector: SecretKeyRingProtector
+    ): SecretKeyRingEditorInterface
+
     /**
      * Revoke the subkey binding signature of a subkey. The subkey with the provided fingerprint
      * will be revoked. If no suitable subkey is found, a [NoSuchElementException] will be thrown.
@@ -262,7 +282,9 @@ interface SecretKeyRingEditorInterface {
         protector: SecretKeyRingProtector,
         revocationAttributes: RevocationAttributes? = null
     ): SecretKeyRingEditorInterface =
-        revokeSubKey(fingerprint.keyId, protector, revocationAttributes)
+        if (revocationAttributes != null)
+            revokeSubkey(fingerprint.keyIdentifier, protector, revocationAttributes)
+        else revokeSubkey(fingerprint.keyIdentifier, protector)
 
     /**
      * Revoke the subkey binding signature of a subkey. The subkey with the provided key-id will be
@@ -274,6 +296,7 @@ interface SecretKeyRingEditorInterface {
      * @throws PGPException in case we cannot generate a revocation signature for the subkey
      */
     @Throws(PGPException::class)
+    @Deprecated("Pass in a KeyIdentifier instead.")
     fun revokeSubKey(subkeyId: Long, protector: SecretKeyRingProtector) =
         revokeSubKey(subkeyId, protector, null as RevocationAttributes?)
 
@@ -288,11 +311,15 @@ interface SecretKeyRingEditorInterface {
      * @throws PGPException in case we cannot generate a revocation signature for the subkey
      */
     @Throws(PGPException::class)
+    @Deprecated("Pass in a KeyIdentifier instead.")
     fun revokeSubKey(
         subkeyId: Long,
         protector: SecretKeyRingProtector,
         revocationAttributes: RevocationAttributes? = null
-    ): SecretKeyRingEditorInterface
+    ): SecretKeyRingEditorInterface =
+        if (revocationAttributes != null)
+            revokeSubkey(KeyIdentifier(subkeyId), protector, revocationAttributes)
+        else revokeSubkey(KeyIdentifier(subkeyId), protector)
 
     /**
      * Revoke the subkey binding signature of a subkey. The subkey with the provided key-id will be
@@ -308,11 +335,12 @@ interface SecretKeyRingEditorInterface {
      * @throws PGPException in case we cannot generate a revocation signature for the subkey
      */
     @Throws(PGPException::class)
+    @Deprecated("Pass in a KeyIdentifier instead.")
     fun revokeSubKey(
         subkeyId: Long,
         protector: SecretKeyRingProtector,
         callback: RevocationSignatureSubpackets.Callback?
-    ): SecretKeyRingEditorInterface
+    ): SecretKeyRingEditorInterface = revokeSubkey(KeyIdentifier(subkeyId), protector, callback)
 
     /**
      * Hard-revoke the given userID.

pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/BaseSignatureSubpackets.kt

@@ -10,6 +10,7 @@ import java.util.*
 import org.bouncycastle.bcpg.sig.*
 import org.bouncycastle.openpgp.PGPPublicKey
 import org.bouncycastle.openpgp.PGPSignature
+import org.bouncycastle.openpgp.api.OpenPGPCertificate.OpenPGPComponentKey
 import org.pgpainless.algorithm.HashAlgorithm
 import org.pgpainless.algorithm.OpenPGPKeyVersion
 import org.pgpainless.algorithm.PublicKeyAlgorithm
@@ -18,6 +19,8 @@ interface BaseSignatureSubpackets {
 
     interface Callback : SignatureSubpacketCallback<BaseSignatureSubpackets>
 
+    fun setAppropriateIssuerInfo(key: OpenPGPComponentKey): BaseSignatureSubpackets
+
     fun setAppropriateIssuerInfo(key: PGPPublicKey): BaseSignatureSubpackets
 
     /**

pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/SignatureSubpackets.kt

@@ -16,6 +16,7 @@ import org.bouncycastle.openpgp.PGPPublicKey
 import org.bouncycastle.openpgp.PGPSignature
 import org.bouncycastle.openpgp.PGPSignatureSubpacketGenerator
 import org.bouncycastle.openpgp.PGPSignatureSubpacketVector
+import org.bouncycastle.openpgp.api.OpenPGPCertificate.OpenPGPComponentKey
 import org.pgpainless.algorithm.*
 import org.pgpainless.key.util.RevocationAttributes
 
@@ -372,6 +373,10 @@ class SignatureSubpackets(
         features?.let { subpacketsGenerator.setFeature(it.isCritical, it.features) }
     }
 
+    override fun setAppropriateIssuerInfo(key: OpenPGPComponentKey): SignatureSubpackets = apply {
+        setAppropriateIssuerInfo(key.pgpPublicKey)
+    }
+
     override fun setAppropriateIssuerInfo(key: PGPPublicKey) = apply {
         setAppropriateIssuerInfo(key, OpenPGPKeyVersion.from(key.version))
     }

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

@@ -150,7 +150,7 @@ public class RevokeSubKeyTest {
     }
 
     @Test
-    public void inspectSubpacketsOnModifiedRevocationSignature() {
+    public void inspectSubpacketsOnModifiedRevocationSignature() throws PGPException {
         PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("Alice")
                 .getPGPSecretKeyRing();
         SecretKeyRingProtector protector = SecretKeyRingProtector.unprotectedKeys();