From 5ff3043903456e4c626053bd918c9f61b84b100c Mon Sep 17 00:00:00 2001
From: Vincent Breitmoser <valodim@mugenguild.com>
Date: Sat, 30 Aug 2014 17:00:58 +0200
Subject: canonicalize: add check for algorithm type

closes #797
---
 .../keychain/pgp/UncachedKeyRing.java              | 34 ++++++++++++++++++++--
 .../keychain/service/OperationResultParcel.java    |  6 ++--
 2 files changed, 36 insertions(+), 4 deletions(-)

(limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain')

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 f00383e0f..9fcd21b4b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -19,6 +19,7 @@
 package org.sufficientlysecure.keychain.pgp;
 
 import org.spongycastle.bcpg.ArmoredOutputStream;
+import org.spongycastle.bcpg.PublicKeyAlgorithmTags;
 import org.spongycastle.bcpg.SignatureSubpacketTags;
 import org.spongycastle.bcpg.sig.KeyFlags;
 import org.spongycastle.openpgp.PGPKeyFlags;
@@ -219,6 +220,19 @@ public class UncachedKeyRing {
         aos.close();
     }
 
+    // An array of known algorithms. Note this must be numerically sorted for binarySearch() to work!
+    static final int[] KNOWN_ALGORITHMS = new int[] {
+        PublicKeyAlgorithmTags.RSA_GENERAL, // 1
+        PublicKeyAlgorithmTags.RSA_ENCRYPT, // 2
+        PublicKeyAlgorithmTags.RSA_SIGN, // 3
+        PublicKeyAlgorithmTags.ELGAMAL_ENCRYPT, // 16
+        PublicKeyAlgorithmTags.DSA, // 17
+        PublicKeyAlgorithmTags.ECDH, // 18
+        PublicKeyAlgorithmTags.ECDSA, // 19
+        PublicKeyAlgorithmTags.ELGAMAL_GENERAL, // 20
+        // PublicKeyAlgorithmTags.DIFFIE_HELLMAN, // 21
+    };
+
     /** "Canonicalizes" a public key, removing inconsistencies in the process.
      *
      * More specifically:
@@ -250,7 +264,7 @@ public class UncachedKeyRing {
 
         // do not accept v3 keys
         if (getVersion() <= 3) {
-            log.add(LogLevel.ERROR, LogType.MSG_KC_V3_KEY, indent);
+            log.add(LogLevel.ERROR, LogType.MSG_KC_ERROR_V3, indent);
             return null;
         }
 
@@ -262,6 +276,12 @@ public class UncachedKeyRing {
         PGPPublicKey masterKey = mRing.getPublicKey();
         final long masterKeyId = masterKey.getKeyID();
 
+        if (Arrays.binarySearch(KNOWN_ALGORITHMS, masterKey.getAlgorithm()) < 0) {
+            log.add(LogLevel.ERROR, LogType.MSG_KC_ERROR_MASTER_ALGO, indent,
+                    Integer.toString(masterKey.getAlgorithm()));
+            return null;
+        }
+
         {
             log.add(LogLevel.DEBUG, LogType.MSG_KC_MASTER,
                     indent, PgpKeyHelper.convertKeyIdToHex(masterKey.getKeyID()));
@@ -490,7 +510,7 @@ public class UncachedKeyRing {
 
             // If NO user ids remain, error out!
             if (!modified.getUserIDs().hasNext()) {
-                log.add(LogLevel.ERROR, LogType.MSG_KC_FATAL_NO_UID, indent);
+                log.add(LogLevel.ERROR, LogType.MSG_KC_ERROR_NO_UID, indent);
                 return null;
             }
 
@@ -513,6 +533,16 @@ public class UncachedKeyRing {
             log.add(LogLevel.DEBUG, LogType.MSG_KC_SUB,
                     indent, PgpKeyHelper.convertKeyIdToHex(key.getKeyID()));
             indent += 1;
+
+            if (Arrays.binarySearch(KNOWN_ALGORITHMS, key.getAlgorithm()) < 0) {
+                ring = removeSubKey(ring, key);
+
+                log.add(LogLevel.ERROR, LogType.MSG_KC_SUB_UNKNOWN_ALGO, indent,
+                        Integer.toString(key.getAlgorithm()));
+                indent -= 1;
+                continue;
+            }
+
             // A subkey needs exactly one subkey binding certificate, and optionally one revocation
             // certificate.
             PGPPublicKey modified = key;
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
index 142bf65cc..fe699224b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -287,10 +287,11 @@ public class OperationResultParcel implements Parcelable {
         MSG_IS_SUCCESS (R.string.msg_is_success),
 
         // keyring canonicalization
-        MSG_KC_V3_KEY (R.string.msg_kc_v3_key),
         MSG_KC_PUBLIC (R.string.msg_kc_public),
         MSG_KC_SECRET (R.string.msg_kc_secret),
-        MSG_KC_FATAL_NO_UID (R.string.msg_kc_fatal_no_uid),
+        MSG_KC_ERROR_V3 (R.string.msg_kc_error_v3),
+        MSG_KC_ERROR_NO_UID (R.string.msg_kc_error_no_uid),
+        MSG_KC_ERROR_MASTER_ALGO (R.string.msg_kc_error_master_algo),
         MSG_KC_MASTER (R.string.msg_kc_master),
         MSG_KC_REVOKE_BAD_ERR (R.string.msg_kc_revoke_bad_err),
         MSG_KC_REVOKE_BAD_LOCAL (R.string.msg_kc_revoke_bad_local),
@@ -314,6 +315,7 @@ public class OperationResultParcel implements Parcelable {
         MSG_KC_SUB_REVOKE_BAD_ERR (R.string.msg_kc_sub_revoke_bad_err),
         MSG_KC_SUB_REVOKE_BAD (R.string.msg_kc_sub_revoke_bad),
         MSG_KC_SUB_REVOKE_DUP (R.string.msg_kc_sub_revoke_dup),
+        MSG_KC_SUB_UNKNOWN_ALGO (R.string.msg_kc_sub_unknown_algo),
         MSG_KC_SUCCESS_BAD (R.plurals.msg_kc_success_bad),
         MSG_KC_SUCCESS_BAD_AND_RED (R.string.msg_kc_success_bad_and_red),
         MSG_KC_SUCCESS_REDUNDANT (R.plurals.msg_kc_success_redundant),
-- 
cgit v1.2.3