Last Comment Bug 478839 - Firefox should support South Korean SEED crypto cipher suites
: Firefox should support South Korean SEED crypto cipher suites
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla1.9.2b1
Assigned To: Kai Engert (:kaie)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-16 23:26 PST by Nelson Bolyard (seldom reads bugmail)
Modified: 2009-10-01 11:56 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.4+
.4-fixed


Attachments
Patch v1 (2.59 KB, patch)
2009-02-17 07:22 PST, Kai Engert (:kaie)
nelson: review+
dveditz: approval1.9.1.4+
Details | Diff | Splinter Review

Description Nelson Bolyard (seldom reads bugmail) 2009-02-16 23:26:28 PST
Now that NSS supports the South Korean SEED crypto algorithm in SSL/TLS
cipher suites, Firefox should also make it possible for users to enable
and disable this cipher suite, ASAP.

My apologies if this is a duplicate.  I searched for an open bug on "SEED"
and found nothing relevant.
Comment 1 Kai Engert (:kaie) 2009-02-17 06:10:17 PST
Please provide a list of cipher IDs.

Two files need to be changed:
mozilla/netwerk/base/public/security-prefs.js
mozilla/security/manager/ssl/src/nsNSSComponent.cpp

With the list of SEED cipher IDs, and the entries for e.g. the camellia ciphers, someone should easily be able to produce the patch.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2009-02-17 07:09:10 PST
I'm not sure what you're requesting, Kai, but I'll guess it's this:

#define TLS_RSA_WITH_SEED_CBC_SHA               0x0096
Comment 3 Kai Engert (:kaie) 2009-02-17 07:22:18 PST
I conclude there is only one cipher suite for SEED?
For example, I see 6 entries related to Camellia.

I looked for a similar ID and produced an entry similar to 

 {"security.ssl3.rsa_aes_256_sha", TLS_RSA_WITH_AES_256_CBC_SHA}, // 256-bit AES encryption with RSA and a SHA1 MAC
Comment 4 Kai Engert (:kaie) 2009-02-17 07:22:58 PST
Created attachment 362720 [details] [diff] [review]
Patch v1
Comment 5 Nelson Bolyard (seldom reads bugmail) 2009-02-17 07:26:00 PST
Yes, presently we implement only one single SEED ciphersuite
Comment 6 Nelson Bolyard (seldom reads bugmail) 2009-02-17 09:54:38 PST
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Kai, in these files, is there any significance to the order in which the ciphersuite prefs appear?  
Does placing the new SEED ciphersuite after the "null" ciphersuites imply that it is considered less secure than they are?  

Finally, is this really all that is needed?
Comment 7 Wan-Teh Chang 2009-02-17 21:03:46 PST
Added the new PSM module peer Honza to the cc
list.

Kai, it's better to list the SEED cipher suite
next to the Camellia cipher suites because they
belong to the same category.
Comment 8 Kai Engert (:kaie) 2009-02-19 08:57:26 PST
(In reply to comment #6)
> Kai, in these files, is there any significance to the order in which the
> ciphersuite prefs appear?  

order does not imply meaning


> Does placing the new SEED ciphersuite after the "null" ciphersuites imply that
> it is considered less secure than they are?

I didn't mean to imply anything, I simply added at the end.


> Finally, is this really all that is needed?

I think it is, PSM will initially loop over this array and call NSS enable functions, with the enable flag according to the pref.

Unless NSS has additional unusual enabling requirements for SEED...
Comment 9 Kai Engert (:kaie) 2009-02-19 08:58:08 PST
(In reply to comment #7)
> Kai, it's better to list the SEED cipher suite
> next to the Camellia cipher suites because they
> belong to the same category.

If you don't like the current position, please propose an exact place in file nsNSSComponent.cpp
Comment 10 Nelson Bolyard (seldom reads bugmail) 2009-02-19 11:50:24 PST
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Kai, thanks for your answers.  r=nelson
Comment 11 Samuel Sidler (old account; do not CC) 2009-07-22 20:24:30 PDT
This should get landed on trunk before landing on branch, although I see no reason not to approve it for branch. Please land on trunk asap.
Comment 12 Mike Beltzner [:beltzner, not reading bugmail] 2009-07-23 09:55:22 PDT
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Trunk before branch, please, though we're very very interested in taking this, so renominate as soon as it lands there!
Comment 13 bluecey 2009-08-02 21:01:58 PDT
To progress this bug continuosly, what should we do next?
Comment 14 Masatoshi Kimura [:emk] 2009-08-02 21:05:16 PDT
The patch is already reviewed.
Comment 15 Dão Gottwald [:dao] 2009-08-09 00:21:26 PDT
http://hg.mozilla.org/mozilla-central/rev/248010090120
Comment 16 Samuel Sidler (old account; do not CC) 2009-08-28 12:11:33 PDT
Kai: Is this ready for branch? Please request approval if so.
Comment 17 Kai Engert (:kaie) 2009-08-28 12:58:01 PDT
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Landing on 1.9.2 branch for Firefox 3.6.x sounds good to me.

Landing on 1.9.1 for Firefox 3.5.x might be fine, too.
Comment 18 Dão Gottwald [:dao] 2009-08-28 13:16:22 PDT
This should already be on 1.9.2. It landed before 1.9.2 branched.
Comment 19 Samuel Sidler (old account; do not CC) 2009-08-28 13:17:49 PDT
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Yep. This isn't needed for 1.9.2. I was asking about 1.9.1.
Comment 20 Kai Engert (:kaie) 2009-08-28 13:30:39 PDT
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Landing this patch on mozilla 1.9.1 will enable a new feature in firefox 3.5.x

Are drivers willing to do that?
Comment 21 Daniel Veditz [:dveditz] 2009-08-31 15:20:00 PDT
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Approved for 1.9.1.4, a=dveditz for release-drivers
Comment 22 bluecey 2009-09-09 18:04:02 PDT
I work at KISA. 
After creating TLS_RSA_WITH_SEED_CBC_SHA,  we recently created a new patch including TLS_RSA_WITH_SEED_CBC_SHA, TLS_DHE_DSS_WITH_SEED_CBC_SHA and TLS_DHE_RSA_WITH_SEED_CBC_SHA.
Could I upload it now? Or Should I create a new Bugzilla process to adopt the patch file that is modified for enabling TLS_DHE_DSS and TLS_DHE_RSA?
Comment 23 Robert Relyea 2009-09-09 18:27:15 PDT
Seed is in the softoken. It's currently frozen for FIPS, but you can still provide your patch. We'll put it in the softoken 3.13 branch to be queued for the next FIPS validation.

bob
Comment 24 Nelson Bolyard (seldom reads bugmail) 2009-09-09 23:21:37 PDT
But not in this bug.  
Please file a new bug, product NSS, component libraries, version trunk,
importance: enhancement, and attach your patch to that.
Comment 25 Kai Engert (:kaie) 2009-09-19 09:28:48 PDT
pushed to mozilla-1.9.1 tree
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/653b75a8f46f

What keyword should we add for fixed1.9.1.4 ?
Comment 26 Al Billings [:abillings] 2009-10-01 11:56:44 PDT
Is there a means to test this fix and verify it?

Kai, you just change the status1.9.1 to .4-fixed, which has been done.

Note You need to log in before you can comment on or make changes to this bug.