478839 - Firefox should support South Korean SEED crypto cipher suites

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement)

Description

[Nelson Bolyard (seldom reads bugmail)]

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:)]

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)]

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:)]

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:)]

Attachment: Patch v1

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Yes, presently we implement only one single SEED ciphersuite

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

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]

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:)]

(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:)]

(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)]

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)]

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]]

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]

To progress this bug continuosly, what should we do next?

Comment 14

[Masatoshi Kimura [:emk]]

The patch is already reviewed.

Comment 15

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/248010090120

Comment 16

[Samuel Sidler (old account; do not CC)]

Kai: Is this ready for branch? Please request approval if so.

Comment 17

[Kai Engert (:KaiE:)]

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]]

This should already be on 1.9.2. It landed before 1.9.2 branched.

Comment 19

[Samuel Sidler (old account; do not CC)]

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:)]

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]]

Comment on attachment 362720 [details] [diff] [review]
Patch v1

Approved for 1.9.1.4, a=dveditz for release-drivers

Comment 22

[bluecey]

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]

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)]

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:)]

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 - ex-MoCo]]

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.