491914 - Pre-specified data ignored by the NSS CMS Encoder

Status: NEW

(NSS :: Libraries, defect, P5)

Description

[Kai Engert [:KaiE:]]

Attachment: Reproducer, see comment 0

Forwarded from https://bugzilla.redhat.com/show_bug.cgi?id=499440

If NSS_CMSContentInfo_SetContent_Data() is called without specifing a SECItem
for the data, and data is later supplied in by NSS_CMSEncoder_Update(),
everything works correctly.

If NSS_CMSContentInfo_SetContent_Data() is called with a SECItem, the data in
the SECItem does not appear in the output at all.

To reproduce, compile the attached file with
   gcc $(pkg-config --cflags --libs nss glib-2.0) repro.c
and run
   ./a.out a/path/to/a/certificate.pem > x
"x" will be 1360 bytes large, and (cmsutil -D) will correctly display the
content.

If you redefine INLINE_DATA in the file to "1" and recompile, after
   ./a.out a/path/to/a/certificate.pem > x
"x" will be only 310 bytes large, and it won't contain the content.  derdump
will show that the octetstring that would contain the encrypted data is simply
missing.


AFAICS this is going on:
* When secasn1.e finishes encoding the contentEncAlg field of
NSSCMSEncryptedContentInfoTemplate, sec_asn1e_next_in_sequence() gets called.
* ...next_in_sequence does the "after" field notification, advances the "child"
template (to point to the rawContent field), and does the "before"
notification.  Note that the "current ASN1 state" is not changed, only the
template pointer is - the ASN1 state is inconsistent.
* the "before" notification ends up in the "if (before && dest ==
&(cinfo->rawContent))" condition of nss_cms_encoder_notify(), which correctly
notices that the raw data was pre-supplied, and calls nss_cms_encoder_work_data
().
* nss_cms_encoder_work_data () encrypts the pre-supplied raw data, and calls
SEC_ASN1EncoderUpdate() with it
* SEC_ASN1EncoderUpdate is in an inconsistent state - as noted above; it will
call sec_asn1e_next_in_sequence() *again*
* sec_asn1e_next_in_sequence() does the "after" field notification - for
rawContent (destroying the encryption context), advances the "child" template
*after* rawContent
* ... The ASN1 encoder continues encoding, but the "raw" field was already
stepped over and the supplied data will never be used.

Comment 1

[Benjamin Beurdouche [:beurdouche]]

Kai, can you please check the triage and update if needed ? : ) Thanks !

Comment 2

[Kai Engert [:KaiE:]]

This is a report that the API doesn't behave as it is documented.

There are two ways to call NSS_CMSContentInfo_SetContent_Data, but only one of them works correctly.

As of today, Thunderbird only uses the API in the way that works.

This means, it would be nice to fix this bug, but it isn't urgent.