From 5d5d2c3c36c77b22a90acae33adb6908c4d7b5d9 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 17 May 2016 21:13:41 +0200 Subject: refactor BackupOperation a bit, make more illegal states explicit --- .../keychain/operations/BackupOperation.java | 115 +++++++++++---------- .../keychain/util/CountingOutputStream.java | 65 ++++++++++++ .../keychain/operations/ExportTest.java | 7 +- 3 files changed, 126 insertions(+), 61 deletions(-) create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/CountingOutputStream.java 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 21c519925..d46d8a25f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java @@ -20,7 +20,6 @@ package org.sufficientlysecure.keychain.operations; import java.io.BufferedOutputStream; -import java.io.DataOutputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -59,6 +58,7 @@ import org.sufficientlysecure.keychain.provider.TemporaryFileProvider; import org.sufficientlysecure.keychain.service.BackupKeyringParcel; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; +import org.sufficientlysecure.keychain.util.CountingOutputStream; import org.sufficientlysecure.keychain.util.InputData; import org.sufficientlysecure.keychain.util.Log; @@ -112,74 +112,43 @@ public class BackupOperation extends BaseOperation { log.add(LogType.MSG_BACKUP_ALL, 0); } - if (backupInput.mIsEncrypted && cryptoInput == null) { - throw new IllegalStateException("Encrypted backup must supply cryptoInput parameter"); - } - try { Uri plainUri = null; OutputStream plainOut; if (backupInput.mIsEncrypted) { + if (cryptoInput == null) { + throw new IllegalStateException("Encrypted backup must supply cryptoInput parameter"); + } + plainUri = TemporaryFileProvider.createFile(mContext); plainOut = mContext.getContentResolver().openOutputStream(plainUri); } else { - if (backupInput.mOutputUri == null) { + if (backupInput.mOutputUri == null || outputStream != null) { throw new IllegalArgumentException("Unencrypted export to output stream is not supported!"); } else { plainOut = mContext.getContentResolver().openOutputStream(backupInput.mOutputUri); } } - int exportedDataSize; - { // export key data, and possibly return if we don't encrypt - DataOutputStream outStream = new DataOutputStream(new BufferedOutputStream(plainOut)); - - boolean backupSuccess = exportKeysToStream( - log, backupInput.mMasterKeyIds, backupInput.mExportSecret, outStream); + CountingOutputStream outStream = new CountingOutputStream(new BufferedOutputStream(plainOut)); + boolean backupSuccess = exportKeysToStream( + log, backupInput.mMasterKeyIds, backupInput.mExportSecret, outStream); - exportedDataSize = outStream.size(); - - if (!backupSuccess) { - // if there was an error, it will be in the log so we just have to return - return new ExportResult(ExportResult.RESULT_ERROR, log); - } - - if (nonEncryptedOutput) { - // log.add(LogType.MSG_EXPORT_NO_ENCRYPT, 1); - log.add(LogType.MSG_BACKUP_SUCCESS, 1); - return new ExportResult(ExportResult.RESULT_OK, log); - } + if (!backupSuccess) { + // if there was an error, it will be in the log so we just have to return + return new ExportResult(ExportResult.RESULT_ERROR, log); } - PgpSignEncryptOperation pseOp = new PgpSignEncryptOperation(mContext, mProviderHelper, mProgressable, mCancelled); - - PgpSignEncryptData data = new PgpSignEncryptData(); - data.setSymmetricPassphrase(cryptoInput.getPassphrase()); - data.setEnableAsciiArmorOutput(true); - data.setAddBackupHeader(true); - PgpSignEncryptInputParcel inputParcel = new PgpSignEncryptInputParcel(data); - - InputStream inStream = mContext.getContentResolver().openInputStream(plainUri); - - String filename; - if (backupInput.mMasterKeyIds != null && backupInput.mMasterKeyIds.length == 1) { - filename = Constants.FILE_BACKUP_PREFIX + KeyFormattingUtils.convertKeyIdToHex(backupInput.mMasterKeyIds[0]); - } else { - filename = Constants.FILE_BACKUP_PREFIX + new SimpleDateFormat("yyyy-MM-dd", Locale.getDefault()).format(new Date()); + if (!backupInput.mIsEncrypted) { + // log.add(LogType.MSG_EXPORT_NO_ENCRYPT, 1); + log.add(LogType.MSG_BACKUP_SUCCESS, 1); + return new ExportResult(ExportResult.RESULT_OK, log); } - filename += backupInput.mExportSecret ? Constants.FILE_EXTENSION_BACKUP_SECRET : Constants.FILE_EXTENSION_BACKUP_PUBLIC; - - InputData inputData = new InputData(inStream, exportedDataSize, filename); - OutputStream outStream; - if (backupInput.mOutputUri == null) { - outStream = outputStream; - } else { - outStream = mContext.getContentResolver().openOutputStream(backupInput.mOutputUri); - } - outStream = new BufferedOutputStream(outStream); + long exportedDataSize = outStream.getCount(); + PgpSignEncryptResult encryptResult = + encryptBackupData(backupInput, cryptoInput, outputStream, plainUri, exportedDataSize); - PgpSignEncryptResult encryptResult = pseOp.execute(inputParcel, new CryptoInputParcel(), inputData, outStream); if (!encryptResult.success()) { log.addByMerge(encryptResult, 1); // log.add(LogType.MSG_EXPORT_ERROR_ENCRYPT, 1); @@ -193,13 +162,52 @@ public class BackupOperation extends BaseOperation { } catch (FileNotFoundException e) { log.add(LogType.MSG_BACKUP_ERROR_URI_OPEN, 1); return new ExportResult(ExportResult.RESULT_ERROR, log); + } + + } + @NonNull + private PgpSignEncryptResult encryptBackupData(@NonNull BackupKeyringParcel backupInput, + @NonNull CryptoInputParcel cryptoInput, @Nullable OutputStream outputStream, Uri plainUri, long exportedDataSize) + throws FileNotFoundException { + PgpSignEncryptOperation signEncryptOperation = new PgpSignEncryptOperation(mContext, mProviderHelper, mProgressable, mCancelled); + + PgpSignEncryptData data = new PgpSignEncryptData(); + data.setSymmetricPassphrase(cryptoInput.getPassphrase()); + data.setEnableAsciiArmorOutput(true); + data.setAddBackupHeader(true); + PgpSignEncryptInputParcel inputParcel = new PgpSignEncryptInputParcel(data); + + InputStream inStream = mContext.getContentResolver().openInputStream(plainUri); + + String filename; + if (backupInput.mMasterKeyIds != null && backupInput.mMasterKeyIds.length == 1) { + filename = Constants.FILE_BACKUP_PREFIX + KeyFormattingUtils.convertKeyIdToHex(backupInput.mMasterKeyIds[0]); + } else { + filename = Constants.FILE_BACKUP_PREFIX + new SimpleDateFormat("yyyy-MM-dd", Locale + .getDefault()).format(new Date()); } + filename += backupInput.mExportSecret ? Constants.FILE_EXTENSION_BACKUP_SECRET : Constants.FILE_EXTENSION_BACKUP_PUBLIC; + InputData inputData = new InputData(inStream, exportedDataSize, filename); + + OutputStream outStream; + if (backupInput.mOutputUri == null) { + if (outputStream == null) { + throw new IllegalArgumentException("If output uri is not set, outputStream must not be null!"); + } + outStream = outputStream; + } else { + if (outputStream != null) { + throw new IllegalArgumentException("If output uri is set, outputStream must null!"); + } + outStream = mContext.getContentResolver().openOutputStream(backupInput.mOutputUri); + } + + return signEncryptOperation.execute(inputParcel, new CryptoInputParcel(), inputData, outStream); } boolean exportKeysToStream(OperationLog log, long[] masterKeyIds, boolean exportSecret, OutputStream outStream) { - // noinspection unused TODO use these in a log entry int okSecret = 0, okPublic = 0; @@ -257,12 +265,10 @@ public class BackupOperation extends BaseOperation { } return true; - } private boolean writePublicKeyToStream(OperationLog log, OutputStream outStream, Cursor cursor) throws IOException { - ArmoredOutputStream arOutStream = null; try { @@ -283,7 +289,6 @@ public class BackupOperation extends BaseOperation { private boolean writeSecretKeyToStream(OperationLog log, OutputStream outStream, Cursor cursor) throws IOException { - ArmoredOutputStream arOutStream = null; try { @@ -303,7 +308,6 @@ public class BackupOperation extends BaseOperation { } private Cursor queryForKeys(long[] masterKeyIds) { - String selection = null, selectionArgs[] = null; if (masterKeyIds != null) { @@ -326,7 +330,6 @@ public class BackupOperation extends BaseOperation { KeyRings.buildUnifiedKeyRingsUri(), PROJECTION, selection, selectionArgs, Tables.KEYS + "." + KeyRings.MASTER_KEY_ID ); - } } \ No newline at end of file diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/CountingOutputStream.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/CountingOutputStream.java new file mode 100644 index 000000000..8bfe26aeb --- /dev/null +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/CountingOutputStream.java @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2007 The Guava Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.sufficientlysecure.keychain.util; + + +import java.io.FilterOutputStream; +import java.io.IOException; +import java.io.OutputStream; + +import android.support.annotation.NonNull; + + +public final class CountingOutputStream extends FilterOutputStream { + + private long count; + + /** + * Wraps another output stream, counting the number of bytes written. + * + * @param out + * the output stream to be wrapped + */ + public CountingOutputStream(@NonNull OutputStream out) { + super(out); + } + + /** + * Returns the number of bytes written. + */ + public long getCount() { + return count; + } + + @Override + public void write(@NonNull byte[] b, int off, int len) throws IOException { + out.write(b, off, len); + count += len; + } + + @Override + public void write(int b) throws IOException { + out.write(b); + count++; + } + + // Overriding close() because FilterOutputStream's close() method pre-JDK8 has bad behavior: + // it silently ignores any exception thrown by flush(). Instead, just close the delegate stream. + // It should flush itself if necessary. + @Override + public void close() throws IOException { + out.close(); + } +} \ No newline at end of file diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java index 6fab979ed..142d2f594 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java @@ -242,7 +242,6 @@ public class ExportTest { @Test public void testExportUnencrypted() throws Exception { - ContentResolver mockResolver = mock(ContentResolver.class); Uri fakeOutputUri = Uri.parse("content://fake/out/1"); @@ -256,7 +255,7 @@ public class ExportTest { new ProviderHelper(RuntimeEnvironment.application), null); BackupKeyringParcel parcel = new BackupKeyringParcel( - new long[] { mStaticRing1.getMasterKeyId() }, false, fakeOutputUri); + new long[] { mStaticRing1.getMasterKeyId() }, false, false, fakeOutputUri); ExportResult result = op.execute(parcel, null); @@ -284,8 +283,6 @@ public class ExportTest { @Test public void testExportEncrypted() throws Exception { - - Application spyApplication; ContentResolver mockResolver = mock(ContentResolver.class); @@ -315,7 +312,7 @@ public class ExportTest { new ProviderHelper(RuntimeEnvironment.application), null); BackupKeyringParcel parcel = new BackupKeyringParcel( - new long[] { mStaticRing1.getMasterKeyId() }, false, fakeOutputUri); + new long[] { mStaticRing1.getMasterKeyId() }, false, true, fakeOutputUri); CryptoInputParcel inputParcel = new CryptoInputParcel(passphrase); ExportResult result = op.execute(parcel, inputParcel); -- cgit v1.2.3