From c121657c2cf17ecd3d59809ff86f758b7b1a592c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Thu, 5 Mar 2015 10:49:57 +0100 Subject: Better selection of preferred algorithm --- .../org/sufficientlysecure/keychain/Constants.java | 14 ++++++-- .../keychain/pgp/CanonicalizedSecretKey.java | 38 ++++++++++++++------ .../keychain/pgp/CanonicalizedSecretKeyRing.java | 4 +-- .../keychain/pgp/PgpKeyOperation.java | 2 +- .../keychain/pgp/PgpSignEncryptInput.java | 4 +-- .../keychain/pgp/PgpSignEncryptOperation.java | 12 ++++--- .../keychain/remote/OpenPgpService.java | 9 ++--- .../keychain/ui/EncryptFilesActivity.java | 2 ++ .../keychain/ui/EncryptTextActivity.java | 2 ++ .../keychain/ui/SettingsActivity.java | 42 +++++++++++++++------- .../keychain/util/Preferences.java | 10 +++++- 11 files changed, 99 insertions(+), 40 deletions(-) (limited to 'OpenKeychain/src/main/java/org') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java index 2d3ee6188..9f8226c05 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java @@ -19,6 +19,8 @@ package org.sufficientlysecure.keychain; import android.os.Environment; +import org.spongycastle.bcpg.HashAlgorithmTags; +import org.spongycastle.bcpg.SymmetricKeyAlgorithmTags; import org.spongycastle.jce.provider.BouncyCastleProvider; import java.io.File; @@ -77,16 +79,24 @@ public final class Constants { public static final String SEARCH_KEYSERVER = "search_keyserver_pref"; public static final String SEARCH_KEYBASE = "search_keybase_pref"; public static final String USE_DEFAULT_YUBIKEY_PIN = "useDefaultYubikeyPin"; - public static final String USE_NUMKEYPAD_FOR_YUBIKEY_PIN="useNumKeypadForYubikeyPin"; + public static final String USE_NUMKEYPAD_FOR_YUBIKEY_PIN = "useNumKeypadForYubikeyPin"; } public static final class Defaults { public static final String KEY_SERVERS = "hkps://hkps.pool.sks-keyservers.net, hkps://pgp.mit.edu"; - public static final int PREF_VERSION = 4; + public static final int PREF_VERSION = 5; } public static final class key { public static final int none = 0; public static final int symmetric = -1; } + + public static interface OpenKeychainSymmetricKeyAlgorithmTags extends SymmetricKeyAlgorithmTags { + public static final int USE_PREFERRED = -1; + } + + public static interface OpenKeychainHashAlgorithmTags extends HashAlgorithmTags { + public static final int USE_PREFERRED = -1; + } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java index 40f2f48ad..0fab4c747 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java @@ -20,6 +20,7 @@ package org.sufficientlysecure.keychain.pgp; import org.spongycastle.bcpg.HashAlgorithmTags; import org.spongycastle.bcpg.S2K; +import org.spongycastle.bcpg.SymmetricKeyAlgorithmTags; import org.spongycastle.openpgp.PGPException; import org.spongycastle.openpgp.PGPPrivateKey; import org.spongycastle.openpgp.PGPPublicKey; @@ -137,7 +138,7 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { // It means the passphrase is empty return SecretKeyType.PASSPHRASE_EMPTY; } catch (PGPException e) { - HashMap notation = getRing().getLocalNotationData(); + HashMap notation = getRing().getLocalNotationData(); if (notation.containsKey("unlock.pin@sufficientlysecure.org") && "1".equals(notation.get("unlock.pin@sufficientlysecure.org"))) { return SecretKeyType.PIN; @@ -179,7 +180,7 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { * Returns a list of all supported hash algorithms. This list is currently hardcoded to return * a limited set of algorithms supported by Yubikeys. * - * @return + * TODO: look into preferred algos of this key? */ public LinkedList getSupportedHashAlgorithms() { LinkedList supported = new LinkedList<>(); @@ -187,24 +188,41 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { if (mPrivateKeyState == PRIVATE_KEY_STATE_DIVERT_TO_CARD) { // No support for MD5 supported.add(HashAlgorithmTags.RIPEMD160); - supported.add(HashAlgorithmTags.SHA1); + // don't allow SHA1 supported.add(HashAlgorithmTags.SHA224); - supported.add(HashAlgorithmTags.SHA256); supported.add(HashAlgorithmTags.SHA384); - supported.add(HashAlgorithmTags.SHA512); // preferred is latest + supported.add(HashAlgorithmTags.SHA512); + supported.add(HashAlgorithmTags.SHA256); // preferred is latest } else { - supported.add(HashAlgorithmTags.MD5); + // NOTE: List of hash algorithms OpenKeychain wants to support! + + // don't allow MD5 supported.add(HashAlgorithmTags.RIPEMD160); - supported.add(HashAlgorithmTags.SHA1); + // don't allow SHA1 supported.add(HashAlgorithmTags.SHA224); - supported.add(HashAlgorithmTags.SHA256); supported.add(HashAlgorithmTags.SHA384); - supported.add(HashAlgorithmTags.SHA512); // preferred is latest + supported.add(HashAlgorithmTags.SHA512); + supported.add(HashAlgorithmTags.SHA256); // preferred is latest + // some application don't support SHA512, thus preferred is SHA-256 (Mailvelope?) } return supported; } + /** + * TODO: look into preferred algos of this key? + */ + public static LinkedList getSupportedEncryptionAlgorithms() { + LinkedList supported = new LinkedList<>(); + + supported.add(SymmetricKeyAlgorithmTags.TWOFISH); + supported.add(SymmetricKeyAlgorithmTags.AES_128); + supported.add(SymmetricKeyAlgorithmTags.AES_192); + supported.add(SymmetricKeyAlgorithmTags.AES_256); // preferred is latest + + return supported; + } + private PGPContentSignerBuilder getContentSignerBuilder(int hashAlgo, byte[] nfcSignedHash, Date nfcCreationTimestamp) { if (mPrivateKeyState == PRIVATE_KEY_STATE_DIVERT_TO_CARD) { @@ -358,7 +376,7 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { } // HACK, for TESTING ONLY!! - PGPPrivateKey getPrivateKey () { + PGPPrivateKey getPrivateKey() { return mPrivateKey; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java index b5f6a5b09..97b5fa6fe 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java @@ -19,11 +19,11 @@ package org.sufficientlysecure.keychain.pgp; import org.spongycastle.openpgp.PGPKeyRing; -import org.spongycastle.openpgp.PGPObjectFactory; import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; import org.spongycastle.openpgp.PGPSignature; +import org.spongycastle.openpgp.jcajce.JcaPGPObjectFactory; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.util.IterableIterator; @@ -45,7 +45,7 @@ public class CanonicalizedSecretKeyRing extends CanonicalizedKeyRing { public CanonicalizedSecretKeyRing(byte[] blob, boolean isRevoked, int verified) { super(verified); - PGPObjectFactory factory = new PGPObjectFactory(blob); + JcaPGPObjectFactory factory = new JcaPGPObjectFactory(blob); PGPKeyRing keyRing = null; try { if ((keyRing = (PGPKeyRing) factory.nextObject()) == null) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index da0394573..a54fa189a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -95,7 +95,7 @@ public class PgpKeyOperation { SymmetricKeyAlgorithmTags.AES_256, SymmetricKeyAlgorithmTags.AES_192, SymmetricKeyAlgorithmTags.AES_128, - SymmetricKeyAlgorithmTags.CAST5 + SymmetricKeyAlgorithmTags.TWOFISH }; private static final int[] PREFERRED_HASH_ALGORITHMS = new int[]{ HashAlgorithmTags.SHA512, diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptInput.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptInput.java index 9318be006..f22d107e4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptInput.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptInput.java @@ -12,10 +12,10 @@ public class PgpSignEncryptInput { protected int mCompressionId = CompressionAlgorithmTags.UNCOMPRESSED; protected long[] mEncryptionMasterKeyIds = null; protected String mSymmetricPassphrase = null; - protected int mSymmetricEncryptionAlgorithm = 0; + protected int mSymmetricEncryptionAlgorithm = Constants.OpenKeychainSymmetricKeyAlgorithmTags.USE_PREFERRED; protected long mSignatureMasterKeyId = Constants.key.none; protected Long mSignatureSubKeyId = null; - protected int mSignatureHashAlgorithm = 0; + protected int mSignatureHashAlgorithm = Constants.OpenKeychainHashAlgorithmTags.USE_PREFERRED; protected String mSignaturePassphrase = null; protected long mAdditionalEncryptId = Constants.key.none; protected byte[] mNfcSignedHash = null; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java index 2fa01d241..d420ddb5a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java @@ -25,7 +25,6 @@ import org.spongycastle.bcpg.ArmoredOutputStream; import org.spongycastle.bcpg.BCPGOutputStream; import org.spongycastle.bcpg.CompressionAlgorithmTags; import org.spongycastle.openpgp.PGPCompressedDataGenerator; -import org.spongycastle.openpgp.PGPEncryptedData; import org.spongycastle.openpgp.PGPEncryptedDataGenerator; import org.spongycastle.openpgp.PGPException; import org.spongycastle.openpgp.PGPLiteralData; @@ -206,10 +205,10 @@ public class PgpSignEncryptOperation extends BaseOperation { return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log); } - // check if hash algo is supported + // Use preferred hash algo int requestedAlgorithm = input.getSignatureHashAlgorithm(); LinkedList supported = signingKey.getSupportedHashAlgorithms(); - if (requestedAlgorithm == 0) { + if (requestedAlgorithm == Constants.OpenKeychainHashAlgorithmTags.USE_PREFERRED) { // get most preferred input.setSignatureHashAlgorithm(supported.getLast()); } else if (!supported.contains(requestedAlgorithm)) { @@ -222,9 +221,12 @@ public class PgpSignEncryptOperation extends BaseOperation { /* Initialize PGPEncryptedDataGenerator for later usage */ PGPEncryptedDataGenerator cPk = null; if (enableEncryption) { + + // Use preferred encryption algo int algo = input.getSymmetricEncryptionAlgorithm(); - if (algo == 0) { - algo = PGPEncryptedData.AES_128; + if (algo == Constants.OpenKeychainSymmetricKeyAlgorithmTags.USE_PREFERRED) { + // get most preferred + algo = CanonicalizedSecretKey.getSupportedEncryptionAlgorithms().getLast(); } // has Integrity packet enabled! JcePGPDataEncryptorBuilder encryptorBuilder = 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 1a65b1bee..5db382174 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -52,6 +52,7 @@ import org.sufficientlysecure.keychain.ui.PassphraseDialogActivity; import org.sufficientlysecure.keychain.ui.ViewKeyActivity; import org.sufficientlysecure.keychain.util.InputData; import org.sufficientlysecure.keychain.util.Log; +import org.sufficientlysecure.keychain.util.Preferences; import java.io.IOException; import java.io.InputStream; @@ -259,7 +260,7 @@ public class OpenPgpService extends RemoteService { .setCleartextSignature(cleartextSign) .setDetachedSignature(!cleartextSign) .setVersionHeader(PgpHelper.getVersionForHeader(this)) - .setSignatureHashAlgorithm(accSettings.getHashAlgorithm()) + .setSignatureHashAlgorithm(Preferences.getPreferences(this).getDefaultHashAlgorithm()) .setSignatureMasterKeyId(accSettings.getKeyId()) .setNfcState(nfcSignedHash, nfcCreationDate); @@ -357,8 +358,8 @@ public class OpenPgpService extends RemoteService { PgpSignEncryptInput pseInput = new PgpSignEncryptInput(); pseInput.setEnableAsciiArmorOutput(asciiArmor) .setVersionHeader(PgpHelper.getVersionForHeader(this)) - .setCompressionId(accSettings.getCompression()) - .setSymmetricEncryptionAlgorithm(accSettings.getEncryptionAlgorithm()) + .setCompressionId(Preferences.getPreferences(this).getDefaultFileCompression()) + .setSymmetricEncryptionAlgorithm(Preferences.getPreferences(this).getDefaultEncryptionAlgorithm()) .setEncryptionMasterKeyIds(keyIds) .setFailOnMissingEncryptionKeyIds(true) .setAdditionalEncryptId(accSettings.getKeyId()); // add acc key for encryption @@ -374,7 +375,7 @@ public class OpenPgpService extends RemoteService { } // sign and encrypt - pseInput.setSignatureHashAlgorithm(accSettings.getHashAlgorithm()) + pseInput.setSignatureHashAlgorithm(Preferences.getPreferences(this).getDefaultHashAlgorithm()) .setSignatureMasterKeyId(accSettings.getKeyId()) .setNfcState(nfcSignedHash, nfcCreationDate); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesActivity.java index 1286617d3..fab6c19f9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesActivity.java @@ -197,6 +197,8 @@ public class EncryptFilesActivity extends EncryptActivity implements EncryptActi data.addOutputUris(mOutputUris); data.setCompressionId(Preferences.getPreferences(this).getDefaultMessageCompression()); + data.setSymmetricEncryptionAlgorithm(Preferences.getPreferences(this).getDefaultEncryptionAlgorithm()); + data.setSignatureHashAlgorithm(Preferences.getPreferences(this).getDefaultHashAlgorithm()); // Always use armor for messages data.setEnableAsciiArmorOutput(mUseArmor); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java index 2dd861d07..05da7a3ba 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java @@ -190,6 +190,8 @@ public class EncryptTextActivity extends EncryptActivity implements EncryptActiv data.setCleartextSignature(true); data.setCompressionId(Preferences.getPreferences(this).getDefaultMessageCompression()); + data.setSymmetricEncryptionAlgorithm(Preferences.getPreferences(this).getDefaultEncryptionAlgorithm()); + data.setSignatureHashAlgorithm(Preferences.getPreferences(this).getDefaultHashAlgorithm()); // Always use armor for messages data.setEnableAsciiArmorOutput(true); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SettingsActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SettingsActivity.java index 53986a392..d82be2571 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SettingsActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/SettingsActivity.java @@ -33,8 +33,6 @@ import android.view.ViewGroup; import android.widget.LinearLayout; import org.spongycastle.bcpg.CompressionAlgorithmTags; -import org.spongycastle.bcpg.HashAlgorithmTags; -import org.spongycastle.openpgp.PGPEncryptedData; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.ui.widget.IntegerListPreference; @@ -345,12 +343,20 @@ public class SettingsActivity extends PreferenceActivity { } private static void initializeEncryptionAlgorithm(final IntegerListPreference mEncryptionAlgorithm) { - int valueIds[] = {PGPEncryptedData.AES_128, PGPEncryptedData.AES_192, - PGPEncryptedData.AES_256, PGPEncryptedData.BLOWFISH, PGPEncryptedData.TWOFISH, - PGPEncryptedData.CAST5, PGPEncryptedData.DES, PGPEncryptedData.TRIPLE_DES, - PGPEncryptedData.IDEA,}; - String entries[] = {"AES-128", "AES-192", "AES-256", "Blowfish", "Twofish", "CAST5", - "DES", "Triple DES", "IDEA",}; + int valueIds[] = { + Constants.OpenKeychainSymmetricKeyAlgorithmTags.USE_PREFERRED, + Constants.OpenKeychainSymmetricKeyAlgorithmTags.AES_256, + Constants.OpenKeychainSymmetricKeyAlgorithmTags.AES_192, + Constants.OpenKeychainSymmetricKeyAlgorithmTags.AES_128, + Constants.OpenKeychainSymmetricKeyAlgorithmTags.TWOFISH, + }; + String entries[] = { + "Use preferred algorithm", + "AES-256", + "AES-192", + "AES-128", + "Twofish", + }; String values[] = new String[valueIds.length]; for (int i = 0; i < values.length; ++i) { values[i] = "" + valueIds[i]; @@ -372,11 +378,21 @@ public class SettingsActivity extends PreferenceActivity { } private static void initializeHashAlgorithm(final IntegerListPreference mHashAlgorithm) { - int[] valueIds = new int[]{HashAlgorithmTags.RIPEMD160, - HashAlgorithmTags.SHA1, HashAlgorithmTags.SHA224, HashAlgorithmTags.SHA256, - HashAlgorithmTags.SHA384, HashAlgorithmTags.SHA512,}; - String[] entries = new String[]{"RIPEMD-160", "SHA-1", "SHA-224", "SHA-256", "SHA-384", - "SHA-512",}; + int[] valueIds = new int[]{ + Constants.OpenKeychainHashAlgorithmTags.USE_PREFERRED, + Constants.OpenKeychainHashAlgorithmTags.SHA512, + Constants.OpenKeychainHashAlgorithmTags.SHA384, + Constants.OpenKeychainHashAlgorithmTags.SHA256, + Constants.OpenKeychainHashAlgorithmTags.SHA224, + Constants.OpenKeychainHashAlgorithmTags.RIPEMD160, + }; + String[] entries = new String[]{ + "Use preferred algorithm", + "SHA-512", + "SHA-384", + "SHA-256", + "SHA-224", + "RIPEMD-160"}; String[] values = new String[valueIds.length]; for (int i = 0; i < values.length; ++i) { values[i] = "" + valueIds[i]; 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 a36af5c87..f1e9ccb28 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/Preferences.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/Preferences.java @@ -210,6 +210,7 @@ public class Preferences { } return servers.toArray(chunks); } + public String getPreferredKeyserver() { return getKeyServers()[0]; } @@ -246,6 +247,7 @@ public class Preferences { editor.putBoolean(Pref.SEARCH_KEYSERVER, searchKeyserver); editor.commit(); } + public void setSearchKeybase(boolean searchKeybase) { SharedPreferences.Editor editor = mSharedPreferences.edit(); editor.putBoolean(Pref.SEARCH_KEYBASE, searchKeybase); @@ -253,7 +255,7 @@ public class Preferences { } public CloudSearchPrefs getCloudSearchPrefs() { - return new CloudSearchPrefs(mSharedPreferences.getBoolean(Pref.SEARCH_KEYSERVER, true), + return new CloudSearchPrefs(mSharedPreferences.getBoolean(Pref.SEARCH_KEYSERVER, true), mSharedPreferences.getBoolean(Pref.SEARCH_KEYBASE, true), getPreferredKeyserver()); } @@ -322,6 +324,12 @@ public class Preferences { setDefaultHashAlgorithm(HashAlgorithmTags.SHA256); } } + // fall through + case 5: { + // use preferred hash and encryption algo setting + setDefaultHashAlgorithm(Constants.OpenKeychainHashAlgorithmTags.USE_PREFERRED); + setDefaultEncryptionAlgorithm(Constants.OpenKeychainSymmetricKeyAlgorithmTags.USE_PREFERRED); + } } // write new preference version -- cgit v1.2.3