Closed Bug 380744 Opened 17 years ago Closed 17 years ago

Thunderbird reports "unable to decrypt" on truncated decryptable messages

Categories

(Thunderbird :: Security, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

(Keywords: fixed1.8.1.8, regression)

Attachments

(1 file, 2 obsolete files)

This is a regression from bug 284369.

I am not yet able to reproduce myself, but a good friend of mine experiences this problem on his machine.

With Thunderbird 1.0x it works fine, as follows:
- receive an encrypted message
- Thunderbird displays the message and its attachments

With Thunderbird 1.5x and 2.0x we get a broken behaviour, as follows:
- receive an encrypted message
- Thunderbird displays the "can not decrypt" error message that got introduced with bug 284369
- however, Thunderbird correctly displays the attachments (which indicates we ARE able to decrypt)


This seems to be related to special kind of encoding.
I do not yet understand what makes a message special and trigger the behaviour.

The problem was seen with messages sent by Thunderbird 2.0, so it should not be related to an unknown encoding.


When the code from bug 284369 decides we're unable to decrypt, it replaces the message pane contents with an error message.

I produced an experimental build. In that build, I disabled that code. This means, whatever text has been added to the message pane display during decoding of the MIME data stream, it will remain in the message content. Instead, I used an alert dialog to inform the user we are running in this special case, where TB thinks it is unable to decrypt.

I asked my friend to use that build. The alert dialog box was displayed, but the decrypted message was displayed correctly!


My friend was kind enough to provide me with such a message. It has confidential contents, so I will NOT attach it here.

I think the message is indeed encrypted, because I can not decrypt or display the message using command line tools (I don't have the private key).


That engineering build also produced some trace information. I'll attach the patch I used.

trace_smime_decrypt: MimeCMS_eof, aRelativeNestLevel: 1, failed: 0, prev_rv: 0
trace_smime_decrypt: MimeCMS_eof, decoder_context->Finish, rv: 0
trace_smime_decrypt: no content_info


This tells us that decoding succeed, but for some reason the decoding did not provide us with content_info.
Attached patch patch used for testing purposes (obsolete) — Splinter Review
We were unable to reproduce the bug. The original message was sent to me, with my email address as an additional receipient.

Unfortunately I do not get that error message.

The only trace I get is:
trace_smime_decrypt: MimeCMS_eof, aRelativeNestLevel: 1, failed: 0, prev_rv: 0
trace_smime_decrypt: MimeCMS_eof, decoder_context->Finish, rv: 0

I do not get the additional trace message about "no content_info".


Our data decoding process works as this:

- crypto decoder gets constructed
- mail data bytes are piped through the mime decoder
- mail content area gets updated with decoded bytes
- after all data bytes are processed, decoder->Finish gets called, which is expected to produce a summary record, called content_info

Only the final step seems to fail on some systems.

I'll try to produce a build that produces more trace information about that final step.

I received an S/Mime message that has multiple recipients, I'm one of them.

- I am able to decrypt it
- my friend gets the error message

Does that mean the error depends on the recipients private key???

I received a log file from an attempt to decrypt one of the failing messages.

We fail in SEC_ASN1DecoderFinish, which is a small function and only fails if the ASN.1 decoder's state is in "needBytes".

How could decoding the same message
- always result in state needBytes for recipient A
- a state other than state needBytes for recipient B
???

I'm doing another build with DEBUG_ASN1D_STATES and CMSDEBUG.

Another suspicion: The receiver who is unable to decrypt received an incomplete message.
Kai, the situation you describe in comment 3 is certainly a different one
than the one in comment 0.  If it is a bug at all, (and I doubt that it is),
then it should be in a separate bug report.

If the received email message was truncated, then it cannot be completely 
decoded, and any signature cannot be verified.  The signatures themselves
and the signers' certs (with which the signatures can be verified) come
at the tail end of the ASN.1 BER-encoded SignedData sequence.  The decoder 
will correctly declare an error that some piece of encoded data is not as 
long as the header says it should be, and the decoder will fail.  Even if 
it didn't, the signature could not be tested without all the data present.
You can spent a LOT of effort understanding the voluminous output of 
DEBUG_ASN1D_STATES, (which will be further complicated by the fact that 
there are TWO ASN.1 BER decoders operating on the message simultaneously,
on on the encrypted data and one on the decrypted data), but you will find 
that it all comes down to the data being shorter than the ASN.1 BER encoding 
says it should be.  That's what "needBytes" means.

Whether signed or not, when the data is encrypted, the encrypted data 
itself is the last piece of the ASN.1 EncryptedData sequence.
One could imagine modifying the decoder used to decode the EncryptedData
message (which is encrypted) to look ahead in the ASN.1 templates to see 
if it is decoding the last element of the ASN.1 encoded data, and to be 
lenient about short data in the last block.  But that would be a 
significant enhancement to the ASN.1 BER encoder, which is one of the 
difficult pieces of NSS to keep correct, and it would be non-standard 
behavior.  

My take on this is that the inability to decode and decrypt a truncated 
message is presently how the code is designed to work.  It's not a bug.
If we contemplate enhancing the code to provide incomplete decryptions 
of incomplete encrypted messages, then at the least we need to provide 
some UI informing the reader that the message was incomplete.

But I think all of this is different from the original problem in comment 0.
In answer to Kai's question in comment 3:
> Does that mean the error depends on the recipients private key???

The success or failure of the decryption does indeed depend on each 
recipient's certificate and private key.  Each recipient receives his 
own special block of data, encrypted using the public key in the 
certificate named therein.  If the recipient cannot find the cert that
the sender claimed to have used in encrypting the message, or finds 
the wrong cert, or if the value decrypted with the private key is wrong
for any reason (which could include encryption failure by the sender or
decryption failure by the receiver) then NO part of the message can be 
decrypted.
(In reply to comment #5)
> Kai, the situation you describe in comment 3 is certainly a different one
> than the one in comment 0.


Nelson, so far it is a suspicion only that the messages are truncated on the receivers side.

In concluded that myself, because of the fact the ans.1 decoder reports NeedBytes state.


> If the received email message was truncated, then it cannot be completely 
> decoded, and any signature cannot be verified.

Sure, this is obvious.


>  The signatures themselves
> and the signers' certs (with which the signatures can be verified) come
> at the tail end of the ASN.1 BER-encoded SignedData sequence.  The decoder 
> will correctly declare an error that some piece of encoded data is not as 
> long as the header says it should be, and the decoder will fail.  Even if 
> it didn't, the signature could not be tested without all the data present.
> You can spent a LOT of effort understanding the voluminous output of 
> DEBUG_ASN1D_STATES, (which will be further complicated by the fact that 
> there are TWO ASN.1 BER decoders operating on the message simultaneously,
> on on the encrypted data and one on the decrypted data), but you will find 
> that it all comes down to the data being shorter than the ASN.1 BER encoding 
> says it should be.  That's what "needBytes" means.

Ok, thanks.


> My take on this is that the inability to decode and decrypt a truncated 
> message is presently how the code is designed to work.  It's not a bug.

I agree, if the messages arrive incomplete, they can't work correctly, that's clear. But so far this is only a suspicion.

At this point I'm still trying to find out what's really going wrong.
Let's wait for some more feedback from the people who reported the problem.


> If we contemplate enhancing the code to provide incomplete decryptions 
> of incomplete encrypted messages, 

So far I am not proposing to do that.

I based my earlier proposal on the assumption, the messages arrive complete and for some reason there is an internal failure in decoding the final block.

Because that's what I had been told so far.

> then at the least we need to provide 
> some UI informing the reader that the message was incomplete.

It would be helpful to be able to detect that an encoded message arrived incomplete, like some obvious end marker, so that we could report the end marker is missing and therefore the message obviously truncated.


> But I think all of this is different from the original problem in comment 0.

Comment 0 is an attempt to explain what is seen on some remote system.
As we still don't know what's really going on, comment 3 is another attempt to explain what might go on.
(In reply to comment #6)
> In answer to Kai's question in comment 3:
> > Does that mean the error depends on the recipients private key???
> 
> The success or failure of the decryption does indeed depend on each 
> recipient's certificate and private key.  Each recipient receives his 
> own special block of data, encrypted using the public key in the 
> certificate named therein.  If the recipient cannot find the cert that
> the sender claimed to have used in encrypting the message, or finds 
> the wrong cert, or if the value decrypted with the private key is wrong
> for any reason (which could include encryption failure by the sender or
> decryption failure by the receiver) then NO part of the message can be 
> decrypted.

You are right.

Because receipients are able to decrypt the message (at least using the special debug build that ignores failures at the end) we know that all recipients went were able to obtain the common message key.

My question was rather "could the numeric attributes of a message decryption key influence the decoding process".

But I immediately take this back.

Because decryption of the message contents seem to succeed (at least partially) we know that all receipients use the same message key.


Nelson, thanks for your explanation.

Let's see whether I can get confirmation that we really experience truncated messages.

Kai, as I understand comment 0, the user was able to see the attachments, 
but not the original message.  Was the multi-part S/MIME message perhaps 
arranged so that the attachments came BEFORE the main message body?  

Given the typical arrangement with the message body first and the attachments
after, the fact that any attachment could be decrypted would surely suggest 
that the main message was also correctly decrypted.  

Can you and the other recipients of that message compare the raw (unparsed,
undecrypted) messages, as they came over the wire, to see how they differ?
Assuming Kai is correct, the regression is: In the case of a truncated encrypted message, we used to display a partial result and now we display a "can not decrypt" message.

This does make sense. The can not decrypt message in the UI comes from the returned status from the encryption engine. In TB 1.0 the partial result was fed to the next layer in the client and displayed, however in the case where you couldn't decrypt anything, the user would just get a blank page.

In TB 1.5 and beyond we display an error page in the pane where the message would normally appear.

I see a couple of possible solutions:

1) try to detect output from the S/MIME engine and not overlay a partial result with an error page.
2) take a finer grain look a the error code coming back from S/MIME and only put up the error page on certain error results (this would also allow us to tailor the error page for those results).
3) consider that it's not a bug not display the partial result.

If we do 1 or 2, I would also be tempted to look for a way to display the fact the message has been truncated for some reason.

bob
We have confirmation that at least some messages are truncated.
It seems likely that all such messages were truncated.

Here's an offer for help with certificates -- if you folks already have this figured out just ignore it.

I wanted to offer some help with generating test certificates if you have any need for that. I've figured out how to use OpenSSL to generate a valid chain of certificates for e-mail usage which means you don't need to pay for certs from VeriSign or whoever. 

So, if it's of any use, I can generate test certs or send instructions on how to do it yourself with OpenSSL. Again, if you're already up to speed on this then never mind...
Changed summary to indicate truncation is the likely suspect.
Summary: Thunderbird reports "unable to decrypt" on messages that can be decrypted → Thunderbird reports "unable to decrypt" on truncated decryptable messages
I would like to propose that we start with a minimal fix for the stable Thunderbird 2.0 branch.

Given the fact that we can not add new wordings on the stable branch...:

Whenever we detect such a broken message, let's not replace the displayed message body, but use the same error string and display it in an alert box. This way, people are at least aware something is broken with the message, even though they get some (or most) content.
Attachment #271967 - Flags: review?(rrelyea)
Hmm. I notice the string contains some html tags. Should we be ok to have a couple of tags displayed inline in the error message? Or should we attempt to strip them out? Note we don't know what localizations are doing...

mozilla/mail/locales/en-US/chrome/messenger-smime/msgReadSMIMEOverlay.properties:CantDecryptTitle=%brand% cannot decrypt this message
mozilla/mail/locales/en-US/chrome/messenger-smime/msgReadSMIMEOverlay.properties:CantDecryptBody=The sender encrypted this message to you using one of your digital certificates, however %brand% was not able to find this certificate and corresponding private key. <br> Possible solutions: <br><ul><li>If you have a smartcard, please insert it now. <li>If you are using a new machine, or if you are using a new %brand% profile, you will need to restore your certificate and private key from a backup. Certificate backups usually end in ".p12".</ul>
Assignee: dveditz → kengert
Blocks: 284369
Keywords: regression
Comment on attachment 271967 [details] [diff] [review]
Patch v2 - workaround for stable branch

I'm pretty sure this will break the smart card insertion/removal stuff.

bob
Attachment #271967 - Flags: review?(rrelyea) → review-
(In reply to comment #18)
> (From update of attachment 271967 [details] [diff] [review])
> I'm pretty sure this will break the smart card insertion/removal stuff.

I conclude you don't like the "alert box" approach.

I conclude the only other possible fix for stable branches is:
- only show the error message if we decrypted zero bytes
- do not show any error message on truncated messages
  (this will produce a display with no encryption icon
   and no signature icons in the chrome)

Bob, do you concur?
yes,

I would say this is a corner case anyway.

bob
The easiest option is to not show the truncated message,

The best option is to show the error message if we decrypted zero bytes. That will give us the same semantic as the pre-smartcard patch.

bob
Attached patch Patch v3Splinter Review
Ok, after more thinking, I hope I have a much better patch, still suitable for 1.8 branch, too.

I realized that we have an icon for bad encryption. And we do have some wordings already like "message got altered", which seems appropriate for our scenario of a truncated message.


The patch introduces a new error code 
nsICMSMessageErrors::ENCRYPT_INCOMPLETE

During MIME decoding, we look at the number of bytes that got decoded.
If the summary fails, then
  if zero bytes got decoded, 
         we keep the current behaviour and error code
  if some bytes got decoded,
         we set the new error code ENCRYPT_INCOMPLETE

When we see this new error code, we keep the message body contents.

Because the error code is != success, we automatically get the "bad encryption" icon in the UI, making the user aware there is a problem.

In the advanced "message security info" that the user can open, I added a new section to handle this new error code. The window will use our existing strings:

  Message Cannot Be Decrypted
  The message contents appear to have been altered during transmission.


Note this will improve SeaMonkey as well (which never replaces the body with the error message), because SeaMonkey will show the "bad encryption" icon, too.


I did some testing of this patch by copying a signed and encrypted message, with a PDF file attachment, to a local folder mailbox file.

- when the private key for decrypting is not present,
  we still get the error message display in the message body

- I edited the mailbox and removed some trailing lines.
  The decrypted content was shown.
  We get the "bad encryption" status icon.

- If I remove sufficient bytes, the "is signed" icon goes away

- If I remove more bytes, the PDF attachment is still shown,
  however when saving to a file, its truncated
Attachment #264881 - Attachment is obsolete: true
Attachment #271967 - Attachment is obsolete: true
Attachment #272291 - Flags: review?(rrelyea)
Comment on attachment 272291 [details] [diff] [review]
Patch v3

r+

I like this solution.
Attachment #272291 - Flags: review?(rrelyea) → review+
Comment on attachment 272291 [details] [diff] [review]
Patch v3

Scott, can you please sr?
Attachment #272291 - Flags: superreview?(mscott)
Comment on attachment 272291 [details] [diff] [review]
Patch v3

looks good. Thanks kaie!
Attachment #272291 - Flags: superreview?(mscott) → superreview+
fixed on trunk
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment on attachment 272291 [details] [diff] [review]
Patch v3

I'm proposing to add this usability/regression fix to the stable branch.
Attachment #272291 - Flags: approval1.8.1.6?
Comment on attachment 272291 [details] [diff] [review]
Patch v3

approved for 1.8.1.7, a=dveditz for release-drivers
Attachment #272291 - Flags: approval1.8.1.7? → approval1.8.1.7+
fixed1.8.1.7
Keywords: fixed1.8.1.7
I have checked this with the 3.0b3pre release of Thunderbird (Shredder). Perhaps I am missing something but it still does not seem to work. Here's the sequence I used:

1) Under account settings choose not to download messages larger than 50kb.
2) Send yourself a signed, encrypted message larger then 50kb.
3) Ask Thunderbird to "Get Mail". The new message is shown with an error, stating that "Shredder cannot decrypt this message".

At this point I cannot figure out how to successfully download the full message from the POP server. If the message is not encrypted, there will be a "Click here to download the rest of the message" link. When the message is encrypted this link is not available. 

Am I missing something or is there still a problem here? Please tell me if there is some way to finish downloading the entire (signed & encrypted) message from the POP server. Or is this fix not included in the Shredder release I am using?

Sorry if this is redundant, but this used to work just fine with the SeaMonkey e-mail client.
zoltan, 

What version of seamonkey are you using (greater than 1.8.1.7?). If so, this is likely a completely different problem -- and probably needs a new bug.

bob
Bob,

I don't actively use SeaMonkey any more. The last version of SeaMonkey I used in which this problem did not exist was version 1.1.

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