From c1ba764ce8d85297601e20d9c3d82acbd17be63c Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 27 Apr 2017 21:48:30 +0200 Subject: [PATCH] change security problem structure --- .../results/DecryptVerifyResult.java | 37 +++---------- .../pgp/DecryptVerifySecurityProblem.java | 54 +++++++++++++++++++ .../pgp/OpenPgpDecryptionResultBuilder.java | 29 +++------- .../pgp/OpenPgpSignatureResultBuilder.java | 21 +++----- .../pgp/PgpDecryptVerifyOperation.java | 32 +++++++---- .../keychain/pgp/PgpSecurityConstants.java | 14 ++--- .../keychain/pgp/PgpSignatureChecker.java | 41 +++++++------- .../keychain/pgp/SecurityProblem.java | 12 ++--- .../remote/ApiPendingIntentFactory.java | 6 +-- .../keychain/remote/OpenPgpService.java | 11 ++-- .../keychain/ui/util/KeyFormattingUtils.java | 17 +++--- 11 files changed, 150 insertions(+), 124 deletions(-) create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/DecryptVerifySecurityProblem.java diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java index 65f36cc70..fc9d22210 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/DecryptVerifyResult.java @@ -19,16 +19,12 @@ package org.sufficientlysecure.keychain.operations.results; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - import android.os.Parcel; import org.openintents.openpgp.OpenPgpDecryptionResult; import org.openintents.openpgp.OpenPgpMetadata; import org.openintents.openpgp.OpenPgpSignatureResult; -import org.sufficientlysecure.keychain.pgp.SecurityProblem; +import org.sufficientlysecure.keychain.pgp.DecryptVerifySecurityProblem; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel; @@ -40,7 +36,7 @@ public class DecryptVerifyResult extends InputPendingResult { OpenPgpSignatureResult mSignatureResult; OpenPgpDecryptionResult mDecryptionResult; OpenPgpMetadata mDecryptionMetadata; - ArrayList mSecurityProblems; + DecryptVerifySecurityProblem mSecurityProblem; CryptoInputParcel mCachedCryptoInputParcel; @@ -73,7 +69,7 @@ public class DecryptVerifyResult extends InputPendingResult { mCachedCryptoInputParcel = source.readParcelable(CryptoInputParcel.class.getClassLoader()); mSkippedDisallowedKeys = source.createLongArray(); - mSecurityProblems = (ArrayList) source.readSerializable(); + mSecurityProblem = (DecryptVerifySecurityProblem) source.readSerializable(); } @@ -137,7 +133,7 @@ public class DecryptVerifyResult extends InputPendingResult { dest.writeParcelable(mCachedCryptoInputParcel, flags); dest.writeLongArray(mSkippedDisallowedKeys); - dest.writeSerializable(mSecurityProblems); + dest.writeSerializable(mSecurityProblem); } public static final Creator CREATOR = new Creator() { @@ -150,28 +146,11 @@ public class DecryptVerifyResult extends InputPendingResult { } }; - public void addSecurityProblem(SecurityProblem securityProblem) { - if (securityProblem == null) { - return; - } - if (mSecurityProblems == null) { - mSecurityProblems = new ArrayList<>(); - } - mSecurityProblems.add(securityProblem); + public DecryptVerifySecurityProblem getSecurityProblem() { + return mSecurityProblem; } - public void addSecurityProblems(List securityProblems) { - if (securityProblems == null) { - return; - } - if (mSecurityProblems == null) { - mSecurityProblems = new ArrayList<>(); - } - mSecurityProblems.addAll(securityProblems); - } - - public List getSecurityProblems() { - return mSecurityProblems != null ? - Collections.unmodifiableList(mSecurityProblems) : Collections.emptyList(); + public void setSecurityProblemResult(DecryptVerifySecurityProblem securityProblem) { + mSecurityProblem = securityProblem; } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/DecryptVerifySecurityProblem.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/DecryptVerifySecurityProblem.java new file mode 100644 index 000000000..36b90b93f --- /dev/null +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/DecryptVerifySecurityProblem.java @@ -0,0 +1,54 @@ +package org.sufficientlysecure.keychain.pgp; + + +import java.io.Serializable; + +import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureSigningAlgorithm; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.KeySecurityProblem; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.EncryptionAlgorithmProblem; + + +public class DecryptVerifySecurityProblem implements Serializable { + public final KeySecurityProblem encryptionKeySecurityProblem; + public final KeySecurityProblem signingKeySecurityProblem; + public final EncryptionAlgorithmProblem symmetricSecurityProblem; + public final InsecureSigningAlgorithm signatureSecurityProblem; + + private DecryptVerifySecurityProblem(DecryptVerifySecurityProblemBuilder builder) { + encryptionKeySecurityProblem = builder.encryptionKeySecurityProblem; + signingKeySecurityProblem = builder.signingKeySecurityProblem; + symmetricSecurityProblem = builder.symmetricSecurityProblem; + signatureSecurityProblem = builder.signatureSecurityProblem; + } + + static class DecryptVerifySecurityProblemBuilder { + private KeySecurityProblem encryptionKeySecurityProblem; + private KeySecurityProblem signingKeySecurityProblem; + private EncryptionAlgorithmProblem symmetricSecurityProblem; + private InsecureSigningAlgorithm signatureSecurityProblem; + + void addEncryptionKeySecurityProblem(KeySecurityProblem encryptionKeySecurityProblem) { + this.encryptionKeySecurityProblem = encryptionKeySecurityProblem; + } + + void addSigningKeyProblem(KeySecurityProblem keySecurityProblem) { + this.signingKeySecurityProblem = keySecurityProblem; + } + + void addSymmetricSecurityProblem(EncryptionAlgorithmProblem symmetricSecurityProblem) { + this.symmetricSecurityProblem = symmetricSecurityProblem; + } + + void addSignatureSecurityProblem(InsecureSigningAlgorithm signatureSecurityProblem) { + this.signatureSecurityProblem = signatureSecurityProblem; + } + + public DecryptVerifySecurityProblem build() { + if (encryptionKeySecurityProblem == null && signingKeySecurityProblem == null && + symmetricSecurityProblem == null && signatureSecurityProblem == null) { + return null; + } + return new DecryptVerifySecurityProblem(this); + } + } +} diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpDecryptionResultBuilder.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpDecryptionResultBuilder.java index e2c0b6b63..199f7c445 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpDecryptionResultBuilder.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpDecryptionResultBuilder.java @@ -17,33 +17,20 @@ package org.sufficientlysecure.keychain.pgp; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - import org.openintents.openpgp.OpenPgpDecryptionResult; import org.sufficientlysecure.keychain.Constants; -import org.sufficientlysecure.keychain.pgp.SecurityProblem.SymmetricAlgorithmProblem; import org.sufficientlysecure.keychain.util.Log; -public class OpenPgpDecryptionResultBuilder { +class OpenPgpDecryptionResultBuilder { // builder + private boolean isInsecure = false; private boolean isEncrypted = false; private byte[] sessionKey; private byte[] decryptedSessionKey; - private ArrayList securityProblems; - public void addSecurityProblem(SecurityProblem securityProblem) { - if (securityProblems == null) { - securityProblems = new ArrayList<>(); - } - securityProblems.add(securityProblem); - } - - public List getKeySecurityProblems() { - return securityProblems != null ? Collections.unmodifiableList(securityProblems) : null; + public void setInsecure(boolean insecure) { + this.isInsecure = insecure; } public void setEncrypted(boolean encrypted) { @@ -51,15 +38,14 @@ public class OpenPgpDecryptionResultBuilder { } public OpenPgpDecryptionResult build() { - if (securityProblems != null && !securityProblems.isEmpty()) { + if (isInsecure) { Log.d(Constants.TAG, "RESULT_INSECURE"); return new OpenPgpDecryptionResult(OpenPgpDecryptionResult.RESULT_INSECURE, sessionKey, decryptedSessionKey); } if (isEncrypted) { Log.d(Constants.TAG, "RESULT_ENCRYPTED"); - return new OpenPgpDecryptionResult( - OpenPgpDecryptionResult.RESULT_ENCRYPTED, sessionKey, decryptedSessionKey); + return new OpenPgpDecryptionResult(OpenPgpDecryptionResult.RESULT_ENCRYPTED, sessionKey, decryptedSessionKey); } Log.d(Constants.TAG, "RESULT_NOT_ENCRYPTED"); @@ -67,12 +53,11 @@ public class OpenPgpDecryptionResultBuilder { } - public void setSessionKey(byte[] sessionKey, byte[] decryptedSessionKey) { + void setSessionKey(byte[] sessionKey, byte[] decryptedSessionKey) { if ((sessionKey == null) != (decryptedSessionKey == null)) { throw new AssertionError("sessionKey must be null iff decryptedSessionKey is null!"); } this.sessionKey = sessionKey; this.decryptedSessionKey = decryptedSessionKey; } - } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java index 83b99776f..9c04c5394 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OpenPgpSignatureResultBuilder.java @@ -19,9 +19,7 @@ package org.sufficientlysecure.keychain.pgp; import java.util.ArrayList; -import java.util.Collections; import java.util.Date; -import java.util.List; import org.openintents.openpgp.OpenPgpSignatureResult; import org.openintents.openpgp.OpenPgpSignatureResult.SenderStatusResult; @@ -55,9 +53,9 @@ public class OpenPgpSignatureResultBuilder { private boolean mIsSignatureKeyCertified = false; private boolean mIsKeyRevoked = false; private boolean mIsKeyExpired = false; + private boolean mInsecure = false; private String mSenderAddress; private Date mSignatureTimestamp; - private ArrayList mSecurityProblems; public OpenPgpSignatureResultBuilder(KeyRepository keyRepository) { this.mKeyRepository = keyRepository; @@ -83,15 +81,8 @@ public class OpenPgpSignatureResultBuilder { this.mValidSignature = validSignature; } - public void addSecurityProblem(SecurityProblem securityProblem) { - if (mSecurityProblems == null) { - mSecurityProblems = new ArrayList<>(); - } - mSecurityProblems.add(securityProblem); - } - - public List getSecurityProblems() { - return mSecurityProblems != null ? Collections.unmodifiableList(mSecurityProblems) : null; + public void setInsecure(boolean insecure) { + this.mInsecure = insecure; } public void setSignatureKeyCertified(boolean isSignatureKeyCertified) { @@ -115,6 +106,10 @@ public class OpenPgpSignatureResultBuilder { this.mConfirmedUserIds = confirmedUserIds; } + public boolean isInsecure() { + return mInsecure; + } + public void initValid(CanonicalizedPublicKey signingKey) { setSignatureAvailable(true); setKnownKey(true); @@ -189,7 +184,7 @@ public class OpenPgpSignatureResultBuilder { } else if (mIsKeyExpired) { Log.d(Constants.TAG, "RESULT_INVALID_KEY_EXPIRED"); signatureStatus = OpenPgpSignatureResult.RESULT_INVALID_KEY_EXPIRED; - } else if (mSecurityProblems != null && !mSecurityProblems.isEmpty()) { + } else if (mInsecure) { Log.d(Constants.TAG, "RESULT_INVALID_INSECURE"); signatureStatus = OpenPgpSignatureResult.RESULT_INVALID_KEY_INSECURE; } else if (mIsSignatureKeyCertified) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerifyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerifyOperation.java index d728b185c..bd7118250 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerifyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerifyOperation.java @@ -68,9 +68,11 @@ import org.sufficientlysecure.keychain.operations.results.DecryptVerifyResult; import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType; import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType; +import org.sufficientlysecure.keychain.pgp.DecryptVerifySecurityProblem.DecryptVerifySecurityProblemBuilder; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.InsecureBitStrength; import org.sufficientlysecure.keychain.pgp.SecurityProblem.KeySecurityProblem; import org.sufficientlysecure.keychain.pgp.SecurityProblem.MissingMdc; -import org.sufficientlysecure.keychain.pgp.SecurityProblem.SymmetricAlgorithmProblem; +import org.sufficientlysecure.keychain.pgp.SecurityProblem.EncryptionAlgorithmProblem; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException; import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing; @@ -301,6 +303,7 @@ public class PgpDecryptVerifyOperation extends BaseOperation getSecurityProblems() { - return signatureResultBuilder.getSecurityProblems(); - } - /** * Mostly taken from ClearSignedFileProcessor in Bouncy Castle */ diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/SecurityProblem.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/SecurityProblem.java index 58819636a..f97d60c30 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/SecurityProblem.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/SecurityProblem.java @@ -34,7 +34,7 @@ public abstract class SecurityProblem implements Serializable { } } - public static abstract class SymmetricAlgorithmProblem extends SecurityProblem { + public static abstract class EncryptionAlgorithmProblem extends SecurityProblem { } @@ -62,23 +62,23 @@ public abstract class SecurityProblem implements Serializable { } } - public static class InsecureHashAlgorithm extends SecurityProblem { + public static class InsecureSigningAlgorithm extends SecurityProblem { public final int hashAlgorithm; - InsecureHashAlgorithm(int hashAlgorithm) { + InsecureSigningAlgorithm(int hashAlgorithm) { this.hashAlgorithm = hashAlgorithm; } } - public static class InsecureSymmetricAlgorithm extends SymmetricAlgorithmProblem { + public static class InsecureEncryptionAlgorithm extends EncryptionAlgorithmProblem { public final int symmetricAlgorithm; - InsecureSymmetricAlgorithm(int symmetricAlgorithm) { + InsecureEncryptionAlgorithm(int symmetricAlgorithm) { this.symmetricAlgorithm = symmetricAlgorithm; } } - public static class MissingMdc extends SymmetricAlgorithmProblem { + public static class MissingMdc extends EncryptionAlgorithmProblem { } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java index e9d5e87ec..a6a93e404 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java @@ -25,7 +25,7 @@ import android.content.Context; import android.content.Intent; import android.os.Build; -import org.sufficientlysecure.keychain.pgp.SecurityProblem; +import org.sufficientlysecure.keychain.pgp.DecryptVerifySecurityProblem; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.remote.ui.RemoteBackupActivity; import org.sufficientlysecure.keychain.remote.ui.RemoteErrorActivity; @@ -143,10 +143,10 @@ public class ApiPendingIntentFactory { return createInternal(data, intent); } - PendingIntent createSecurityProblemIntent(String packageName, SecurityProblem keySecurityProblem) { + PendingIntent createSecurityProblemIntent(String packageName, DecryptVerifySecurityProblem securityProblem) { Intent intent = new Intent(mContext, RemoteSecurityProblemDialogActivity.class); intent.putExtra(RemoteSecurityProblemDialogActivity.EXTRA_PACKAGE_NAME, packageName); - intent.putExtra(RemoteSecurityProblemDialogActivity.EXTRA_SECURITY_PROBLEM, keySecurityProblem); + intent.putExtra(RemoteSecurityProblemDialogActivity.EXTRA_SECURITY_PROBLEM, securityProblem); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { //noinspection ResourceType, looks like lint is missing FLAG_IMMUTABLE 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 46192a4d4..e634be9b6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -53,6 +53,7 @@ import org.sufficientlysecure.keychain.operations.results.ExportResult; import org.sufficientlysecure.keychain.operations.results.OperationResult.LogEntryParcel; import org.sufficientlysecure.keychain.operations.results.PgpSignEncryptResult; 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; @@ -60,7 +61,6 @@ import org.sufficientlysecure.keychain.pgp.PgpSignEncryptData; import org.sufficientlysecure.keychain.pgp.PgpSignEncryptInputParcel; import org.sufficientlysecure.keychain.pgp.PgpSignEncryptOperation; import org.sufficientlysecure.keychain.pgp.Progressable; -import org.sufficientlysecure.keychain.pgp.SecurityProblem; import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException; import org.sufficientlysecure.keychain.provider.ApiDataAccessObject; import org.sufficientlysecure.keychain.provider.KeyRepository; @@ -430,17 +430,14 @@ public class OpenPgpService extends Service { } private void processSecurityProblemsPendingIntent(Intent result, DecryptVerifyResult decryptVerifyResult) { - List securityProblems = decryptVerifyResult.getSecurityProblems(); - if (securityProblems.isEmpty()) { + DecryptVerifySecurityProblem securityProblem = decryptVerifyResult.getSecurityProblem(); + if (securityProblem == null) { return; } - // TODO what if there is multiple? - SecurityProblem keySecurityProblem = securityProblems.get(0); - String packageName = mApiPermissionHelper.getCurrentCallingPackage(); result.putExtra(OpenPgpApi.RESULT_INSECURE_DETAIL_INTENT, - mApiPendingIntentFactory.createSecurityProblemIntent(packageName, keySecurityProblem)); + mApiPendingIntentFactory.createSecurityProblemIntent(packageName, securityProblem)); } private void processDecryptionResultForResultIntent(int targetApiVersion, Intent result, diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java index c6c4b6954..a884fcb32 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java @@ -18,6 +18,15 @@ package org.sufficientlysecure.keychain.ui.util; + +import java.math.BigInteger; +import java.nio.ByteBuffer; +import java.security.DigestException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Collection; +import java.util.Locale; + import android.content.Context; import android.content.res.Resources; import android.graphics.Color; @@ -48,14 +57,6 @@ import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm; import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Curve; import org.sufficientlysecure.keychain.util.Log; -import java.math.BigInteger; -import java.nio.ByteBuffer; -import java.security.DigestException; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.util.Collection; -import java.util.Locale; - public class KeyFormattingUtils { public static String getAlgorithmInfo(int algorithm, Integer keySize, String oid) {