274132 - allow disabling 3DES wrapping of keys in SSL session cache

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Julien Pierre]

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.

Comment 1

[Julien Pierre]

Targetting to 3.10 .

Comment 2

[Julien Pierre]

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%.

Comment 3

[Julien Pierre]

Attachment: 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

[Robert Relyea]

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

[Robert Relyea]

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

Comment 6

[Julien Pierre]

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.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

This work is planned for 3.11

Comment 8

[Julien Pierre]

This work was completed as part of the SSL bypass. Marking fixed.