Some code cleanup

pgpainless-core/src/main/java/org/pgpainless/key/modification/KeyRingEditor.java

@@ -15,6 +15,8 @@
  */
 package org.pgpainless.key.modification;
 
+import static org.pgpainless.key.util.KeyUtils.unlockSecretKey;
+
 import java.security.InvalidAlgorithmParameterException;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
@@ -56,7 +58,7 @@ import org.pgpainless.key.protection.SecretKeyRingProtector;
 import org.pgpainless.key.protection.UnprotectedKeysProtector;
 import org.pgpainless.key.protection.passphrase_provider.SolitaryPassphraseProvider;
 import org.pgpainless.key.util.KeyRingUtils;
-import org.pgpainless.key.util.OpenPgpKeyAttributeUtil;
+import org.pgpainless.key.util.SignatureUtils;
 import org.pgpainless.util.NotYetImplementedException;
 import org.pgpainless.util.Passphrase;
 
@@ -115,8 +117,7 @@ public class KeyRingEditor implements KeyRingEditorInterface {
             throw new IllegalArgumentException("Key-ID mismatch!");
         }
         // Create signature with new user-id and add it to the public key
-        PGPSignatureGenerator signatureGenerator = new PGPSignatureGenerator(
+        PGPSignatureGenerator signatureGenerator = SignatureUtils.getSignatureGeneratorFor(publicKey);
-                getPgpContentSignerBuilderForKey(publicKey));
         signatureGenerator.init(SignatureType.POSITIVE_CERTIFICATION.getCode(), privateKey);
 
         PGPSignature userIdSignature = signatureGenerator.generateCertification(userId, publicKey);
@@ -126,26 +127,6 @@ public class KeyRingEditor implements KeyRingEditorInterface {
         return publicKey;
     }
 
-    private static BcPGPContentSignerBuilder getPgpContentSignerBuilderForKey(PGPPublicKey publicKey) {
-        List<HashAlgorithm> preferredHashAlgorithms = OpenPgpKeyAttributeUtil.getPreferredHashAlgorithms(publicKey);
-        HashAlgorithm hashAlgorithm = negotiateHashAlgorithm(preferredHashAlgorithms);
-
-        return new BcPGPContentSignerBuilder(publicKey.getAlgorithm(), hashAlgorithm.getAlgorithmId());
-    }
-
-    private static HashAlgorithm negotiateHashAlgorithm(List<HashAlgorithm> preferredHashAlgorithms) {
-        // TODO: Match our list of supported hash algorithms against the list, to determine the best suitable algo.
-        //  For now we just take the first algorithm in the list and hope that BC has support for it.
-        return preferredHashAlgorithms.get(0);
-    }
-
-    // TODO: Move to utility class
-    private static PGPPrivateKey unlockSecretKey(PGPSecretKey secretKey, SecretKeyRingProtector protector) throws PGPException {
-        PBESecretKeyDecryptor secretKeyDecryptor = protector.getDecryptor(secretKey.getKeyID());
-        PGPPrivateKey privateKey = secretKey.extractPrivateKey(secretKeyDecryptor);
-        return privateKey;
-    }
-
     // TODO: Move to utility class?
     private String sanitizeUserId(String userId) {
         userId = userId.trim();
@@ -266,7 +247,7 @@ public class KeyRingEditor implements KeyRingEditorInterface {
 
         PGPSecretKey deleteMe = secretKeyRing.getSecretKey(subKeyId);
         if (deleteMe == null) {
-            throw new NoSuchElementException("KeyRing does not contain such a key.");
+            throw new NoSuchElementException("KeyRing does not contain a key with keyId " + Long.toHexString(subKeyId));
         }
 
         PGPSecretKeyRing newKeyRing = PGPSecretKeyRing.removeSecretKey(secretKeyRing, deleteMe);
@@ -285,10 +266,7 @@ public class KeyRingEditor implements KeyRingEditorInterface {
             throw new NoSuchElementException("No subkey with fingerprint " + fingerprint + " found.");
         }
 
-        PGPContentSignerBuilder contentSignerBuilder = new BcPGPContentSignerBuilder(
+        PGPSignatureGenerator signatureGenerator = SignatureUtils.getSignatureGeneratorFor(primaryKey);
-                primaryKey.getPublicKey().getAlgorithm(),
-                defaultDigestHashAlgorithm.getAlgorithmId());
-        PGPSignatureGenerator signatureGenerator = new PGPSignatureGenerator(contentSignerBuilder);
         signatureGenerator.init(SignatureType.SUBKEY_REVOCATION.getCode(), privateKey);
 
         // Generate revocation

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

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2020 Paul Schaub.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.pgpainless.key.util;
+
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPPrivateKey;
+import org.bouncycastle.openpgp.PGPSecretKey;
+import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor;
+import org.pgpainless.key.protection.SecretKeyRingProtector;
+
+public class KeyUtils {
+
+    public static PGPPrivateKey unlockSecretKey(PGPSecretKey secretKey, SecretKeyRingProtector protector) throws PGPException {
+        PBESecretKeyDecryptor secretKeyDecryptor = protector.getDecryptor(secretKey.getKeyID());
+        PGPPrivateKey privateKey = secretKey.extractPrivateKey(secretKeyDecryptor);
+        return privateKey;
+    }
+}

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

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2020 Paul Schaub.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.pgpainless.key.util;
+
+import java.util.List;
+
+import org.bouncycastle.openpgp.PGPPublicKey;
+import org.bouncycastle.openpgp.PGPSecretKey;
+import org.bouncycastle.openpgp.PGPSignatureGenerator;
+import org.bouncycastle.openpgp.operator.bc.BcPGPContentSignerBuilder;
+import org.pgpainless.algorithm.HashAlgorithm;
+
+public class SignatureUtils {
+
+    public static PGPSignatureGenerator getSignatureGeneratorFor(PGPSecretKey singingKey) {
+        return getSignatureGeneratorFor(singingKey.getPublicKey());
+    }
+
+    public static PGPSignatureGenerator getSignatureGeneratorFor(PGPPublicKey signingPubKey) {
+        PGPSignatureGenerator signatureGenerator = new PGPSignatureGenerator(
+                getPgpContentSignerBuilderForKey(signingPubKey));
+        return signatureGenerator;
+    }
+
+    private static BcPGPContentSignerBuilder getPgpContentSignerBuilderForKey(PGPPublicKey publicKey) {
+        List<HashAlgorithm> preferredHashAlgorithms = OpenPgpKeyAttributeUtil.getPreferredHashAlgorithms(publicKey);
+        HashAlgorithm hashAlgorithm = negotiateHashAlgorithm(preferredHashAlgorithms);
+
+        return new BcPGPContentSignerBuilder(publicKey.getAlgorithm(), hashAlgorithm.getAlgorithmId());
+    }
+
+    private static HashAlgorithm negotiateHashAlgorithm(List<HashAlgorithm> preferredHashAlgorithms) {
+        // TODO: Match our list of supported hash algorithms against the list, to determine the best suitable algo.
+        //  For now we just take the first algorithm in the list and hope that BC has support for it.
+        return preferredHashAlgorithms.get(0);
+    }
+}