From dba145f5dfb7f7a43be41b3d01eeb68c4253ae7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Thu, 30 Jul 2015 22:46:36 +0200 Subject: Follow some of coorus recommendations: better selection of algo whitelist, ignore recipients preferred algos --- .../operations/results/OperationResult.java | 1 - .../keychain/pgp/CanonicalizedSecretKey.java | 10 ---- .../keychain/pgp/PgpConstants.java | 68 ++++++++++++++++------ .../keychain/pgp/PgpSignEncryptOperation.java | 13 +---- 4 files changed, 53 insertions(+), 39 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain') 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 04013e9ed..b063e1e74 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 @@ -660,7 +660,6 @@ public abstract class OperationResult implements Parcelable { MSG_PSE_COMPRESSING (LogLevel.DEBUG, R.string.msg_pse_compressing), MSG_PSE_ENCRYPTING (LogLevel.DEBUG, R.string.msg_pse_encrypting), MSG_PSE_ERROR_BAD_PASSPHRASE (LogLevel.ERROR, R.string.msg_pse_error_bad_passphrase), - MSG_PSE_ERROR_HASH_ALGO (LogLevel.ERROR, R.string.msg_pse_error_hash_algo), MSG_PSE_ERROR_IO (LogLevel.ERROR, R.string.msg_pse_error_io), MSG_PSE_ERROR_SIGN_KEY(LogLevel.ERROR, R.string.msg_pse_error_sign_key), MSG_PSE_ERROR_KEY_SIGN (LogLevel.ERROR, R.string.msg_pse_error_key_sign), 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 4aa05c9f8..23f02ac08 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java @@ -177,16 +177,6 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { return true; } - /** - * Returns a list of all supported hash algorithms. - */ - public ArrayList getSupportedHashAlgorithms() { - // TODO: intersection between preferred hash algos of this key and PgpConstants.PREFERRED_HASH_ALGORITHMS - // choose best algo - - return new ArrayList<>(); - } - private PGPContentSignerBuilder getContentSignerBuilder(int hashAlgo, Map signedHashes) { if (mPrivateKeyState == PRIVATE_KEY_STATE_DIVERT_TO_CARD) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpConstants.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpConstants.java index 7c009921d..7162a3366 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpConstants.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpConstants.java @@ -29,6 +29,8 @@ import java.util.HashSet; * - ECC: 224 bit * - Symmetric: 3TDEA * - Digital Signature (hash A): SHA-224 - SHA-512 + * + * Many decisions are based on https://gist.github.com/coruus/68a8c65571e2b4225a69 */ public class PgpConstants { @@ -39,26 +41,36 @@ public class PgpConstants { // } // https://tools.ietf.org/html/rfc6637#section-13 + /* + PgpDecryptVerify: Secure Algorithms Whitelist + all other algorithms will be rejected with OpenPgpDecryptionResult.RESULT_INSECURE - // PgpDecryptVerify: Secure Algorithms Whitelist - // all other algorithms will be rejected with OpenPgpDecryptionResult.RESULT_INSECURE + No broken ciphers or ciphers with key length smaller than 128 bit are allowed! + */ public static HashSet sSymmetricAlgorithmsWhitelist = new HashSet<>(); static { sSymmetricAlgorithmsWhitelist.add(SymmetricKeyAlgorithmTags.AES_256); sSymmetricAlgorithmsWhitelist.add(SymmetricKeyAlgorithmTags.AES_192); sSymmetricAlgorithmsWhitelist.add(SymmetricKeyAlgorithmTags.AES_128); - sSymmetricAlgorithmsWhitelist.add(SymmetricKeyAlgorithmTags.TWOFISH); + sSymmetricAlgorithmsWhitelist.add(SymmetricKeyAlgorithmTags.TWOFISH); // 128 bit } // all other algorithms will be rejected with OpenPgpSignatureResult.RESULT_INVALID_INSECURE public static HashSet sHashAlgorithmsWhitelist = new HashSet<>(); static { - sHashAlgorithmsWhitelist.add(HashAlgorithmTags.SHA256); sHashAlgorithmsWhitelist.add(HashAlgorithmTags.SHA512); sHashAlgorithmsWhitelist.add(HashAlgorithmTags.SHA384); + /* + TODO: SHA256 and SHA224 are still allowed even though + coruus advises against it, to enable better backward compatibility + + coruus: + Implementations MUST NOT sign SHA-224 hashes. They SHOULD NOT accept signatures over SHA-224 hashes. + ((collision resistance of 112-bits)) + Implementations SHOULD NOT sign SHA-256 hashes. They MUST NOT default to signing SHA-256 hashes. + */ + sHashAlgorithmsWhitelist.add(HashAlgorithmTags.SHA256); sHashAlgorithmsWhitelist.add(HashAlgorithmTags.SHA224); - sHashAlgorithmsWhitelist.add(HashAlgorithmTags.SHA1); - sHashAlgorithmsWhitelist.add(HashAlgorithmTags.RIPEMD160); } /* @@ -70,15 +82,15 @@ public class PgpConstants { SymmetricKeyAlgorithmTags.AES_256, SymmetricKeyAlgorithmTags.AES_192, SymmetricKeyAlgorithmTags.AES_128, - SymmetricKeyAlgorithmTags.TWOFISH }; - // NOTE: some implementations do not support SHA512, thus we choose SHA256 as default (Mailvelope?) + /* + coorus: + Implementations SHOULD use SHA-512 for RSA or DSA signatures. They SHOULD NOT use SHA-384. + ((cite to affine padding attacks; unproven status of RSA-PKCSv15)) + */ public static final int[] PREFERRED_HASH_ALGORITHMS = new int[]{ - HashAlgorithmTags.SHA256, HashAlgorithmTags.SHA512, - HashAlgorithmTags.SHA384, - HashAlgorithmTags.SHA224, }; /* @@ -89,19 +101,41 @@ public class PgpConstants { */ public static final int[] PREFERRED_COMPRESSION_ALGORITHMS = new int[]{ CompressionAlgorithmTags.ZIP, - CompressionAlgorithmTags.ZLIB, - CompressionAlgorithmTags.BZIP2 }; - public static final int CERTIFY_HASH_ALGO = HashAlgorithmTags.SHA256; + public static final int CERTIFY_HASH_ALGO = HashAlgorithmTags.SHA512; + + /* + Always use AES-256! Ignore the preferred encryption algos of the recipient! + + coorus: + Implementations SHOULD ignore the symmetric algorithm preferences of a recipient's public key; + in particular, implementations MUST NOT choose an algorithm forbidden by this + document because a recipient prefers it. + NEEDCITE downgrade attacks on TLS, other protocols + */ public static final int DEFAULT_SYMMETRIC_ALGORITHM = SymmetricKeyAlgorithmTags.AES_256; public interface OpenKeychainSymmetricKeyAlgorithmTags extends SymmetricKeyAlgorithmTags { int USE_DEFAULT = -1; } - public static final int DEFAULT_HASH_ALGORITHM = HashAlgorithmTags.SHA256; + /* + Always use SHA-512! Ignore the preferred hash algos of the recipient! + + coorus: + Implementations MUST ignore the hash algorithm preferences of a recipient when signing + a message to a recipient. The difficulty of forging a signature under a given key, + using generic attacks on hash functions, is the difficulty of the weakest hash signed by that key. + + Implementations MUST default to using SHA-512 for RSA signatures, + + and either SHA-512 or the matched instance of SHA-2 for ECDSA signatures. + TODO: Ed25519 + CITE: zooko's hash function table CITE: distinguishers on SHA-256 + */ + public static final int DEFAULT_HASH_ALGORITHM = HashAlgorithmTags.SHA512; public interface OpenKeychainHashAlgorithmTags extends HashAlgorithmTags { int USE_DEFAULT = -1; } @@ -130,9 +164,9 @@ public class PgpConstants { * OpenKeychain: 0x90 */ public static final int SECRET_KEY_ENCRYPTOR_S2K_COUNT = 0x90; - public static final int SECRET_KEY_ENCRYPTOR_HASH_ALGO = HashAlgorithmTags.SHA256; + public static final int SECRET_KEY_ENCRYPTOR_HASH_ALGO = HashAlgorithmTags.SHA512; public static final int SECRET_KEY_ENCRYPTOR_SYMMETRIC_ALGO = SymmetricKeyAlgorithmTags.AES_256; - public static final int SECRET_KEY_BINDING_SIGNATURE_HASH_ALGO = HashAlgorithmTags.SHA256; + public static final int SECRET_KEY_BINDING_SIGNATURE_HASH_ALGO = HashAlgorithmTags.SHA512; // NOTE: only SHA1 is supported for key checksum calculations in OpenPGP, // see http://tools.ietf.org/html/rfc488 0#section-5.5.3 public static final int SECRET_KEY_SIGNATURE_CHECKSUM_HASH_ALGO = HashAlgorithmTags.SHA1; 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 a2c5fa30d..8701c27dc 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java @@ -226,17 +226,10 @@ public class PgpSignEncryptOperation extends BaseOperation { return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log); } - // Use preferred hash algo + // Use requested hash algo int requestedAlgorithm = input.getSignatureHashAlgorithm(); - ArrayList supported = signingKey.getSupportedHashAlgorithms(); if (requestedAlgorithm == PgpConstants.OpenKeychainHashAlgorithmTags.USE_DEFAULT) { input.setSignatureHashAlgorithm(PgpConstants.DEFAULT_HASH_ALGORITHM); - // TODO - // get most preferred -// input.setSignatureHashAlgorithm(supported.get(0)); - } else if (!supported.contains(requestedAlgorithm)) { - log.add(LogType.MSG_PSE_ERROR_HASH_ALGO, indent); - return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log); } } updateProgress(R.string.progress_preparing_streams, 2, 100); @@ -245,11 +238,9 @@ public class PgpSignEncryptOperation extends BaseOperation { PGPEncryptedDataGenerator cPk = null; if (enableEncryption) { - // Use preferred encryption algo + // Use requested encryption algo int algo = input.getSymmetricEncryptionAlgorithm(); if (algo == PgpConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT) { - // get most preferred - // TODO: get from recipients algo = PgpConstants.DEFAULT_SYMMETRIC_ALGORITHM; } // has Integrity packet enabled! -- cgit v1.2.3