From 3c62d0e2428090d4ca0aee72c9342caeb2207a1a Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Tue, 11 Mar 2025 20:11:28 +0100 Subject: [PATCH] WIP: Port SecretKeyRingEditor to use OpenPGPKeyEditor --- .../secretkeyring/SecretKeyRingEditor.kt | 234 ++++++++---------- .../SecretKeyRingEditorInterface.kt | 34 ++- .../subpackets/BaseSignatureSubpackets.kt | 3 + .../subpackets/SignatureSubpackets.kt | 5 + .../key/modification/RevokeSubKeyTest.java | 2 +- 5 files changed, 147 insertions(+), 131 deletions(-) diff --git a/pgpainless-core/src/main/kotlin/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.kt b/pgpainless-core/src/main/kotlin/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.kt index eb19941c..bbf48e82 100644 --- a/pgpainless-core/src/main/kotlin/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.kt +++ b/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 { - 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 - } - } - }) + toSignatureParametersCallback(callback, referenceTime)) .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) - val primaryKey = secretKeyRing.publicKey - 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, + return addUserId( + userId, object : SelfSignatureSubpackets.Callback { override fun modifyHashedSubpackets(hashedSubpackets: SelfSignatureSubpackets) { - hashedSubpackets.apply { - setPrimaryUserId() - if (previousKeyExpiration != null) - setKeyExpirationTime(primaryKey, previousKeyExpiration) - else setKeyExpirationTime(null) - } + hashedSubpackets.setPrimaryUserId() } }, 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 keyPair = KeyRingBuilder.generateKeyPair(keySpec, OpenPGPKeyVersion.v4, referenceTime) + val version = OpenPGPKeyVersion.from(secretKeyRing.publicKey.version) + 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 { - return revokeSubKey(secretKeyRing.secretKey.keyID, protector, callback) + ): SecretKeyRingEditorInterface = apply { + key = + OpenPGPKeyEditor(key, protector) + .revokeKey(toSignatureParametersCallback(callback, referenceTime)) + .done() } - override fun revokeSubKey( - subkeyId: Long, + override fun revokeSubkey( + keyIdentifier: KeyIdentifier, protector: SecretKeyRingProtector, - revocationAttributes: RevocationAttributes? - ): SecretKeyRingEditorInterface { - return revokeSubKey( - subkeyId, protector, callbackFromRevocationAttributes(revocationAttributes)) + revocationAttributes: RevocationAttributes + ): SecretKeyRingEditor { + return revokeSubkey( + keyIdentifier, + protector, + object : RevocationSignatureSubpackets.Callback { + override fun modifyHashedSubpackets( + hashedSubpackets: RevocationSignatureSubpackets + ) { + hashedSubpackets.setRevocationReason(revocationAttributes) + } + }) } - override fun revokeSubKey( - subkeyId: Long, + override fun revokeSubkey( + keyIdentifier: KeyIdentifier, protector: SecretKeyRingProtector, - callback: RevocationSignatureSubpackets.Callback? - ): SecretKeyRingEditorInterface { - val revokeeSubKey = secretKeyRing.requirePublicKey(subkeyId) - val subkeyRevocation = generateRevocation(protector, revokeeSubKey, callback) - secretKeyRing = injectCertification(secretKeyRing, revokeeSubKey, subkeyRevocation) - return this + revocationSignatureCallback: RevocationSignatureSubpackets.Callback? + ): SecretKeyRingEditor = apply { + key = + OpenPGPKeyEditor(key, protector) + .revokeComponentKey( + 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( - SignatureType.CERTIFICATION_REVOCATION, key.primarySecretKey, protector) - .apply { - hashedSubpackets.setSignatureCreationTime(referenceTime) - applyCallback(callback) - } - .let { - secretKeyRing = - injectCertification(secretKeyRing, userId, it.build(userId.toString())) - } + val uid = key.getUserId(userId.toString())!! + key = + OpenPGPKeyEditor(key, protector) + .revokeIdentity(uid, toSignatureParametersCallback(callback, referenceTime)) + .done() return this } + private fun toSignatureParametersCallback( + callback: SignatureSubpacketCallback?, + 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 diff --git a/pgpainless-core/src/main/kotlin/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.kt b/pgpainless-core/src/main/kotlin/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.kt index ad8e36ff..38e38560 100644 --- a/pgpainless-core/src/main/kotlin/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.kt +++ b/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. diff --git a/pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/BaseSignatureSubpackets.kt b/pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/BaseSignatureSubpackets.kt index 4b4c40d5..699f15d7 100644 --- a/pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/BaseSignatureSubpackets.kt +++ b/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 + fun setAppropriateIssuerInfo(key: OpenPGPComponentKey): BaseSignatureSubpackets + fun setAppropriateIssuerInfo(key: PGPPublicKey): BaseSignatureSubpackets /** diff --git a/pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/SignatureSubpackets.kt b/pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/SignatureSubpackets.kt index e30c6100..0e53e1ef 100644 --- a/pgpainless-core/src/main/kotlin/org/pgpainless/signature/subpackets/SignatureSubpackets.kt +++ b/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)) } diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeSubKeyTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeSubKeyTest.java index 729c2151..dec5661d 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/modification/RevokeSubKeyTest.java +++ b/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();