Closed Bug 326159 Opened 18 years ago Closed 18 years ago

ECC support for KEYGEN and crypto.generateCRMFRequest

Categories

(Core :: Security: PSM, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

Attachments

(1 file, 8 obsolete files)

Elliptic Curve Cryptography uses a new class of certificates.

The html tag KEYGEN and the JavaScript function crypto.generateCRMFRequest should be enhanced to support generation of appropriate keys and requests.

documentation:
  http://wp.netscape.com/eng/security/comm4-keygen.html
  http://developer.mozilla.org/en/docs/generateCRMFRequest
Looks like the patch attached to bug 235773 already provides support for EC KEYGEN.
I did some testing of the KEYGEN tag patch currently contained in bug 235773.

First, I created a "classic" RSA key using http://kuix.de/misc/test3/rsa.php

The result looks like this:

   0 30  580: SEQUENCE {
   4 30  300: . SEQUENCE {
   8 30  290: . . SEQUENCE {
  12 30   13: . . . SEQUENCE {
  14 06    9: . . . . OBJECT IDENTIFIER rsaEncryption (1 2 840 113549 1 1 1)
            : . . . . . (PKCS #1)
  25 05    0: . . . . NULL
            : . . . . }
  27 03  271: . . . BIT STRING 0 unused bits, encapsulates {
  32 30  266: . . . . . SEQUENCE {
  36 02  257: . . . . . . INTEGER
            : . . . . . . . 00 AF 69 B5 52 4C A9 89 57 3D D9 33 33 E3 90 69
            : . . . . . . . 52 56 CA AE 58 94 BC 3B F9 CA 68 A1 8A 02 96 DD
            : . . . . . . . 67 5D 06 4F 12 5F 0C 21 85 B3 20 42 A8 48 73 8D
            : . . . . . . . B3 52 1E D0 69 18 C2 F3 76 1C AA AC F1 B6 66 0F
            : . . . . . . . 9E BA C5 34 05 FB FC 7E 9D E2 BD 32 86 3C 05 CC
            : . . . . . . . 77 33 78 41 21 46 8A 5F 81 F1 E6 F6 CE A5 5D 2A
            : . . . . . . . 7A 1F 43 A9 CE 63 5C 57 35 71 7C DE A8 D0 9B C3
            : . . . . . . . 75 86 94 03 5E BF CB 2D 2F 95 50 D5 3E 33 1D 72
            : . . . . . . . . . . . [ Another 129 bytes skipped ]
 297 02    3: . . . . . . INTEGER 65537
            : . . . . . . }
            : . . . . . }
            : . . . }
 302 16    4: . . IA5String 'none'
            : . . }
 308 30   13: . SEQUENCE {
 310 06    9: . . OBJECT IDENTIFIER md5withRSAEncryption (1 2 840 113549 1 1 4)
            : . . . (PKCS #1)
 321 05    0: . . NULL
            : . . }
 323 03  257: . BIT STRING 0 unused bits
            : . . 38 20 B1 BD A4 D4 57 4A 37 E9 8C 42 A2 7A 2A D4
            : . . 39 0E 19 FA 71 A7 1A A1 0E 24 8E F2 63 32 B7 57
            : . . C1 AE 6E 1D ED BD D1 91 87 5F E4 54 8B 28 FC 8E
            : . . 4C DE 21 34 5E 2F E2 AA 82 82 4D 7E 01 77 93 E0
            : . . 6E D4 5B DA A4 65 4A 3B 9F 29 BC 4E 27 41 87 51
            : . . 50 07 EE 15 A1 26 DC D5 FE 63 D6 3F C6 1A CC B1
            : . . 6A 73 6A 82 11 6B 5A 5C A8 F8 D7 AF C7 B7 56 59
            : . . DA BC 9F 88 8E 3B 5A 18 59 B2 89 23 E7 45 0D 17
            : . . . . . . [ Another 128 bytes skipped ]
            : . }

Netscape SPKI:
  Public Key Algorithm: rsaEncryption
  RSA Public Key: (2048 bit)
  Modulus (2048 bit):
      00:af:69:b5:52:4c:a9:89:57:3d:d9:33:33:e3:90:
      69:52:56:ca:ae:58:94:bc:3b:f9:ca:68:a1:8a:02:
      96:dd:67:5d:06:4f:12:5f:0c:21:85:b3:20:42:a8:
      48:73:8d:b3:52:1e:d0:69:18:c2:f3:76:1c:aa:ac:
      f1:b6:66:0f:9e:ba:c5:34:05:fb:fc:7e:9d:e2:bd:
      32:86:3c:05:cc:77:33:78:41:21:46:8a:5f:81:f1:
      e6:f6:ce:a5:5d:2a:7a:1f:43:a9:ce:63:5c:57:35:
      71:7c:de:a8:d0:9b:c3:75:86:94:03:5e:bf:cb:2d:
      2f:95:50:d5:3e:33:1d:72:6a:2f:1f:73:42:86:09:
      1c:36:88:15:e9:b7:2a:e8:f2:87:0c:e0:55:0d:43:
      6f:ce:cd:47:1c:56:0b:9d:cc:cf:cb:d4:53:02:35:
      7b:2c:15:a4:0e:32:d3:34:91:52:e0:bb:72:b6:33:
      89:cf:15:a8:74:a6:a6:92:db:d9:52:3b:97:99:a5:
      81:02:d5:24:38:ee:5b:9a:e4:6e:93:aa:f2:7d:99:
      3e:7c:8e:11:7b:bd:74:1e:35:75:f1:78:9a:39:30:
      55:ba:fe:65:fd:db:0b:0d:7c:cf:76:04:de:74:7a:
      32:cf:1e:db:55:4d:c1:ff:0a:a8:c0:c2:2f:28:81:
      5d:79
  Exponent: 65537 (0x10001)
  Challenge String: none
  Signature Algorithm: md5WithRSAEncryption
      38:20:b1:bd:a4:d4:57:4a:37:e9:8c:42:a2:7a:2a:d4:39:0e:
      19:fa:71:a7:1a:a1:0e:24:8e:f2:63:32:b7:57:c1:ae:6e:1d:
      ed:bd:d1:91:87:5f:e4:54:8b:28:fc:8e:4c:de:21:34:5e:2f:
      e2:aa:82:82:4d:7e:01:77:93:e0:6e:d4:5b:da:a4:65:4a:3b:
      9f:29:bc:4e:27:41:87:51:50:07:ee:15:a1:26:dc:d5:fe:63:
      d6:3f:c6:1a:cc:b1:6a:73:6a:82:11:6b:5a:5c:a8:f8:d7:af:
      c7:b7:56:59:da:bc:9f:88:8e:3b:5a:18:59:b2:89:23:e7:45:
      0d:17:91:e4:60:cd:84:75:45:14:db:b0:6c:1c:d1:69:f5:0a:
      17:c0:74:1d:c3:8a:e3:f1:23:51:5e:ad:a9:48:61:fe:c9:ad:
      1d:5d:09:d7:fa:ef:e4:bb:91:56:f1:57:81:32:48:ba:ba:fc:
      49:2f:1a:aa:61:8f:39:89:52:14:83:c5:a6:a9:d9:c9:82:b7:
      52:23:85:6e:39:00:9c:f9:de:1c:1f:70:94:05:e4:32:25:9b:
      4a:5a:30:44:e5:2b:81:97:41:af:47:ba:fb:53:d0:11:b1:49:
      7e:a6:b3:9e:7b:58:68:b8:ce:8a:92:85:32:b7:dd:41:73:95:
      f7:ff:f8:de
Next, I created a EC key using this webpage and a ECC enabled build of Mozilla:

   0 30  245: SEQUENCE {
   3 30  126: . SEQUENCE {
   5 30  118: . . SEQUENCE {
   7 30   16: . . . SEQUENCE {
   9 06    7: . . . . OBJECT IDENTIFIER ecPublicKey (1 2 840 10045 2 1)
            : . . . . . (ANSI X9.62 public key type)
  18 06    5: . . . . OBJECT IDENTIFIER '1 3 132 0 34'
            : . . . . }
  25 03   98: . . . BIT STRING 0 unused bits
            : . . . . 04 09 4A 0B 8D 4A 80 A1 98 B0 08 CC 78 B2 78 DE
            : . . . . DC 90 F7 9B 3D 2E DB 8F 9C D2 FD F6 29 4F A9 C0
            : . . . . 7B 07 3C 15 1D E7 3E 22 E6 70 44 6D D8 78 F3 E5
            : . . . . 1A 56 46 EF 00 AE 10 4E 0B 17 D8 40 4C 73 08 71
            : . . . . 2C B4 49 02 55 F2 25 EB BB 77 6D A4 47 DB FF 2D
            : . . . . BE 99 06 96 FA 4B ED C9 76 14 60 FB 30 E2 F0 0B
            : . . . . 1B
            : . . . }
 125 16    4: . . IA5String 'none'
            : . . }
 131 30    9: . SEQUENCE {
 133 06    7: . . OBJECT IDENTIFIER '1 2 840 10045 4 1'
            : . . }
 142 03  104: . BIT STRING 0 unused bits, encapsulates {
 145 30  101: . . . SEQUENCE {
 147 02   49: . . . . INTEGER
            : . . . . . 00 9F F3 03 DD 11 49 55 13 2A 36 B0 91 F4 C0 AA
            : . . . . . 47 C3 71 46 93 2F 8A DE DF 09 A9 91 48 38 48 97
            : . . . . . 1A E6 C0 74 25 38 43 75 E7 8C 26 BC 26 FF 0F 22
            : . . . . . 77
 198 02   48: . . . . INTEGER
            : . . . . . 4F 0E 2F B6 04 4B 2D 93 9C 7D BE 34 CD ED E8 09
            : . . . . . B0 74 8F F9 09 02 B9 14 11 0E EF 73 6F 69 BE F3
            : . . . . . 83 7B D3 0F 16 E4 E6 DB BA 5B 2C 35 C0 D4 E5 71
            : . . . . }
            : . . . }
            : . }

Netscape SPKI:
  Public Key Algorithm: id-ecPublicKey
  EC Public Key:
  pub:
      04:09:4a:0b:8d:4a:80:a1:98:b0:08:cc:78:b2:78:
      de:dc:90:f7:9b:3d:2e:db:8f:9c:d2:fd:f6:29:4f:
      a9:c0:7b:07:3c:15:1d:e7:3e:22:e6:70:44:6d:d8:
      78:f3:e5:1a:56:46:ef:00:ae:10:4e:0b:17:d8:40:
      4c:73:08:71:2c:b4:49:02:55:f2:25:eb:bb:77:6d:
      a4:47:db:ff:2d:be:99:06:96:fa:4b:ed:c9:76:14:
      60:fb:30:e2:f0:0b:1b
  ASN1 OID: secp384r1
  Challenge String: none
  Signature Algorithm: ecdsa-with-SHA1
      30:65:02:31:00:9f:f3:03:dd:11:49:55:13:2a:36:b0:91:f4:
      c0:aa:47:c3:71:46:93:2f:8a:de:df:09:a9:91:48:38:48:97:
      1a:e6:c0:74:25:38:43:75:e7:8c:26:bc:26:ff:0f:22:77:02:
      30:4f:0e:2f:b6:04:4b:2d:93:9c:7d:be:34:cd:ed:e8:09:b0:
      74:8f:f9:09:02:b9:14:11:0e:ef:73:6f:69:be:f3:83:7b:d3:
      0f:16:e4:e6:db:ba:5b:2c:35:c0:d4:e5:71
link for creating a ECC request using KEYGEN: http://kuix.de/misc/test3/ecc.php
I was able to produce a ECC certificate based on the SPKAC request from comment 3. I uploaded it to a webserver, made it available using content-type 
application/x-x509-user-cert. After accessing the cert with the browser, it got successfully installed, cert manager displayed it correclty, and I was able to backup it to a p12 file. Looks great to me.

I conclude the patch for ECC keygen is working fine already.
Next, I will try to produce a patch that adds support for crypto.generateCRMFRequest().

As the KEYGEN patch has chosen to use "ec" as the key type identifier, we should be consistent and use that for crypto.generateCRMFRequest, too.

According to the documentation and source code, for RSA we currently support 5 key usages. Without knowing all the background, I suspect we want a corresponding EC key usage for each of them:

- "rsa-ex" - generate an ECC key for key exchange only
- "rsa-dual-use" - generate a single ECC key for both signing and encryption
- "rsa-sign" - generate an ECC key for signing only
- "rsa-sign-nonrepudiation" - generate a single ECC key for nonRepudiation only
- "rsa-nonrepudiation" - generate a single ECC key use for both signing and nonRepudiation


I created web pages that can be used to create a request and will dump it on screen.

http://kuix.de/misc/test3/crmf-rsa.php (already works)
http://kuix.de/misc/test3/crmf-ecc.php
(In reply to comment #5)

> I conclude the patch for ECC keygen is working fine already.

This is as expected. The only major outstanding thing
with that patch is UI related. Currently, the tag is 
rendered as a pull-down menu with two options
"High grade" or "Medium grade" but it might make more sense to
display more information like "High Grade (RSA 2048)"
or "High Grade (ECC secp521r1)". The tricky part that 
I couldn't figure out was how to influence the display
string based on the arguments (e.g. key type and
keyparams) passed to the tag.

Perhaps someone more familiar with the existing
code can help out here.

vipul
Depends on: 326503
Attached patch Patch v1 (obsolete) — Splinter Review
This is a first patch to add EC CRMF support.

Using this patch and the fix contained in the dependent NSS bug, I was able to produce some CRMF request, but have not yet been able to test, whether the created request is valid.
In the patch attached to this bug, I added a SECITEM_FreeItem call, but commented it out because of a crashing experience.

I just retested and the crash seems to have been a followup problem, because now that I have a fix for bug 326503, I no longer crash.

In the next revision of the patch I'll enable the free call, and change FALSE to TRUE.


The attached patch reuses the decode_ec_params function added in the patch in bug 235773, we'll have to make that function non-static and declare it to the nsKeygenHandler.h file, will do so in a later release of the patch.
> Currently, the tag is rendered as a pull-down menu with two options
> "High grade" or "Medium grade" but it might make more sense to
> display more information like "High Grade (RSA 2048)"
> or "High Grade (ECC secp521r1)". The tricky part that 
> I couldn't figure out was how to influence the display
> string based on the arguments (e.g. key type and
> keyparams) passed to the tag.


If I understand correctly, this would be nice to have, but it's not absolutely required.


I look at how this could be done.

The dropdown elements are created at the time the HTML parser encounters the KEYGEN tag, in CNavDTD::HandleKeyGen().

It calls nsIFormProcessor::ProvideContent, implemented by PSM. Unfortunately, this interface does not provide any information that could be used by PSM to find the key type.

In general, we are not allowed to change the interface on the branch to Firefox 2.

Interface nsIFormProcessor seems only to be used for interaction between Mozilla's content module and PSM's keygen handler. I'm not sure if that could grant as an exception from the "do not change interfaces on branch" rule.

Assuming we are not granted an exception, we'd have to create a new additional interface.

A solution would have to:
- create a new interface nsIFormProcessor2, in its ProvideContent function add parameters for key-type and key-parameters
- change nsKeygenFormProcessor to implement both interfaces, and make use of key-type and key-parameters when provided
- change HTML parser to obtain the key-type and -parameter values from HTML
- change HTML parser to try QueryInterface on the form processor object to our new interface. If that fails, use the old interface, if it succeeds, use the new interface and provide the optional parameters.


I would like to propose that we don't try to solve this in the very first implementation. This saves us the hassle of creating additional interfaces.

We are fine to change the interface on the trunk and add additional parameters to the existing interface.

I would like to propose that we file an enhancement request bug and to work on it indepently.
We need a fix for bug 328909 before this patch can be tested.
Depends on: 328909
Depends on: eccui
Attached patch Patch v2 (obsolete) — Splinter Review
I learned that for EC certs, we don't use KU_KEY_ENCIPHERMENT.
What we want is KU_KEY_AGREEMENT.
Attachment #211235 - Attachment is obsolete: true
Attached patch Patch v3 (obsolete) — Splinter Review
When running a 64 bit client, the request ID integer contained as the very first piece of the CRMF request is treated as a signed value. This can result in negative 64 bit numbers, which confuses the server I'm testing with.

The real fix is to change the request encoding to use a signed number.
This will be done in NSS bug 331648 and will make sure that interaction between 32 bit clients and 64 bit servers works correctly, too.

To start with a fix in the client, I'm changing the type of our random integer from signed to unsigned.

Here is a new patch that adds this small change.
Attachment #215957 - Attachment is obsolete: true
Attachment #216178 - Flags: review?(rrelyea)
Comment on attachment 216178 [details] [diff] [review]
Patch v3

r+=rrelyea
Attachment #216178 - Flags: review?(rrelyea) → review+
Attached patch Patch v4 (obsolete) — Splinter Review
I would like to track the key generation code (both KEYGEN html tag and crypto.generateCRMF) within this bug.

Bug 235773 attachment 218288 [details] [diff] [review] (named Patch v12) contains code that really belongs into this bug, because it implements the key generation functionality. I'm moving those portions over here, and adding them to the previous patch.

In addition to the move, I made two small changes, the loop to iterate over curve names now does a zero string length check, and we map the 512 bit key size to the 256 bit curve. (The 512 bit key size does not appear in the UI anyway).
Attachment #216178 - Attachment is obsolete: true
(In reply to comment #15)

> curve names now does a zero string length check, and we map the 512 bit key
> size to the 256 bit curve. (The 512 bit key size does not appear in the UI
> anyway).

  Kai,

   Does this mean that the user cannot select the 256-bit curve?
  I think it would be a mistake a change that precludes the user
  from choosing 256-bit curves.

  The 256-bit curves provide more than adequate security for the
  forseeable future (as per NIST Pub 800-56, ECC 256 is as secure
  as RSA 3072 which is considered adequate until 2030) and have
  a significant performance advantage over the longer curves.

  In fact, from what I remember from a recent ECC-forum concall,
  Verisign plans to issue primarliy 256-bit  certs via their
  ECC CA (their root will use a 384-bit curve).

  I recommend retaining the UI I've seen in recent builds 
  where medium is mapped to 256-bits and high to 384 bits.
  (actually, lower curves would have been fine but since
  Vista will not have support for them, we shouldn't go
  lower than 256).

  thoughts/comments?

  vipul


> Does this mean that the user cannot select the 256-bit curve?
> I think it would be a mistake a change that precludes the user
> from choosing 256-bit curves.

No!
Sorry for not being clear. Let me try to rephrase.

Web pages are able to request curves explicitly, by specifying them as parameters.

When using the KEYGEN html tag, in the past, the UI offered three key size choices:
- high 2048 bit
- medium 1024 bit
- low 512 bit

The above key sizes were related to RSA keys.

Some time ago the "low 512 bit" choice was eliminated from the UI, and the user was left with a choice between
- high 2048 bit
- medium 1024 bit

In the patch, attached to this bug, when using EC as the key type, we have a mapping from RSA equivalent key sizes to EC curves.
- we map 2048 RSA key size to the 384 bit curve
- we map 1024 RSA key size to the 256 bit curve

The latest patch continues to contain this mapping.

In the earlier patches, we also mapped the 512 RSA key size to a 160 or 192 bit curve. Since our UI no longer offers the equalivent of the 512 RSA key size to the user, Bob and my thought, we could just map the 512 key size to the lowest curve we offer to the user, too - which is the 256 bit curve.
Attachment #219897 - Flags: review?(rrelyea)
(In reply to comment #17)

> No!

   Great. Thanks for the clarification.

> Web pages are able to request curves explicitly, by specifying them as
> parameters.

   What does the user interface show when an explicit curve
   has been specified in the parameters? The original patch
   did not handle this situation properly (because I was unfamiliar
   with the code necessary to display the explicit curves in the
   user visible choices). Is that still true?
 
> In the earlier patches, we also mapped the 512 RSA key size to a 160 or 192 bit
> curve. Since our UI no longer offers the equalivent of the 512 RSA key size to
> the user, Bob and my thought, we could just map the 512 key size to the lowest
> curve we offer to the user, too - which is the 256 bit curve.

   Does this change anything from a user perspective? I'd
   guess not.

   vipul
(In reply to comment #18)
> > Web pages are able to request curves explicitly, by specifying them as
> > parameters.
> 
>    What does the user interface show when an explicit curve
>    has been specified in the parameters? The original patch
>    did not handle this situation properly (because I was unfamiliar
>    with the code necessary to display the explicit curves in the
>    user visible choices). Is that still true?

Yes


>    Does this change anything from a user perspective? I'd
>    guess not.

Right, is does not change things from a user perspective.
Comment on attachment 219897 [details] [diff] [review]
Patch v4

r+ relyea.
The patch is correct.
There are missing functionality
in NSS to allow this patch
to function.
It's still an open question as to when this patch should land.

bob
Attachment #219897 - Flags: review?(rrelyea) → review+
Depends on: 343682
I recently tried a firefox 2.0a3 linux build that Kai produced to see if we could
generate ECC keys using this page : http://kuix.de/misc/test3/crmf-ecc.php. 

firefox crashes while doing keygen.  
Blocks: 343867
Kai, is bug 346551 related to this?
Does it "block" this bug?
If so, feel free to "take" that bug.
Attached patch Patch v5 (obsolete) — Splinter Review
We would like cert generation to succeed, even if adding ProofOfPossession is not possible.

Bob, when comparing the previous patch you already had reviewed to this newer patch, the only difference is the following new change:

@@ -1399,8 +1503,10 @@ nsCreateReqFromKeyPairs(nsKeyPairInfo *k
       goto loser;
 
     rv = nsSetProofOfPossession(certReqMsgs[i], &keyids[i]);
-    if (NS_FAILED(rv))
-      goto loser;
+    rv = NS_OK;
+    // We explicitly ignore a failure in nsSetProofOfPossession.
+    // We would like to produce the cert request, even if 
+    // adding POP has not been possible.
     CRMF_DestroyCertRequest(certReq);
   }
   encodedReq = nsEncodeCertReqMessages(certReqMsgs);
Attachment #219897 - Attachment is obsolete: true
Attachment #232522 - Flags: review?(rrelyea)
(In reply to comment #22)
> Kai, is bug 346551 related to this?
> Does it "block" this bug?
> If so, feel free to "take" that bug.

So far I haven't seen problems caused by bug 346551, but it is a good idea to fix it now and make this one depend on it for tracking purposes.
I attached a patch.
Depends on: 346551
Back when our CRMF code was first implemented, it was done by a member of
the PSM team, and I didn't think about it too much.  Thinking about it now,
it seems to me that for a CA to issue a cert without Proof of Possession (PoP)
is dereliction of duty.  A CA that does so will surely be deceived into 
issuing certs naming others than the private key holders.  It troubles me 
greatly to learn that we are considering facilitating cert issuance without 
PoP.  

It's not necessary to sign for PoP.  PoP can be accomplished through en/decryption.  IIRC, CRMF was designed specifically to provide enrollment 
of certs that cannot be used for signing.  So Surely the CRMF protocol 
developers provided for that! (no?)  
No longer depends on: 346551
Doing a little extra research, the crmf pop field is optional, though pop itself is not. The CRMF spec requires that the CA does do some form of pop as part of issuing the certificate. (this can include things like encrypting the certificate targetting the private key (that is encrypting with the public key for RSA, or key exchanging with the public key for ECC).

There is a specification for doing doing an in line key exchange pop as well. The Spec is only for DH (not necessarily ECDH), and involves the CA passing a public DH (or ECDH) key.

I'm still unclear what kind of attack can be mounted by getting a certificate with my name for a key that someone else owns, though it's clear CA's that want to claim support for CRMF must handle PoP in some way.
(In reply to comment #26)

> I'm still unclear what kind of attack can be mounted by getting a certificate
> with my name for a key that someone else owns, 

If you can get such a cert, then so can someone else, incuding the owner
of that key.  Then he can impersonate you.

Attachment #232522 - Attachment is obsolete: true
Attachment #232522 - Flags: review?(rrelyea)
>> I'm still unclear what kind of attack can be mounted by getting a certificate
>> with my name for a key that someone else owns, 
>
> If you can get such a cert, then so can someone else, incuding the owner
> of that key.  Then he can impersonate you.

Right, so if you *purposefully* get a certificate with someone else's public key, then he can impersonate you. So PoP keeps you from shooting yourself in the foot, not a bad thing, but not necessarily a security hole in itself.

bob
needn't be purposeful.  An attacker can alter your CRMF request in transit,
substituting his key for yours, in the absence of PoP.  This makes automated
MITM attacks very feasible.
If the attacker can modify the key in the crmf request he can also modify the PoP. crmf requires the transport to protect against MITM.

It's clear that the creaters of crmf believe there is an exploit (though I don't know what it would be), and traditional crypto hygene says you should provide PoP, I was just hoping to get a clearer understanding of what the exploit is (the only exploits I've heard involve you claiming to have signed something someone else signed by creating a cert with their key).
In reply to Nelson's comment 25 I filed bug 350200, in order to add the missing code to the CRMF library which is required to do DHMAC based POP as specifed in RFC 2511 Appendix A.
Depends on: dhmac
Attached patch Patch v6 (obsolete) — Splinter Review
Attached patch Patch v6 - reviewer version (obsolete) — Splinter Review
This patch is easier to review, it has more context, and ignores whitespace, thus listing fewer changes.
Attachment #235438 - Flags: review?(rrelyea)
Comment on attachment 235438 [details] [diff] [review]
Patch v6 - reviewer version

r+ with the following caveats.

need to track Bug 339393 wrt curve names.

minor suggestions:

The keyparam parsing code is calling to be an separte function. It should be made one as some point, but for now it's find.

bob
Attachment #235438 - Flags: review?(rrelyea) → review+
Patch has been checked in to the trunk of Mozilla, marking fixed.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Reopening, I backed out this patch because it does not compile on Mac.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The only error I saw was on Windows, although maybe you were looking at a tinderbox I didn't think of:

c:/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/security/manager/ssl/src/nsCrypto.cpp(682) : error C3861: 'strndup': identifier not found
c:/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/security/manager/ssl/src/nsCrypto.cpp(686) : error C3861: 'strndup': identifier not found
Attached patch Patch v7Splinter Review
This patch compiles fine on Windows and Mac. In order to fix the bustage, I replaced strndup/strncmp/free with the NSPR functions PL_strndup/PL_strncmp/PL_strfree.
Attachment #235437 - Attachment is obsolete: true
Attachment #235438 - Attachment is obsolete: true
Attachment #236411 - Flags: review+
(In reply to comment #37)
> The only error I saw was on Windows, although maybe you were looking at a
> tinderbox I didn't think of:

I had seen the same error on the Camino Tinderbox.
Attached patch INVALID (obsolete) — Splinter Review
Looks like the previous patch did not encode the dhmac bit string correctly.
Attachment #236411 - Attachment is obsolete: true
Comment on attachment 239198 [details] [diff] [review]
INVALID

There is a bug in Patch v8.

When calculating the "content length" for the bit string encoding, I forgot to add the initial byte that specifies the number of unused bits in the final byte.

But in addition I wonder if we really need to add those 30 lines of code I had added with Patch v8, because it seems, we can get the very same output by adding the EXPLICIT tag to the dhmac asn.1 template (as contained in the patch bug 350200).

On the other hand Bob R had discouraged me by mail from setting the explicit tag on the template, so I feel this needs to be discussed again.
Attachment #239198 - Attachment is obsolete: true
Comment on attachment 239198 [details] [diff] [review]
INVALID

PLEASE IGNORE comment 40 and comment 41.
They are invalid.
Attachment #239198 - Attachment description: Patch v8 → INVALID
Comment on attachment 236411 [details] [diff] [review]
Patch v7

After the discussion in bug 350200, we have concluded that this earlier patch is exactly what we want.

I'm removing the obsolete flag and changing this patch back to be valid.

It still has r=rrelyea and is ready for check in as soon as we land the NSS backend code from bug 350200.
Attachment #236411 - Attachment is obsolete: false
Depends on: 354756
Patch checked in to trunk (second attempt), marking fixed.
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
QA Contact: psm
Bonjour,

I have a query regarding the "Key Manager Tool – Extension of Mozilla Certificate Manager for Key Generation and Certificate Enrollment ".

I am using this tool version 0.1.3.20090714 with Firefox version 3.6.13.
Problem is that i can not create an ECC Certificate Request with, in the "GenerateCSR" menu of KeyManager ECC is disabled by default. How can i enable it

Further in comment 3, you mentioned about ECC enable Build, is there any difference in normal and ecc enabled build ?
Regards
Scott Thomas
Scott, this bug is not a good place for your question.
You should ask the author of that extension.

In my understanding the binaries provided by Mozilla.org have some ECC enabled.
For general Mozilla crypto questions, a good place to ask is newsgroup mozilla.dev.tech.crypto
You need to log in before you can comment on or make changes to this bug.