treat missing keys (from 404 errors) individually during key import

2017-05-20 21:34:52 +02:00
parent add9bf8973
commit 1331d3960c
6 changed files with 83 additions and 24 deletions
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java
@@ -40,6 +40,7 @@ import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.keyimport.FacebookKeyserver;
 import org.sufficientlysecure.keychain.keyimport.KeybaseKeyserver;
 import org.sufficientlysecure.keychain.keyimport.Keyserver;
+import org.sufficientlysecure.keychain.keyimport.Keyserver.QueryNotFoundException;
 import org.sufficientlysecure.keychain.keyimport.ParcelableHkpKeyserver;
 import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing;
 import org.sufficientlysecure.keychain.network.orbot.OrbotHelper;
@@ -152,7 +153,7 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
            return new ImportKeyResult(ImportKeyResult.RESULT_FAIL_NOTHING, log);
        }

-        int newKeys = 0, updatedKeys = 0, badKeys = 0, secret = 0;
+        int newKeys = 0, updatedKeys = 0, missingKeys = 0, badKeys = 0, secret = 0;
        ArrayList<Long> importedMasterKeyIds = new ArrayList<>();

        ArrayList<CanonicalizedKeyRing> canKeyRings = new ArrayList<>();
@@ -170,6 +171,8 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
                break;
            }

+            boolean keyWasDownloaded = false;
+
            try {

                UncachedKeyRing key = null;
@@ -178,13 +181,25 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
                if (entry.mBytes != null) {
                    key = UncachedKeyRing.decodeFromData(entry.mBytes);
                } else {
-                    key = fetchKeyFromInternet(hkpKeyserver, proxy, log, entry, key);
+                    try {
+                        key = fetchKeyFromInternet(hkpKeyserver, proxy, log, entry, key);
+                    } catch (QueryNotFoundException e) {
+                        // note that this does NOT fire on network errors! those will be logged inline and return in null
+                        log.add(LogType.MSG_IMPORT_FETCH_ERROR_NOT_FOUND, 2);
+                        missingKeys += 1;

-                    if (key.isSecret()) {
-                        log.add(LogType.MSG_IMPORT_FETCH_ERROR_KEYSERVER_SECRET, 2);
-                        badKeys += 1;
                        continue;
                    }
+
+                    if (key != null) {
+                        keyWasDownloaded = true;
+
+                        if (key.isSecret()) {
+                            log.add(LogType.MSG_IMPORT_FETCH_ERROR_KEYSERVER_SECRET, 2);
+                            badKeys += 1;
+                            continue;
+                        }
+                    }
                }

                if (key == null) {
@@ -266,6 +281,7 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>

        // special return case: no new keys at all
        if (badKeys == 0 && newKeys == 0 && updatedKeys == 0) {
+            // if keys merely aren't on keyservers, it's just a warning
            resultType = ImportKeyResult.RESULT_FAIL_NOTHING;
        } else {
            if (newKeys > 0) {
@@ -296,19 +312,28 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
            }
        }

-        ImportKeyResult result = new ImportKeyResult(resultType, log, newKeys, updatedKeys, badKeys, secret,
-                importedMasterKeyIdsArray);
+        ImportKeyResult result = new ImportKeyResult(
+                resultType, log, newKeys, updatedKeys, missingKeys, badKeys, secret, importedMasterKeyIdsArray);

        result.setCanonicalizedKeyRings(canKeyRings);
        return result;
    }

    private UncachedKeyRing fetchKeyFromInternet(ParcelableHkpKeyserver hkpKeyserver, @NonNull ParcelableProxy proxy,
-            OperationLog log, ParcelableKeyRing entry, UncachedKeyRing key) throws PgpGeneralException, IOException {
+            OperationLog log, ParcelableKeyRing entry, UncachedKeyRing key)
+            throws PgpGeneralException, IOException, QueryNotFoundException {
+        QueryNotFoundException queryNotFoundException = null;
+
        boolean canFetchFromKeyservers =
                hkpKeyserver != null && (entry.mKeyIdHex != null || entry.mExpectedFingerprint != null);
        if (canFetchFromKeyservers) {
-            UncachedKeyRing keyserverKey = fetchKeyFromKeyserver(hkpKeyserver, proxy, log, entry);
+            UncachedKeyRing keyserverKey = null;
+            try {
+                keyserverKey = fetchKeyFromKeyserver(hkpKeyserver, proxy, log, entry);
+            } catch (QueryNotFoundException e) {
+                queryNotFoundException = e;
+            }
+
            if (keyserverKey != null) {
                key = keyserverKey;
            }
@@ -329,12 +354,17 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
                key = mergeKeysOrUseEither(log, 3, key, facebookKey);
            }
        }
+
+        if (key == null && queryNotFoundException != null) {
+            throw queryNotFoundException;
+        }
+
        return key;
    }

    @Nullable
    private UncachedKeyRing fetchKeyFromKeyserver(ParcelableHkpKeyserver hkpKeyserver, @NonNull ParcelableProxy proxy,
-            OperationLog log, ParcelableKeyRing entry) throws PgpGeneralException, IOException {
+            OperationLog log, ParcelableKeyRing entry) throws PgpGeneralException, IOException, Keyserver.QueryNotFoundException {
        try {
            byte[] data;
            log.add(LogType.MSG_IMPORT_KEYSERVER, 1, hkpKeyserver);
@@ -357,6 +387,8 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
            }

            return keyserverKey;
+        } catch (Keyserver.QueryNotFoundException e) {
+            throw e;
        } catch (Keyserver.QueryFailedException e) {
            Log.d(Constants.TAG, "query failed", e);
            log.add(LogType.MSG_IMPORT_FETCH_ERROR_KEYSERVER, 3, e.getMessage());
@@ -473,7 +505,6 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
    private ImportKeyResult multiThreadedKeyImport(ArrayList<ParcelableKeyRing> keyList,
                                                   final ParcelableHkpKeyserver keyServer, final ParcelableProxy proxy,
                                                   final boolean skipSave) {
-
        Log.d(Constants.TAG, "Multi-threaded key import starting");

        final Iterator<ParcelableKeyRing> keyListIterator = keyList.iterator();
@@ -540,6 +571,7 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
        private int mBadKeys = 0;
        private int mNewKeys = 0;
        private int mUpdatedKeys = 0;
+        private int mMissingKeys = 0;
        private int mSecret = 0;
        private int mResultType = 0;
        private boolean mHasCancelledResult;
@@ -586,6 +618,7 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
            mBadKeys += result.mBadKeys;
            mNewKeys += result.mNewKeys;
            mUpdatedKeys += result.mUpdatedKeys;
+            mMissingKeys += result.mMissingKeys;
            mSecret += result.mSecret;

            long[] masterKeyIds = result.getImportedMasterKeyIds();
@@ -635,7 +668,7 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
            }

            ImportKeyResult result = new ImportKeyResult(mResultType, mImportLog, mNewKeys,
-                    mUpdatedKeys, mBadKeys, mSecret, masterKeyIds);
+                    mUpdatedKeys, mMissingKeys, mBadKeys, mSecret, masterKeyIds);

            result.setCanonicalizedKeyRings(mCanonicalizedKeyRings);
            return result;
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/ImportKeyResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/ImportKeyResult.java
@@ -18,6 +18,9 @@

 package org.sufficientlysecure.keychain.operations.results;

+
+import java.util.ArrayList;
+
 import android.app.Activity;
 import android.content.Intent;
 import android.os.Parcel;
@@ -33,11 +36,9 @@ import org.sufficientlysecure.keychain.ui.util.Notify.ActionListener;
 import org.sufficientlysecure.keychain.ui.util.Notify.Showable;
 import org.sufficientlysecure.keychain.ui.util.Notify.Style;

-import java.util.ArrayList;
-
 public class ImportKeyResult extends InputPendingResult {

-    public final int mNewKeys, mUpdatedKeys, mBadKeys, mSecret;
+    public final int mNewKeys, mUpdatedKeys, mMissingKeys, mBadKeys, mSecret;
    public final long[] mImportedMasterKeyIds;

    // NOT PARCELED
@@ -74,6 +75,10 @@ public class ImportKeyResult extends InputPendingResult {
        return (mResult & RESULT_FAIL_NOTHING) == RESULT_FAIL_NOTHING;
    }

+    public boolean isFailMissing() {
+        return isFailNothing() && mMissingKeys > 0;
+    }
+
    public long[] getImportedMasterKeyIds() {
        return mImportedMasterKeyIds;
    }
@@ -82,21 +87,23 @@ public class ImportKeyResult extends InputPendingResult {
        super(source);
        mNewKeys = source.readInt();
        mUpdatedKeys = source.readInt();
+        mMissingKeys = source.readInt();
        mBadKeys = source.readInt();
        mSecret = source.readInt();
        mImportedMasterKeyIds = source.createLongArray();
    }

    public ImportKeyResult(int result, OperationLog log) {
-        this(result, log, 0, 0, 0, 0, new long[]{});
+        this(result, log, 0, 0, 0, 0, 0, new long[]{});
    }

    public ImportKeyResult(int result, OperationLog log,
-                           int newKeys, int updatedKeys, int badKeys, int secret,
+                           int newKeys, int updatedKeys, int missingKeys, int badKeys, int secret,
                           long[] importedMasterKeyIds) {
        super(result, log);
        mNewKeys = newKeys;
        mUpdatedKeys = updatedKeys;
+        mMissingKeys = missingKeys;
        mBadKeys = badKeys;
        mSecret = secret;
        mImportedMasterKeyIds = importedMasterKeyIds;
@@ -108,6 +115,7 @@ public class ImportKeyResult extends InputPendingResult {
        // just assign default values, we won't use them anyway
        mNewKeys = 0;
        mUpdatedKeys = 0;
+        mMissingKeys = 0;
        mBadKeys = 0;
        mSecret = 0;
        mImportedMasterKeyIds = new long[]{};
@@ -122,6 +130,7 @@ public class ImportKeyResult extends InputPendingResult {
        super.writeToParcel(dest, flags);
        dest.writeInt(mNewKeys);
        dest.writeInt(mUpdatedKeys);
+        dest.writeInt(mMissingKeys);
        dest.writeInt(mBadKeys);
        dest.writeInt(mSecret);
        dest.writeLongArray(mImportedMasterKeyIds);
@@ -190,17 +199,20 @@ public class ImportKeyResult extends InputPendingResult {
            }

        } else {
-            duration = 0;
-            style = Style.ERROR;
-            if (isFailNothing()) {
+            if (isFailMissing()) {
+                duration = 0;
+                style = Style.WARN;
+                str = activity.getResources().getString(R.string.import_warn_missing);
+            } else if (isFailNothing()) {
+                duration = 0;
+                style = Style.ERROR;
                str = activity.getString((resultType & ImportKeyResult.RESULT_CANCELLED) > 0
                        ? R.string.import_error_nothing_cancelled
                        : R.string.import_error_nothing);
            } else {
-                str = activity.getResources().getQuantityString(
-                        R.plurals.import_error,
-                        mBadKeys,
-                        mBadKeys);
+                duration = 0;
+                style = Style.ERROR;
+                str = activity.getResources().getQuantityString(R.plurals.import_error, mBadKeys, mBadKeys);
            }
        }

--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
@@ -769,6 +769,7 @@ public abstract class OperationResult implements Parcelable {

        MSG_IMPORT_FETCH_ERROR (LogLevel.ERROR, R.string.msg_import_fetch_error),
        MSG_IMPORT_FETCH_ERROR_DECODE (LogLevel.ERROR, R.string.msg_import_fetch_error_decode),
+        MSG_IMPORT_FETCH_ERROR_NOT_FOUND (LogLevel.ERROR, R.string.msg_import_fetch_error_not_found),
        MSG_IMPORT_FETCH_ERROR_KEYSERVER(LogLevel.ERROR, R.string.msg_import_fetch_error_keyserver),
        MSG_IMPORT_FETCH_ERROR_KEYSERVER_SECRET (LogLevel.ERROR, R.string.msg_import_fetch_error_keyserver_secret),
        MSG_IMPORT_FETCH_KEYBASE (LogLevel.INFO, R.string.msg_import_fetch_keybase),