Closed Bug 274132 Opened 15 years ago Closed 15 years ago
allow disabling 3DES wrapping of keys in SSL session cache
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.
Targetting to 3.10 .
Priority: -- → P1
Target Milestone: --- → 3.10
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%.
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 .
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.
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
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.
QA Contact: bishakhabanerjee → jason.m.reid
This work is planned for 3.11
Target Milestone: 3.10 → 3.11
This work was completed as part of the SSL bypass. Marking fixed.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.