540498 - Optionally include cert chain root in signed emails

Status: NEW

(MailNews Core :: Security: S/MIME, defect)

Description

[Bob Lord]

This topic is mentioned in Bug 354273 but needs to be called out separately.

It is legal for an S/MIME client to send a cert chain that includes the root. There are some circumstances where that behavior is desirable. Today, TB only sends the cert chain up to, but not including the root cert.

Comment 1

[Frederik Vermeulen]

Microsoft Outlook seems to need the root certificate.

I think this requires the following change in mozilla/security/nss/cmd/smimetools/cmsutil.c 

-    if (NSS_CMSSignerInfo_IncludeCerts(signerinfo, NSSCMSCM_CertChain,
+    if (NSS_CMSSignerInfo_IncludeCerts(signerinfo, NSSCMSCM_CertChainWithRoot,

I didn't test this yet.

Comment 2

[Frederik Vermeulen]

Attachment: Include cert chain root in signed emails

Comment 3

[Benjamin Smedberg]

Comment on attachment 813716 [details] [diff] [review]
Include cert chain root in signed emails

I'm not sure how you picked me as a reviewer, but I'm totally unfamiliar with any of this. Perhaps you meant bsmith?

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 813716 [details] [diff] [review]
Include cert chain root in signed emails

Review of attachment 813716 [details] [diff] [review]:
-----------------------------------------------------------------

I am not so familiar with the S/MIME code. Pushing to rrelyea.

Comment 5

[Kaspar]

Attachment: Include the root cert based on the "sign_include_root" pref, c-c part

(In reply to Frederik Vermeulen from comment #2)
> Created attachment 813716 [details] [diff] [review]
> Include cert chain root in signed emails

This is patching the wrong code (cmsutil is a standalone tool, completely separate from the Tb/Sm code). Furthermore, the root certificate shouldn't be included by default (contrary to what comment 1 says, it isn't needed for Outlook), it should be configurable instead.

(In reply to bug 354273 comment 2)
> I wrote a patch for this sometime in 2006, which I might be able to dig up
> again, if there's an interest in it

I'm attaching the c-c and m-c parts of that patch (unbitrotten), but without the UI part. Also, I'm not sure if it's ok to change the signature of CreateSigned in nsICMSMessage.idl, or whether that needs a new uuid etc. (can't remember that stuff).

Comment 6

[Kaspar]

Attachment: Include the root cert based on the "sign_include_root" pref, m-c part

Comment 7

[Magnus Melin [:mkmelin]]

Yes you should reve the uuid. Don't forget to ask for reviews.

Comment 8

[Kaspar]

Attachment: Include the root cert based on the "sign_include_root" pref, UI part (c-c)

Here's the UI part, for the sake of completeness.

Anybody who wants to drive this change through (Frederik?) should feel free to do so, I don't claim any specific authorship for these patches or modifications thereof. (Personally, I think there's very little need for such an option.)