379753 - S/MIME should support AES

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P2)

Description

[Nicholas Sushkin]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.5; Linux) KHTML/3.5.4 (like Gecko)
Build Identifier: 

S/MIME should support AES. AES has been around for a while and is supported by other S/MIME implementations like KDE KMail/gpgsm. Most recently, AES (RFC 3565) has been added as a SHOULD implement to S/MIME 3.1 (RFC 3851). It's time for NSS AES support to move from planning to the implementation stage. ;)

http://www.apps.ietf.org/rfc/rfc3851.html#sec-2.7
http://www.apps.ietf.org/rfc/rfc3565.html

2.7 ContentEncryptionAlgorithmIdentifier

Sending and receiving agents MUST support encryption and decryption with DES
EDE3 CBC, hereinafter called "tripleDES" [CMSALG]. Receiving agents SHOULD
support encryption and decryption using the RC2 [CMSALG] or a compatible
algorithm at a key size of 40 bits, hereinafter called "RC2/40". Sending and
receiving agents SHOULD support encryption and decryption with AES [CMSAES] at
a key size of 128, 192, and 256 bits.

...

   [CMSAES]      Schaad, J., "Use of the Advanced Encryption Standard
                 (AES) Encryption Algorithm in Cryptographic Message
                 Syntax (CMS)", RFC 3565, July 2003.

Reproducible: Always

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1 (checked in on trunk)

This patch causes
a) the CMIME capabilities record in outgoing signed emails to say that I 
can receive (and prefer) AES_128_CBC,
b) when looking for the cipher to use for sending encrypted messages, it 
recognizes AES_128_CBC in other people's SMIME capabilities (stored in 
their SMIME profiles),

But it DOES NOT enable me to send a message encrypted with AES_128_CBC.

And I think the reason is due to another latent bug, undiscovered since
libSMIME was written, until now.

The problem occurs in function smime_choose_cipher() in file 
lib/smime/smimeutil.c.  That function has a loop (see the comment 
" /* walk all the recipient's certs */" ) that tries to get the stored
SMIME profile for every recipient.  If it fails to find a profile for 
ANY recipient, then it ends up using a "default" cipher, which is either
RC2-40 or 3DES.  

The problem is that it ALWAYS fails to find the stored SMIME profile for 
the sending user, that is, for any recipient with a "user" cert.  
The function CERT_FindSMimeProfile() always returns NULL for the user's
own cert.  This causes libSMIME to always send an encrypted message using
the "default" cipher, which is never AES, always RC2-40 or 3DES.   

It seems that the cert DB has an SMIME profile record, but it contains a
zero length SMIME capabilities, and a zero-length capabilities timestamp.

Now, I'm not sure how this is SUPPOSED to work.  When we're looking for 
our own profile, the profile of a cert for which we are the user, should
we look for it in the cert DB?  Is the SMIME profile record for a user 
cert supposed to contain a valid set of SMIME capabilities?  

Or should we say "hey, this is MY cert, I'm going to generate an SMIME 
capabilities with my current capabilities and substitute that"?

It seems to me that we do NOT want to store oru own capabilities in the 
cert DB, because our capabilities can change.  So, I think we want to 
generate our own capabilities and use them, rather than using old stores
ones, for our own cert.  

If that analysis is correct, then a bunch of code needs to be written, 
because neither smime_choose_cipher() nor CERT_FindSMimeProfile() has any
logic that does anything like that.  

Suggestions?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Maybe Sean will have some ideas. 

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

I was mistaken.  CERT_FindSMimeProfile() does not return NULL.
It returns a pointer to a SECItem with a zero len value.
Same effect as if it returned a NULL.

When I try to send an encrypted message, pipnss calls
CERT_SaveSMimeProfile() with NULL pointers for emailProfile and
profileTime.  THAT appears to be the reason why my cert DB doesn't
have a valid set of SMIME capabilities in the SMIME profile record
for my user certs.

For some reason, while trying to send an encrypted message, pipnss
calls NSS_SMIMEUtil_FindBulkAlgForRecipients more than once.
Seems wasteful to recompute all that over and over for a single
sent message.  I *suspect* it is regenerating the message a second time
to store it in the SENT folder.  That's evil, if true, because it means 
the message saved in the SENT folder is NOT a true copy of the message 
actually sent!

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

I have successfully read and decrypted an AES_128_CBC message with the
first patch above.  So, the only remaining issue is why pipnss 
clobbers the SMIME capabilities in the SMIME profile for my user cert
when I send a signed or encrypted email message (as described in the 
previous comment).  :-/  There probably should be a separate bug filed
on that.  

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 264085 [details] [diff] [review]
patch v1 (checked in on trunk)

I believe this patch actually is complete.  There is another separate bug that must also be fixed before AES encryption can be done, but that is the subject of bug 380091.  There is a workaround patch for that bug, which I shall attach to this bug shortly.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Attachment: NSS workaround for bug 380091 (checked in)

This patch, copied from bug 380091, works around a PSM problem that 
clobbers the SMIME capabilities for local user certs.  This patch is
NOT for checkin, but is useful for testing the other patch attached to 
this bug, which IS for checkin.

Comment 9

[Robert Relyea]

Comment on attachment 264085 [details] [diff] [review]
patch v1 (checked in on trunk)

r+=rrelyea

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 264085 [details] [diff] [review]
patch v1 (checked in on trunk)

First patch checked in on trunk

Checking in util/ciferfam.h;   new revision: 1.3;  previous revision: 1.2
Checking in smime/smimeutil.c; new revision: 1.20; previous revision: 1.19

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 264204 [details] [diff] [review]
NSS workaround for bug 380091 (checked in)

I've changed my mind about this patch.  
I still regard it as something of a hack, but it has been working
for me ever since I wrote it, without ill effects that I can see.

I had originally thought that this patch might stop the code from
saving an SMIME profile record for some user cert in some 
circumstance where it would be desirable to do save the profile
anyway. But I can no longer think of any case where that would occur.  

So, now I'm requesting two reviews.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

(I marked 3 prior comments in this bug as private, because they were 
comments made in the wrong bug.  Marking them private reduces confusion 
in this bug.)

Comment 14

[Robert Relyea]

Comment on attachment 264204 [details] [diff] [review]
NSS workaround for bug 380091 (checked in)

r+=relyea

Julian, if you get here before kai, you can take his review slot.

While this patch works around an existing bug, it is also not an unreasonable sanity check.

my only concern is the 'bootstrap' case. What happens when you first get your cert (can you encrypt to yourself?).

bob

Comment 15

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 264204 [details] [diff] [review]
NSS workaround for bug 380091 (checked in)

Checking in certdb/stanpcertdb.c; new revision: 1.75; previous revision: 1.74