Firefox should support South Korean SEED crypto cipher suites

RESOLVED FIXED in mozilla1.9.2b1

Status

()

Core
Security: PSM
--
enhancement
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: kaie)

Tracking

Trunk
mozilla1.9.2b1
Points:
---

Firefox Tracking Flags

(blocking1.9.1 .4+, status1.9.1 .4-fixed)

Details

Attachments

(1 attachment)

2.59 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
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.
(Assignee)

Comment 1

9 years ago
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.
I'm not sure what you're requesting, Kai, but I'll guess it's this:

#define TLS_RSA_WITH_SEED_CBC_SHA               0x0096
(Assignee)

Comment 3

9 years ago
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
(Assignee)

Comment 4

9 years ago
Created attachment 362720 [details] [diff] [review]
Patch v1
Attachment #362720 - Flags: review?(nelson)
Yes, presently we implement only one single SEED ciphersuite
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

9 years ago
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.
(Assignee)

Comment 8

9 years ago
(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...
(Assignee)

Comment 9

9 years ago
(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 on attachment 362720 [details] [diff] [review]
Patch v1

Kai, thanks for your answers.  r=nelson
Attachment #362720 - Flags: review?(nelson) → review+
(Reporter)

Updated

8 years ago
Attachment #362720 - Flags: approval1.9.1.2?
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.
blocking1.9.1: --- → ?
Attachment #362720 - Flags: approval1.9.1.2?
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!
blocking1.9.1: ? → needed
status1.9.1: --- → wanted

Comment 13

8 years ago
To progress this bug continuosly, what should we do next?
The patch is already reviewed.
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/248010090120
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2b1
Kai: Is this ready for branch? Please request approval if so.
(Assignee)

Comment 17

8 years ago
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.
Attachment #362720 - Flags: approval1.9.2?
This should already be on 1.9.2. It landed before 1.9.2 branched.
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.
Attachment #362720 - Flags: approval1.9.2?
(Assignee)

Comment 20

8 years ago
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?
Attachment #362720 - Flags: approval1.9.1.4?
Attachment #362720 - Flags: approval1.9.1.4? → approval1.9.1.4+
Comment on attachment 362720 [details] [diff] [review]
Patch v1

Approved for 1.9.1.4, a=dveditz for release-drivers

Comment 22

8 years ago
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

8 years ago
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
But not in this bug.  
Please file a new bug, product NSS, component libraries, version trunk,
importance: enhancement, and attach your patch to that.
blocking1.9.1: needed → .4+
(Assignee)

Comment 25

8 years ago
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 ?

Updated

8 years ago
status1.9.1: wanted → .4-fixed
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.
You need to log in before you can comment on or make changes to this bug.