From ab3ae15719bdb300a76d5f5841011805d80a6952 Mon Sep 17 00:00:00 2001
From: Paul Schaub <vanitasvitae@fsfe.org>
Date: Fri, 12 Nov 2021 15:07:09 +0100
Subject: [PATCH] Ensure keyflags are set when adding userid

---
 .../secretkeyring/SecretKeyRingEditor.java    | 17 +++---
 .../builder/AbstractSignatureBuilder.java     | 53 +++++++++++++------
 .../subpackets/SignatureSubpackets.java       |  4 ++
 3 files changed, 47 insertions(+), 27 deletions(-)

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 cc20a156..1ceea650 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
@@ -37,7 +37,7 @@ import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor;
 import org.bouncycastle.openpgp.operator.PBESecretKeyEncryptor;
 import org.bouncycastle.openpgp.operator.PGPContentSignerBuilder;
 import org.bouncycastle.openpgp.operator.PGPDigestCalculator;
-import org.pgpainless.algorithm.HashAlgorithm;
+import org.pgpainless.PGPainless;
 import org.pgpainless.algorithm.KeyFlag;
 import org.pgpainless.algorithm.PublicKeyAlgorithm;
 import org.pgpainless.algorithm.SignatureType;
@@ -45,6 +45,7 @@ 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;
 import org.pgpainless.key.protection.CachingSecretKeyRingProtector;
 import org.pgpainless.key.protection.KeyRingProtectionSettings;
 import org.pgpainless.key.protection.PasswordBasedSecretKeyRingProtector;
@@ -58,6 +59,7 @@ import org.pgpainless.key.util.RevocationAttributes;
 import org.pgpainless.signature.SignatureUtils;
 import org.pgpainless.signature.builder.PrimaryKeyBindingSignatureBuilder;
 import org.pgpainless.signature.builder.SelfSignatureBuilder;
+import org.pgpainless.signature.builder.SignatureFactory;
 import org.pgpainless.signature.builder.SubkeyBindingSignatureBuilder;
 import org.pgpainless.signature.subpackets.SelfSignatureSubpackets;
 import org.pgpainless.signature.subpackets.SignatureSubpacketGeneratorUtil;
@@ -67,11 +69,6 @@ import org.pgpainless.util.Passphrase;
 
 public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
 
-    // Default algorithm for calculating private key checksums
-    // While I'd like to use something else, eg. SHA256, BC seems to lack support for
-    // calculating secret key checksums with algorithms other than SHA1.
-    private static final HashAlgorithm defaultDigestHashAlgorithm = HashAlgorithm.SHA1;
-
     private PGPSecretKeyRing secretKeyRing;
 
     public SecretKeyRingEditor(PGPSecretKeyRing secretKeyRing) {
@@ -98,8 +95,9 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         // add user-id certificate to primary key
         PGPSecretKey primaryKey = secretKeyIterator.next();
         PGPPublicKey publicKey = primaryKey.getPublicKey();
-
-        SelfSignatureBuilder builder = new SelfSignatureBuilder(primaryKey, protector);
+        KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing);
+        List<KeyFlag> keyFlags = info.getKeyFlagsOf(info.getKeyId());
+        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);
@@ -155,8 +153,7 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface {
         PBESecretKeyDecryptor ringDecryptor = keyRingProtector.getDecryptor(primaryKey.getKeyID());
         PBESecretKeyEncryptor subKeyEncryptor = subKeyProtector.getEncryptor(secretSubKey.getKeyID());
 
-        PGPDigestCalculator digestCalculator =
-                ImplementationFactory.getInstance().getPGPDigestCalculator(defaultDigestHashAlgorithm);
+        PGPDigestCalculator digestCalculator = ImplementationFactory.getInstance().getV4FingerprintCalculator();
         PGPContentSignerBuilder contentSignerBuilder =
                 SignatureUtils.getPgpContentSignerBuilderForKey(primaryKey);
 
diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java
index d919d49b..7b92ab02 100644
--- a/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java
+++ b/pgpainless-core/src/main/java/org/pgpainless/signature/builder/AbstractSignatureBuilder.java
@@ -24,6 +24,8 @@ import org.pgpainless.key.util.OpenPgpKeyAttributeUtil;
 import org.pgpainless.signature.subpackets.SignatureSubpackets;
 import org.pgpainless.signature.subpackets.SignatureSubpacketsHelper;
 
+import javax.annotation.Nonnull;
+
 public abstract class AbstractSignatureBuilder<B extends AbstractSignatureBuilder<B>> {
     protected final PGPPrivateKey privateSigningKey;
     protected final PGPPublicKey publicSigningKey;
@@ -34,7 +36,12 @@ public abstract class AbstractSignatureBuilder<B extends AbstractSignatureBuilde
     protected SignatureSubpackets unhashedSubpackets;
     protected SignatureSubpackets hashedSubpackets;
 
-    public AbstractSignatureBuilder(SignatureType signatureType, PGPSecretKey signingKey, SecretKeyRingProtector protector)
+    protected AbstractSignatureBuilder(SignatureType signatureType,
+                                       PGPSecretKey signingKey,
+                                       SecretKeyRingProtector protector,
+                                       HashAlgorithm hashAlgorithm,
+                                       SignatureSubpackets hashedSubpackets,
+                                       SignatureSubpackets unhashedSubpackets)
             throws WrongPassphraseException {
         if (!isValidSignatureType(signatureType)) {
             throw new IllegalArgumentException("Invalid signature type.");
@@ -42,26 +49,33 @@ public abstract class AbstractSignatureBuilder<B extends AbstractSignatureBuilde
         this.signatureType = signatureType;
         this.privateSigningKey = UnlockSecretKey.unlockSecretKey(signingKey, protector);
         this.publicSigningKey = signingKey.getPublicKey();
-        this.hashAlgorithm = negotiateHashAlgorithm(publicSigningKey);
+        this.hashAlgorithm = hashAlgorithm;
+        this.hashedSubpackets = hashedSubpackets;
+        this.unhashedSubpackets = unhashedSubpackets;
+    }
 
-        unhashedSubpackets = new SignatureSubpackets();
-        // Prepopulate hashed subpackets with default values (key-id etc.)
-        hashedSubpackets = SignatureSubpackets.createHashedSubpackets(publicSigningKey);
+    public AbstractSignatureBuilder(SignatureType signatureType, PGPSecretKey signingKey, SecretKeyRingProtector protector)
+            throws WrongPassphraseException {
+        this(
+                signatureType,
+                signingKey,
+                protector,
+                negotiateHashAlgorithm(signingKey.getPublicKey()),
+                SignatureSubpackets.createHashedSubpackets(signingKey.getPublicKey()),
+                SignatureSubpackets.createEmptySubpackets()
+        );
     }
 
     public AbstractSignatureBuilder(PGPSecretKey certificationKey, SecretKeyRingProtector protector, PGPSignature archetypeSignature)
             throws WrongPassphraseException {
-        SignatureType type = SignatureType.valueOf(archetypeSignature.getSignatureType());
-        if (!isValidSignatureType(type)) {
-            throw new IllegalArgumentException("Invalid signature type.");
-        }
-        this.signatureType = SignatureType.valueOf(archetypeSignature.getSignatureType());
-        this.privateSigningKey = UnlockSecretKey.unlockSecretKey(certificationKey, protector);
-        this.publicSigningKey = certificationKey.getPublicKey();
-        this.hashAlgorithm = negotiateHashAlgorithm(publicSigningKey);
-
-        unhashedSubpackets = SignatureSubpackets.refreshUnhashedSubpackets(archetypeSignature);
-        hashedSubpackets = SignatureSubpackets.refreshHashedSubpackets(publicSigningKey, archetypeSignature);
+        this(
+                SignatureType.valueOf(archetypeSignature.getSignatureType()),
+                certificationKey,
+                protector,
+                negotiateHashAlgorithm(certificationKey.getPublicKey()),
+                SignatureSubpackets.refreshHashedSubpackets(certificationKey.getPublicKey(), archetypeSignature),
+                SignatureSubpackets.refreshUnhashedSubpackets(archetypeSignature)
+        );
     }
 
     /**
@@ -70,12 +84,17 @@ public abstract class AbstractSignatureBuilder<B extends AbstractSignatureBuilde
      * @param publicKey signing public key
      * @return hash algorithm
      */
-    protected HashAlgorithm negotiateHashAlgorithm(PGPPublicKey publicKey) {
+    protected static HashAlgorithm negotiateHashAlgorithm(PGPPublicKey publicKey) {
         Set<HashAlgorithm> hashAlgorithmPreferences = OpenPgpKeyAttributeUtil.getOrGuessPreferredHashAlgorithms(publicKey);
         return HashAlgorithmNegotiator.negotiateSignatureHashAlgorithm(PGPainless.getPolicy())
                 .negotiateHashAlgorithm(hashAlgorithmPreferences);
     }
 
+    public B overrideHashAlgorithm(@Nonnull HashAlgorithm hashAlgorithm) {
+        this.hashAlgorithm = hashAlgorithm;
+        return (B) this;
+    }
+
     /**
      * Set the builders {@link SignatureType}.
      * Note that only those types who are valid for the concrete subclass of this {@link AbstractSignatureBuilder}
diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/subpackets/SignatureSubpackets.java b/pgpainless-core/src/main/java/org/pgpainless/signature/subpackets/SignatureSubpackets.java
index a7a5b71d..7076873f 100644
--- a/pgpainless-core/src/main/java/org/pgpainless/signature/subpackets/SignatureSubpackets.java
+++ b/pgpainless-core/src/main/java/org/pgpainless/signature/subpackets/SignatureSubpackets.java
@@ -103,6 +103,10 @@ public class SignatureSubpackets
         return wrapper;
     }
 
+    public static SignatureSubpackets createEmptySubpackets() {
+        return new SignatureSubpackets();
+    }
+
     @Override
     public SignatureSubpackets setIssuerFingerprintAndKeyId(PGPPublicKey key) {
         setIssuerKeyId(key.getKeyID());