canonicalize: support secret keys

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java

@@ -178,8 +178,7 @@ public class UncachedKeyRing {
         return result;
     }
 
-    /** "Canonicalizes" a key, removing inconsistencies in the process. This operation can be
+    /** "Canonicalizes" a key, removing inconsistencies in the process.
-     * applied to public keyrings only.
      *
      * More specifically:
      *  - Remove all non-verifying self-certificates
@@ -193,6 +192,7 @@ public class UncachedKeyRing {
      *   - certifications and certification revocations for user ids
      *  - If a subkey retains no valid subkey binding certificate, remove it
      *  - If a user id retains no valid self certificate, remove it
+     *  - If the key is a secret key, remove all certificates by foreign keys
      *
      * This operation writes an OperationLog which can be used as part of a OperationResultParcel.
      *
@@ -200,12 +200,8 @@ public class UncachedKeyRing {
      *
      */
     public UncachedKeyRing canonicalize(OperationLog log, int indent) {
-        if (isSecret()) {
-            throw new RuntimeException("Tried to canonicalize non-secret keyring. " +
-                    "This is a programming error and should never happen!");
-        }
 
-        log.add(LogLevel.START, LogType.MSG_KC,
+        log.add(LogLevel.START, isSecret() ? LogType.MSG_KC_SECRET : LogType.MSG_KC_PUBLIC,
                 new String[]{PgpKeyHelper.convertKeyIdToHex(getMasterKeyId())}, indent);
         indent += 1;
 
@@ -213,7 +209,7 @@ public class UncachedKeyRing {
 
         int redundantCerts = 0, badCerts = 0;
 
-        PGPPublicKeyRing ring = (PGPPublicKeyRing) mRing;
+        PGPKeyRing ring = mRing;
         PGPPublicKey masterKey = mRing.getPublicKey();
         final long masterKeyId = masterKey.getKeyID();
 
@@ -334,8 +330,15 @@ public class UncachedKeyRing {
                         continue;
                     }
 
-                    // If this is a foreign signature, never mind any further
+                    // If this is a foreign signature, ...
                     if (certId != masterKeyId) {
+                        // never mind any further for public keys, but remove them from secret ones
+                        if (isSecret()) {
+                            log.add(LogLevel.WARN, LogType.MSG_KC_UID_FOREIGN,
+                                    new String[] { PgpKeyHelper.convertKeyIdToHex(certId) }, indent);
+                            modified = PGPPublicKey.removeCertification(modified, userId, zert);
+                            badCerts += 1;
+                        }
                         continue;
                     }
 
@@ -433,7 +436,7 @@ public class UncachedKeyRing {
             }
 
             // Replace modified key in the keyring
-            ring = PGPPublicKeyRing.insertPublicKey(ring, modified);
+            ring = replacePublicKey(ring, modified);
             indent -= 1;
 
         }
@@ -578,7 +581,7 @@ public class UncachedKeyRing {
 
             // it is not properly bound? error!
             if (selfCert == null) {
-                ring = PGPPublicKeyRing.removePublicKey(ring, modified);
+                ring = replacePublicKey(ring, modified);
 
                 log.add(LogLevel.ERROR, LogType.MSG_KC_SUB_NO_CERT,
                         new String[]{ PgpKeyHelper.convertKeyIdToHex(key.getKeyID()) }, indent);
@@ -593,7 +596,7 @@ public class UncachedKeyRing {
                 modified = PGPPublicKey.addCertification(modified, revocation);
             }
             // replace pubkey in keyring
-            ring = PGPPublicKeyRing.insertPublicKey(ring, modified);
+            ring = replacePublicKey(ring, modified);
             indent -= 1;
         }
 
@@ -614,5 +617,14 @@ public class UncachedKeyRing {
         return new UncachedKeyRing(ring);
     }
 
+    private static PGPKeyRing replacePublicKey(PGPKeyRing ring, PGPPublicKey key) {
+        if (ring instanceof PGPPublicKeyRing) {
+            return PGPPublicKeyRing.insertPublicKey((PGPPublicKeyRing) ring, key);
+        }
+        PGPSecretKeyRing secRing = (PGPSecretKeyRing) ring;
+        PGPSecretKey sKey = secRing.getSecretKey(key.getKeyID());
+        sKey = PGPSecretKey.replacePublicKey(sKey, key);
+        return PGPSecretKeyRing.insertSecretKey(secRing, sKey);
+    }
 
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java

@@ -609,6 +609,12 @@ public class ProviderHelper {
             return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
         }
 
+        // Canonicalize this key, to assert a number of assumptions made about it.
+        keyRing = keyRing.canonicalize(mLog, mIndent);
+        if (keyRing == null) {
+            return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+        }
+
         long masterKeyId = keyRing.getMasterKeyId();
         log(LogLevel.START, LogType.MSG_IS,
                 new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) });

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java

@@ -179,7 +179,8 @@ public class OperationResultParcel implements Parcelable {
         MSG_IS_SUCCESS (R.string.msg_is_success),
 
         // keyring canonicalization
-        MSG_KC (R.string.msg_kc),
+        MSG_KC_PUBLIC (R.string.msg_kc_public),
+        MSG_KC_SECRET (R.string.msg_kc_secret),
         MSG_KC_FATAL_NO_UID (R.string.msg_kc_fatal_no_uid),
         MSG_KC_MASTER (R.string.msg_kc_master),
         MSG_KC_REVOKE_BAD_ERR (R.string.msg_kc_revoke_bad_err),
@@ -212,6 +213,7 @@ public class OperationResultParcel implements Parcelable {
         MSG_KC_UID_BAD_TYPE (R.string.msg_kc_uid_bad_type),
         MSG_KC_UID_BAD (R.string.msg_kc_uid_bad),
         MSG_KC_UID_DUP (R.string.msg_kc_uid_dup),
+        MSG_KC_UID_FOREIGN (R.string.msg_kc_uid_foreign),
         MSG_KC_UID_NO_CERT (R.string.msg_kc_uid_no_cert),
         MSG_KC_UID_REVOKE_DUP (R.string.msg_kc_uid_revoke_dup),
         MSG_KC_UID_REVOKE_OLD (R.string.msg_kc_uid_revoke_old),

OpenKeychain/src/main/res/values/strings.xml

@@ -563,7 +563,8 @@
     <string name="msg_is_success">Successfully imported secret keyring</string>
 
     <!-- Keyring Canonicalization log entries -->
-    <string name="msg_kc">Canonicalizing keyring %s</string>
+    <string name="msg_kc_public">Canonicalizing public keyring %s</string>
+    <string name="msg_kc_secret">Canonicalizing secret keyring %s</string>
     <string name="msg_kc_fatal_no_uid">Keyring canonicalization failed: Keyring has no valid user ids</string>
     <string name="msg_kc_master">Processing master key</string>
     <string name="msg_kc_revoke_bad_err">Removing bad keyring revocation certificate</string>
@@ -596,6 +597,7 @@
     <string name="msg_kc_uid_bad_type">Removing user id certificate of unknown type (%s)</string>
     <string name="msg_kc_uid_bad">Removing bad self certificate for user id "%s"</string>
     <string name="msg_kc_uid_dup">Removing outdated self certificate for user id "%s"</string>
+    <string name="msg_kc_uid_foreign">Removing foreign user id certificate by %s</string>
     <string name="msg_kc_uid_revoke_dup">Removing redundant revocation certificate for user id "%s"</string>
     <string name="msg_kc_uid_revoke_old">Removing outdated revocation certificate for user id "%s"</string>
     <string name="msg_kc_uid_no_cert">No valid self-certificate found for user id %s, removing from ring</string>