Enable additional SSL ciphers, add configuration UI

VERIFIED FIXED in psm2.4

Status

Core Graveyard
Security: UI
--
enhancement
VERIFIED FIXED
15 years ago
a year ago

People

(Reporter: kaie, Assigned: kaie)

Tracking

Other Branch
psm2.4

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Assignee)

Description

15 years ago
The intention of this bug is to:
- make Mozilla's SSL code use additional ciphers available in NSS
- provide an appropriate UI to control which ciphers are enabled
- enable additional new ciphers by default

The dialog shown "Edit/Preferences/Privacy/SSL/Edit Ciphers" should get
changed appropriately.

(This bug was originally filed as bug 102633, and some historical information
can be found there. However, because we are now using a different strategy for
implementation, and given the fact the other bug is already long and complex,
I've decided to use a fresh bug.)
(Assignee)

Comment 1

15 years ago
*** Bug 102633 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

15 years ago
Target Milestone: --- → 2.4
(Assignee)

Comment 2

15 years ago
Instead of the complex UI using scrolling checkboxes attempted in bug 102633, we
would like to use a simple UI that avoid scrolling, but distributes the
checkboxes on three panes in a tabbed dialog.

The wording for the checkboxes (including for the new ciphers) has been cleaned
up and optimized by Nelson. Thanks!

I'm reusing some code from the the former patches in bug 102633. We provide the
new ability to open a small detailed information dialog that provides more
information about a cipher in a structured layout.

Here is the list of new ciphers and the names of their preference strings, that
will be ENABLED BY DEFAULT:
security.ssl3.rsa_rc4_128_sha      SSL_RSA_WITH_RC4_128_SHA
security.ssl3.dhe_rsa_aes_256_sha  TLS_DHE_RSA_WITH_AES_256_CBC_SHA
security.ssl3.dhe_dss_aes_256_sha  TLS_DHE_DSS_WITH_AES_256_CBC_SHA
security.ssl3.rsa_aes_256_sha      TLS_RSA_WITH_AES_256_CBC_SHA
security.ssl3.dhe_rsa_aes_128_sha  TLS_DHE_RSA_WITH_AES_128_CBC_SHA
security.ssl3.dhe_dss_aes_128_sha  TLS_DHE_DSS_WITH_AES_128_CBC_SHA
security.ssl3.rsa_aes_128_sha      TLS_RSA_WITH_AES_128_CBC_SHA
security.ssl3.dhe_rsa_des_ede3_sha SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA
security.ssl3.dhe_dss_des_ede3_sha SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA
security.ssl3.dhe_rsa_des_sha      SSL_DHE_RSA_WITH_DES_CBC_SHA
security.ssl3.dhe_dss_des_sha      SSL_DHE_DSS_WITH_DES_CBC_SHA

For completenes, here is one new cipher that will be supported, but will be
DISABLED by default:
security.ssl3.rsa_null_sha         SSL_RSA_WITH_NULL_SHA

(Assignee)

Comment 3

15 years ago
Created attachment 109067 [details] [diff] [review]
Patch v1
(Assignee)

Comment 4

15 years ago
Created attachment 109069 [details]
Screenshot 1/4
(Assignee)

Comment 5

15 years ago
Created attachment 109070 [details]
Screenshot 2/4
(Assignee)

Comment 6

15 years ago
Created attachment 109071 [details]
Screenshot 3/4
(Assignee)

Comment 7

15 years ago
Comment on attachment 109069 [details]
Screenshot 1/4

Marking obsolete, wrong content-type used...
Attachment #109069 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #109070 - Attachment is obsolete: true
(Assignee)

Comment 8

15 years ago
Created attachment 109073 [details]
Screenshot 4/4
(Assignee)

Comment 9

15 years ago
Created attachment 109074 [details]
Screenshot 1/4
(Assignee)

Comment 10

15 years ago
Created attachment 109075 [details]
Screenshot 2/4
(Assignee)

Comment 11

15 years ago
Comment on attachment 109067 [details] [diff] [review]
Patch v1

Javi, can you please review?
Attachment #109067 - Flags: review?(javi)
(Assignee)

Comment 12

15 years ago
*** Bug 144510 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 13

15 years ago
*** Bug 85788 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

15 years ago
Attachment #109069 - Attachment is obsolete: false
Attachment #109069 - Attachment is patch: false
Attachment #109069 - Attachment mime type: text/plain → image/jpeg
(Assignee)

Updated

15 years ago
Attachment #109070 - Attachment is obsolete: false
Attachment #109070 - Attachment is patch: false
Attachment #109070 - Attachment mime type: text/plain → image/jpeg
(Assignee)

Updated

15 years ago
Attachment #109074 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #109075 - Attachment is obsolete: true

Comment 14

15 years ago
Comment on attachment 109067 [details] [diff] [review]
Patch v1

r=javi
Attachment #109067 - Flags: review?(javi) → review+
(Assignee)

Comment 15

15 years ago
Comment on attachment 109067 [details] [diff] [review]
Patch v1

Jag, can you please review?
Attachment #109067 - Flags: superreview?(jaggernaut)
Kai,  Do you have a binary for Win2K you want me to test?
(Assignee)

Comment 17

15 years ago
I have uploaded a win32 test build to:
  http://www.kuix.de/mozilla/mozilla-win32-184940.zip

Just unzip and run bin/mozilla.exe

md5sum:
f407e14b4faf0222efcd1db2bf6209a8 *mozilla-win32-184940.zip
(Assignee)

Comment 18

15 years ago
Sean looked at the screenshots, and asks, whether we should change
"Ciphersuites" into "Cipher Suites".

Nelson, do you have a preference? Sean suggests, we could leave the current
wording "Ciphersuites" if it appears to be more common.
I used the term "ciphersuites" (no space) because that is the term defined 
in the SSL3 and TLS specifications.  However, for localization purposes, 
I'm sure it will be less confusing if it's separated into two words.

Comment 20

15 years ago
The TLS RFC uses both "cipher suite" and "CipherSuite"
(note the capitalization).  I think the latter is the
name of a type in the formal protocol specification.
In plain English prose I think it is better to use the
former.
(Assignee)

Comment 21

15 years ago
Thanks for your comments. After the superreview, I will change it to "cipher suite".

Comment 22

15 years ago
Comment on attachment 109067 [details] [diff] [review]
Patch v1

>Index: mozilla/security/manager/pki/resources/content/cipherinfo.js
>===================================================================

>+  var info_name;
>+  var info_encryption;
>+  var info_authAlg;
>+  var info_keyAlg;
>+  var info_keySize;
>+  var info_macAlg;
>+  var info_fips;
>+  var info_exportable;
>+
>+  info_name = document.getElementById("name");
>+  info_encryption = document.getElementById("encryption");
>+  info_authAlg = document.getElementById("authAlg");
>+  info_keyAlg = document.getElementById("keyAlg");
>+  info_keySize = document.getElementById("keySize");
>+  info_macAlg = document.getElementById("macAlg");
>+  info_fips = document.getElementById("fips");
>+  info_exportable = document.getElementById("exportable");

This is a little odd, I'd merge these two blocks.

>+    if (typeof(prefValue) == "string") prefValue = (prefValue == "true");

Split this into two lines.

>Index: mozilla/security/manager/ssl/src/nsCipherInfo.cpp
>===================================================================

>+nsCipherInfoService::nsCipherInfoService()
>+{
>+  NS_INIT_ISUPPORTS();

NS_INIT_ISUPPORTS() is no longer needed (see its definition).
Attachment #109067 - Flags: superreview?(jaggernaut) → superreview+
(Assignee)

Comment 23

15 years ago
Created attachment 110829 [details] [diff] [review]
Patch v2

New patch with all requests addressed.
Attachment #109067 - Attachment is obsolete: true
(Assignee)

Comment 24

15 years ago
Comment on attachment 110829 [details] [diff] [review]
Patch v2

carrying forward reviews
Attachment #110829 - Flags: superreview+
Attachment #110829 - Flags: review+
(Assignee)

Comment 25

15 years ago
Updating summary to describe better what we are doing.
Summary: PSM should allow the user to enable/disable additional available ciphers → Enable additional SSL ciphers, add configuration UI
(Assignee)

Comment 26

15 years ago
Checked in, marking fixed.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 27

15 years ago
Reopening.
Status: RESOLVED → REOPENED
Depends on: 190160
Resolution: FIXED → ---
(Assignee)

Comment 28

15 years ago
John, I think something must have gone wrong when you reopened the bug.
You reopened without explaining why. I don't understand why you did.
You added a dependency to unconfirmed bug 190160, but I don't see any relation.

I'm removing the dependency and marking the bug fixed again.
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
No longer depends on: 190160
Resolution: --- → FIXED

Comment 29

15 years ago
Verified. I must not have had my coffee yet and thought I was on a different bug.
Status: RESOLVED → VERIFIED

Updated

13 years ago
Component: Security: UI → Security: UI
Product: PSM → Core
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.