From 1331d3960c1dd9335e1fd29c8d76f41ae9796156 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sat, 20 May 2017 21:34:52 +0200 Subject: [PATCH] treat missing keys (from 404 errors) individually during key import --- .../keychain/keyimport/Keyserver.java | 8 +++ .../keyimport/ParcelableHkpKeyserver.java | 3 + .../keychain/operations/ImportOperation.java | 57 +++++++++++++++---- .../operations/results/ImportKeyResult.java | 36 ++++++++---- .../operations/results/OperationResult.java | 1 + OpenKeychain/src/main/res/values/strings.xml | 2 + 6 files changed, 83 insertions(+), 24 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java index f5970ef8a..85e250a6d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java @@ -47,6 +47,14 @@ public abstract class Keyserver { } } + public static class QueryNotFoundException extends QueryFailedException { + private static final long serialVersionUID = 2693768928624654513L; + + public QueryNotFoundException(String message) { + super(message); + } + } + public static class QueryNeedsRepairException extends CloudSearchFailureException { private static final long serialVersionUID = 2693768928624654512L; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableHkpKeyserver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableHkpKeyserver.java index 8d2aa054e..9db602d6a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableHkpKeyserver.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ParcelableHkpKeyserver.java @@ -402,6 +402,9 @@ public class ParcelableHkpKeyserver extends Keyserver implements Parcelable { throw new Keyserver.QueryFailedException("Unsupported keyserver URI"); } catch (HttpError httpError) { Log.d(Constants.TAG, "Failed to get key at HkpKeyserver", httpError); + if (httpError.getCode() == 404) { + throw new Keyserver.QueryNotFoundException("not found"); + } throw new Keyserver.QueryFailedException("not found"); } if (data == null) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportOperation.java index d08b22a24..25fb4cdbb 100644 --- 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 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 importedMasterKeyIds = new ArrayList<>(); ArrayList canKeyRings = new ArrayList<>(); @@ -170,6 +171,8 @@ public class ImportOperation extends BaseReadWriteOperation break; } + boolean keyWasDownloaded = false; + try { UncachedKeyRing key = null; @@ -178,13 +181,25 @@ public class ImportOperation extends BaseReadWriteOperation 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 // 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 } } - 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 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 } 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 private ImportKeyResult multiThreadedKeyImport(ArrayList keyList, final ParcelableHkpKeyserver keyServer, final ParcelableProxy proxy, final boolean skipSave) { - Log.d(Constants.TAG, "Multi-threaded key import starting"); final Iterator keyListIterator = keyList.iterator(); @@ -540,6 +571,7 @@ public class ImportOperation extends BaseReadWriteOperation 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 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 } ImportKeyResult result = new ImportKeyResult(mResultType, mImportLog, mNewKeys, - mUpdatedKeys, mBadKeys, mSecret, masterKeyIds); + mUpdatedKeys, mMissingKeys, mBadKeys, mSecret, masterKeyIds); result.setCanonicalizedKeyRings(mCanonicalizedKeyRings); return result; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/ImportKeyResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/ImportKeyResult.java index 71072f029..b00841789 100644 --- 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); } } 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 e5840d9f5..a20b5e492 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 @@ -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), diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 0362b5121..27f131f4f 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -585,6 +585,7 @@ "Import failed!" "Import of %d keys failed!" + "Key not found on keyservers." "Nothing to import." "Import cancelled." @@ -1337,6 +1338,7 @@ "Error decoding retrieved keyring!" "Key could not be retrieved! (Network problems?)" + "Key not found!" "Could not retrieve key from keyservers: %s" "Cannot import secret key from keyserver!" "Retrieving from keybase.io: %s"