import-log: minor improvements

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

@@ -127,9 +127,7 @@ public class PgpImportExport {
 
         updateProgress(R.string.progress_importing, 0, 100);
 
-        int newKeys = 0;
-        int oldKeys = 0;
-        int badKeys = 0;
+        int newKeys = 0, oldKeys = 0, badKeys = 0;
 
         int position = 0;
         for (ParcelableKeyRing entry : entries) {
@@ -147,7 +145,12 @@ public class PgpImportExport {
                     }
                 }
 
-                SaveKeyringResult result = mProviderHelper.savePublicKeyRing(key);
+                SaveKeyringResult result;
+                if (key.isSecret()) {
+                    result = mProviderHelper.saveSecretKeyRing(key);
+                } else {
+                    result = mProviderHelper.savePublicKeyRing(key);
+                }
                 if (!result.success()) {
                     badKeys += 1;
                 } else if (result.updated()) {

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

@@ -416,9 +416,6 @@ public class UncachedKeyRing {
                         cert.init(masterKey);
                         if (!cert.verifySignature(masterKey, key)) {
                             log.add(LogLevel.WARN, LogType.MSG_KC_SUB_BAD, null, indent);
-                            log.add(LogLevel.WARN, LogType.MSG_KC_SUB, new String[] {
-                                cert.getCreationTime().toString()
-                            }, indent);
                             continue;
                         }
                     } catch (PgpGeneralException e) {

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

@@ -296,20 +296,14 @@ public class ProviderHelper {
             secretRing = null;
         }
 
-        // delete old version of this keyRing, which also deletes all keys and userIds on cascade
-        int deleted = mContentResolver.delete(
-                KeyRingData.buildPublicKeyRingUri(Long.toString(masterKeyId)), null, null);
-        if (deleted > 0) {
-            log(LogLevel.DEBUG, LogType.MSG_IP_DELETE_OLD_OK);
-            result |= SaveKeyringResult.UPDATED;
-        } else {
-            log(LogLevel.DEBUG, LogType.MSG_IP_DELETE_OLD_FAIL);
-        }
-
+        ArrayList<ContentProviderOperation> operations;
         try {
 
+            log(LogLevel.DEBUG, LogType.MSG_IP_PREPARE);
+            mIndent += 1;
+
             // save all keys and userIds included in keyRing object in database
-            ArrayList<ContentProviderOperation> operations = new ArrayList<ContentProviderOperation>();
+            operations = new ArrayList<ContentProviderOperation>();
 
             log(LogLevel.INFO, LogType.MSG_IP_INSERT_KEYRING);
             { // insert keyring
@@ -354,26 +348,26 @@ public class ProviderHelper {
                     values.put(Keys.IS_REVOKED, key.isRevoked());
                     if (c) {
                         if (e) {
-                            log(LogLevel.DEBUG,s ? LogType.MSG_IP_SUBKEY_FLAGS_CES
-                                                 : LogType.MSG_IP_SUBKEY_FLAGS_CEX, null);
+                            log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_CES
+                                    : LogType.MSG_IP_SUBKEY_FLAGS_CEX, null);
                         } else {
                             log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_CXS
-                                                  : LogType.MSG_IP_SUBKEY_FLAGS_CXX, null);
+                                    : LogType.MSG_IP_SUBKEY_FLAGS_CXX, null);
                         }
                     } else {
                         if (e) {
                             log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_XES
-                                                  : LogType.MSG_IP_SUBKEY_FLAGS_XEX, null);
+                                    : LogType.MSG_IP_SUBKEY_FLAGS_XEX, null);
                         } else {
                             log(LogLevel.DEBUG, s ? LogType.MSG_IP_SUBKEY_FLAGS_XXS
-                                                  : LogType.MSG_IP_SUBKEY_FLAGS_XXX, null);
+                                    : LogType.MSG_IP_SUBKEY_FLAGS_XXX, null);
                         }
                     }
 
                     Date creation = key.getCreationTime();
                     values.put(Keys.CREATION, creation.getTime() / 1000);
                     if (creation.after(new Date())) {
-                        log(LogLevel.ERROR, LogType.MSG_IP_SUBKEY_FUTURE, new String[] {
+                        log(LogLevel.ERROR, LogType.MSG_IP_SUBKEY_FUTURE, new String[]{
                                 creation.toString()
                         });
                         return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
@@ -382,11 +376,11 @@ public class ProviderHelper {
                     if (expiryDate != null) {
                         values.put(Keys.EXPIRY, expiryDate.getTime() / 1000);
                         if (key.isExpired()) {
-                            log(LogLevel.INFO, LogType.MSG_IP_SUBKEY_EXPIRED, new String[] {
+                            log(LogLevel.DEBUG, LogType.MSG_IP_SUBKEY_EXPIRED, new String[]{
                                     expiryDate.toString()
                             });
                         } else {
-                            log(LogLevel.DEBUG, LogType.MSG_IP_SUBKEY_EXPIRES, new String[] {
+                            log(LogLevel.DEBUG, LogType.MSG_IP_SUBKEY_EXPIRES, new String[]{
                                     expiryDate.toString()
                             });
                         }
@@ -402,8 +396,8 @@ public class ProviderHelper {
             // get a list of owned secret keys, for verification filtering
             LongSparseArray<UncachedPublicKey> trustedKeys =
                     getUncachedMasterKeys(KeyRingData.buildSecretKeyRingUri());
-            log(LogLevel.INFO, LogType.MSG_IP_TRUST_USING, new String[] {
-                Integer.toString(trustedKeys.size())
+            log(LogLevel.INFO, LogType.MSG_IP_TRUST_USING, new String[]{
+                    Integer.toString(trustedKeys.size())
             });
 
             // classify and order user ids. primary are moved to the front, revoked to the back,
@@ -419,7 +413,7 @@ public class ProviderHelper {
 
                 int unknownCerts = 0;
 
-                log(LogLevel.INFO, LogType.MSG_IP_UID_PROCESSING, new String[] { userId });
+                log(LogLevel.INFO, LogType.MSG_IP_UID_PROCESSING, new String[]{ userId });
                 mIndent += 1;
                 // look through signatures for this specific key
                 for (WrappedSignature cert : new IterableIterator<WrappedSignature>(
@@ -453,7 +447,7 @@ public class ProviderHelper {
                             item.isPrimary = cert.isPrimaryUserId();
                             if (cert.isRevocation()) {
                                 item.isRevoked = true;
-                                log(LogLevel.INFO, LogType.MSG_IP_UID_REVOKED);
+                                log(LogLevel.DEBUG, LogType.MSG_IP_UID_REVOKED);
                             } else {
                                 item.isRevoked = false;
                             }
@@ -467,7 +461,8 @@ public class ProviderHelper {
                             if (cert.verifySignature(masterKey, userId)) {
                                 item.trustedCerts.add(cert);
                                 log(LogLevel.INFO, LogType.MSG_IP_UID_CERT_GOOD, new String[] {
-                                    PgpKeyHelper.convertKeyIdToHex(trustedKey.getKeyId())
+                                        PgpKeyHelper.convertKeyIdToHexShort(trustedKey.getKeyId()),
+                                        trustedKey.getPrimaryUserId()
                                 });
                             } else {
                                 log(LogLevel.WARN, LogType.MSG_IP_UID_CERT_BAD);
@@ -485,7 +480,7 @@ public class ProviderHelper {
                 mIndent -= 1;
 
                 if (unknownCerts > 0) {
-                    log(LogLevel.DEBUG, LogType.MSG_IP_UID_CERTS_UNKNOWN, new String[] {
+                    log(LogLevel.DEBUG, LogType.MSG_IP_UID_CERTS_UNKNOWN, new String[]{
                             Integer.toString(unknownCerts)
                     });
                 }
@@ -517,13 +512,43 @@ public class ProviderHelper {
                 }
             }
 
-            log(LogLevel.DEBUG, LogType.MSG_IP_APPLY_BATCH);
-            mContentResolver.applyBatch(KeychainContract.CONTENT_AUTHORITY, operations);
+            log(LogLevel.DEBUG, LogType.MSG_IP_PREPARE_SUCCESS);
+            mIndent -= 1;
+
         } catch (IOException e) {
             log(LogLevel.ERROR, LogType.MSG_IP_FAIL_IO_EXC);
             Log.e(Constants.TAG, "IOException during import", e);
             mIndent -= 1;
             return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+        }
+
+        try {
+            // delete old version of this keyRing, which also deletes all keys and userIds on cascade
+            int deleted = mContentResolver.delete(
+                    KeyRingData.buildPublicKeyRingUri(Long.toString(masterKeyId)), null, null);
+            if (deleted > 0) {
+                log(LogLevel.DEBUG, LogType.MSG_IP_DELETE_OLD_OK);
+                result |= SaveKeyringResult.UPDATED;
+            } else {
+                log(LogLevel.DEBUG, LogType.MSG_IP_DELETE_OLD_FAIL);
+            }
+
+            log(LogLevel.DEBUG, LogType.MSG_IP_APPLY_BATCH);
+            mContentResolver.applyBatch(KeychainContract.CONTENT_AUTHORITY, operations);
+
+            // Save the saved keyring (if any)
+            if (secretRing != null) {
+                log(LogLevel.DEBUG, LogType.MSG_IP_REINSERT_SECRET);
+                mIndent += 1;
+                saveSecretKeyRing(secretRing);
+                result |= SaveKeyringResult.SAVED_SECRET;
+                mIndent -= 1;
+            }
+
+            mIndent -= 1;
+            log(LogLevel.OK, LogType.MSG_IP_SUCCESS);
+            return new SaveKeyringResult(result, mLog);
+
         } catch (RemoteException e) {
             log(LogLevel.ERROR, LogType.MSG_IP_FAIL_REMOTE_EX);
             Log.e(Constants.TAG, "RemoteException during import", e);
@@ -536,19 +561,6 @@ public class ProviderHelper {
             return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
         }
 
-        // Save the saved keyring (if any)
-        if (secretRing != null) {
-            log(LogLevel.DEBUG, LogType.MSG_IP_REINSERT_SECRET);
-            mIndent += 1;
-            saveSecretKeyRing(secretRing);
-            result |= SaveKeyringResult.SAVED_SECRET;
-            mIndent -= 1;
-        }
-
-        mIndent -= 1;
-        log(LogLevel.OK, LogType.MSG_IP_SUCCESS);
-        return new SaveKeyringResult(result, mLog);
-
     }
 
     private static class UserIdItem implements Comparable<UserIdItem> {
@@ -575,19 +587,24 @@ public class ProviderHelper {
     /**
      * Saves a PGPSecretKeyRing in the DB. This will only work if a corresponding public keyring
      * is already in the database!
+     *
+     * TODO allow adding secret keys where no public key exists (ie, consolidate keys)
      */
-    public OperationResultParcel saveSecretKeyRing(UncachedKeyRing keyRing) {
-
-        long masterKeyId = keyRing.getMasterKeyId();
-        log(LogLevel.START, LogType.MSG_IS,
-                new String[]{PgpKeyHelper.convertKeyIdToHex(masterKeyId)});
-        mIndent += 1;
+    public SaveKeyringResult saveSecretKeyRing(UncachedKeyRing keyRing) {
 
         if (!keyRing.isSecret()) {
             log(LogLevel.ERROR, LogType.MSG_IS_BAD_TYPE_PUBLIC);
-            return new OperationResultParcel(1, mLog);
+            return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
         }
 
+        long masterKeyId = keyRing.getMasterKeyId();
+        log(LogLevel.START, LogType.MSG_IS,
+                new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) });
+        mIndent += 1;
+
+        // IF this is successful, it's a secret key
+        int result = SaveKeyringResult.SAVED_SECRET;
+
         // save secret keyring
         try {
             ContentValues values = new ContentValues();
@@ -599,7 +616,7 @@ public class ProviderHelper {
         } catch (IOException e) {
             Log.e(Constants.TAG, "Failed to encode key!", e);
             log(LogLevel.ERROR, LogType.MSG_IS_IO_EXCPTION);
-            return new OperationResultParcel(1, mLog);
+            return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
         }
 
         {
@@ -643,7 +660,7 @@ public class ProviderHelper {
         }
 
         log(LogLevel.OK, LogType.MSG_IS_SUCCESS);
-        return new OperationResultParcel(0, mLog);
+        return new SaveKeyringResult(result, mLog);
 
     }
 

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

@@ -115,6 +115,8 @@ public class OperationResultParcel implements Parcelable {
         MSG_IP_FAIL_REMOTE_EX (R.string.msg_ip_fail_remote_ex),
         MSG_IP_INSERT_KEYRING (R.string.msg_ip_insert_keyring),
         MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_subkeys),
+        MSG_IP_PREPARE (R.string.msg_ip_prepare),
+        MSG_IP_PREPARE_SUCCESS(R.string.msg_ip_prepare_success),
         MSG_IP_PRESERVING_SECRET (R.string.msg_ip_preserving_secret),
         MSG_IP_REINSERT_SECRET (R.string.msg_ip_reinsert_secret),
         MSG_IP_SUBKEY (R.string.msg_ip_subkey),

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

@@ -516,6 +516,8 @@
     <string name="msg_ip">Importing public keyring %s</string>
     <string name="msg_ip_insert_keyring">Inserting keyring data</string>
     <string name="msg_ip_insert_subkeys">Inserting subkeys</string>
+    <string name="msg_ip_prepare">Preparing database operations</string>
+    <string name="msg_ip_prepare_success">OK</string>
     <string name="msg_ip_preserving_secret">Preserving available secret key</string>
     <string name="msg_ip_subkey">Processing subkey %s</string>
     <string name="msg_ip_subkey_expired">Subkey expired on %s</string>
@@ -536,7 +538,7 @@
     <string name="msg_ip_trust_using">Using %s trusted keys</string>
     <string name="msg_ip_uid_cert_bad">Encountered bad certificate!</string>
     <string name="msg_ip_uid_cert_error">Error processing certificate!</string>
-    <string name="msg_ip_uid_cert_good">Found good certificate from %s</string>
+    <string name="msg_ip_uid_cert_good">Found good certificate from %2$s (%2$s)</string>
     <string name="msg_ip_uid_certs_unknown">Ignored %s certificates from unknown pubkeys</string>
     <string name="msg_ip_uid_classifying">Classifying user ids</string>
     <string name="msg_ip_uid_insert">Inserting user ids</string>