Bug 1507218 - Don't use a hardcoded SEC_OID_SHA1 parameter. r=rrelyea a=jorgk
authorKai Engert <kaie@kuix.de>
Fri, 08 Feb 2019 18:08:20 +0100
changeset 33481 77a16669b9c8
parent 33480 8069a80f2074
child 33482 bb057edd9bbb
push id2375
push usermozilla@jorgk.com
push dateSat, 09 Feb 2019 11:27:00 +0000
treeherdercomm-beta@bb057edd9bbb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrrelyea, jorgk
bugs1507218
Bug 1507218 - Don't use a hardcoded SEC_OID_SHA1 parameter. r=rrelyea a=jorgk
mailnews/mime/public/nsICMSMessage.idl
mailnews/mime/public/nsICMSMessage2.idl
mailnews/mime/src/mimecms.cpp
mailnews/mime/src/mimemcms.cpp
mailnews/mime/src/nsCMS.cpp
mailnews/mime/src/nsCMS.h
--- a/mailnews/mime/public/nsICMSMessage.idl
+++ b/mailnews/mime/public/nsICMSMessage.idl
@@ -23,17 +23,19 @@ interface nsICMSMessage : nsISupports
 {
   void contentIsSigned(out boolean aSigned);
   void contentIsEncrypted(out boolean aEncrypted);
   void getSignerCommonName(out string aName);
   void getSignerEmailAddress(out string aEmail);
   void getSignerCert(out nsIX509Cert scert);
   void getEncryptionCert(out nsIX509Cert ecert);
   void verifySignature();
-  void verifyDetachedSignature(in UnsignedCharPtr aDigestData, in unsigned long aDigestDataLen);
+  void verifyDetachedSignature(in UnsignedCharPtr aDigestData,
+                               in unsigned long aDigestDataLen,
+                               in int16_t aDigestType);
   void CreateEncrypted(in nsIArray aRecipientCerts);
 
   /* The parameter aDigestType must be one of the values in nsICryptoHash */
   void CreateSigned(in nsIX509Cert scert, in nsIX509Cert ecert,
                     in UnsignedCharPtr aDigestData,
                     in unsigned long aDigestDataLen, in int16_t aDigestType);
 };
 
--- a/mailnews/mime/public/nsICMSMessage2.idl
+++ b/mailnews/mime/public/nsICMSMessage2.idl
@@ -37,20 +37,23 @@ interface nsICMSMessage2 : nsISupports
     * should be the identical. Cleaning up nsICMSMessages needs to be
     * postponed, because this async version is needed on MOZILLA_1_8_BRANCH.
     *
     * Once both interfaces get cleaned up, the function signature should
     * look like:
     *     [array, length_is(aDigestDataLen)]
     *     in octet aDigestData,
     *     in unsigned long aDigestDataLen);
+    *
+    * Set aDigestType to one of the values from nsICryptoHash.
    */
   void asyncVerifyDetachedSignature(in nsISMimeVerificationListener listener,
-                                     in UnsignedCharPtr aDigestData,
-                                     in unsigned long aDigestDataLen);
+                                    in UnsignedCharPtr aDigestData,
+                                    in unsigned long aDigestDataLen,
+                                    in int16_t aDigestType);
 };
 
 [uuid(5226d698-0773-4f25-b94c-7944b3fc01d3)]
 interface nsISMimeVerificationListener : nsISupports {
 
   /**
    *  Notify that results are ready, that have been requested
    *  using nsICMSMessage2::asyncVerify[Detached]Signature()
--- a/mailnews/mime/src/mimecms.cpp
+++ b/mailnews/mime/src/mimecms.cpp
@@ -568,30 +568,31 @@ void MimeCMSGetFromSender(MimeObject *ob
   if (!s.IsEmpty())
     ExtractFirstAddress(EncodedHeader(s), sender_name, sender_addr);
 }
 
 void MimeCMSRequestAsyncSignatureVerification(nsICMSMessage *aCMSMsg,
                                               const char *aFromAddr, const char *aFromName,
                                               const char *aSenderAddr, const char *aSenderName,
                                               nsIMsgSMIMEHeaderSink *aHeaderSink, int32_t aMimeNestingLevel,
-                                              unsigned char* item_data, uint32_t item_len)
+                                              unsigned char* item_data, uint32_t item_len,
+                                              int16_t digest_type)
 {
   nsCOMPtr<nsICMSMessage2> msg2 = do_QueryInterface(aCMSMsg);
   if (!msg2)
     return;
 
   RefPtr<nsSMimeVerificationListener> listener =
     new nsSMimeVerificationListener(aFromAddr, aFromName, aSenderAddr, aSenderName,
                                     aHeaderSink, aMimeNestingLevel);
   if (!listener)
     return;
 
   if (item_data)
-    msg2->AsyncVerifyDetachedSignature(listener, item_data, item_len);
+    msg2->AsyncVerifyDetachedSignature(listener, item_data, item_len, digest_type);
   else
     msg2->AsyncVerifySignature(listener);
 }
 
 static int
 MimeCMS_eof (void *crypto_closure, bool abort_p)
 {
   MimeCMSdata *data = (MimeCMSdata *) crypto_closure;
@@ -684,17 +685,17 @@ MimeCMS_eof (void *crypto_closure, bool 
       MimeCMSGetFromSender(data->self,
                            from_addr, from_name,
                            sender_addr, sender_name);
 
       MimeCMSRequestAsyncSignatureVerification(data->content_info,
                                                from_addr.get(), from_name.get(),
                                                sender_addr.get(), sender_name.get(),
                                                data->smimeHeaderSink, aRelativeNestLevel,
-                                               nullptr, 0);
+                                               nullptr, 0, 0);
     }
   }
 
   if (data->ci_is_encrypted)
   {
     data->smimeHeaderSink->EncryptionStatus(
       aRelativeNestLevel,
       status,
--- a/mailnews/mime/src/mimemcms.cpp
+++ b/mailnews/mime/src/mimemcms.cpp
@@ -120,17 +120,18 @@ extern void MimeCMSGetFromSender(MimeObj
                                  nsCString &sender_name);
 extern bool MimeCMSHeadersAndCertsMatch(MimeObject *obj,
                                           nsICMSMessage *,
                                           bool *signing_cert_without_email_address);
 extern void MimeCMSRequestAsyncSignatureVerification(nsICMSMessage *aCMSMsg,
                                                      const char *aFromAddr, const char *aFromName,
                                                      const char *aSenderAddr, const char *aSenderName,
                                                      nsIMsgSMIMEHeaderSink *aHeaderSink, int32_t aMimeNestingLevel,
-                                                     unsigned char* item_data, uint32_t item_len);
+                                                     unsigned char* item_data, uint32_t item_len,
+                                                     int16_t digest_type);
 extern char *MimeCMS_MakeSAURL(MimeObject *obj);
 extern char *IMAP_CreateReloadAllPartsUrl(const char *url);
 extern int MIMEGetRelativeCryptoNestLevel(MimeObject *obj);
 
 static void *
 MimeMultCMS_init (MimeObject *obj)
 {
   MimeHeaders *hdrs = obj->headers;
@@ -449,17 +450,18 @@ MimeMultCMS_generate (void *crypto_closu
   MimeCMSGetFromSender(data->self,
                        from_addr, from_name,
                        sender_addr, sender_name);
 
   MimeCMSRequestAsyncSignatureVerification(data->content_info,
                                            from_addr.get(), from_name.get(),
                                            sender_addr.get(), sender_name.get(),
                                            data->smimeHeaderSink, aRelativeNestLevel,
-                                           data->item_data, data->item_len);
+                                           data->item_data, data->item_len,
+                                           data->hash_type);
 
   if (data->content_info)
   {
 #if 0 // XXX Fix this. What do we do here? //
     if (SEC_CMSContainsCertsOrCrls(data->content_info))
     {
       /* #### call libsec telling it to import the certs */
     }
--- a/mailnews/mime/src/nsCMS.cpp
+++ b/mailnews/mime/src/nsCMS.cpp
@@ -58,17 +58,17 @@ void nsCMSMessage::destructorSafeDestroy
 {
   if (m_cmsMsg) {
     NSS_CMSMessage_Destroy(m_cmsMsg);
   }
 }
 
 NS_IMETHODIMP nsCMSMessage::VerifySignature()
 {
-  return CommonVerifySignature(nullptr, 0);
+  return CommonVerifySignature(nullptr, 0, 0);
 }
 
 NSSCMSSignerInfo* nsCMSMessage::GetTopLevelSignerInfo()
 {
   if (!m_cmsMsg)
     return nullptr;
 
   if (!NSS_CMSMessage_IsSigned(m_cmsMsg))
@@ -167,58 +167,94 @@ NS_IMETHODIMP nsCMSMessage::GetSignerCer
   return NS_OK;
 }
 
 NS_IMETHODIMP nsCMSMessage::GetEncryptionCert(nsIX509Cert**)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
-NS_IMETHODIMP nsCMSMessage::VerifyDetachedSignature(unsigned char* aDigestData, uint32_t aDigestDataLen)
+NS_IMETHODIMP
+nsCMSMessage::VerifyDetachedSignature(unsigned char* aDigestData,
+                                      uint32_t aDigestDataLen,
+                                      int16_t aDigestType)
 {
   if (!aDigestData || !aDigestDataLen)
     return NS_ERROR_FAILURE;
 
-  return CommonVerifySignature(aDigestData, aDigestDataLen);
+  return CommonVerifySignature(aDigestData, aDigestDataLen, aDigestType);
 }
 
-nsresult nsCMSMessage::CommonVerifySignature(unsigned char* aDigestData, uint32_t aDigestDataLen)
+nsresult
+nsCMSMessage::CommonVerifySignature(unsigned char* aDigestData,
+                                    uint32_t aDigestDataLen,
+                                    int16_t aDigestType)
 {
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsCMSMessage::CommonVerifySignature, content level count %d\n", NSS_CMSMessage_ContentLevelCount(m_cmsMsg)));
   NSSCMSContentInfo *cinfo = nullptr;
   NSSCMSSignedData *sigd = nullptr;
   NSSCMSSignerInfo *si;
   int32_t nsigners;
   RefPtr<SharedCertVerifier> certVerifier;
   nsresult rv = NS_ERROR_FAILURE;
 
   if (!NSS_CMSMessage_IsSigned(m_cmsMsg)) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsCMSMessage::CommonVerifySignature - not signed\n"));
     return NS_ERROR_CMS_VERIFY_NOT_SIGNED;
   }
 
   cinfo = NSS_CMSMessage_ContentLevel(m_cmsMsg, 0);
   if (cinfo) {
-    // I don't like this hard cast. We should check in some way, that we really have this type.
-    sigd = (NSSCMSSignedData*)NSS_CMSContentInfo_GetContent(cinfo);
+    switch (NSS_CMSContentInfo_GetContentTypeTag(cinfo)) {
+      case SEC_OID_PKCS7_SIGNED_DATA:
+        sigd = reinterpret_cast<NSSCMSSignedData*>(NSS_CMSContentInfo_GetContent(cinfo));
+        break;
+
+      case SEC_OID_PKCS7_ENVELOPED_DATA:
+      case SEC_OID_PKCS7_ENCRYPTED_DATA:
+      case SEC_OID_PKCS7_DIGESTED_DATA:
+      default: {
+        MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsCMSMessage::CommonVerifySignature - unexpected ContentTypeTag\n"));
+        rv = NS_ERROR_CMS_VERIFY_NO_CONTENT_INFO;
+        goto loser;
+      }
+    }
   }
 
   if (!sigd) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsCMSMessage::CommonVerifySignature - no content info\n"));
     rv = NS_ERROR_CMS_VERIFY_NO_CONTENT_INFO;
     goto loser;
   }
 
+  if (NSS_CMSSignedData_HasDigests(sigd)) {
+    SECAlgorithmID **existingAlgs = NSS_CMSSignedData_GetDigestAlgs(sigd);
+    if (existingAlgs) {
+      while (*existingAlgs) {
+        SECAlgorithmID *alg = *existingAlgs;
+        SECOidTag algOIDTag = SECOID_FindOIDTag(&alg->algorithm);
+        NSS_CMSSignedData_SetDigestValue(sigd, algOIDTag, NULL);
+        ++existingAlgs;
+      }
+    }
+  }
+
   if (aDigestData && aDigestDataLen)
   {
+    SECOidTag oidTag;
     SECItem digest;
     digest.data = aDigestData;
     digest.len = aDigestDataLen;
 
-    if (NSS_CMSSignedData_SetDigestValue(sigd, SEC_OID_SHA1, &digest)) {
+    if (!GetIntHashToOidHash(aDigestType, oidTag)) {
+      rv = NS_ERROR_CMS_VERIFY_BAD_DIGEST;
+      goto loser;
+    }
+
+    if (NSS_CMSSignedData_SetDigestValue(sigd, oidTag, &digest)) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsCMSMessage::CommonVerifySignature - bad digest\n"));
       rv = NS_ERROR_CMS_VERIFY_BAD_DIGEST;
       goto loser;
     }
   }
 
   // Import certs. Note that import failure is not a signature verification failure. //
   if (NSS_CMSSignedData_ImportCerts(sigd, CERT_GetDefaultCertDB(), certUsageEmailRecipient, true) != SECSuccess) {
@@ -312,40 +348,45 @@ nsresult nsCMSMessage::CommonVerifySigna
   rv = NS_OK;
 loser:
   return rv;
 }
 
 NS_IMETHODIMP nsCMSMessage::AsyncVerifySignature(
                               nsISMimeVerificationListener *aListener)
 {
-  return CommonAsyncVerifySignature(aListener, nullptr, 0);
+  return CommonAsyncVerifySignature(aListener, nullptr, 0, 0);
 }
 
 NS_IMETHODIMP nsCMSMessage::AsyncVerifyDetachedSignature(
                               nsISMimeVerificationListener *aListener,
-                              unsigned char* aDigestData, uint32_t aDigestDataLen)
+                              unsigned char* aDigestData,
+                              uint32_t aDigestDataLen,
+                              int16_t aDigestType)
 {
   if (!aDigestData || !aDigestDataLen)
     return NS_ERROR_FAILURE;
 
-  return CommonAsyncVerifySignature(aListener, aDigestData, aDigestDataLen);
+  return CommonAsyncVerifySignature(aListener, aDigestData, aDigestDataLen, aDigestType);
 }
 
 class SMimeVerificationTask final : public CryptoTask
 {
 public:
   SMimeVerificationTask(nsICMSMessage *aMessage,
                         nsISMimeVerificationListener *aListener,
-                        unsigned char *aDigestData, uint32_t aDigestDataLen)
+                        unsigned char *aDigestData,
+                        uint32_t aDigestDataLen,
+                        int16_t aDigestType)
   {
     MOZ_ASSERT(NS_IsMainThread());
     mMessage = aMessage;
     mListener = aListener;
     mDigestData.Assign(reinterpret_cast<char *>(aDigestData), aDigestDataLen);
+    mDigestType = aDigestType;
   }
 
   static void InitStaticLock()
   {
     // If we ensure this is only executed on the main thread, we know
     // there cannot be a race to allocate the lock multiple times.
     MOZ_ASSERT(NS_IsMainThread());
     if (!mLock) {
@@ -359,17 +400,17 @@ private:
   {
     MOZ_ASSERT(!NS_IsMainThread());
 
     MutexAutoLock mon(*mLock);
     nsresult rv;
     if (!mDigestData.IsEmpty()) {
       rv = mMessage->VerifyDetachedSignature(
         reinterpret_cast<uint8_t*>(const_cast<char *>(mDigestData.get())),
-        mDigestData.Length());
+        mDigestData.Length(), mDigestType);
     } else {
       rv = mMessage->VerifySignature();
     }
 
     return rv;
   }
   virtual void CallCallback(nsresult rv) override
   {
@@ -377,27 +418,29 @@ private:
 
     nsCOMPtr<nsICMSMessage2> m2 = do_QueryInterface(mMessage);
     mListener->Notify(m2, rv);
   }
 
   nsCOMPtr<nsICMSMessage> mMessage;
   nsCOMPtr<nsISMimeVerificationListener> mListener;
   nsCString mDigestData;
+  int16_t mDigestType;
 
   static mozilla::Mutex *mLock;
 };
 
 mozilla::Mutex * SMimeVerificationTask::mLock = nullptr;
 
 nsresult nsCMSMessage::CommonAsyncVerifySignature(nsISMimeVerificationListener *aListener,
-                                                  unsigned char* aDigestData, uint32_t aDigestDataLen)
+                                                  unsigned char* aDigestData, uint32_t aDigestDataLen,
+                                                  int16_t aDigestType)
 {
   SMimeVerificationTask::InitStaticLock();
-  RefPtr<CryptoTask> task = new SMimeVerificationTask(this, aListener, aDigestData, aDigestDataLen);
+  RefPtr<CryptoTask> task = new SMimeVerificationTask(this, aListener, aDigestData, aDigestDataLen, aDigestType);
   return task->Dispatch("SMimeVerify");
 }
 
 class nsZeroTerminatedCertArray
 {
 public:
   nsZeroTerminatedCertArray()
   :mCerts(nullptr), mPoolp(nullptr), mSize(0)
@@ -565,16 +608,61 @@ loser:
   if (m_cmsMsg) {
     NSS_CMSMessage_Destroy(m_cmsMsg);
     m_cmsMsg = nullptr;
   }
 
   return rv;
 }
 
+bool nsCMSMessage::IsAllowedHash(const int16_t aCryptoHashInt)
+{
+  switch (aCryptoHashInt) {
+    case nsICryptoHash::SHA1:
+    case nsICryptoHash::SHA256:
+    case nsICryptoHash::SHA384:
+    case nsICryptoHash::SHA512:
+      return true;
+    case nsICryptoHash::MD2:
+    case nsICryptoHash::MD5:
+    default:
+      return false;
+  }
+}
+
+bool nsCMSMessage::GetIntHashToOidHash(const int16_t aCryptoHashInt, SECOidTag &aOidTag)
+{
+  bool rv = true;
+
+  switch (aCryptoHashInt) {
+    case nsICryptoHash::MD2:
+      aOidTag = SEC_OID_MD2;
+      break;
+    case nsICryptoHash::MD5:
+      aOidTag = SEC_OID_MD5;
+      break;
+    case nsICryptoHash::SHA1:
+      aOidTag = SEC_OID_SHA1;
+      break;
+    case nsICryptoHash::SHA256:
+      aOidTag = SEC_OID_SHA256;
+      break;
+    case nsICryptoHash::SHA384:
+      aOidTag = SEC_OID_SHA384;
+      break;
+    case nsICryptoHash::SHA512:
+      aOidTag = SEC_OID_SHA512;
+      break;
+    default:
+      rv = false;
+  }
+
+  return rv;
+}
+
 NS_IMETHODIMP
 nsCMSMessage::CreateSigned(nsIX509Cert* aSigningCert, nsIX509Cert* aEncryptCert,
                            unsigned char* aDigestData, uint32_t aDigestDataLen,
                            int16_t aDigestType)
 {
   NS_ENSURE_ARG(aSigningCert);
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsCMSMessage::CreateSigned\n"));
   NSSCMSContentInfo *cinfo;
@@ -588,30 +676,18 @@ nsCMSMessage::CreateSigned(nsIX509Cert* 
     return NS_ERROR_FAILURE;
   }
 
   if (aEncryptCert) {
     ecert = UniqueCERTCertificate(aEncryptCert->GetCert());
   }
 
   SECOidTag digestType;
-  switch (aDigestType) {
-    case nsICryptoHash::SHA1:
-      digestType = SEC_OID_SHA1;
-      break;
-    case nsICryptoHash::SHA256:
-      digestType = SEC_OID_SHA256;
-      break;
-    case nsICryptoHash::SHA384:
-      digestType = SEC_OID_SHA384;
-      break;
-    case nsICryptoHash::SHA512:
-      digestType = SEC_OID_SHA512;
-      break;
-    default:
+  if (!IsAllowedHash(aDigestType) ||
+      !GetIntHashToOidHash(aDigestType, digestType)) {
       return NS_ERROR_INVALID_ARG;
   }
 
   /*
    * create the message object
    */
   m_cmsMsg = NSS_CMSMessage_Create(nullptr); /* create a message on its own pool */
   if (!m_cmsMsg) {
--- a/mailnews/mime/src/nsCMS.h
+++ b/mailnews/mime/src/nsCMS.h
@@ -33,20 +33,24 @@ public:
 
   void referenceContext(nsIInterfaceRequestor* aContext) {m_ctx = aContext;}
   NSSCMSMessage* getCMS() {return m_cmsMsg;}
 private:
   virtual ~nsCMSMessage();
   nsCOMPtr<nsIInterfaceRequestor> m_ctx;
   NSSCMSMessage * m_cmsMsg;
   NSSCMSSignerInfo* GetTopLevelSignerInfo();
-  nsresult CommonVerifySignature(unsigned char* aDigestData, uint32_t aDigestDataLen);
+  nsresult CommonVerifySignature(unsigned char* aDigestData, uint32_t aDigestDataLen,
+                                 int16_t aDigestType);
 
   nsresult CommonAsyncVerifySignature(nsISMimeVerificationListener *aListener,
-                                      unsigned char* aDigestData, uint32_t aDigestDataLen);
+                                      unsigned char* aDigestData, uint32_t aDigestDataLen,
+                                      int16_t aDigestType);
+  bool GetIntHashToOidHash(const int16_t aCryptoHashInt, SECOidTag &aOidTag);
+  bool IsAllowedHash(const int16_t aCryptoHashInt);
 
   void destructorSafeDestroyNSSReference();
 
 };
 
 // ===============================================
 // nsCMSDecoder - implementation of nsICMSDecoder
 // ===============================================