allow disabling 3DES wrapping of keys in SSL session cache

RESOLVED FIXED in 3.11

Status

P1
enhancement
RESOLVED FIXED
14 years ago
12 years ago

People

(Reporter: julien.pierre, Assigned: nelson)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

14 years ago
We currently wrap the SSL session cache keys with DES every time there is a full
handshake . On restart handshake, we unwrap them . This is a security measure
designed to prevent keys from being visible in the clear in core files. It is a
good one, but not always necessary depending on the environment, in particular
for benchmarking . The use of DES was measured with dtrace to cost 9% in restart
SSL handshakes .
We need a way to disable this SSL session key wrapping for performance minded
environments and benchmarks . The default should probably remain the same as it
is today.
(Reporter)

Comment 1

14 years ago
Targetting to 3.10 .
Priority: -- → P1
Target Milestone: --- → 3.10
(Reporter)

Comment 2

14 years ago
I have also measured that the cost of wrapping SSL session cache keys with DES
in a full handshake test is only 2.2% ; but of course in this case there is more
CPU time spent in RSA and far fewer sockets . A hardware accelerator full
handshake test would probably show this cost as being much higher than 2%.
(Reporter)

Comment 3

14 years ago
Created attachment 168748 [details] [diff] [review]
hack

This is a quick hack to try to implement this RFE . It works , but only when
using NSS softoken, because in one place, I wasn't sure how to obtain the slot
to pass to PK11_ImportSymKey . And in the other place, I may not be using the
proper slot either . Nelson, hopefully you can help resolve this.

This patch is based on a macro, SSL_WRAP, which is not set by default . This is
of course not suitable for checkin, but it was convenient to get this to
quickly run in a lab . For checkin, we'll probably want a runtime toggle rather
than compile-time .

I measured the performance improvement of using this patch and got about 3% for
restart handshakes. I had a baseline of 4114 ops/s in the particular build I
was using, and this patch brought it up to 4240 . This is less than I expected.
I verified using dtrace that no DES functions were being called at all anymore
in this test, so this patch achieved the stated goal . However, the function
that is used to restore the master secret to a PK11SymKey*, PK11_ImportKey,
comes with its own overhead . I used my latest dtrace script to measure it and
got :

CPU time (us) - per CPU second :

 PK11_ImportSymKey				     Global		       
				   32016
 Total						     Global		       
				 1000955

Ie., this function (which is only being called from my patch) takes 3.2% of the
CPU time. In theory, it should only amount to a memcpy of 48 bytes (the size of
the master secret) for each restart handshake, but the pk11wrap + softoken
overhead to create this session object is apparently quite high. 

This might be a case where we could optimize the SSL session cache and simply
store a PK11SymKey* pointer in the cache entry, rather than a copy of the raw
key material, as this patch does. However, this can only work properly for
single-process SSL servers, and even in this case it will require additional
cleanup code to free the old keys when an old SSL session cache entry is
overwritten . The benefit would be avoiding the call to PK11_ImportKey
altogether .

Comment 4

14 years ago
Just a note, there were 2 reasons for wrapping the keys... 1) to prevent keys in
the clear on the disk (which Julien already meantioned), and 2) to provide
better performance for FIPs tokens. In the FIPS case, if we aren't wrapping the
key in with a known key, then we have to do an RSA key exchange.

You will want to specify this in your documentation if you expose this option to
the user.

Comment 5

14 years ago
Julien,

You patch appears to find the slot from the SSL cache, which should be correct
(the master key should wind up in the same slot it was in previously). Are you
sure your patch doesn't work on other tokens. (Actually even if you imported it
into the wrong slot, it will almost certainly continue to work -- NSS moves keys
around if it needs to to get it's job done).

bob
(Reporter)

Comment 6

14 years ago
Bob,

We have had some discussions about dtrace lately and we aren't confident about
half the time being spent in DES unwrap vs RSA anymore . It could well be much
less, but it is still significant. The matter still needs to be looked into.

I agree this needs to be made optional for security reasons. Based on your
input, it sounds like we should keep the current wrapping behavior the default
for the FIPS case . For the non-FIPS case, I'm not sure what the default should
be, but we'll definitely want to make it configurable.

Regarding the patch, I'm not sure the patch fails on other tokens - I haven't
actually had a chance to try it yet.

(Assignee)

Updated

14 years ago
QA Contact: bishakhabanerjee → jason.m.reid
(Assignee)

Comment 7

14 years ago
This work is planned for 3.11
Target Milestone: 3.10 → 3.11
(Reporter)

Comment 8

13 years ago
This work was completed as part of the SSL bypass. Marking fixed.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.