Bug 1638645 - OpenPGP: Missing MDC protection not discovered. r=PatrickBrunschwig a=wsmwk
authorKai Engert <kaie@kuix.de>
Sat, 27 Jun 2020 22:32:55 +0200
changeset 39478 967707e1200c71b210cc9018dd10e8f773c33adf
parent 39477 16ffc077a58d10cc44d72faa1d9fe322eedc36a9
child 39479 7783b3d24305817f20a3ac803be3e4eeb4e15d9a
push id402
push userclokep@gmail.com
push dateMon, 29 Jun 2020 20:48:04 +0000
reviewersPatrickBrunschwig, wsmwk
bugs1638645
Bug 1638645 - OpenPGP: Missing MDC protection not discovered. r=PatrickBrunschwig a=wsmwk Differential Revision: https://phabricator.services.mozilla.com/D81446
mail/extensions/openpgp/content/modules/RNP.jsm
mail/extensions/openpgp/content/modules/RNPLib.jsm
mail/extensions/openpgp/content/modules/mimeDecrypt.jsm
--- a/mail/extensions/openpgp/content/modules/RNP.jsm
+++ b/mail/extensions/openpgp/content/modules/RNP.jsm
@@ -284,16 +284,53 @@ var RNP = {
   getSecretAvailableFromHandle(handle) {
     let have_secret = new ctypes.bool();
     if (RNPLib.rnp_key_have_secret(handle, have_secret.address())) {
       throw new Error("rnp_key_have_secret failed");
     }
     return have_secret.value;
   },
 
+  // We already know sub_handle is a subkey
+  getPrimaryKeyHandleFromSub(ffi, sub_handle) {
+    let newHandle = new RNPLib.rnp_key_handle_t();
+    // test my expectation is correct
+    if (!newHandle.isNull()) {
+      throw new Error("unexpected, new handle isn't null");
+    }
+    let primary_grip = new ctypes.char.ptr();
+    if (RNPLib.rnp_key_get_primary_grip(sub_handle, primary_grip.address())) {
+      throw new Error("rnp_key_get_primary_grip failed");
+    }
+    if (primary_grip.isNull()) {
+      return newHandle;
+    }
+    if (RNPLib.rnp_locate_key(ffi, "grip", primary_grip, newHandle.address())) {
+      throw new Error("rnp_locate_key failed");
+    }
+    return newHandle;
+  },
+
+  // We don't know if handle is a subkey. If it's not, return null handle
+  getPrimaryKeyHandleIfSub(ffi, handle) {
+    let is_subkey = new ctypes.bool();
+    if (RNPLib.rnp_key_is_sub(handle, is_subkey.address())) {
+      throw new Error("rnp_key_is_sub failed");
+    }
+    if (!is_subkey.value) {
+      let nullHandle = new RNPLib.rnp_key_handle_t();
+      // test my expectation is correct
+      if (!nullHandle.isNull()) {
+        throw new Error("unexpected, new handle isn't null");
+      }
+      return nullHandle;
+    }
+    return this.getPrimaryKeyHandleFromSub(ffi, handle);
+  },
+
   // return false if handle refers to subkey and should be ignored
   getKeyInfoFromHandle(
     ffi,
     handle,
     keyObj,
     usePrimaryIfSubkey,
     forListing,
     onlyIfSecret
@@ -307,48 +344,34 @@ var RNP = {
     let is_subkey = new ctypes.bool();
     let sub_count = new ctypes.size_t();
     let uid_count = new ctypes.size_t();
 
     if (RNPLib.rnp_key_is_sub(handle, is_subkey.address())) {
       throw new Error("rnp_key_is_sub failed");
     }
     if (is_subkey.value) {
-      let primary_grip = new ctypes.char.ptr();
-      if (RNPLib.rnp_key_get_primary_grip(handle, primary_grip.address())) {
-        throw new Error("rnp_key_get_primary_grip failed");
+      if (!usePrimaryIfSubkey) {
+        return false;
       }
-      if (!primary_grip.isNull()) {
-        let rv = false;
-        if (usePrimaryIfSubkey) {
-          let newHandle = new RNPLib.rnp_key_handle_t();
-          if (
-            RNPLib.rnp_locate_key(
-              ffi,
-              "grip",
-              primary_grip,
-              newHandle.address()
-            )
-          ) {
-            throw new Error("rnp_locate_key failed");
-          }
-          // recursively call ourselves to get primary key info
-          rv = this.getKeyInfoFromHandle(
-            ffi,
-            newHandle,
-            keyObj,
-            false,
-            forListing,
-            onlyIfSecret
-          );
-          RNPLib.rnp_key_handle_destroy(newHandle);
-        }
-        RNPLib.rnp_buffer_destroy(primary_grip);
-        return rv;
+      let rv = false;
+      let newHandle = this.getPrimaryKeyHandleFromSub(ffi, handle);
+      if (!newHandle.isNull()) {
+        // recursively call ourselves to get primary key info
+        rv = this.getKeyInfoFromHandle(
+          ffi,
+          newHandle,
+          keyObj,
+          false,
+          forListing,
+          onlyIfSecret
+        );
+        RNPLib.rnp_key_handle_destroy(newHandle);
       }
+      return rv;
     }
 
     if (onlyIfSecret) {
       let have_secret = new ctypes.bool();
       if (RNPLib.rnp_key_have_secret(handle, have_secret.address())) {
         throw new Error("rnp_key_have_secret failed");
       }
       if (!have_secret.value) {
@@ -629,16 +652,22 @@ var RNP = {
     } catch (ex) {
       console.log(ex);
     } finally {
       RNPLib.rnp_key_handle_destroy(handle);
     }
     return rList;
   },
 
+  policyForbidsAlg(alg) {
+    // TODO: implement policy
+    // Currently, all algorithms are allowed
+    return false;
+  },
+
   async decrypt(encrypted, options) {
     let input_from_memory = new RNPLib.rnp_input_t();
 
     var tmp_array = ctypes.char.array()(encrypted);
     var encrypted_array = ctypes.cast(
       tmp_array,
       ctypes.uint8_t.array(encrypted.length)
     );
@@ -667,61 +696,141 @@ var RNP = {
     let verify_op = new RNPLib.rnp_op_verify_t();
     result.exitCode = RNPLib.rnp_op_verify_create(
       verify_op.address(),
       RNPLib.ffi,
       input_from_memory,
       output_to_memory
     );
 
-    RNPLib.password_cb_collected_info = {};
     result.exitCode = RNPLib.rnp_op_verify_execute(verify_op);
-    let rnplib_context = RNPLib.password_cb_collected_info;
-    if (rnplib_context.context == "decrypt") {
-      result.encToDetails = rnplib_context.keyId;
-    }
 
-    let useDecryptedData;
+    let isEncrypted = false;
+    let rejectedDecryption = false;
+
+    let useDecodedData;
     let processSignature;
     switch (result.exitCode) {
       case RNPLib.RNP_SUCCESS:
-        useDecryptedData = true;
+        useDecodedData = true;
         processSignature = true;
         break;
       case RNPLib.RNP_ERROR_SIGNATURE_INVALID:
         result.statusFlags |= EnigmailConstants.BAD_SIGNATURE;
-        useDecryptedData = true;
+        useDecodedData = true;
         processSignature = false;
         break;
       case RNPLib.RNP_ERROR_SIGNATURE_EXPIRED:
-        useDecryptedData = true;
+        useDecodedData = true;
         processSignature = false;
         result.statusFlags |= EnigmailConstants.EXPIRED_SIGNATURE;
         break;
       case RNPLib.RNP_ERROR_DECRYPT_FAILED:
-        useDecryptedData = false;
+        useDecodedData = false;
         processSignature = false;
         result.statusFlags |= EnigmailConstants.DECRYPTION_FAILED;
         break;
       case RNPLib.RNP_ERROR_NO_SUITABLE_KEY:
-        useDecryptedData = false;
+        useDecodedData = false;
         processSignature = false;
         result.statusFlags |=
           EnigmailConstants.DECRYPTION_FAILED | EnigmailConstants.NO_SECKEY;
         break;
       default:
-        useDecryptedData = false;
+        useDecodedData = false;
         processSignature = false;
         console.debug(
           "rnp_op_verify_execute returned unexpected: " + result.exitCode
         );
         break;
     }
 
-    if (useDecryptedData) {
+    if (useDecodedData) {
+      let prot_mode_str = new ctypes.char.ptr();
+      let prot_cipher_str = new ctypes.char.ptr();
+      let prot_is_valid = new ctypes.bool();
+
+      if (
+        RNPLib.rnp_op_verify_get_protection_info(
+          verify_op,
+          prot_mode_str.address(),
+          prot_cipher_str.address(),
+          prot_is_valid.address()
+        )
+      ) {
+        throw new Error("rnp_op_verify_get_protection_info failed");
+      }
+      let mode = prot_mode_str.readString();
+      let cipher = prot_cipher_str.readString();
+      let validIntegrityProtection = prot_is_valid.value;
+
+      if (mode != "none") {
+        isEncrypted = true;
+
+        if (!validIntegrityProtection) {
+          useDecodedData = false;
+          rejectedDecryption = true;
+          result.statusFlags |=
+            EnigmailConstants.MISSING_MDC | EnigmailConstants.DECRYPTION_FAILED;
+        } else if (mode == "null" || this.policyForbidsAlg(cipher)) {
+          // don't indicate decryption, because a non-protecting or insecure cipher was used
+          rejectedDecryption = true;
+          result.statusFlags |= EnigmailConstants.UNKNOWN_ALGO;
+        } else {
+          let recip_handle = new RNPLib.rnp_recipient_handle_t();
+          let rv = RNPLib.rnp_op_verify_get_used_recipient(
+            verify_op,
+            recip_handle.address()
+          );
+          if (rv) {
+            throw new Error("rnp_op_verify_get_used_recipient failed");
+          }
+
+          let c_alg = new ctypes.char.ptr();
+          rv = RNPLib.rnp_recipient_get_alg(recip_handle, c_alg.address());
+          if (rv) {
+            throw new Error("rnp_recipient_get_alg failed");
+          }
+
+          if (this.policyForbidsAlg(c_alg.readString())) {
+            rejectedDecryption = true;
+            result.statusFlags |= EnigmailConstants.UNKNOWN_ALGO;
+          } else {
+            let c_key_id = new ctypes.char.ptr();
+            rv = RNPLib.rnp_recipient_get_keyid(
+              recip_handle,
+              c_key_id.address()
+            );
+            if (rv) {
+              throw new Error("rnp_recipient_get_keyid failed");
+            }
+            let recip_key_id = c_key_id.readString();
+
+            let recip_key_handle = this.getKeyHandleByKeyIdOrFingerprint(
+              RNPLib.ffi,
+              "0x" + recip_key_id
+            );
+            let primary_signer_handle = this.getPrimaryKeyHandleIfSub(
+              RNPLib.ffi,
+              recip_key_handle
+            );
+            if (!primary_signer_handle.isNull()) {
+              recip_key_id = this.getKeyIDFromHandle(primary_signer_handle);
+              RNPLib.rnp_key_handle_destroy(primary_signer_handle);
+            }
+            RNPLib.rnp_key_handle_destroy(recip_key_handle);
+
+            result.encToDetails = recip_key_id;
+            result.statusFlags |= EnigmailConstants.DECRYPTION_OKAY;
+          }
+        }
+      }
+    }
+
+    if (useDecodedData) {
       let result_buf = new ctypes.uint8_t.ptr();
       let result_len = new ctypes.size_t();
       let rv = RNPLib.rnp_output_memory_get_buf(
         output_to_memory,
         result_buf.address(),
         result_len.address(),
         false
       );
@@ -746,16 +855,18 @@ var RNP = {
       }
     }
 
     RNPLib.rnp_input_destroy(input_from_memory);
     RNPLib.rnp_output_destroy(output_to_memory);
     RNPLib.rnp_op_verify_destroy(verify_op);
 
     if (
+      isEncrypted &&
+      !rejectedDecryption &&
       result.exitCode &&
       !("alreadyUsedGPGME" in options) &&
       Services.prefs.getBoolPref("mail.openpgp.allow_external_gnupg") &&
       GPGME.allDependenciesLoaded()
     ) {
       // failure processing with RNP, attempt decryption with GPGME
       let r2 = await GPGME.decrypt(encrypted, RNP.enArmor);
       if (!r2.exitCode && r2.decryptedData) {
--- a/mail/extensions/openpgp/content/modules/RNPLib.jsm
+++ b/mail/extensions/openpgp/content/modules/RNPLib.jsm
@@ -101,16 +101,18 @@ const rnp_uid_handle_t = ctypes.void_t.p
 const rnp_identifier_iterator_t = ctypes.void_t.ptr;
 const rnp_op_generate_t = ctypes.void_t.ptr;
 const rnp_op_encrypt_t = ctypes.void_t.ptr;
 const rnp_op_sign_t = ctypes.void_t.ptr;
 const rnp_op_sign_signature_t = ctypes.void_t.ptr;
 const rnp_op_verify_t = ctypes.void_t.ptr;
 const rnp_op_verify_signature_t = ctypes.void_t.ptr;
 const rnp_signature_handle_t = ctypes.void_t.ptr;
+const rnp_recipient_handle_t = ctypes.void_t.ptr;
+const rnp_symenc_handle_t = ctypes.void_t.ptr;
 
 const rnp_password_cb_t = ctypes.FunctionType(abi, ctypes.bool, [
   rnp_ffi_t,
   ctypes.void_t.ptr,
   rnp_key_handle_t,
   ctypes.char.ptr,
   ctypes.char.ptr,
   ctypes.size_t,
@@ -237,77 +239,17 @@ function enableRNPLibJS() {
       this.rnp_output_destroy(out2);
 
       output_to_path = null;
       out2 = null;
     },
 
     keep_password_cb_alive: null,
 
-    password_cb_collected_info: {},
-
     password_cb(ffi, app_ctx, key, pgp_context, buf, buf_len) {
-      // Before we fullfil the request, we collect context information.
-
-      RNPLib.password_cb_collected_info.context = pgp_context.readString();
-
-      if (RNPLib.password_cb_collected_info.context == "decrypt") {
-        // Get the key ID of the key used to decrypt. If it's a subkey,
-        // we use the ID of the primary key.
-
-        let usePrimaryHandle = false;
-        let primaryHandle = null;
-
-        let is_subkey = new ctypes.bool();
-        if (RNPLib.rnp_key_is_sub(key, is_subkey.address())) {
-          throw new Error("rnp_key_is_sub failed");
-        }
-        if (is_subkey.value) {
-          let primary_grip = new ctypes.char.ptr();
-          if (RNPLib.rnp_key_get_primary_grip(key, primary_grip.address())) {
-            throw new Error("rnp_key_get_primary_grip failed");
-          }
-          if (!primary_grip.isNull()) {
-            primaryHandle = new RNPLib.rnp_key_handle_t();
-            if (
-              RNPLib.rnp_locate_key(
-                ffi,
-                "grip",
-                primary_grip,
-                primaryHandle.address()
-              )
-            ) {
-              throw new Error("rnp_locate_key failed");
-            }
-            if (!primaryHandle.isNull()) {
-              usePrimaryHandle = true;
-            }
-            RNPLib.rnp_buffer_destroy(primary_grip);
-          }
-        }
-
-        let key_id = new ctypes.char.ptr();
-        if (
-          RNPLib.rnp_key_get_keyid(
-            usePrimaryHandle ? primaryHandle : key,
-            key_id.address()
-          )
-        ) {
-          throw new Error("rnp_key_get_keyid failed");
-        }
-        RNPLib.password_cb_collected_info.keyId = key_id.readString();
-        RNPLib.rnp_buffer_destroy(key_id);
-
-        if (primaryHandle) {
-          RNPLib.rnp_key_handle_destroy(primaryHandle);
-        }
-      }
-
-      // Now do what we've been asked to do.
-
       let pass = OpenPGPMasterpass.retrieveOpenPGPPassword();
       var passCTypes = ctypes.char.array()(pass); // UTF-8
       let passLen = passCTypes.length;
 
       if (buf_len < passLen) {
         return false;
       }
 
@@ -1132,31 +1074,140 @@ function enableRNPLibJS() {
       "rnp_enarmor",
       abi,
       rnp_result_t,
       rnp_input_t,
       rnp_output_t,
       ctypes.char.ptr
     ),
 
+    rnp_op_verify_get_protection_info: librnp.declare(
+      "rnp_op_verify_get_protection_info",
+      abi,
+      rnp_result_t,
+      rnp_op_verify_t,
+      ctypes.char.ptr.ptr,
+      ctypes.char.ptr.ptr,
+      ctypes.bool.ptr
+    ),
+
+    rnp_op_verify_get_recipient_count: librnp.declare(
+      "rnp_op_verify_get_recipient_count",
+      abi,
+      rnp_result_t,
+      rnp_op_verify_t,
+      ctypes.size_t.ptr
+    ),
+
+    rnp_op_verify_get_used_recipient: librnp.declare(
+      "rnp_op_verify_get_used_recipient",
+      abi,
+      rnp_result_t,
+      rnp_op_verify_t,
+      rnp_recipient_handle_t.ptr
+    ),
+
+    rnp_recipient_get_keyid: librnp.declare(
+      "rnp_recipient_get_keyid",
+      abi,
+      rnp_result_t,
+      rnp_recipient_handle_t,
+      ctypes.char.ptr.ptr
+    ),
+
+    rnp_recipient_get_alg: librnp.declare(
+      "rnp_recipient_get_alg",
+      abi,
+      rnp_result_t,
+      rnp_recipient_handle_t,
+      ctypes.char.ptr.ptr
+    ),
+
+    rnp_op_verify_get_symenc_count: librnp.declare(
+      "rnp_op_verify_get_symenc_count",
+      abi,
+      rnp_result_t,
+      rnp_op_verify_t,
+      ctypes.size_t.ptr
+    ),
+
+    rnp_op_verify_get_used_symenc: librnp.declare(
+      "rnp_op_verify_get_used_symenc",
+      abi,
+      rnp_result_t,
+      rnp_op_verify_t,
+      rnp_symenc_handle_t.ptr
+    ),
+
+    rnp_op_verify_get_symenc_at: librnp.declare(
+      "rnp_op_verify_get_symenc_at",
+      abi,
+      rnp_result_t,
+      rnp_op_verify_t,
+      ctypes.size_t,
+      rnp_symenc_handle_t.ptr
+    ),
+
+    rnp_symenc_get_cipher: librnp.declare(
+      "rnp_symenc_get_cipher",
+      abi,
+      rnp_result_t,
+      rnp_symenc_handle_t,
+      ctypes.char.ptr.ptr
+    ),
+
+    rnp_symenc_get_aead_alg: librnp.declare(
+      "rnp_symenc_get_aead_alg",
+      abi,
+      rnp_result_t,
+      rnp_symenc_handle_t,
+      ctypes.char.ptr.ptr
+    ),
+
+    rnp_symenc_get_hash_alg: librnp.declare(
+      "rnp_symenc_get_hash_alg",
+      abi,
+      rnp_result_t,
+      rnp_symenc_handle_t,
+      ctypes.char.ptr.ptr
+    ),
+
+    rnp_symenc_get_s2k_type: librnp.declare(
+      "rnp_symenc_get_s2k_type",
+      abi,
+      rnp_result_t,
+      rnp_symenc_handle_t,
+      ctypes.char.ptr.ptr
+    ),
+
+    rnp_symenc_get_s2k_iterations: librnp.declare(
+      "rnp_symenc_get_s2k_iterations",
+      abi,
+      rnp_result_t,
+      rnp_symenc_handle_t,
+      ctypes.uint32_t.ptr
+    ),
+
     rnp_result_t,
     rnp_ffi_t,
     rnp_password_cb_t,
     rnp_input_t,
     rnp_output_t,
     rnp_key_handle_t,
     rnp_uid_handle_t,
     rnp_identifier_iterator_t,
     rnp_op_generate_t,
     rnp_op_encrypt_t,
     rnp_op_sign_t,
     rnp_op_sign_signature_t,
     rnp_op_verify_t,
     rnp_op_verify_signature_t,
     rnp_signature_handle_t,
+    rnp_recipient_handle_t,
+    rnp_symenc_handle_t,
 
     RNP_LOAD_SAVE_PUBLIC_KEYS: 1,
     RNP_LOAD_SAVE_SECRET_KEYS: 2,
     RNP_LOAD_SAVE_PERMISSIVE: 256,
 
     RNP_KEY_REMOVE_PUBLIC: 1,
     RNP_KEY_REMOVE_SECRET: 2,
     RNP_KEY_REMOVE_SUBKEYS: 4,
--- a/mail/extensions/openpgp/content/modules/mimeDecrypt.jsm
+++ b/mail/extensions/openpgp/content/modules/mimeDecrypt.jsm
@@ -572,17 +572,19 @@ MimeDecryptHandler.prototype = {
       if (!this.returnStatus) {
         this.returnStatus = {
           decryptedData: "",
           exitCode: -1,
           statusFlags: EnigmailConstants.DECRYPTION_FAILED,
         };
       }
 
-      this.returnStatus.statusFlags |= EnigmailConstants.PGP_MIME_ENCRYPTED;
+      if (this.returnStatus.statusFlags & EnigmailConstants.DECRYPTION_OKAY) {
+        this.returnStatus.statusFlags |= EnigmailConstants.PGP_MIME_ENCRYPTED;
+      }
 
       if (this.returnStatus.exitCode) {
         // Failure
         if (this.returnStatus.decryptedData.length) {
           // However, we got decrypted data.
           // Did we get any verification failure flags?
           // If yes, then conclude only verification failed.
           if (
@@ -596,20 +598,16 @@ MimeDecryptHandler.prototype = {
           } else {
             this.returnStatus.statusFlags |=
               EnigmailConstants.DECRYPTION_FAILED;
           }
         } else {
           // no data
           this.returnStatus.statusFlags |= EnigmailConstants.DECRYPTION_FAILED;
         }
-      } else if (
-        !(this.returnStatus.statusFlags & EnigmailConstants.DECRYPTION_FAILED)
-      ) {
-        this.returnStatus.statusFlags |= EnigmailConstants.DECRYPTION_OKAY;
       }
 
       this.decryptedData = this.returnStatus.decryptedData;
       this.handleResult(this.returnStatus.exitCode);
 
       let decError =
         this.returnStatus.statusFlags & EnigmailConstants.DECRYPTION_FAILED;