337519 - Certutil/blapitest usage mentions unimplemented curves

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Vipul Gupta]

Some higher level applications like certutil and blapitest mention ONB elliptic curves in their usage instructions but these curves are not implemented by the underlying elliptic curve library. The usage instructions should not mention these curves.

Comment 1

[Vipul Gupta]

This bug was first discussed in late Feb 2006 (see attached emails) but I unintentionally dropped the ball and never got around to filing a bug in bugzilla.

vipul

Message thread 1:

Begin forwarded message:
From: Douglas Stebila <[e-mail]>
Date: February 20, 2006 5:49:15 PM PST
To: Wan-Teh Chang <[e-mail]>
Cc: Vipul Gupta <[e-mail]>, Nelson B Bolyard <[e-mail]>
Subject: Re: Not all of the ANSI X9.62 curves are listed in lib/freebl/ecl/ecl-curve.h

> Yes, the following are also missing.

> +    case SEC_OID_ANSIX962_EC_C2ONB191V4: /* XXX */
> +    case SEC_OID_ANSIX962_EC_C2ONB191V5: /* XXX */
> +    case SEC_OID_ANSIX962_EC_C2ONB239V4: /* XXX */
> +    case SEC_OID_ANSIX962_EC_C2ONB239V5: /* XXX */

These curves use an ONB, optimal normal basis, representation for the irreducible binary polynomial.  I think we made a decision to not implement these curves because the arithmetic for them is more difficult to implement, but I don't remember for sure.  It may be the case that these curves can use the normal arithmetic code, but that there are other more optimized algorithms; I don't know.  Vipul, Sheueling might know the answer to this.

However, I wouldn't be too worried about these curves being missing from the implementation.  I think most people are leaning toward the NIST and SEC curves in terms of favoured standards.

Douglas


Message thread 2:

Begin forwarded message:
From: Douglas Stebila <[e-mail]>
Date: February 28, 2006 3:12:12 PM PST
To: Vipul Gupta <[e-mail]>
Subject: Re: ONB curves in NSS

Removing them seems fine with me.  I doubt anyone will actually use the ANSI X9.62 curves anyway.

Douglas

On 28-Feb-06, at 6:09 PM, Vipul Gupta wrote:

Yes. I wanted to bring this up for today but we ran out of time.
Instead of trying to add code for these curves, I'd lobby for
simply removing these names from the higher level apps, e.g.
certutil.

What do you think?

vipul

On Feb 28, 2006, at 3:04 PM, Douglas Stebila wrote:

Should this be filed in bugzilla?

Douglas

On 22-Feb-06, at 8:46 PM, Vipul Gupta wrote:

I noticed that the certutil command:

http://lxr.mozilla.org/security/source/security/nss/cmd/certutil/certutil.c#1147
http://lxr.mozilla.org/security/source/security/nss/cmd/certutil/certutil.c#1149

lists four onb curves but, from what I understand now, these aren't
really implemented by ecl.

Do you know what happens today if one invokes certutil with one
of these curves?

Looks like there are several places where NSS makes references to these
curves. Any recommendations on how to address this issue?

vipul

Comment 2

[Vipul Gupta]

I investigated this in late Feb/early Mar and here
are my notes on what needs to be done:

In blapitest.c:
  - fix Usage() to not mention any onb curves (~line 193)
  - fix nameTagPair[] by removing the lines for onb curves 
   (~lines 520-527)

In certutil.c/keystuff.c:

  - fix LongUsage() (~lines 1148-1150) to remove mention of onb curves
  - fix nameTagPair[] by removing the lines for onb curves 
   (~lines 433-440)

In fipstest:
  - fix nameTagPair by removing the lines for onb curves
    (~lines 1951-1958)

In seckey.c:
  - remove onb curves from SECKEY_ECParamsToKeySize

(One could also use this opportunity to refactor
nameTagPair so there is just one instance of it).

vipul

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Vipul, besides the onb curves, do we support ANY of these?

433   { "c2onb191v4", SEC_OID_ANSIX962_EC_C2ONB191V4 },
434   { "c2onb191v5", SEC_OID_ANSIX962_EC_C2ONB191V5 },
439   { "c2onb239v4", SEC_OID_ANSIX962_EC_C2ONB239V4 },
440   { "c2onb239v5", SEC_OID_ANSIX962_EC_C2ONB239V5 },

426   { "c2pnb163v1", SEC_OID_ANSIX962_EC_C2PNB163V1 },
427   { "c2pnb163v2", SEC_OID_ANSIX962_EC_C2PNB163V2 },
428   { "c2pnb163v3", SEC_OID_ANSIX962_EC_C2PNB163V3 },
429   { "c2pnb176v1", SEC_OID_ANSIX962_EC_C2PNB176V1 },
435   { "c2pnb208w1", SEC_OID_ANSIX962_EC_C2PNB208W1 },
441   { "c2pnb272w1", SEC_OID_ANSIX962_EC_C2PNB272W1 },
442   { "c2pnb304w1", SEC_OID_ANSIX962_EC_C2PNB304W1 },
444   { "c2pnb368w1", SEC_OID_ANSIX962_EC_C2PNB368W1 },

430   { "c2tnb191v1", SEC_OID_ANSIX962_EC_C2TNB191V1 },
431   { "c2tnb191v2", SEC_OID_ANSIX962_EC_C2TNB191V2 },
432   { "c2tnb191v3", SEC_OID_ANSIX962_EC_C2TNB191V3 },
436   { "c2tnb239v1", SEC_OID_ANSIX962_EC_C2TNB239V1 },
437   { "c2tnb239v2", SEC_OID_ANSIX962_EC_C2TNB239V2 },
438   { "c2tnb239v3", SEC_OID_ANSIX962_EC_C2TNB239V3 },
443   { "c2tnb359v1", SEC_OID_ANSIX962_EC_C2TNB359V1 },
445   { "c2tnb431r1", SEC_OID_ANSIX962_EC_C2TNB431R1 },

419   { "prime192v1", SEC_OID_ANSIX962_EC_PRIME192V1 },
420   { "prime192v2", SEC_OID_ANSIX962_EC_PRIME192V2 },
421   { "prime192v3", SEC_OID_ANSIX962_EC_PRIME192V3 },
422   { "prime239v1", SEC_OID_ANSIX962_EC_PRIME239V1 },
423   { "prime239v2", SEC_OID_ANSIX962_EC_PRIME239V2 },
424   { "prime239v3", SEC_OID_ANSIX962_EC_PRIME239V3 },

447   { "secp112r1", SEC_OID_SECG_EC_SECP112R1},
448   { "secp112r2", SEC_OID_SECG_EC_SECP112R2},
449   { "secp128r1", SEC_OID_SECG_EC_SECP128R1},
450   { "secp128r2", SEC_OID_SECG_EC_SECP128R2},

452   { "sect113r1", SEC_OID_SECG_EC_SECT113R1},
453   { "sect113r2", SEC_OID_SECG_EC_SECT113R2},
454   { "sect131r1", SEC_OID_SECG_EC_SECT131R1},
455   { "sect131r2", SEC_OID_SECG_EC_SECT131R2},

Comment 4

[Douglas Stebila]

(In reply to comment #3)
> Vipul, besides the onb curves, do we support ANY of these?

We support all of those curves except the ONB ones.  You can see the list of curves supported in http://lxr.mozilla.org/security/source/security/nss/lib/freebl/ecl/ecl-exp.h.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Both usage messages also mention one other curve that does not exist:

c2tnb359w1

This appears to just be a typo.  The correct name should be:

c2tnb359v1
        ^  v, not w

Comment 6

[Wan-Teh Chang]

c2pnb176v1 is another typo.  The correct name should be:

c2pnb176w1
        ^  w, not v

Comment 7

[Wan-Teh Chang]

I should point out that the typo c2pnb176v1 is more
serious because its OID tag SEC_OID_ANSIX962_EC_C2PNB176V1
and string "ANSI X9.62 elliptic curve c2pnb176v1" have the
same typo, and this typo has propagated to PSM.  Since this
curve is disallowed in X9.62-2005, perhaps it's not worth
our time to fix this typo.

Comment 8

[Julien Pierre]

Retargetting all P2s to 3.11.3 .

Comment 9

[Wan-Teh Chang]

I listed all elliptic curves supported by NSS and
documented how to generate that list in the NSS
Security Policy:
http://wiki.mozilla.org/Security_Policy#Non-NIST-Recommended_Elliptic_Curves.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Neil, here's another bug that should be quick to fix.

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

While you're fixing certutil's list of supported curves, by removing the 
names of unsupported curves and correcting typos in others, here's another
thing to fix.  

certutil's list of curve names is the "extended" list, not the "basic" list.
Even when certutil is being built to feature "basic" ECC support, it 
displays the list of names for "extended" support.  That needs to be fixed,
so that for "basic" support builds, certutil shows only the names of the 
three basic curves.  

Comment 12

[Neil Williams]

Attachment: fixes 3 problems with help message

This patch ifdefs curve names listed by the help message. Curve c2tnb359w1 has already been renamed to c2tnb359v1; no change was made to c2pnb176v1 after comment 7.

Also added a note to -A and -R to indicate applicable options that are described in other commands (-E and -S, respectively).

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 252131 [details] [diff] [review]
fixes 3 problems with help message

Neil, did you mean to delete this line?  I suspect you did not.  
r+ if you did, r- if you didn't.  

>     FPS "%-20s nistb163, sect193r1, sect193r2, sect233k1, nistk233,\n", "");
>-    FPS "%-20s sect233r1, nistb233, sect239k1, sect283k1, nistk283,\n", "");
>     FPS "%-20s sect283r1, nistb283, sect409k1, nistk409, sect409r1,\n", "");

Comment 14

[Neil Williams]

Attachment: patch with 2 more fixes in easily checkable form

This patch and the next are equivalent. The next one, which is the one to be checked in, merely has excess blank space removed from it. This is done in hopes it will be easier to review.

Comment 15

[Neil Williams]

Attachment: final patch

This is equivalent to previous patch. It fixes the accidental deletion Nelson mentioned (thanks). It also removes the ONB curves. I have verified that if you try to create a CSR with one of the ONB curves EC_DecodeParams() returns SEC_ERROR_UNSUPPORTED_ELLIPTIC_CURVE but certutil fails with the unhelpful ": An I/O error occurred during security authorization".

Comment 16

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 252395 [details] [diff] [review]
final patch

r=nelson
Requesting second review for 3.11 branch.

Comment 17

[Neil Williams]

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.107; previous revision: 1.106
done

checked in on trunk.

Comment 18

[Neil Williams]

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.97.2.8; previous revision: 1.97.2.7
done

Checked in on 3.11 branch.