From 38fe066bb424ab3ebf4297b9dcde12e5f8057612 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sun, 10 Nov 2019 23:49:16 -0500 Subject: Bugs 1507218 and 1528615 Tag #1273 --- mailnews/mime/public/nsICMSMessage.idl | 4 +- mailnews/mime/public/nsICMSMessage2.idl | 19 +++-- mailnews/mime/src/mimecms.cpp | 7 +- mailnews/mime/src/mimemcms.cpp | 6 +- mailnews/mime/src/nsCMS.cpp | 132 +++++++++++++++++++++++++------- mailnews/mime/src/nsCMS.h | 8 +- 6 files changed, 132 insertions(+), 44 deletions(-) diff --git a/mailnews/mime/public/nsICMSMessage.idl b/mailnews/mime/public/nsICMSMessage.idl index 1d67bf51d..ffa2aac7e 100644 --- a/mailnews/mime/public/nsICMSMessage.idl +++ b/mailnews/mime/public/nsICMSMessage.idl @@ -28,7 +28,9 @@ interface nsICMSMessage : nsISupports 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 */ diff --git a/mailnews/mime/public/nsICMSMessage2.idl b/mailnews/mime/public/nsICMSMessage2.idl index 9360279c6..f353c32a2 100644 --- a/mailnews/mime/public/nsICMSMessage2.idl +++ b/mailnews/mime/public/nsICMSMessage2.idl @@ -11,7 +11,7 @@ interface nsISMimeVerificationListener; /* * This interface is currently not marked scriptable, * because its verification functions are meant to look like those - * in nsICMSMessage. At the time the ptr type is eliminated in both + * in nsICMSMessage. At the time the ptr type is eliminated in both * interfaces, both should be made scriptable. */ @@ -21,19 +21,19 @@ interface nsICMSMessage2 : nsISupports /** * Async version of nsICMSMessage::VerifySignature. * Code will be executed on a background thread and - * availability of results will be notified using a + * availability of results will be notified using a * call to nsISMimeVerificationListener. */ void asyncVerifySignature(in nsISMimeVerificationListener listener); - + /** * Async version of nsICMSMessage::VerifyDetachedSignature. * Code will be executed on a background thread and - * availability of results will be notified using a + * availability of results will be notified using a * call to nsISMimeVerificationListener. * - * We are using "native unsigned char" ptr, because the function - * signatures of this one and nsICMSMessage::verifyDetachedSignature + * We are using "native unsigned char" ptr, because the function + * signatures of this one and nsICMSMessage::verifyDetachedSignature * should be the identical. Cleaning up nsICMSMessages needs to be * postponed, because this async version is needed on MOZILLA_1_8_BRANCH. * @@ -42,10 +42,13 @@ interface nsICMSMessage2 : nsISupports * [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)] diff --git a/mailnews/mime/src/mimecms.cpp b/mailnews/mime/src/mimecms.cpp index 18d88acaa..a3a3c4739 100644 --- a/mailnews/mime/src/mimecms.cpp +++ b/mailnews/mime/src/mimecms.cpp @@ -567,7 +567,8 @@ 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 msg2 = do_QueryInterface(aCMSMsg); if (!msg2) @@ -580,7 +581,7 @@ void MimeCMSRequestAsyncSignatureVerification(nsICMSMessage *aCMSMsg, return; if (item_data) - msg2->AsyncVerifyDetachedSignature(listener, item_data, item_len); + msg2->AsyncVerifyDetachedSignature(listener, item_data, item_len, digest_type); else msg2->AsyncVerifySignature(listener); } @@ -683,7 +684,7 @@ MimeCMS_eof (void *crypto_closure, bool abort_p) from_addr.get(), from_name.get(), sender_addr.get(), sender_name.get(), data->smimeHeaderSink, aRelativeNestLevel, - nullptr, 0); + nullptr, 0, 0); } } diff --git a/mailnews/mime/src/mimemcms.cpp b/mailnews/mime/src/mimemcms.cpp index d67f69899..b8b6a8669 100644 --- a/mailnews/mime/src/mimemcms.cpp +++ b/mailnews/mime/src/mimemcms.cpp @@ -125,7 +125,8 @@ 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); @@ -454,7 +455,8 @@ MimeMultCMS_generate (void *crypto_closure) 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) { diff --git a/mailnews/mime/src/nsCMS.cpp b/mailnews/mime/src/nsCMS.cpp index c7cfb31ed..6717c0840 100644 --- a/mailnews/mime/src/nsCMS.cpp +++ b/mailnews/mime/src/nsCMS.cpp @@ -75,7 +75,7 @@ void nsCMSMessage::destructorSafeDestroyNSSReference() NS_IMETHODIMP nsCMSMessage::VerifySignature() { - return CommonVerifySignature(nullptr, 0); + return CommonVerifySignature(nullptr, 0, 0); } NSSCMSSignerInfo* nsCMSMessage::GetTopLevelSignerInfo() @@ -210,15 +210,21 @@ NS_IMETHODIMP nsCMSMessage::GetEncryptionCert(nsIX509Cert **ecert) 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) { nsNSSShutDownPreventionLock locker; if (isAlreadyShutDown()) @@ -239,8 +245,20 @@ nsresult nsCMSMessage::CommonVerifySignature(unsigned char* aDigestData, uint32_ 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(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) { @@ -251,11 +269,29 @@ nsresult nsCMSMessage::CommonVerifySignature(unsigned char* aDigestData, uint32_ if (aDigestData && aDigestDataLen) { + SECOidTag oidTag; SECItem digest; digest.data = aDigestData; digest.len = aDigestDataLen; - if (NSS_CMSSignedData_SetDigestValue(sigd, SEC_OID_SHA1, &digest)) { + 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 (!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; @@ -357,17 +393,19 @@ loser: 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 @@ -375,12 +413,15 @@ 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(aDigestData), aDigestDataLen); + mDigestType = aDigestType; } private: @@ -393,7 +434,7 @@ private: if (!mDigestData.IsEmpty()) { rv = mMessage->VerifyDetachedSignature( reinterpret_cast(const_cast(mDigestData.get())), - mDigestData.Length()); + mDigestData.Length(), mDigestType); } else { rv = mMessage->VerifySignature(); } @@ -411,12 +452,14 @@ private: nsCOMPtr mMessage; nsCOMPtr mListener; nsCString mDigestData; + int16_t mDigestType; }; nsresult nsCMSMessage::CommonAsyncVerifySignature(nsISMimeVerificationListener *aListener, - unsigned char* aDigestData, uint32_t aDigestDataLen) + unsigned char* aDigestData, uint32_t aDigestDataLen, + int16_t aDigestType) { - RefPtr task = new SMimeVerificationTask(this, aListener, aDigestData, aDigestDataLen); + RefPtr task = new SMimeVerificationTask(this, aListener, aDigestData, aDigestDataLen, aDigestType); return task->Dispatch("SMimeVerify"); } @@ -620,6 +663,51 @@ loser: 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, @@ -647,20 +735,8 @@ nsCMSMessage::CreateSigned(nsIX509Cert* aSigningCert, nsIX509Cert* aEncryptCert, } 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; } diff --git a/mailnews/mime/src/nsCMS.h b/mailnews/mime/src/nsCMS.h index 7c862eb46..393344277 100644 --- a/mailnews/mime/src/nsCMS.h +++ b/mailnews/mime/src/nsCMS.h @@ -42,10 +42,14 @@ private: nsCOMPtr 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); virtual void virtualDestroyNSSReference() override; void destructorSafeDestroyNSSReference(); -- cgit v1.2.3