Closed Bug 848384 Opened 12 years ago Closed 11 years ago

Change the default value of the 'policy' setting from SSL_NOT_ALLOWED to SSL_ALLOWED for all cipher suites

Categories

(NSS :: Libraries, defect, P2)

3.14.5
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.15.2

People

(Reporter: agl, Assigned: wtc)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Attached file patch (obsolete) —
The cipher suite policy code has been ineffective for some time, except to make people wonder why nothing works until they call NSS_SetDomesticPolicy. This change removes the last bits of the policy support. Ciphersuites can still be enabled/disabled by setting the preference, of course.
Attachment #721733 - Flags: review?(wtc)
Attachment #721733 - Attachment is obsolete: true
Attachment #721733 - Flags: review?(wtc)
Attachment #789053 - Flags: review+
Attachment #789053 - Flags: checked-in+
Summary of changes: 1. Originally, each cipher suite was controlled by three settings: * enabled * isPresent * policy The 'policy' setting was initialized to SSL_NOT_ALLOWED for all cipher suites. (This is why an NSS-based program must call NSS_SetDomesticPolicy() for SSL/TLS to work.) 2. In NSS 3.13.2 (bug 651523), NSS_SetExportPolicy() and NSS_SetFrancePolicy() became synonyms of NSS_SetDomesticPolicy(). 3. In patch v2 of this bug (comment 1), the cipher suite policy code was completely removed. This means each cipher suite is now controlled by only two settings: * enabled * isPresent The 'enabled' setting is the cipher suite preference. The 'isPresent' setting is determined at run time by the presence of a crypto token that supports the cipher suite. It is no longer necessary to call NSS_SetDomesticPolicy(). It is as if the 'policy' setting were hardcoded to be SSL_ALLOWED for all cipher suites. SSL_CipherPolicySet() does nothing and returns SECSuccess. SSL_CipherPolicyGet sets the output argument *policy to SSL_ALLOWED and returns SECSuccess. Question: an alternative is to simply initialize the 'policy' setting to SSL_ALLOWED for all cipher suites and keep the cipher suite policy code. This will also make it unnecessary to call NSS_SetDomesticPolicy(). I support Adam's patch to remove the cipher suite policy code completely because I think the cipher suite preference code is sufficient to control which cipher suites should be enabled. But I'd like to know if there are objections. Thanks.
Assignee: nobody → agl
Severity: enhancement → normal
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → 3.15.2
I support removal of cipher policy code. Security policy is hard enough for most people to understand, even those who need to. Having two bits of enable information where one is sufficient was a design error. However, I notice bug 848384 was closed as a duplicate of this bug. The gist of that bug was to provide a way to have the enable bits for cipher suites set to useful/appropriate defaults that will change over time. For example, the API proposed in that bug probably should turn off RSA/RC4/MD5 cipher suites by default today as they aren't appropriate for most uses (except when unavoidable for backwards compatibility). And I could see RSA/RC4/SHA1 suites going away in some number of years. The proposal was to have an API with that behavior so applications that wanted complete interface stability over security could choose not to call it and those that wanted security over interface stability could call it. I would like to understand how that will be addressed by this bug. Thanks.
(In reply to Chris Newman from comment #5) > However, I notice bug 848384 was closed as a duplicate of this bug. The gist > of that bug was to provide a way to have the enable bits for cipher suites > set to useful/appropriate defaults that will change over time. For example, > the API proposed in that bug probably should turn off RSA/RC4/MD5 cipher > suites by default today as they aren't appropriate for most uses (except > when unavoidable for backwards compatibility). RC4 is actually unavoidable for backwards compatibility. Browsers which do not support RC4 cannot connect to the web site of the German Federal Information Security Agency <https://www.bsi.bund.de>, for example. > And I could see RSA/RC4/SHA1 > suites going away in some number of years. The proposal was to have an API > with that behavior so applications that wanted complete interface stability > over security could choose not to call it and those that wanted security > over interface stability could call it. I think the desire for complete behavioral stability is a bit at odds with security updates. To fix protocol-level issues, we have to make externally visible changes. From a distribution perspective, we'd rather have the behavioral upgrades by default. We'll have to patch all NSS users to call something like NSS_SetContemporaryPolicy otherwise. This doesn't apply just to cipher suites, but also to TLS version upgrades.
My personal preference is that a "secure-by-default-at-the-API-layer" philosophy trumps interface stability (but only barely -- interface stability has been a huge benefit of NSS relative to other SSL libraries). In the past NSS designers considered interface stability more important than secure-by-default. The proposal in bug 842307 was a compromise between these two positions so a consuming application could choose a philosophy, but if NSS designers have switched to the secure-by-default design philosophy then I'm personally fine with the outcome. The downside to the secure-by-default philosophy for cipher suites is applications can't hardcode cipher suite information; they have to query the NSS layer using SSL_ImplementedCiphers, SSL_NumImplementedCiphers, SSL_GetCipherSuiteInfo to get the list of ciphers that can be enabled/disabled by admin configuration and provide a way to adjust the defaults. Sample code to do that is in bug 358440. Secure-by-default will make it necessary to disable RC4 by default in the near future (http://blog.cryptographyengineering.com/2013/03/attack-of-week-rc4-is-kind-of-broken-in.html), so end-users who want to use sites that require use of insecure ciphers will need a way to turn that back on manually until the sites get fixed.
(In reply to Chris Newman from comment #5) > > However, I notice bug 848384 was closed as a duplicate of this bug. The gist > of that bug was to provide a way to have the enable bits for cipher suites > set to useful/appropriate defaults that will change over time. I don't know the current goal of bug 848384. I thought it was a complaint about the need to call NSS_SetDomesticPolicy() in order to use any cipher suite. I will reopen bug 848384, and limit the scope of this bug to the fate of the SSL cipher suite policy code.
Bob: this patch implements the changes we discussed in the NSS meeting this morning. 1. It brings back the 'policy' setting for cipher suites. 2. It changes the default value of the 'policy' setting from SSL_NOT_ALLOWED to SSL_ALLOWED for all cipher suites. The default value of the 'policy' setting of all supported cipher suites is specified in the 'cipherSuites' array in ssl3con.c. 3. It changes NSS_SetDomesticPolicy() to use the SSL_ImplementedCiphers array rather than an 'ssl_ciphers' array defined locally in sslsock.c. 4. Please check the documentation of the three NSS_SetXXXPolicy() functions in ssl.h. I will try to generate another patch that shows the diffs against the original code before the removal patch.
Attachment #790949 - Flags: review?(rrelyea)
Bob: this custom patch is generated against the state of the tree before the removal patch. It helps you review the net changes.
Attachment #790954 - Flags: feedback?(rrelyea)
Comment on attachment 790954 [details] [diff] [review] [Custom patch to help review] Resurrect the SSL cipher policy framework, but change default to SSL_ALLOWED This custom patch (the net changes from the original code before the SSL cipher policy code was removed) can also be reviewed at https://codereview.chromium.org/23279007/.
Attachment #790954 - Flags: review?(ryan.sleevi)
Comment on attachment 790949 [details] [diff] [review] Resurrect the SSL cipher policy framework, but change default to SSL_ALLOWED I checked in this patch before obtaining Bob's review because it mostly adds back the code that was removed: https://hg.mozilla.org/projects/nss/rev/87b9b62d8028 Please review 1. the comments in ssl.h that describe the current state of NSS_SetExportPolicy and NSS_SetFrancePolicy, and 2. the new implementation of NSS_SetDomesticPolicy in sslsock.c and approve the change to initialize the 'policy' setting of all cipher suites to SSL_ALLOWED instead of SSL_NOT_ALLOWED. It is easier to review these changes using the custom patch (attachment 790954 [details] [diff] [review]).
Attachment #790949 - Flags: checked-in+
Updated the bug's summary to reflect the final change.
Assignee: agl → wtc
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Summary: Remove vestigial cipher suite policy code. → Change the default value of the 'policy' setting from SSL_NOT_ALLOWED to SSL_ALLOWED for all cipher suites
Blocks: 935597
Attachment #790954 - Flags: review?(ryan.sleevi) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: