Closed Bug 1273505 Opened 8 years ago Closed 7 years ago

Remove NSS_DISABLE_ECC from areas outside of freebl/softoken

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: mt, Assigned: mt)

References

Details

Attachments

(2 files, 1 obsolete file)

These only complicate things.
Attached patch bug1273505-1.patch (obsolete) — Splinter Review
Here's what I have.
Attachment #8753370 - Flags: review?(emaldona)
Comment on attachment 8753370 [details] [diff] [review]
bug1273505-1.patch

Review of attachment 8753370 [details] [diff] [review]:
-----------------------------------------------------------------

Martin: thanks for the patch. I suggest two changes.

::: cmd/bltest/blapitest.c
@@ -20,5 @@
>  #include "secport.h"
>  #include "secoid.h"
>  #include "nssutil.h"
>  
> -#ifndef NSS_DISABLE_ECC

I recommend putting the changes to the "cmd" directory in
a separate patch, because I am less certain about those
changes.

For example, blapitest.c is a test for lib/freebl, so it
seems reasonable that blapitest.c and lib/freebl should
be built in the same way: both should check NSS_DISABLE_ECC.

::: lib/nss/nss.h
@@ +7,5 @@
>  
>  #ifndef __nss_h_
>  #define __nss_h_
>  
>  /* The private macro _NSS_ECC_STRING is for NSS internal use only. */

Based on this MXR query:
 
http://mxr.mozilla.org/nss/search?string=NSS_ECC_MORE_THAN_SUITE_B
 
no code in libnss3.so, libssl3.so, and libsmime3.so (the three
shared libraries that use the NSS version macros defined here) depends
on NSS_ECC_MORE_THAN_SUITE_B. We should remove _NSS_ECC_STRING
altogether.

This change to the NSS version string needs to be documented in the
release notes.
Just in lib/
Attachment #8753370 - Attachment is obsolete: true
Attachment #8753370 - Flags: review?(emaldona)
Attachment #8753432 - Flags: review?(wtc)
Comment on attachment 8753432 [details] [diff] [review]
bug1273505-1.patch

Review of attachment 8753432 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc.

::: lib/ssl/sslcert.h
@@ +26,5 @@
>          /* For ssl_auth_ecdsa and ssl_auth_ecdh_*.  This is only the named curve
>           * of the end-entity certificate key.  The keys in other certificates in
>           * the chain aren't directly relevant to the operation of TLS (though it
>           * might make certificate validation difficult, libssl doesn't care). */
>          ECName namedCurve;

This is a side comment: it is strange to have a union with only
one member.
Attachment #8753432 - Flags: review?(wtc) → review+
As Wan-Teh notes, we probably need to retain these guards in bltest and fipstest.  I've also left them in cmd/lib/secutil.c too.
Attachment #8753446 - Flags: review?(wtc)
Assignee: nobody → martin.thomson
Comment on attachment 8753446 [details] [diff] [review]
bug1273505-2.patch

OBE
Attachment #8753446 - Flags: review?(wtc)
See Also: → 1339720
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: