extend canonicalize to strip local certificates on export

3 files changed, 72 insertions, 22 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java
index da532d2dc..6ca28142f 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java
@@ -31,6 +31,7 @@ import org.sufficientlysecure.keychain.keyimport.Keyserver;
 import org.sufficientlysecure.keychain.keyimport.Keyserver.AddKeyException;
 import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing;
 import org.sufficientlysecure.keychain.operations.results.ExportResult;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpHelper;
 import org.sufficientlysecure.keychain.pgp.Progressable;
@@ -399,7 +400,7 @@ public class ImportExportOperation extends BaseOperation {
 
     }
 
-    private ExportResult exportKeyRings(OperationLog log, long[] masterKeyIds, boolean exportSecret,
+    ExportResult exportKeyRings(OperationLog log, long[] masterKeyIds, boolean exportSecret,
                                  OutputStream outStream) {
 
         /* TODO isn't this checked above, with the isStorageMounted call?
@@ -469,12 +470,16 @@ public class ImportExportOperation extends BaseOperation {
 
                     log.add(LogType.MSG_EXPORT_PUBLIC, 1, KeyFormattingUtils.beautifyKeyId(keyId));
 
-                    { // export public key part
-                        byte[] data = cursor.getBlob(1);
-                        arOutStream.write(data);
+                    byte[] data = cursor.getBlob(1);
+                    CanonicalizedKeyRing ring =
+                            UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true);
+                    ring.encode(arOutStream);
 
-                        okPublic += 1;
-                    }
+                    okPublic += 1;
+                } catch (PgpGeneralException e) {
+                    log.add(LogType.MSG_EXPORT_ERROR_KEY, 2);
+                    updateProgress(progress++, numKeys);
+                    continue;
                 } finally {
                     // make sure this is closed
                     if (arOutStream != null) {
@@ -491,12 +496,18 @@ public class ImportExportOperation extends BaseOperation {
                             arOutStream.setHeader("Version", version);
                         }
 
-                    // export secret key part
+                        // export secret key part
                         log.add(LogType.MSG_EXPORT_SECRET, 2, KeyFormattingUtils.beautifyKeyId(keyId));
                         byte[] data = cursor.getBlob(2);
-                        arOutStream.write(data);
+                        CanonicalizedKeyRing ring =
+                                UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true);
+                        ring.encode(arOutStream);
 
                         okSecret += 1;
+                    } catch (PgpGeneralException e) {
+                        log.add(LogType.MSG_EXPORT_ERROR_KEY, 2);
+                        updateProgress(progress++, numKeys);
+                        continue;
                     } finally {
                         // make sure this is closed
                         if (arOutStream != null) {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
index 426b0827e..bd73a9609 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
@@ -371,12 +371,13 @@ public abstract class OperationResult implements Parcelable {
         MSG_KC_ERROR_MASTER_ALGO (LogLevel.ERROR, R.string.msg_kc_error_master_algo),
         MSG_KC_ERROR_DUP_KEY (LogLevel.ERROR, R.string.msg_kc_error_dup_key),
         MSG_KC_MASTER (LogLevel.DEBUG, R.string.msg_kc_master),
-        MSG_KC_BAD_TYPE(LogLevel.WARN, R.string.msg_kc_bad_type),
-        MSG_KC_REVOKE_BAD_ERR (LogLevel.WARN, R.string.msg_kc_revoke_bad_err),
-        MSG_KC_REVOKE_BAD_LOCAL (LogLevel.WARN, R.string.msg_kc_revoke_bad_local),
-        MSG_KC_REVOKE_BAD_TIME (LogLevel.WARN, R.string.msg_kc_revoke_bad_time),
-        MSG_KC_REVOKE_BAD_TYPE_UID (LogLevel.WARN, R.string.msg_kc_revoke_bad_type_uid),
-        MSG_KC_REVOKE_BAD (LogLevel.WARN, R.string.msg_kc_revoke_bad),
+        MSG_KC_MASTER_BAD_TYPE(LogLevel.WARN, R.string.msg_kc_master_bad_type),
+        MSG_KC_MASTER_BAD_LOCAL(LogLevel.WARN, R.string.msg_kc_master_bad_local),
+        MSG_KC_MASTER_BAD_ERR(LogLevel.WARN, R.string.msg_kc_master_bad_err),
+        MSG_KC_MASTER_BAD_TIME(LogLevel.WARN, R.string.msg_kc_master_bad_time),
+        MSG_KC_MASTER_BAD_TYPE_UID(LogLevel.WARN, R.string.msg_kc_master_bad_type_uid),
+        MSG_KC_MASTER_BAD(LogLevel.WARN, R.string.msg_kc_master_bad),
+        MSG_KC_MASTER_LOCAL(LogLevel.WARN, R.string.msg_kc_master_local),
         MSG_KC_REVOKE_DUP (LogLevel.DEBUG, R.string.msg_kc_revoke_dup),
         MSG_KC_NOTATION_DUP (LogLevel.DEBUG, R.string.msg_kc_notation_dup),
         MSG_KC_NOTATION_EMPTY (LogLevel.DEBUG, R.string.msg_kc_notation_empty),
@@ -618,6 +619,7 @@ public abstract class OperationResult implements Parcelable {
         MSG_EXPORT_ERROR_STORAGE (LogLevel.ERROR, R.string.msg_export_error_storage),
         MSG_EXPORT_ERROR_DB (LogLevel.ERROR, R.string.msg_export_error_db),
         MSG_EXPORT_ERROR_IO (LogLevel.ERROR, R.string.msg_export_error_io),
+        MSG_EXPORT_ERROR_KEY (LogLevel.ERROR, R.string.msg_export_error_key),
         MSG_EXPORT_SUCCESS (LogLevel.OK, R.string.msg_export_success),
 
         MSG_CRT_UPLOAD_SUCCESS (LogLevel.OK, R.string.msg_crt_upload_success),
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
index 5e5a28e83..c4cd0b3e5 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -266,6 +266,35 @@ public class UncachedKeyRing {
      */
     @SuppressWarnings("ConstantConditions")
     public CanonicalizedKeyRing canonicalize(OperationLog log, int indent) {
+        return canonicalize(log, indent, false);
+    }
+
+
+    /** "Canonicalizes" a public key, removing inconsistencies in the process.
+     *
+     * More specifically:
+     *  - Remove all non-verifying self-certificates
+     *  - Remove all "future" self-certificates
+     *  - Remove all certificates flagged as "local"
+     *  - Remove all certificates which are superseded by a newer one on the same target,
+     *      including revocations with later re-certifications.
+     *  - Remove all certificates in other positions if not of known type:
+     *   - key revocation signatures on the master key
+     *   - subkey binding signatures for subkeys
+     *   - 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
+     *  - If no valid user id remains, log an error and return null
+     *
+     * This operation writes an OperationLog which can be used as part of an OperationResultParcel.
+     *
+     * @param forExport if this is true, non-exportable signatures will be removed
+     * @return A canonicalized key, or null on fatal error (log will include a message in this case)
+     *
+     */
+    @SuppressWarnings("ConstantConditions")
+    public CanonicalizedKeyRing canonicalize(OperationLog log, int indent, boolean forExport) {
 
         log.add(isSecret() ? LogType.MSG_KC_SECRET : LogType.MSG_KC_PUBLIC,
                 indent, KeyFormattingUtils.convertKeyIdToHex(getMasterKeyId()));
@@ -311,7 +340,7 @@ public class UncachedKeyRing {
                         || type == PGPSignature.CASUAL_CERTIFICATION
                         || type == PGPSignature.POSITIVE_CERTIFICATION
                         || type == PGPSignature.CERTIFICATION_REVOCATION) {
-                    log.add(LogType.MSG_KC_REVOKE_BAD_TYPE_UID, indent);
+                    log.add(LogType.MSG_KC_MASTER_BAD_TYPE_UID, indent);
                     modified = PGPPublicKey.removeCertification(modified, zert);
                     badCerts += 1;
                     continue;
@@ -320,7 +349,7 @@ public class UncachedKeyRing {
 
                 if (type != PGPSignature.KEY_REVOCATION && type != PGPSignature.DIRECT_KEY) {
                     // Unknown type, just remove
-                    log.add(LogType.MSG_KC_BAD_TYPE, indent, "0x" + Integer.toString(type, 16));
+                    log.add(LogType.MSG_KC_MASTER_BAD_TYPE, indent, "0x" + Integer.toString(type, 16));
                     modified = PGPPublicKey.removeCertification(modified, zert);
                     badCerts += 1;
                     continue;
@@ -328,7 +357,7 @@ public class UncachedKeyRing {
 
                 if (cert.getCreationTime().after(nowPlusOneDay)) {
                     // Creation date in the future? No way!
-                    log.add(LogType.MSG_KC_REVOKE_BAD_TIME, indent);
+                    log.add(LogType.MSG_KC_MASTER_BAD_TIME, indent);
                     modified = PGPPublicKey.removeCertification(modified, zert);
                     badCerts += 1;
                     continue;
@@ -337,23 +366,31 @@ public class UncachedKeyRing {
                 try {
                     cert.init(masterKey);
                     if (!cert.verifySignature(masterKey)) {
-                        log.add(LogType.MSG_KC_REVOKE_BAD, indent);
+                        log.add(LogType.MSG_KC_MASTER_BAD, indent);
                         modified = PGPPublicKey.removeCertification(modified, zert);
                         badCerts += 1;
                         continue;
                     }
                 } catch (PgpGeneralException e) {
-                    log.add(LogType.MSG_KC_REVOKE_BAD_ERR, indent);
+                    log.add(LogType.MSG_KC_MASTER_BAD_ERR, indent);
                     modified = PGPPublicKey.removeCertification(modified, zert);
                     badCerts += 1;
                     continue;
                 }
 
-                // special case: direct key signatures!
+                // if this is for export, we always remove any non-exportable certs
+                if (forExport && cert.isLocal()) {
+                    // Remove revocation certs with "local" flag
+                    log.add(LogType.MSG_KC_MASTER_LOCAL, indent);
+                    modified = PGPPublicKey.removeCertification(modified, zert);
+                    continue;
+                }
+
+                // special case: non-exportable, direct key signatures for notations!
                 if (cert.getSignatureType() == PGPSignature.DIRECT_KEY) {
                     // must be local, otherwise strip!
                     if (!cert.isLocal()) {
-                        log.add(LogType.MSG_KC_BAD_TYPE, indent);
+                        log.add(LogType.MSG_KC_MASTER_BAD_TYPE, indent);
                         modified = PGPPublicKey.removeCertification(modified, zert);
                         badCerts += 1;
                         continue;
@@ -376,7 +413,7 @@ public class UncachedKeyRing {
                     continue;
                 } else if (cert.isLocal()) {
                     // Remove revocation certs with "local" flag
-                    log.add(LogType.MSG_KC_REVOKE_BAD_LOCAL, indent);
+                    log.add(LogType.MSG_KC_MASTER_BAD_LOCAL, indent);
                     modified = PGPPublicKey.removeCertification(modified, zert);
                     badCerts += 1;
                     continue;