Last Comment Bug 465351 - Wrong message and reason reported with untrusted CA roots when signing email
: Wrong message and reason reported with untrusted CA roots when signing email
Status: RESOLVED FIXED
[good first bug]
: regression
Product: MailNews Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 22.0
Assigned To: sakshi
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-17 08:35 PST by Eddy Nigg (StartCom)
Modified: 2013-03-26 09:23 PDT (History)
5 users (show)
standard8: wanted‑thunderbird3+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Message received with untrusted root (email flag not set) (11.99 KB, image/png)
2008-11-20 02:36 PST, Eddy Nigg (StartCom)
no flags Details
fix text (3.58 KB, patch)
2008-12-29 01:44 PST, timeless
standard8: review-
Details | Diff | Review
New patch (5.23 KB, patch)
2013-03-12 03:42 PDT, sakshi
mkmelin+mozilla: review-
Details | Diff | Review
patch with caller (8.67 KB, patch)
2013-03-12 05:28 PDT, sakshi
standard8: review-
Details | Diff | Review
Patch2 (8.67 KB, patch)
2013-03-24 10:43 PDT, sakshi
standard8: review+
Details | Diff | Review

Description Eddy Nigg (StartCom) 2008-11-17 08:35:57 PST
Signing messages with digital certificate fails since today. This is build Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081113 Shredder/3.0b1pre . Same settings worked previously. No co clue how to debug.
Comment 1 Magnus Melin 2008-11-19 12:32:11 PST
Signing seems to work for me on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081119 Lightning/1.0pre Shredder/3.0b1pre

In what way does it "not work" for you?
Comment 2 Eddy Nigg (StartCom) 2008-11-19 12:38:05 PST
That was a weird case...I had the CA certificates on the smart card I used and they had trust only enabled for web sites. Not sure what's the reason for this default trust setting, however once I changed the trust settings to include email (as in the builtin root), it obviously worked again.
Comment 3 Eddy Nigg (StartCom) 2008-11-20 02:33:58 PST
The error message is highly misleading and should be corrected.
Comment 4 Eddy Nigg (StartCom) 2008-11-20 02:36:05 PST
Created attachment 349157 [details]
Message received with untrusted root (email flag not set)

The message indicates that the settings of the certificate are incorrect, however the CA certificates have email trust bit set to off. The message should indicate that - it took me two days to figure what's going on.
Comment 5 Mark Banner (:standard8) 2008-11-20 02:39:04 PST
Comment on attachment 349157 [details]
Message received with untrusted root (email flag not set)

approval1.9.1 is for getting patches into the tree, not necessary here.
Comment 6 Mark Banner (:standard8) 2008-11-20 02:51:49 PST
I think I had this problem the other day.

We're getting a generic error message (ErrorCanNotSign from am-smime.properties) for a bunch of failure cases.

Could you set NSPR_LOG_MODULES to pipnss:5 (see https://wiki.mozilla.org/MailNews:Logging for how to do it), and post the relevant part of the log on the bug?

I'm expecting it'll be somewhere in nsCMSMessage::CreateSigned that it is failing, getting the log will confirm this and help us work out what we need to change.
Comment 7 timeless 2008-12-29 01:44:35 PST
Created attachment 354669 [details] [diff] [review]
fix text
Comment 8 Axel Hecht [:Pike] 2008-12-31 03:44:25 PST
Comment on attachment 354669 [details] [diff] [review]
fix text

This should be caught by localizations, too, so it should get a change in the keys.

I'd suggest to use ErrorCanNotSignMail (and EncryptMail), which is a proper change carrying over the semantics of the change in the text.
Comment 9 Mark Banner (:standard8) 2008-12-31 03:44:34 PST
Comment on attachment 354669 [details] [diff] [review]
fix text

L10n request that because of the context change we also change the name of the string, this is so that we'll force localisations to reconsider the change to the string for their language.
Comment 10 sakshi 2013-03-11 06:33:33 PDT
Hello,

I would like to fix this bug.
Comment 11 sakshi 2013-03-12 03:42:09 PDT
Created attachment 723883 [details] [diff] [review]
New patch

Who should I set for review?
Comment 12 Magnus Melin 2013-03-12 04:13:36 PDT
You'll need a mailnews/ reviewer - https://wiki.mozilla.org/Modules/All#MailNews
standard8, or IanN perhaps?
Comment 13 Magnus Melin 2013-03-12 04:14:21 PDT
Comment on attachment 723883 [details] [diff] [review]
New patch

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

You'll need to update the "callers" too.
Comment 14 sakshi 2013-03-12 04:17:45 PDT
I guess those are placed in mailnews/extensions/smime/src/nsMsgComposeSecure.cpp ?
Comment 16 sakshi 2013-03-12 05:28:09 PDT
Created attachment 723898 [details] [diff] [review]
patch with caller
Comment 17 sakshi 2013-03-16 20:40:59 PDT
Any updates on the patch?
Comment 18 Mark Banner (:standard8) 2013-03-22 06:33:19 PDT
Comment on attachment 723898 [details] [diff] [review]
patch with caller

Sorry for the delay.

I'd really like to give this r+, but the new labels seem to be the wrong way around:

+ErrorCanNotSignMail=Unable to encrypt message. Please check that you have a valid email certificate for each recipient. Please check that the certificates specified in Mail & Newsgroups Account Settings for this mail account are valid and trusted for mail.
+ErrorEncryptMail=Unable to sign message. Please check that the certificates specified in Mail & Newsgroups Account Settings for this mail account are valid and trusted for mail.

ErrorEncryptMail should be for "Unable to encrypt message..."
Comment 19 sakshi 2013-03-24 10:43:34 PDT
Created attachment 728755 [details] [diff] [review]
Patch2
Comment 20 Mark Banner (:standard8) 2013-03-24 14:26:12 PDT
Comment on attachment 728755 [details] [diff] [review]
Patch2

That's better, thanks, r=Standard8.
Comment 21 Mark Banner (:standard8) 2013-03-26 09:23:35 PDT
https://hg.mozilla.org/comm-central/rev/fd78e7d7cda7

Note You need to log in before you can comment on or make changes to this bug.