349949 - unable to get CMS library to create messages with definite-length encodings

Status: RESOLVED WONTFIX

(NSS :: Libraries, enhancement, P5)

Description

[Nalin Dahyabhai]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.6) Gecko/20060808 Fedora/1.5.0.6-3 Firefox/1.5.0.6 pango-text
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.6) Gecko/20060808 Fedora/1.5.0.6-3 Firefox/1.5.0.6 pango-text

RFC 4556 notes that CMS messages which use indefinite-length encoding may not be able to be parsed by other (unspecified) CMS implementations, and it appears that the CMS API uses indefinite-length encodings even when the calling application doesn't need or request that it do so.  I'd like some way to force the behavior.

Reproducible: Always

Comment 1

[brenan99]

Attachment: Patch for proper non-streaming definite length encoding

This is a patch for NSS 3.12.4 to do proper DER encoding of CMS messages. It is not as efficient as streaming indefinite length but DER is not meant to be streamed.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Does Your patch break Thunderbird's ability to send LARGE signed and/or 
encrypted email messages in a single pass?

Comment 3

[brenan99]

(In reply to comment #2)
> Does Your patch break Thunderbird's ability to send LARGE signed and/or 
> encrypted email messages in a single pass?

I can not speak for Thunderbird as I have not tested the patch with it; however, I can tell you that the patch removes "streaming mode" for DER. Instead it buffers the data until it has ALL the bytes; only then, does it start encoding. This is of course can be less efficient than streaming, especially with large messages since they will instead be buffered in memory.

It is kind of an ugly patch if you want to keep streaming "DER" functionality as it rips that out, but I need properly formed DER that could be parsed by another application and so made the changes as they are and then released the patch in case anyone else wants to run with it.

Comment 4

[Vincent Agriesti]

Attachment: Patch uses the code from the first patch in this bug, but isolates the CMS DER Encoder code into it's own file (cmsderencode.c). Some refactoring is also applied to distinguish BER from DER encoding.

Patch uses the code from the first patch in this bug, but isolates the CMS DER Encoder code into it's own file (cmsderencode.c). Some refactoring is also applied to distinguish BER from DER encoding.

This patch essentially adds 4 new calls to the CMS API. NSS_CMSDEREncoder_(Start/Update/Finish/Cancel) are the new calls. They incorporate code from a previously submitted patch. Also, the NSSCMSEncoderContext has been duplicated and modified as NSSCMSDEREncoderContext. This is because we don't want BER encoding operations to get mixed up with DER encoding operations.

Patch was applied to 3.12.7 and no tests have been written yet.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 473532 [details] [diff] [review]
Patch uses the code from the first patch in this bug, but isolates the CMS DER Encoder code into it's own file (cmsderencode.c). Some refactoring is also applied to distinguish BER from DER encoding.

I don't know what "feedback:+" means, especially when the patch writer 
applies it to his own patch.  Please explain.

Were you trying to ask for code review?

Comment 6

[Vincent Agriesti]

I have a requirement for the encoder to do proper DER encoding aka definite length encoding. The original patch was not by me, I simply used the same logic but rearranged it to not interfere with the current behavior of the CMS Encoder. This should eliminate concerns with messing up performance and expected behavior with applications like Thunderbird.

I am new to the Mozilla dev community, so I figured feedback:+ meant "I'd like feedback on the approach/patch please." I know the patch itself needs work, but was just hoping that a little feedback on what may or will not fly would go a long way.

Such as, is it bad to add another set of encoder api calls to NSS? So you'd have one set for streaming and one that is more strict? 

Another approach was to break up the NSS_CMSEncoder_Start function into several smaller functions. You could call them *_Init and *_Run. Then between the *_Init and *_Run you could set variables for different behaviors like the more strict RFC definite length encoding rules?

Comment 7

[Kai Engert [:KaiE:]]

I don't know if Thunderbird might need this.

The cited RFC 4556 appears to focus on kerberos, and I don't see a current need to support kerberos-related encryption in Thunderbird.