fix handling of algorithm tags in parcels

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BenchmarkOperation.java

@@ -41,7 +41,6 @@ import org.sufficientlysecure.keychain.operations.results.OperationResult.Operat
 import org.sufficientlysecure.keychain.operations.results.SignEncryptResult;
 import org.sufficientlysecure.keychain.pgp.PgpDecryptVerifyInputParcel;
 import org.sufficientlysecure.keychain.pgp.PgpDecryptVerifyOperation;
-import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags;
 import org.sufficientlysecure.keychain.pgp.PgpSignEncryptData;
 import org.sufficientlysecure.keychain.pgp.Progressable;
 import org.sufficientlysecure.keychain.pgp.SignEncryptParcel;
@@ -85,7 +84,7 @@ public class BenchmarkOperation extends BaseOperation<BenchmarkInputParcel> {
                             new ProgressScaler(mProgressable, i*(50/numRepeats), (i+1)*(50/numRepeats), 100), mCancelled);
             PgpSignEncryptData.Builder data = PgpSignEncryptData.builder();
             data.setSymmetricPassphrase(passphrase);
-            data.setSymmetricEncryptionAlgorithm(OpenKeychainSymmetricKeyAlgorithmTags.AES_128);
+            data.setSymmetricEncryptionAlgorithm(SymmetricKeyAlgorithmTags.AES_128);
             SignEncryptParcel input = SignEncryptParcel.createSignEncryptParcel(data.build(), buf);
             encryptResult = op.execute(input, CryptoInputParcel.createCryptoInputParcel());
             log.add(encryptResult, 1);

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerifyOperation.java

@@ -361,10 +361,8 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
             log.add(LogType.MSG_DC_CLEAR_DECOMPRESS, indent + 1);
 
             PGPCompressedData compressedData = (PGPCompressedData) dataChunk;
-
+            plainFact = new JcaSkipMarkerPGPObjectFactory(compressedData.getDataStream());
-            JcaSkipMarkerPGPObjectFactory fact = new JcaSkipMarkerPGPObjectFactory(compressedData.getDataStream());
+            dataChunk = plainFact.nextObject();
-            dataChunk = fact.nextObject();
-            plainFact = fact;
         }
 
         PgpSignatureChecker signatureChecker = new PgpSignatureChecker(
@@ -378,10 +376,7 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
             dataChunk = plainFact.nextObject();
         }
 
-        OpenPgpMetadata metadata;
+        if (!(dataChunk instanceof PGPLiteralData)) {
-
-        if ( ! (dataChunk instanceof PGPLiteralData)) {
-
             log.add(LogType.MSG_DC_ERROR_INVALID_DATA, indent);
             return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log);
 
@@ -422,6 +417,8 @@ public class PgpDecryptVerifyOperation extends BaseOperation<PgpDecryptVerifyInp
         log.add(LogType.MSG_DC_CLEAR_META_TIME, indent + 1,
                 new Date(literalData.getModificationTime().getTime()).toString());
 
+        OpenPgpMetadata metadata;
+
         // return here if we want to decrypt the metadata only
         if (input.isDecryptMetadataOnly()) {
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptData.java

@@ -28,8 +28,10 @@ import android.os.Parcelable;
 import android.support.annotation.Nullable;
 
 import com.google.auto.value.AutoValue;
-import org.bouncycastle.bcpg.CompressionAlgorithmTags;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainHashAlgorithmTags;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags;
 import org.sufficientlysecure.keychain.util.Passphrase;
 
 @AutoValue
@@ -37,11 +39,9 @@ public abstract class PgpSignEncryptData implements Parcelable {
     @Nullable
     public abstract String getCharset();
     abstract long getAdditionalEncryptId();
-    abstract int getSignatureHashAlgorithm();
     @Nullable
     public abstract Long getSignatureSubKeyId();
     public abstract long getSignatureMasterKeyId();
-    public abstract int getSymmetricEncryptionAlgorithm();
     @Nullable
     public abstract Passphrase getSymmetricPassphrase();
     @Nullable
@@ -49,10 +49,13 @@ public abstract class PgpSignEncryptData implements Parcelable {
     public abstract long[] getEncryptionMasterKeyIds();
     @Nullable
     public abstract List<Long> getAllowedSigningKeyIds();
-    public abstract int getCompressionAlgorithm();
     @Nullable
     public abstract String getVersionHeader();
 
+    public abstract int getCompressionAlgorithm();
+    public abstract int getSignatureHashAlgorithm();
+    public abstract int getSymmetricEncryptionAlgorithm();
+
     public abstract boolean isEnableAsciiArmorOutput();
     public abstract boolean isCleartextSignature();
     public abstract boolean isDetachedSignature();
@@ -61,16 +64,16 @@ public abstract class PgpSignEncryptData implements Parcelable {
 
     public static Builder builder() {
         return new AutoValue_PgpSignEncryptData.Builder()
-                .setCompressionAlgorithm(CompressionAlgorithmTags.UNCOMPRESSED)
-                .setSymmetricEncryptionAlgorithm(PgpSecurityConstants.DEFAULT_SYMMETRIC_ALGORITHM)
                 .setSignatureMasterKeyId(Constants.key.none)
-                .setSignatureHashAlgorithm(PgpSecurityConstants.DEFAULT_HASH_ALGORITHM)
                 .setAdditionalEncryptId(Constants.key.none)
                 .setEnableAsciiArmorOutput(false)
                 .setCleartextSignature(false)
                 .setDetachedSignature(false)
                 .setAddBackupHeader(false)
-                .setHiddenRecipients(false);
+                .setHiddenRecipients(false)
+                .setCompressionAlgorithm(OpenKeychainCompressionAlgorithmTags.USE_DEFAULT)
+                .setSignatureHashAlgorithm(OpenKeychainHashAlgorithmTags.USE_DEFAULT)
+                .setSymmetricEncryptionAlgorithm(OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT);
     }
 
     @AutoValue.Builder
@@ -79,15 +82,16 @@ public abstract class PgpSignEncryptData implements Parcelable {
 
         public abstract Builder setCharset(String charset);
         public abstract Builder setAdditionalEncryptId(long additionalEncryptId);
-        public abstract Builder setSignatureHashAlgorithm(int signatureHashAlgorithm);
         public abstract Builder setSignatureSubKeyId(Long signatureSubKeyId);
         public abstract Builder setSignatureMasterKeyId(long signatureMasterKeyId);
-        public abstract Builder setSymmetricEncryptionAlgorithm(int symmetricEncryptionAlgorithm);
         public abstract Builder setSymmetricPassphrase(Passphrase symmetricPassphrase);
         public abstract Builder setEncryptionMasterKeyIds(long[] encryptionMasterKeyIds);
-        public abstract Builder setCompressionAlgorithm(int compressionAlgorithm);
         public abstract Builder setVersionHeader(String versionHeader);
 
+        public abstract Builder setCompressionAlgorithm(int compressionAlgorithm);
+        public abstract Builder setSignatureHashAlgorithm(int signatureHashAlgorithm);
+        public abstract Builder setSymmetricEncryptionAlgorithm(int symmetricEncryptionAlgorithm);
+
         public abstract Builder setAddBackupHeader(boolean isAddBackupHeader);
         public abstract Builder setEnableAsciiArmorOutput(boolean enableAsciiArmorOutput);
         public abstract Builder setCleartextSignature(boolean isCleartextSignature);

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java

@@ -61,6 +61,9 @@ import org.sufficientlysecure.keychain.operations.results.OperationResult.LogTyp
 import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
 import org.sufficientlysecure.keychain.operations.results.PgpSignEncryptResult;
 import org.sufficientlysecure.keychain.operations.results.SignEncryptResult;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainHashAlgorithmTags;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.provider.KeyRepository;
 import org.sufficientlysecure.keychain.provider.KeyWritableRepository;
@@ -178,13 +181,13 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
         boolean enableSignature = data.getSignatureMasterKeyId() != Constants.key.none;
         boolean enableEncryption = ((data.getEncryptionMasterKeyIds() != null && data.getEncryptionMasterKeyIds().length > 0)
                 || data.getSymmetricPassphrase() != null);
-        boolean enableCompression = (data.getCompressionAlgorithm() != CompressionAlgorithmTags.UNCOMPRESSED);
 
-        Log.d(Constants.TAG, "enableSignature:" + enableSignature
+        int compressionAlgorithm = data.getCompressionAlgorithm();
-                + "\nenableEncryption:" + enableEncryption
+        if (compressionAlgorithm == OpenKeychainCompressionAlgorithmTags.USE_DEFAULT) {
-                + "\nenableCompression:" + enableCompression
+            compressionAlgorithm = PgpSecurityConstants.DEFAULT_COMPRESSION_ALGORITHM;
-                + "\nenableAsciiArmorOutput:" + data.isEnableAsciiArmorOutput()
+        }
-                + "\nisHiddenRecipients:" + data.isHiddenRecipients());
+
+        Log.d(Constants.TAG, data.toString());
 
         ArmoredOutputStream armorOut = null;
         OutputStream out;
@@ -299,12 +302,12 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
         if (enableEncryption) {
 
             // Use requested encryption algo
-            int algo = data.getSymmetricEncryptionAlgorithm();
+            int symmetricEncryptionAlgorithm = data.getSymmetricEncryptionAlgorithm();
-            if (algo == PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT) {
+            if (symmetricEncryptionAlgorithm == OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT) {
-                algo = PgpSecurityConstants.DEFAULT_SYMMETRIC_ALGORITHM;
+                symmetricEncryptionAlgorithm = PgpSecurityConstants.DEFAULT_SYMMETRIC_ALGORITHM;
             }
             JcePGPDataEncryptorBuilder encryptorBuilder =
-                    new JcePGPDataEncryptorBuilder(algo)
+                    new JcePGPDataEncryptorBuilder(symmetricEncryptionAlgorithm)
                             .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME)
                             .setWithIntegrityPacket(true);
 
@@ -338,6 +341,11 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
             }
         }
 
+        int signatureHashAlgorithm = data.getSignatureHashAlgorithm();
+        if (signatureHashAlgorithm == OpenKeychainHashAlgorithmTags.USE_DEFAULT) {
+            signatureHashAlgorithm = PgpSecurityConstants.DEFAULT_HASH_ALGORITHM;
+        }
+
         /* Initialize signature generator object for later usage */
         PGPSignatureGenerator signatureGenerator = null;
         if (enableSignature) {
@@ -346,7 +354,7 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
             try {
                 boolean cleartext = data.isCleartextSignature() && data.isEnableAsciiArmorOutput() && !enableEncryption;
                 signatureGenerator = signingKey.getDataSignatureGenerator(
-                        data.getSignatureHashAlgorithm(), cleartext,
+                        signatureHashAlgorithm, cleartext,
                         cryptoInput.getCryptoData(), cryptoInput.getSignatureTime());
             } catch (PgpGeneralException e) {
                 log.add(LogType.MSG_PSE_ERROR_NFC, indent);
@@ -381,15 +389,10 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
 
                 encryptionOut = cPk.open(out, new byte[1 << 16]);
 
-                if (enableCompression) {
+                if (compressionAlgorithm != CompressionAlgorithmTags.UNCOMPRESSED) {
                     log.add(LogType.MSG_PSE_COMPRESSING, indent);
 
-                    // Use preferred compression algo
+                    compressGen = new PGPCompressedDataGenerator(compressionAlgorithm);
-                    int algo = data.getCompressionAlgorithm();
-                    if (algo == PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.USE_DEFAULT) {
-                        algo = PgpSecurityConstants.DEFAULT_COMPRESSION_ALGORITHM;
-                    }
-                    compressGen = new PGPCompressedDataGenerator(algo);
                     bcpgOut = new BCPGOutputStream(compressGen.open(encryptionOut));
                 } else {
                     bcpgOut = new BCPGOutputStream(encryptionOut);
@@ -438,7 +441,7 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
                 log.add(LogType.MSG_PSE_SIGNING_CLEARTEXT, indent);
 
                 // write -----BEGIN PGP SIGNED MESSAGE-----
-                armorOut.beginClearText(data.getSignatureHashAlgorithm());
+                armorOut.beginClearText(signatureHashAlgorithm);
 
                 InputStream in = new BufferedInputStream(inputData.getInputStream());
                 final BufferedReader reader = new BufferedReader(new InputStreamReader(in));
@@ -511,14 +514,10 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
 
                 InputStream in = new BufferedInputStream(inputData.getInputStream());
 
-                if (enableCompression) {
+                if (compressionAlgorithm != CompressionAlgorithmTags.UNCOMPRESSED) {
-                    // Use preferred compression algo
+                    log.add(LogType.MSG_PSE_COMPRESSING, indent);
-                    int algo = data.getCompressionAlgorithm();
-                    if (algo == PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.USE_DEFAULT) {
-                        algo = PgpSecurityConstants.DEFAULT_COMPRESSION_ALGORITHM;
-                    }
 
-                    compressGen = new PGPCompressedDataGenerator(algo);
+                    compressGen = new PGPCompressedDataGenerator(compressionAlgorithm);
                     bcpgOut = new BCPGOutputStream(compressGen.open(out));
                 } else {
                     bcpgOut = new BCPGOutputStream(out);
@@ -573,11 +572,11 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
 
             // closing outputs
             // NOTE: closing needs to be done in the correct order!
-            if (encryptionOut != null) {
+            if (compressGen != null) {
-                if (compressGen != null) {
+                compressGen.close();
-                    compressGen.close();
+            }
-                }
 
+            if (encryptionOut != null) {
                 encryptionOut.close();
             }
             // Note: Closing ArmoredOutputStream does not close the underlying stream
@@ -624,7 +623,7 @@ public class PgpSignEncryptOperation extends BaseOperation<PgpSignEncryptInputPa
             }
             result.setDetachedSignature(detachedByteOut.toByteArray());
             try {
-                String digestName = PGPUtil.getDigestName(data.getSignatureHashAlgorithm());
+                String digestName = PGPUtil.getDigestName(signatureHashAlgorithm);
                 // construct micalg parameter according to https://tools.ietf.org/html/rfc3156#section-5
                 result.setMicAlgDigestName("pgp-" + digestName.toLowerCase());
             } catch (PGPException e) {

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java

@@ -56,7 +56,7 @@ import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
 import org.sufficientlysecure.keychain.pgp.DecryptVerifySecurityProblem;
 import org.sufficientlysecure.keychain.pgp.PgpDecryptVerifyInputParcel;
 import org.sufficientlysecure.keychain.pgp.PgpDecryptVerifyOperation;
-import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags;
 import org.sufficientlysecure.keychain.pgp.PgpSignEncryptData;
 import org.sufficientlysecure.keychain.pgp.PgpSignEncryptOperation;
 import org.sufficientlysecure.keychain.pgp.Progressable;
@@ -112,8 +112,7 @@ public class OpenPgpService extends Service {
             pgpData.setEnableAsciiArmorOutput(asciiArmor)
                     .setCleartextSignature(cleartextSign)
                     .setDetachedSignature(!cleartextSign)
-                    .setVersionHeader(null)
+                    .setVersionHeader(null);
-                    .setSignatureHashAlgorithm(PgpSecurityConstants.OpenKeychainHashAlgorithmTags.USE_DEFAULT);
 
 
             Intent signKeyIdIntent = getSignKeyMasterId(data);
@@ -200,18 +199,14 @@ public class OpenPgpService extends Service {
                 originalFilename = "";
             }
 
-            boolean enableCompression = data.getBooleanExtra(OpenPgpApi.EXTRA_ENABLE_COMPRESSION, true);
+            PgpSignEncryptData.Builder pgpData = PgpSignEncryptData.builder()
-            int compressionId;
+                    .setEnableAsciiArmorOutput(asciiArmor)
-            if (enableCompression) {
+                    .setVersionHeader(null);
-                compressionId = PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.USE_DEFAULT;
-            } else {
-                compressionId = PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.UNCOMPRESSED;
-            }
 
-            PgpSignEncryptData.Builder pgpData = PgpSignEncryptData.builder();
+            boolean enableCompression = data.getBooleanExtra(OpenPgpApi.EXTRA_ENABLE_COMPRESSION, true);
-            pgpData.setEnableAsciiArmorOutput(asciiArmor)
+            if (!enableCompression) {
-                    .setVersionHeader(null)
+                pgpData.setCompressionAlgorithm(OpenKeychainCompressionAlgorithmTags.UNCOMPRESSED);
-                    .setCompressionAlgorithm(compressionId);
+            }
 
             if (sign) {
                 Intent signKeyIdIntent = getSignKeyMasterId(data);

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesFragment.java

@@ -623,19 +623,11 @@ public class EncryptFilesFragment
         // fill values for this action
         PgpSignEncryptData.Builder data = PgpSignEncryptData.builder();
 
-        if (mUseCompression) {
+        if (!mUseCompression) {
-            data.setCompressionAlgorithm(
+            data.setCompressionAlgorithm(PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.UNCOMPRESSED);
-                    PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.USE_DEFAULT);
-        } else {
-            data.setCompressionAlgorithm(
-                    PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.UNCOMPRESSED);
         }
         data.setHiddenRecipients(mHiddenRecipients);
         data.setEnableAsciiArmorOutput(mAfterEncryptAction == AfterEncryptAction.COPY || mUseArmor);
-        data.setSymmetricEncryptionAlgorithm(
-                PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT);
-        data.setSignatureHashAlgorithm(
-                PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT);
 
         EncryptActivity encryptActivity = (EncryptActivity) getActivity();
         EncryptModeFragment modeFragment = encryptActivity.getModeFragment();

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextFragment.java

@@ -43,7 +43,7 @@ import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.operations.results.SignEncryptResult;
 import org.sufficientlysecure.keychain.pgp.KeyRing;
-import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants;
+import org.sufficientlysecure.keychain.pgp.PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags;
 import org.sufficientlysecure.keychain.pgp.PgpSignEncryptData;
 import org.sufficientlysecure.keychain.pgp.SignEncryptParcel;
 import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
@@ -238,18 +238,10 @@ public class EncryptTextFragment
 
         data.setCleartextSignature(true);
 
-        if (mUseCompression) {
+        if (!mUseCompression) {
-            data.setCompressionAlgorithm(
+            data.setCompressionAlgorithm(OpenKeychainCompressionAlgorithmTags.UNCOMPRESSED);
-                    PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.USE_DEFAULT);
-        } else {
-            data.setCompressionAlgorithm(
-                    PgpSecurityConstants.OpenKeychainCompressionAlgorithmTags.UNCOMPRESSED);
         }
         data.setHiddenRecipients(mHiddenRecipients);
-        data.setSymmetricEncryptionAlgorithm(
-                PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT);
-        data.setSignatureHashAlgorithm(
-                PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT);
 
         // Always use armor for messages
         data.setEnableAsciiArmorOutput(true);