From a1330f78e499840b386ea92e4280e9cdff9407f6 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 29 Oct 2015 14:50:29 +0100 Subject: uploadop: clean up, add logging --- .../keychain/keyimport/HkpKeyserver.java | 2 +- .../keychain/operations/BackupOperation.java | 4 +- .../keychain/operations/CertifyOperation.java | 2 +- .../keychain/operations/EditKeyOperation.java | 2 +- .../keychain/operations/UploadOperation.java | 141 +++++++++++++-------- .../operations/results/InputPendingResult.java | 7 +- .../operations/results/OperationResult.java | 15 ++- .../keychain/operations/results/UploadResult.java | 3 +- .../keychain/service/input/CryptoInputParcel.java | 1 + .../keychain/util/ParcelableProxy.java | 2 + .../keychain/util/Preferences.java | 7 + 11 files changed, 120 insertions(+), 66 deletions(-) (limited to 'OpenKeychain/src/main/java/org') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java index 7473705f3..3b52a0306 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java @@ -434,7 +434,7 @@ public class HkpKeyserver extends Keyserver { @Override public String toString() { - return mHost + ":" + mPort; + return getUrlPrefix() + mHost + ":" + mPort; } /** diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java index 206212387..ae9a2c180 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java @@ -252,7 +252,7 @@ public class BackupOperation extends BaseOperation { ring.encode(arOutStream); } catch (PgpGeneralException e) { - log.add(LogType.MSG_BACKUP_ERROR_KEY, 2); + log.add(LogType.MSG_UPLOAD_ERROR_IO, 2); } finally { if (arOutStream != null) { arOutStream.close(); @@ -273,7 +273,7 @@ public class BackupOperation extends BaseOperation { ring.encode(arOutStream); } catch (PgpGeneralException e) { - log.add(LogType.MSG_BACKUP_ERROR_KEY, 2); + log.add(LogType.MSG_UPLOAD_ERROR_IO, 2); } finally { if (arOutStream != null) { arOutStream.close(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java index e1daac874..7d11fa1f1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/CertifyOperation.java @@ -208,7 +208,7 @@ public class CertifyOperation extends BaseOperation { // these variables are used inside the following loop, but they need to be created only once UploadOperation uploadOperation = null; if (parcel.keyServerUri != null) { - uploadOperation = new UploadOperation(mContext, mProviderHelper, mProgressable); + uploadOperation = new UploadOperation(mContext, mProviderHelper, mProgressable, mCancelled); } // Write all certified keys into the database diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java index cf8928768..93ee76f3f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/EditKeyOperation.java @@ -148,7 +148,7 @@ public class EditKeyOperation extends BaseOperation { new UploadKeyringParcel(saveParcel.getUploadKeyserver(), keyringBytes); UploadResult uploadResult = - new UploadOperation(mContext, mProviderHelper, mProgressable) + new UploadOperation(mContext, mProviderHelper, mProgressable, mCancelled) .execute(exportKeyringParcel, cryptoInput); if (uploadResult.isPending()) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/UploadOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/UploadOperation.java index bac9a7975..d4bbaa9fd 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/UploadOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/UploadOperation.java @@ -27,6 +27,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import android.content.Context; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import org.spongycastle.bcpg.ArmoredOutputStream; import org.sufficientlysecure.keychain.Constants; @@ -47,26 +48,17 @@ import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel; import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; import org.sufficientlysecure.keychain.util.Log; +import org.sufficientlysecure.keychain.util.ParcelableProxy; import org.sufficientlysecure.keychain.util.Preferences; +import org.sufficientlysecure.keychain.util.Preferences.ProxyPrefs; import org.sufficientlysecure.keychain.util.orbot.OrbotHelper; /** - * An operation class which implements high level export operations. - * This class receives a source and/or destination of keys as input and performs - * all steps for this export. - * - * @see org.sufficientlysecure.keychain.ui.adapter.ImportKeysAdapter#getSelectedEntries() - * For the export operation, the input consists of a set of key ids and - * either the name of a file or an output uri to write to. + * An operation class which implements the upload of a single key to a key server. */ public class UploadOperation extends BaseOperation { - public UploadOperation(Context context, ProviderHelper providerHelper, Progressable - progressable) { - super(context, providerHelper, progressable); - } - public UploadOperation(Context context, ProviderHelper providerHelper, Progressable progressable, AtomicBoolean cancelled) { super(context, providerHelper, progressable, cancelled); @@ -74,57 +66,99 @@ public class UploadOperation extends BaseOperation { @NonNull public UploadResult execute(UploadKeyringParcel uploadInput, CryptoInputParcel cryptoInput) { + OperationLog log = new OperationLog(); + + log.add(LogType.MSG_UPLOAD, 0); + updateProgress(R.string.progress_uploading, 0, 1); + Proxy proxy; - if (cryptoInput.getParcelableProxy() == null) { - // explicit proxy not set - if (!OrbotHelper.isOrbotInRequiredState(mContext)) { - return new UploadResult(null, RequiredInputParcel.createOrbotRequiredOperation(), cryptoInput); + { + boolean proxyIsTor = false; + + // Proxy priorities: + // 1. explicit proxy + // 2. orbot proxy state + // 3. proxy from preferences + ParcelableProxy parcelableProxy = cryptoInput.getParcelableProxy(); + if (parcelableProxy != null) { + proxy = parcelableProxy.getProxy(); + } else { + if ( ! OrbotHelper.isOrbotInRequiredState(mContext)) { + return new UploadResult(log, RequiredInputParcel.createOrbotRequiredOperation(), cryptoInput); + } + ProxyPrefs proxyPrefs = Preferences.getPreferences(mContext).getProxyPrefs(); + if (proxyPrefs.torEnabled) { + proxyIsTor = true; + } + proxy = proxyPrefs.getProxy(); } - proxy = Preferences.getPreferences(mContext).getProxyPrefs().parcelableProxy.getProxy(); - } else { - proxy = cryptoInput.getParcelableProxy().getProxy(); + + if (proxyIsTor) { + log.add(LogType.MSG_UPLOAD_PROXY_TOR, 1); + } else if (proxy != null && proxy != Proxy.NO_PROXY) { + log.add(LogType.MSG_UPLOAD_PROXY, 1, proxy.toString()); + } else { + log.add(LogType.MSG_UPLOAD_PROXY_DIRECT, 1); + } + + } + + HkpKeyserver hkpKeyserver; + { + hkpKeyserver = new HkpKeyserver(uploadInput.mKeyserver); + log.add(LogType.MSG_UPLOAD_SERVER, 1, hkpKeyserver.toString()); + } + + CanonicalizedPublicKeyRing keyring = getPublicKeyringFromInput(log, uploadInput); + if (keyring == null) { + return new UploadResult(UploadResult.RESULT_ERROR, log); + } + + return uploadKeyRingToServer(log, hkpKeyserver, keyring, proxy); + } + + @Nullable + private CanonicalizedPublicKeyRing getPublicKeyringFromInput(OperationLog log, UploadKeyringParcel uploadInput) { + + boolean hasMasterKeyId = uploadInput.mMasterKeyId != null; + boolean hasKeyringBytes = uploadInput.mUncachedKeyringBytes != null; + if (hasMasterKeyId == hasKeyringBytes) { + throw new IllegalArgumentException("either keyid xor bytes must be non-null for this method call!"); } - HkpKeyserver hkpKeyserver = new HkpKeyserver(uploadInput.mKeyserver); try { - CanonicalizedPublicKeyRing keyring; - if (uploadInput.mMasterKeyId != null) { - keyring = mProviderHelper.getCanonicalizedPublicKeyRing( - uploadInput.mMasterKeyId); - } else if (uploadInput.mUncachedKeyringBytes != null) { - CanonicalizedKeyRing canonicalizedRing = - UncachedKeyRing.decodeFromData(uploadInput.mUncachedKeyringBytes) - .canonicalize(new OperationLog(), 0, true); - if ( ! CanonicalizedPublicKeyRing.class.isInstance(canonicalizedRing)) { - throw new AssertionError("keyring bytes must contain public key ring!"); - } - keyring = (CanonicalizedPublicKeyRing) canonicalizedRing; - } else { - throw new AssertionError("key id or bytes must be non-null!"); + + if (hasMasterKeyId) { + log.add(LogType.MSG_UPLOAD_KEY, 0, KeyFormattingUtils.convertKeyIdToHex(uploadInput.mMasterKeyId)); + return mProviderHelper.getCanonicalizedPublicKeyRing(uploadInput.mMasterKeyId); } - return uploadKeyRingToServer(hkpKeyserver, keyring, proxy); + + CanonicalizedKeyRing canonicalizedRing = + UncachedKeyRing.decodeFromData(uploadInput.mUncachedKeyringBytes) + .canonicalize(new OperationLog(), 0, true); + if ( ! CanonicalizedPublicKeyRing.class.isInstance(canonicalizedRing)) { + throw new IllegalArgumentException("keyring bytes must contain public key ring!"); + } + log.add(LogType.MSG_UPLOAD_KEY, 0, KeyFormattingUtils.convertKeyIdToHex(canonicalizedRing.getMasterKeyId())); + return (CanonicalizedPublicKeyRing) canonicalizedRing; + } catch (ProviderHelper.NotFoundException e) { + log.add(LogType.MSG_UPLOAD_ERROR_NOT_FOUND, 1); + return null; + } catch (IOException | PgpGeneralException e) { + log.add(LogType.MSG_UPLOAD_ERROR_IO, 1); Log.e(Constants.TAG, "error uploading key", e); - return new UploadResult(UploadResult.RESULT_ERROR, new OperationLog()); - } catch (IOException e) { - e.printStackTrace(); - return new UploadResult(UploadResult.RESULT_ERROR, new OperationLog()); - } catch (PgpGeneralException e) { - e.printStackTrace(); - return new UploadResult(UploadResult.RESULT_ERROR, new OperationLog()); + return null; } - } - UploadResult uploadKeyRingToServer(HkpKeyserver server, CanonicalizedPublicKeyRing keyring, Proxy proxy) { + } - mProgressable.setProgress(R.string.progress_uploading, 0, 1); + @NonNull + private UploadResult uploadKeyRingToServer( + OperationLog log, HkpKeyserver server, CanonicalizedPublicKeyRing keyring, Proxy proxy) { ByteArrayOutputStream bos = new ByteArrayOutputStream(); ArmoredOutputStream aos = null; - OperationLog log = new OperationLog(); - log.add(LogType.MSG_BACKUP_UPLOAD_PUBLIC, 0, KeyFormattingUtils.convertKeyIdToHex( - keyring.getPublicKey().getKeyId() - )); try { aos = new ArmoredOutputStream(bos); @@ -134,20 +168,21 @@ public class UploadOperation extends BaseOperation { String armoredKey = bos.toString("UTF-8"); server.add(armoredKey, proxy); - log.add(LogType.MSG_BACKUP_UPLOAD_SUCCESS, 1); + updateProgress(R.string.progress_uploading, 1, 1); + + log.add(LogType.MSG_UPLOAD_SUCCESS, 1); return new UploadResult(UploadResult.RESULT_OK, log); } catch (IOException e) { Log.e(Constants.TAG, "IOException", e); - log.add(LogType.MSG_BACKUP_ERROR_KEY, 1); + log.add(LogType.MSG_UPLOAD_ERROR_IO, 1); return new UploadResult(UploadResult.RESULT_ERROR, log); } catch (AddKeyException e) { Log.e(Constants.TAG, "AddKeyException", e); - log.add(LogType.MSG_BACKUP_ERROR_UPLOAD, 1); + log.add(LogType.MSG_UPLOAD_ERROR_UPLOAD, 1); return new UploadResult(UploadResult.RESULT_ERROR, log); } finally { - mProgressable.setProgress(R.string.progress_uploading, 1, 1); try { if (aos != null) { aos.close(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/InputPendingResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/InputPendingResult.java index 0a8c1f653..ed6674ef7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/InputPendingResult.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/InputPendingResult.java @@ -19,6 +19,7 @@ package org.sufficientlysecure.keychain.operations.results; import android.os.Parcel; +import android.support.annotation.NonNull; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel; @@ -32,13 +33,13 @@ public class InputPendingResult extends OperationResult { // in case operation needs to add to/changes the cryptoInputParcel sent to it public final CryptoInputParcel mCryptoInputParcel; - public InputPendingResult(int result, OperationLog log) { + public InputPendingResult(int result, @NonNull OperationLog log) { super(result, log); mRequiredInput = null; mCryptoInputParcel = null; } - public InputPendingResult(OperationLog log, InputPendingResult result) { + public InputPendingResult(@NonNull OperationLog log, @NonNull InputPendingResult result) { super(RESULT_PENDING, log); if (!result.isPending()) { throw new AssertionError("sub result must be pending!"); @@ -47,7 +48,7 @@ public class InputPendingResult extends OperationResult { mCryptoInputParcel = result.mCryptoInputParcel; } - public InputPendingResult(OperationLog log, RequiredInputParcel requiredInput, + public InputPendingResult(@NonNull OperationLog log, RequiredInputParcel requiredInput, CryptoInputParcel cryptoInputParcel) { super(RESULT_PENDING, log); mRequiredInput = requiredInput; 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 0b8c3e6c7..5b6b719ae 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 @@ -766,17 +766,24 @@ public abstract class OperationResult implements Parcelable { MSG_IMPORT_SUCCESS (LogLevel.OK, R.string.msg_import_success), MSG_BACKUP(LogLevel.START, R.plurals.msg_backup), - MSG_BACKUP_UPLOAD_PUBLIC(LogLevel.START, R.string.msg_backup_upload_public), MSG_BACKUP_PUBLIC(LogLevel.DEBUG, R.string.msg_backup_public), MSG_BACKUP_SECRET(LogLevel.DEBUG, R.string.msg_backup_secret), MSG_BACKUP_ALL(LogLevel.START, R.string.msg_backup_all), MSG_BACKUP_ERROR_URI_OPEN(LogLevel.ERROR, R.string.msg_backup_error_uri_open), MSG_BACKUP_ERROR_DB(LogLevel.ERROR, R.string.msg_backup_error_db), MSG_BACKUP_ERROR_IO(LogLevel.ERROR, R.string.msg_backup_error_io), - MSG_BACKUP_ERROR_KEY(LogLevel.ERROR, R.string.msg_backup_error_key), - MSG_BACKUP_ERROR_UPLOAD(LogLevel.ERROR, R.string.msg_backup_error_upload), MSG_BACKUP_SUCCESS(LogLevel.OK, R.string.msg_backup_success), - MSG_BACKUP_UPLOAD_SUCCESS(LogLevel.OK, R.string.msg_backup_upload_success), + + MSG_UPLOAD(LogLevel.START, R.string.msg_upload), + MSG_UPLOAD_KEY(LogLevel.INFO, R.string.msg_upload_key), + MSG_UPLOAD_PROXY_DIRECT(LogLevel.DEBUG, R.string.msg_upload_proxy_direct), + MSG_UPLOAD_PROXY_TOR(LogLevel.DEBUG, R.string.msg_upload_proxy_tor), + MSG_UPLOAD_PROXY(LogLevel.DEBUG, R.string.msg_upload_proxy), + MSG_UPLOAD_SERVER(LogLevel.DEBUG, R.string.msg_upload_server), + MSG_UPLOAD_SUCCESS(LogLevel.OK, R.string.msg_upload_success), + MSG_UPLOAD_ERROR_NOT_FOUND(LogLevel.ERROR, R.string.msg_upload_error_not_found), + MSG_UPLOAD_ERROR_IO(LogLevel.ERROR, R.string.msg_upload_error_key), + MSG_UPLOAD_ERROR_UPLOAD(LogLevel.ERROR, R.string.msg_upload_error_upload), MSG_CRT_UPLOAD_SUCCESS (LogLevel.OK, R.string.msg_crt_upload_success), diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/UploadResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/UploadResult.java index a88072de3..ea2b373a9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/UploadResult.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/UploadResult.java @@ -18,6 +18,7 @@ package org.sufficientlysecure.keychain.operations.results; import android.os.Parcel; +import android.support.annotation.NonNull; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel; @@ -38,7 +39,7 @@ public class UploadResult extends InputPendingResult { } - public UploadResult(OperationLog log, RequiredInputParcel requiredInputParcel, + public UploadResult(@NonNull OperationLog log, RequiredInputParcel requiredInputParcel, CryptoInputParcel cryptoInputParcel) { super(log, requiredInputParcel, cryptoInputParcel); // we won't use these values diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java index 0d8569fe6..849418905 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/input/CryptoInputParcel.java @@ -23,6 +23,7 @@ import android.os.Parcelable; import org.sufficientlysecure.keychain.util.ParcelableProxy; import org.sufficientlysecure.keychain.util.Passphrase; +import java.net.Proxy; import java.nio.ByteBuffer; import java.util.Date; import java.util.HashMap; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableProxy.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableProxy.java index 7e788d04c..ade75f926 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableProxy.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/ParcelableProxy.java @@ -19,6 +19,7 @@ package org.sufficientlysecure.keychain.util; import android.os.Parcel; import android.os.Parcelable; +import android.support.annotation.Nullable; import java.net.InetSocketAddress; import java.net.Proxy; @@ -47,6 +48,7 @@ public class ParcelableProxy implements Parcelable { return new ParcelableProxy(null, -1, null); } + @Nullable public Proxy getProxy() { if (mProxyHost == null) { return null; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/Preferences.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/Preferences.java index 8b2c3c66a..32d98826d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/Preferences.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/Preferences.java @@ -23,6 +23,8 @@ import android.content.SharedPreferences; import android.content.res.Resources; import android.preference.PreferenceManager; +import android.support.annotation.Nullable; + import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.Constants.Pref; import org.sufficientlysecure.keychain.service.KeyserverSyncAdapterService; @@ -322,6 +324,11 @@ public class Preferences { if (!torEnabled && !normalPorxyEnabled) this.parcelableProxy = new ParcelableProxy(null, -1, null); else this.parcelableProxy = new ParcelableProxy(hostName, port, type); } + + public Proxy getProxy() { + return parcelableProxy.getProxy(); + } + } // cloud prefs -- cgit v1.2.3