Closed
Bug 373276
Opened 17 years ago
Closed 17 years ago
Enhance SSL's Bypass feature to withstand failures
Categories
(NSS :: Libraries, enhancement, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
3.11.7
People
(Reporter: nelson, Assigned: nelson)
Details
Attachments
(1 file)
16.13 KB,
patch
|
alvolkov.bgs
:
review+
neil.williams
:
review+
|
Details | Diff | Splinter Review |
In NSS 3.11 we added a performance feature to libSSL that enables it to optionally "bypass" the overhead of the PK11Wrap layer and the PKCS#11 module and call directly into the basic crypto algorithm code for SOME (not all) operations. In particular, we always do all asymmetric (public/private key) algorithms in a PKCS#11 module. We never bypass PKCS#11 for those algorithms. The performance benefits were SUBSTANTIAL. With the bypass enabled, and with the other performance enhancements we did, NSS 3.11 is over twice as fast as NSS 3.10 in certain benchmarks using SSL_RSA_WITH_RC4_128_MD5 or SSL_RSA_WITH_RC4_128_SHA cipher suites, doing over twice as many connections per second. We knew that this feature did not appeal to all NSS users. Clearly this bypass operation is incompatible with FIPS 140, so we created an SSL socket option to turn this feature on or off, default to off. The application must explicitly enable this feature to use it. The bypass is not compatible with all PKCS#11 modules, and particularly not with some FIPS 140 compliant modules that do no RSA decryption, and do not allow any keys (including derived keys) to be extracted from the module "in the clear" (that is, unwrapped). As originally implemented, if the application enabled the bypass feature, but was using an incompatible PKCS#11 module (as described above), all "full" handshakes would fail. The bypass code had no ability to recover from a failure to get certain values from the PKCS#11 module. It was unable to "fall back" on the original non-bypass pure PKCS#11 code base. This RFE requests that the bypass be made able to fall back when it is used with incompatible PKCS#11 modules. Accomplishing that task will make the bypass code somewhat slower, and will make it use more memory. These effects only occur when bypass is enabled.
Attachment #257912 -
Flags: review?(alexei.volkov.bugs)
Assignee | ||
Updated•17 years ago
|
Priority: -- → P1
Assignee | ||
Comment 1•17 years ago
|
||
Comment on attachment 257912 [details] [diff] [review] patch v1 Some explanation of this patch. Background: There are two places in the flow of an SSL/TLS handshake where the bypass tries to stop using PKCS#11 and do everything after that point without PKCS#11. These two places are known as the "triple bypass" and the "double bypass". The triple bypass is used only in full handshakes that use the RSA key exchange algorithm. It is called the triple bypass because it bypasses PKCS#11 for 3 things: - the derivation of the master secret from the pre-master secret - the derivation of the cipher and MAC keys from the master secret - the bulk data enciphering/deciphering and MACing. The triple bypass attempts to use the PKCS#11 module with the RSA private key to decrypt the PMS into process memory as data, rather than trying to unwrap it into the module as a session key. Without this patch, if that decryption attempt fails, the handshake fails. This patch causes a failed triple bypass to fall back on attempting a double bypass. This is thought to have negligible cost, and I believe it does not reintroduce Bleichebacher's "million question" attack. A double bypass uses the PKCS#11 module to derive the pre-master secret and then to derive the master secret from the pre-master secret. The derived master secret is a session key in the PKCS#11 module. The double bypass attempts to fetch the value of the master secret from the PKCS#11 module "in the clear" (without wrapping it with another key). It is called the "double bypass" because when it succeeds, it bypasses PKCS#11 for two subsequent groups of operations: - the derivation of the cipher and MAC keys from the master secret - the bulk data enciphering/deciphering and MACing. A double bypass failure occurs when and if the derived master secret cannot be extracted from the PKCS#11 module "in the clear". This patch adds support for recovering from a double bypass failure. This support is conditionally compiled, predecated on the definition of a new symbol: NSS_SURVIVE_DOUBLE_BYPASS_FAILURE . It is conditional because it has performance cost. We believe it would be better to detect the conditions for double bypass failure at configuration time and avoid them by disabling the bypass feature. We may yet make it run-time conditional rather than compile time conditional. To recover from the double bypass failure, it is necessary to completely recompute the "handshake hashes", which are the hashes of all the SSL/TLS handshake messages sent or received at the point where they are needed. When the bypass is enabled, we optimistically begin to compute the hashes outside of the PKCS#11 module, but if the double bypass fails, then the hashes must be recomputed INSIDE the PKCS#11 module because those hashes need the value of the master secret. So, being prepared to recover from double bypass failure means keeping a copy of the handshake messages, in order to be able to feed them into the PKCS#11 module for the hashes, if the recovery is needed. Being able to be flexible about the choice of module in which the handshake hashes are computed may have other benefits to NSS. This patch: The code to initialize the handshake hashes was duplicated in several places in ssl3con.c. This patch creates a new function to do that task, and replaces all the duplicated code with calls to the new function. With just one function to initialize the hashes, and just one to update them, the logic for handling the two cases can be minimized. This code also eliminates a bunch of duplicated code whose purpose is to append bytes of data to an SSLBuffer. It creates one new function to do that, and calls it from the places that duplicated it.
Assignee | ||
Comment 2•17 years ago
|
||
Commited on bypass branch: Checking in config.mk; new revision: 1.21.26.1; previous revision: 1.21 Checking in ssl3con.c; new revision: 1.76.2.19.6.1; previous revision: 1.76.2.19 Checking in sslimpl.h; new revision: 1.42.2.9.2.1; previous revision: 1.42.2.9 Checking in sslsecur.c; new revision: 1.34.2.3.2.1; previous revision: 1.34.2.3
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•17 years ago
|
||
Comment on attachment 257912 [details] [diff] [review] patch v1 This needs two reviews for NSS 3.11.7 ASAP
Attachment #257912 -
Flags: superreview?(wtc)
Attachment #257912 -
Flags: review?(neil.williams)
Comment 4•17 years ago
|
||
Comment on attachment 257912 [details] [diff] [review] patch v1 I think ss->ssl3.hs.md5_cx and ss->ssl3.hs.sha_cx will be leaked after recovery from double bypass.
Attachment #257912 -
Flags: review?(alexei.volkov.bugs) → review-
Comment 5•17 years ago
|
||
Comment on attachment 257912 [details] [diff] [review] patch v1 I was wrong: there is no leak bug. sha_cx and md5_cx are the part of SSL3HandshakeState structure and will be freed with it. The found problem is that functions SHA1_DestroyContext and MD5_DestroyContext wont be called for sha_cx and md5_cx buffers in case of recovery from double bypass. The problem is not important, since SHA1_DestroyContext only zero the context of sha1 and MD5_DestroyContext does not do anything at all then second argument is PR_FALSE.
Attachment #257912 -
Flags: review- → review+
Comment 6•17 years ago
|
||
Comment on attachment 257912 [details] [diff] [review] patch v1 r=+, but sp. error in patch ssl3con.c line ~6564; s/PSM/PMS
Attachment #257912 -
Flags: review?(neil.williams) → review+
Assignee | ||
Comment 7•17 years ago
|
||
Committed on trunk: Checking in config.mk; new revision: 1.22; previous revision: 1.21 Checking in ssl3con.c; new revision: 1.101; previous revision: 1.100 Checking in sslimpl.h; new revision: 1.56; previous revision: 1.55 Checking in sslsecur.c; new revision: 1.38; previous revision: 1.37 On branch, next.
Assignee | ||
Comment 8•17 years ago
|
||
On Branch: Checking in config.mk; new revision: 1.21.2.1; previous revision: 1.21 Checking in ssl3con.c; new revision: 1.76.2.20; previous revision: 1.76.2.19 Checking in sslimpl.h; new revision: 1.42.2.10; previous revision: 1.42.2.9 Checking in sslsecur.c; new revision: 1.34.2.4; previous revision: 1.34.2.3
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 9•17 years ago
|
||
Comment on attachment 257912 [details] [diff] [review] patch v1 I didn't review this patch in time. Sorry.
Attachment #257912 -
Flags: superreview?(wtc)
You need to log in
before you can comment on or make changes to this bug.
Description
•