reject keys with duplicate subkeys altogether

closes #870

3 files changed, 17 insertions, 1 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
index cde25c19d..7bf16791d 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -51,6 +51,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 import java.util.TreeSet;
@@ -529,12 +530,25 @@ public class UncachedKeyRing {
 
         }
 
+        // Keep track of ids we encountered so far
+        Set<Long> knownIds = new HashSet<Long>();
+
         // Process all keys
         for (PGPPublicKey key : new IterableIterator<PGPPublicKey>(ring.getPublicKeys())) {
-            // Don't care about the master key here, that one gets special treatment above
+            // Make sure this is not a duplicate, avoid undefined behavior!
+            if (knownIds.contains(key.getKeyID())) {
+                log.add(LogType.MSG_KC_ERROR_DUP_KEY, indent,
+                        KeyFormattingUtils.convertKeyIdToHex(key.getKeyID()));
+                return null;
+            }
+            // Add the key id to known
+            knownIds.add(key.getKeyID());
+
+            // Don't care about the master key any further, that one gets special treatment above
             if (key.isMasterKey()) {
                 continue;
             }
+
             log.add(LogType.MSG_KC_SUB,
                     indent, KeyFormattingUtils.convertKeyIdToHex(key.getKeyID()));
             indent += 1;
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java
index 69516eafd..beaa6f2ba 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java
@@ -316,6 +316,7 @@ public abstract class OperationResult implements Parcelable {
         MSG_KC_ERROR_V3 (LogLevel.ERROR, R.string.msg_kc_error_v3),
         MSG_KC_ERROR_NO_UID (LogLevel.ERROR, R.string.msg_kc_error_no_uid),
         MSG_KC_ERROR_MASTER_ALGO (LogLevel.ERROR, R.string.msg_kc_error_master_algo),
+        MSG_KC_ERROR_DUP_KEY (LogLevel.ERROR, R.string.msg_kc_error_dup_key),
         MSG_KC_MASTER (LogLevel.DEBUG, R.string.msg_kc_master),
         MSG_KC_REVOKE_BAD_ERR (LogLevel.WARN, R.string.msg_kc_revoke_bad_err),
         MSG_KC_REVOKE_BAD_LOCAL (LogLevel.WARN, R.string.msg_kc_revoke_bad_local),
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 28a1507f5..ea55a8932 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -643,6 +643,7 @@
     <string name="msg_kc_error_v3">"This is an OpenPGP version 3 key, which has been deprecated and is no longer supported!"</string>
     <string name="msg_kc_error_no_uid">"Keyring has no valid user ids!"</string>
     <string name="msg_kc_error_master_algo">"The master key uses an unknown (%s) algorithm!"</string>
+    <string name="msg_kc_error_dup_key">"Subkey %s occurs twice in keyring. Keyring is malformed, not importing!"</string>
     <string name="msg_kc_master">"Processing master key"</string>
     <string name="msg_kc_revoke_bad_err">"Removing bad keyring revocation certificate"</string>
     <string name="msg_kc_revoke_bad_local">"Removing keyring revocation certificate with "local" flag"</string>