diff options
author | Dominik Schürmann <dominik@dominikschuermann.de> | 2015-11-13 23:17:38 +0100 |
---|---|---|
committer | Dominik Schürmann <dominik@dominikschuermann.de> | 2015-11-13 23:17:38 +0100 |
commit | 4b594e4e753b4dab82dcb151502d4a0ff156e661 (patch) | |
tree | 453630e820a2cdd8b25e7366a4e3655c140b1405 | |
parent | 1963ca9c2d88f70fdab1c3a696bc832cfc578aba (diff) | |
parent | 4c5fce8f266f44b3c960d309bf8abe7b13455492 (diff) | |
download | open-keychain-4b594e4e753b4dab82dcb151502d4a0ff156e661.tar.gz open-keychain-4b594e4e753b4dab82dcb151502d4a0ff156e661.tar.bz2 open-keychain-4b594e4e753b4dab82dcb151502d4a0ff156e661.zip |
Merge branch 'master' of github.com:open-keychain/open-keychain
2 files changed, 138 insertions, 134 deletions
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java index d6ef1be85..b810f5a6a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -25,6 +25,8 @@ import android.net.Uri; import android.os.IBinder; import android.os.ParcelFileDescriptor; import android.os.Parcelable; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.text.TextUtils; import org.openintents.openpgp.IOpenPgpService; @@ -239,10 +241,8 @@ public class OpenPgpService extends RemoteService { PendingIntent.FLAG_CANCEL_CURRENT); } - private Intent signImpl(Intent data, ParcelFileDescriptor input, - ParcelFileDescriptor output, boolean cleartextSign) { - InputStream is = null; - OutputStream os = null; + private Intent signImpl(Intent data, InputStream inputStream, + OutputStream outputStream, boolean cleartextSign) { try { boolean asciiArmor = cleartextSign || data.getBooleanExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true); @@ -278,14 +278,13 @@ public class OpenPgpService extends RemoteService { } // Get Input- and OutputStream from ParcelFileDescriptor - is = new ParcelFileDescriptor.AutoCloseInputStream(input); - if (cleartextSign) { + if (!cleartextSign) { // output stream only needed for cleartext signatures, // detached signatures are returned as extra - os = new ParcelFileDescriptor.AutoCloseOutputStream(output); + outputStream = null; } - long inputLength = is.available(); - InputData inputData = new InputData(is, inputLength); + long inputLength = inputStream.available(); + InputData inputData = new InputData(inputStream, inputLength); CryptoInputParcel inputParcel = CryptoInputParcelCacheService.getCryptoInputParcel(this, data); if (inputParcel == null) { @@ -299,7 +298,7 @@ public class OpenPgpService extends RemoteService { // execute PGP operation! PgpSignEncryptOperation pse = new PgpSignEncryptOperation(this, new ProviderHelper(getContext()), null); - PgpSignEncryptResult pgpResult = pse.execute(pseInput, inputParcel, inputData, os); + PgpSignEncryptResult pgpResult = pse.execute(pseInput, inputParcel, inputData, outputStream); if (pgpResult.isPending()) { @@ -331,28 +330,11 @@ public class OpenPgpService extends RemoteService { new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); return result; - } finally { - if (is != null) { - try { - is.close(); - } catch (IOException e) { - Log.e(Constants.TAG, "IOException when closing InputStream", e); - } - } - if (os != null) { - try { - os.close(); - } catch (IOException e) { - Log.e(Constants.TAG, "IOException when closing OutputStream", e); - } - } } } - private Intent encryptAndSignImpl(Intent data, ParcelFileDescriptor input, - ParcelFileDescriptor output, boolean sign) { - InputStream is = null; - OutputStream os = null; + private Intent encryptAndSignImpl(Intent data, InputStream inputStream, + OutputStream outputStream, boolean sign) { try { boolean asciiArmor = data.getBooleanExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true); String originalFilename = data.getStringExtra(OpenPgpApi.EXTRA_ORIGINAL_FILENAME); @@ -384,13 +366,9 @@ public class OpenPgpService extends RemoteService { } } - // build InputData and write into OutputStream - // Get Input- and OutputStream from ParcelFileDescriptor - is = new ParcelFileDescriptor.AutoCloseInputStream(input); - os = new ParcelFileDescriptor.AutoCloseOutputStream(output); - - long inputLength = is.available(); - InputData inputData = new InputData(is, inputLength, originalFilename); + // TODO this is not correct! + long inputLength = inputStream.available(); + InputData inputData = new InputData(inputStream, inputLength, originalFilename); PgpSignEncryptInputParcel pseInput = new PgpSignEncryptInputParcel(); pseInput.setEnableAsciiArmorOutput(asciiArmor) @@ -456,7 +434,7 @@ public class OpenPgpService extends RemoteService { PgpSignEncryptOperation op = new PgpSignEncryptOperation(this, new ProviderHelper(getContext()), null); // execute PGP operation! - PgpSignEncryptResult pgpResult = op.execute(pseInput, inputParcel, inputData, os); + PgpSignEncryptResult pgpResult = op.execute(pseInput, inputParcel, inputData, outputStream); if (pgpResult.isPending()) { RequiredInputParcel requiredInput = pgpResult.getRequiredInputParcel(); @@ -483,37 +461,15 @@ public class OpenPgpService extends RemoteService { new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); return result; - } finally { - if (is != null) { - try { - is.close(); - } catch (IOException e) { - Log.e(Constants.TAG, "IOException when closing InputStream", e); - } - } - if (os != null) { - try { - os.close(); - } catch (IOException e) { - Log.e(Constants.TAG, "IOException when closing OutputStream", e); - } - } } } - private Intent decryptAndVerifyImpl(Intent data, ParcelFileDescriptor inputDescriptor, - ParcelFileDescriptor output, boolean decryptMetadataOnly) { - InputStream inputStream = null; - OutputStream outputStream = null; + private Intent decryptAndVerifyImpl(Intent data, InputStream inputStream, + OutputStream outputStream, boolean decryptMetadataOnly) { try { - // Get Input- and OutputStream from ParcelFileDescriptor - inputStream = new ParcelFileDescriptor.AutoCloseInputStream(inputDescriptor); - // output is optional, e.g., for verifying detached signatures - if (decryptMetadataOnly || output == null) { + if (decryptMetadataOnly) { outputStream = null; - } else { - outputStream = new ParcelFileDescriptor.AutoCloseOutputStream(output); } String currentPkg = getCurrentCallingPackage(); @@ -539,6 +495,7 @@ public class OpenPgpService extends RemoteService { PgpDecryptVerifyOperation op = new PgpDecryptVerifyOperation(this, mProviderHelper, null); + // TODO this is not correct! long inputLength = inputStream.available(); InputData inputData = new InputData(inputStream, inputLength); @@ -605,6 +562,7 @@ public class OpenPgpService extends RemoteService { // case RESULT_NOT_ENCRYPTED, but a signature, fallback to deprecated signatureOnly variable if (decryptionResult.getResult() == OpenPgpDecryptionResult.RESULT_NOT_ENCRYPTED && signatureResult.getResult() != OpenPgpSignatureResult.RESULT_NO_SIGNATURE) { + // noinspection deprecation, TODO signatureResult.setSignatureOnly(true); } @@ -666,33 +624,14 @@ public class OpenPgpService extends RemoteService { result.putExtra(OpenPgpApi.RESULT_ERROR, new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage())); result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); return result; - } finally { - if (inputStream != null) { - try { - inputStream.close(); - } catch (IOException e) { - Log.e(Constants.TAG, "IOException when closing InputStream", e); - } - } - if (outputStream != null) { - try { - outputStream.close(); - } catch (IOException e) { - Log.e(Constants.TAG, "IOException when closing OutputStream", e); - } - } } } - private Intent getKeyImpl(Intent data, ParcelFileDescriptor output) { + private Intent getKeyImpl(Intent data, OutputStream outputStream) { try { long masterKeyId = data.getLongExtra(OpenPgpApi.EXTRA_KEY_ID, 0); - // output is optional, for getting the key - OutputStream outputStream = - (output != null) ? new ParcelFileDescriptor.AutoCloseOutputStream(output) : null; - try { // try to find key, throws NotFoundException if not in db! CanonicalizedPublicKeyRing keyRing = @@ -846,7 +785,7 @@ public class OpenPgpService extends RemoteService { OpenPgpError error = new OpenPgpError (OpenPgpError.INCOMPATIBLE_API_VERSIONS, "Incompatible API versions!\n" + "used API version: " + data.getIntExtra(OpenPgpApi.EXTRA_API_VERSION, -1) + "\n" - + "supported API versions: " + supportedVersions.toString()); + + "supported API versions: " + supportedVersions); result.putExtra(OpenPgpApi.RESULT_ERROR, error); result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR); return result; @@ -875,68 +814,88 @@ public class OpenPgpService extends RemoteService { return mBinder; } + @Nullable + protected Intent executeInternal( + @NonNull Intent data, + @Nullable ParcelFileDescriptor input, + @Nullable ParcelFileDescriptor output) { - protected Intent executeInternal(Intent data, ParcelFileDescriptor input, ParcelFileDescriptor output) { - try { - Intent errorResult = checkRequirements(data); - if (errorResult != null) { - return errorResult; - } + OutputStream outputStream = + (output != null) ? new ParcelFileDescriptor.AutoCloseOutputStream(output) : null; + InputStream inputStream = + (input != null) ? new ParcelFileDescriptor.AutoCloseInputStream(input) : null; - String action = data.getAction(); - switch (action) { - case OpenPgpApi.ACTION_CLEARTEXT_SIGN: { - return signImpl(data, input, output, true); - } - case OpenPgpApi.ACTION_SIGN: { - // DEPRECATED: same as ACTION_CLEARTEXT_SIGN - Log.w(Constants.TAG, "You are using a deprecated API call, please use ACTION_CLEARTEXT_SIGN instead of ACTION_SIGN!"); - return signImpl(data, input, output, true); - } - case OpenPgpApi.ACTION_DETACHED_SIGN: { - return signImpl(data, input, output, false); - } - case OpenPgpApi.ACTION_ENCRYPT: { - return encryptAndSignImpl(data, input, output, false); - } - case OpenPgpApi.ACTION_SIGN_AND_ENCRYPT: { - return encryptAndSignImpl(data, input, output, true); - } - case OpenPgpApi.ACTION_DECRYPT_VERIFY: { - return decryptAndVerifyImpl(data, input, output, false); - } - case OpenPgpApi.ACTION_DECRYPT_METADATA: { - return decryptAndVerifyImpl(data, input, output, true); - } - case OpenPgpApi.ACTION_GET_SIGN_KEY_ID: { - return getSignKeyIdImpl(data); - } - case OpenPgpApi.ACTION_GET_KEY_IDS: { - return getKeyIdsImpl(data); - } - case OpenPgpApi.ACTION_GET_KEY: { - return getKeyImpl(data, output); - } - default: { - return null; - } - } + try { + return executeInternalWithStreams(data, inputStream, outputStream); } finally { // always close input and output file descriptors even in error cases - if (input != null) { + if (inputStream != null) { try { - input.close(); + inputStream.close(); } catch (IOException e) { Log.e(Constants.TAG, "IOException when closing input ParcelFileDescriptor", e); } } - if (output != null) { + if (outputStream != null) { try { - output.close(); + outputStream.close(); } catch (IOException e) { Log.e(Constants.TAG, "IOException when closing output ParcelFileDescriptor", e); } } } } + + @Nullable + protected Intent executeInternalWithStreams( + @NonNull Intent data, + @Nullable InputStream inputStream, + @Nullable OutputStream outputStream) { + + Intent errorResult = checkRequirements(data); + if (errorResult != null) { + return errorResult; + } + + String action = data.getAction(); + switch (action) { + case OpenPgpApi.ACTION_CLEARTEXT_SIGN: { + return signImpl(data, inputStream, outputStream, true); + } + case OpenPgpApi.ACTION_SIGN: { + // DEPRECATED: same as ACTION_CLEARTEXT_SIGN + Log.w(Constants.TAG, "You are using a deprecated API call, please use ACTION_CLEARTEXT_SIGN instead of ACTION_SIGN!"); + return signImpl(data, inputStream, outputStream, true); + } + case OpenPgpApi.ACTION_DETACHED_SIGN: { + return signImpl(data, inputStream, outputStream, false); + } + case OpenPgpApi.ACTION_ENCRYPT: { + return encryptAndSignImpl(data, inputStream, outputStream, false); + } + case OpenPgpApi.ACTION_SIGN_AND_ENCRYPT: { + return encryptAndSignImpl(data, inputStream, outputStream, true); + } + case OpenPgpApi.ACTION_DECRYPT_VERIFY: { + return decryptAndVerifyImpl(data, inputStream, outputStream, false); + } + case OpenPgpApi.ACTION_DECRYPT_METADATA: { + return decryptAndVerifyImpl(data, inputStream, outputStream, true); + } + case OpenPgpApi.ACTION_GET_SIGN_KEY_ID: { + return getSignKeyIdImpl(data); + } + case OpenPgpApi.ACTION_GET_KEY_IDS: { + return getKeyIdsImpl(data); + } + case OpenPgpApi.ACTION_GET_KEY: { + return getKeyImpl(data, outputStream); + } + default: { + return null; + } + } + + } + } diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java index be233d0b3..d3c3f1df5 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java @@ -46,6 +46,7 @@ import org.spongycastle.bcpg.PacketTags; import org.spongycastle.bcpg.PublicKeyEncSessionPacket; import org.spongycastle.bcpg.sig.KeyFlags; import org.spongycastle.jce.provider.BouncyCastleProvider; +import org.spongycastle.openpgp.PGPKeyFlags; import org.sufficientlysecure.keychain.WorkaroundBuildConfig; import org.sufficientlysecure.keychain.operations.results.DecryptVerifyResult; import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType; @@ -56,6 +57,7 @@ import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel; +import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel.RequiredInputType; import org.sufficientlysecure.keychain.support.KeyringTestingHelper; @@ -558,8 +560,10 @@ public class PgpEncryptDecryptTest { String plaintext = "dies ist ein plaintext ☭" + TestingUtils.genPassphrase(true); + byte[] ciphertext; + long encKeyId1; + { // encrypt data with key - byte[] ciphertext; ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteArrayInputStream in = new ByteArrayInputStream(plaintext.getBytes()); @@ -589,7 +593,7 @@ public class PgpEncryptDecryptTest { Packet p; p = new BCPGInputStream(new ByteArrayInputStream(enc1.buf)).readPacket(); Assert.assertTrue("first packet must be session packet", p instanceof PublicKeyEncSessionPacket); - long encKeyId1 = ((PublicKeyEncSessionPacket) p).getKeyID(); + encKeyId1 = ((PublicKeyEncSessionPacket) p).getKeyID(); p = new BCPGInputStream(new ByteArrayInputStream(enc2.buf)).readPacket(); Assert.assertTrue("second packet must be session packet", p instanceof PublicKeyEncSessionPacket); @@ -604,6 +608,48 @@ public class PgpEncryptDecryptTest { } + { // strip first encrypted subkey, decryption should skip it + + SaveKeyringParcel parcel = new SaveKeyringParcel(mStaticRing1.getMasterKeyId(), mStaticRing1.getFingerprint()); + parcel.mChangeSubKeys.add(new SubkeyChange(encKeyId1, true, false)); + UncachedKeyRing modified = PgpKeyOperationTest.applyModificationWithChecks(parcel, mStaticRing1, + new ArrayList<RawPacket>(), new ArrayList<RawPacket>(), + new CryptoInputParcel(new Date(), mKeyPhrase1)); + + ProviderHelper providerHelper = new ProviderHelper(RuntimeEnvironment.application); + providerHelper.saveSecretKeyRing(modified, new ProgressScaler()); + + PgpDecryptVerifyOperation op = new PgpDecryptVerifyOperation(RuntimeEnvironment.application, + new ProviderHelper(RuntimeEnvironment.application), null); + PgpDecryptVerifyInputParcel input = new PgpDecryptVerifyInputParcel(ciphertext); + DecryptVerifyResult result = op.execute(input, new CryptoInputParcel(mKeyPhrase1)); + + Assert.assertTrue("decryption must succeed", result.success()); + Assert.assertTrue("decryption must have skipped first key", + result.getLog().containsType(LogType.MSG_DC_ASKIP_UNAVAILABLE)); + } + + { // change flags of second encrypted subkey, decryption should skip it + + SaveKeyringParcel parcel = new SaveKeyringParcel(mStaticRing1.getMasterKeyId(), mStaticRing1.getFingerprint()); + parcel.mChangeSubKeys.add(new SubkeyChange(encKeyId1, PGPKeyFlags.CAN_CERTIFY, null)); + UncachedKeyRing modified = PgpKeyOperationTest.applyModificationWithChecks(parcel, mStaticRing1, + new ArrayList<RawPacket>(), new ArrayList<RawPacket>(), + new CryptoInputParcel(new Date(), mKeyPhrase1)); + + ProviderHelper providerHelper = new ProviderHelper(RuntimeEnvironment.application); + providerHelper.saveSecretKeyRing(modified, new ProgressScaler()); + + PgpDecryptVerifyOperation op = new PgpDecryptVerifyOperation(RuntimeEnvironment.application, + new ProviderHelper(RuntimeEnvironment.application), null); + PgpDecryptVerifyInputParcel input = new PgpDecryptVerifyInputParcel(ciphertext); + DecryptVerifyResult result = op.execute(input, new CryptoInputParcel(mKeyPhrase1)); + + Assert.assertTrue("decryption must succeed", result.success()); + Assert.assertTrue("decryption must have skipped first key", + result.getLog().containsType(LogType.MSG_DC_ASKIP_BAD_FLAGS)); + } + { // revoke first encryption subkey of keyring in database SaveKeyringParcel parcel = new SaveKeyringParcel(mStaticRing1.getMasterKeyId(), mStaticRing1.getFingerprint()); parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(mStaticRing1, 2)); @@ -616,7 +662,6 @@ public class PgpEncryptDecryptTest { } { // encrypt to this keyring, make sure it's not encrypted to the revoked subkey - byte[] ciphertext; ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteArrayInputStream in = new ByteArrayInputStream(plaintext.getBytes()); |