14.23 KB, patch
|Details | Diff | Splinter Review|
21.11 KB, patch
|Details | Diff | Splinter Review|
The NSS cryptographic module should have its own version numbers, independent of the NSS version numbers. This allows us to change the rest of NSS while keeping the NSS cryptographic module at the FIPS validated version. The NSS cryptographic module has two shared libraries/DLLS: - the softoken library - the freebl library The new version numbers need to apply to both of them. The new version numbers only need to be changed when we change one of the files in the NSS cryptographic module, whose source tree consists of four directories (ignoring makefiles): - mozilla/dbm - mozilla/security/nss/lib/util - mozilla/security/nss/lib/freebl - mozilla/security/nss/lib/softoken
Created attachment 246876 [details] [diff] [review] Proposed patch Add a new header file, cryptmod.h, that defines the macros for the NSS cryptographic module's version numbers. Change the files to include cryptmod.h instead of nss.h, and change the NSS_xxx version macros to the CRYPTO_MODULE_xxx version macros. I didn't change anything else, in particular the product name, so that the compiled binaries should be essentially the same as the binaries that were tested by the FIPS testing lab.
I'll go beyond suggesting that softoken have its own version numbers, and suggest that it should be separately built and released, just as NSPR is separately built and released.
Comment on attachment 246876 [details] [diff] [review] Proposed patch I think the name of the proposed new header file, and the names of the symbols defined in that file, are too generic. If NSS had only one cyrypto module, and was going to only ever have one crypto module, then I'd agree that symbols (and file names) that say "crypto module", as is the only one true crypto module, would be OK. But NSS already has two crypto modules, and soon may have more (e.g. a CAPI-based module). So, I think it is more appropriate for the file names and symbol names to clearly indicate WHICH ONE of the crypto modules they are part of.
Attachment #246876 - Flags: review?(nelson) → review-
Created attachment 246995 [details] [diff] [review] Proposed patch v2 In this patch, cryptmod.h is renamed softkver.h and moved to lib/softoken, and the CRYPTO_MODULE_xxx macros are renamed SOFTOKEN_xxx.
Comment on attachment 246876 [details] [diff] [review] Proposed patch Looks good as far as it goes.
Attachment #246876 - Flags: superreview+
Attachment #246995 - Flags: superreview?(neil.williams) → superreview+
Created attachment 247256 [details] [diff] [review] Add NSS 3.11.4's nss.h to lib/freebl and lib/softoken This is a workaround for the NSS_3_11_BRANCH only, to avoid changing any file in lib/freebl and lib/softoken. I found that if there is an nss.h file in the current directory, the compiler (on all platforms) will use that instead of the nss.h file found in any of the directories specified by the -I flags. So if we add the nss.h file from NSS_3_11_4_RTM to the lib/freebl and lib/softoken directories, that copy of nss.h will get used when we are compiling in lib/freebl and lib/softoken.
Comment on attachment 246995 [details] [diff] [review] Proposed patch v2 r=nelson
Attachment #246995 - Flags: review?(nelson) → review+
Comment on attachment 247256 [details] [diff] [review] Add NSS 3.11.4's nss.h to lib/freebl and lib/softoken I gather this patch is a contigency plan, to be used in the event that we learn that we cannot make any changes to the sources between 3.11.4 and 3.11.5, not even to change the version numbers in softokn3. Yes? Please add a LARGE gawdy block comment to both of these new files, explaining that these are duplicates of the real nss.h, and naming all the new duplicates, and explaining that the two new duplicate files MUST be kept in sync with each other. If the intent is that these files NOT be changed when the real nss.h is changed, say that is BIG BOLD letters, and add a comment to the real nss.h, explaining that we must NOT change these dupliate files when we change nss.h. Otherwise, I'm sure someone will do the wrong thing with these files.
Attachment #247256 - Flags: review?(nelson) → review+
Created attachment 247485 [details] [diff] [review] Proposed patch v2 (as checked in to the NSS trunk) I checked in the proposed patch v2 on the NSS trunk (NSS 3.12). The versions in the new file softkver.h need to be changed from 3.11.4 to 3.12 (the version of the NSS trunk), so I attached this new patch. Checking in lib/freebl/freebl.rc; /cvsroot/mozilla/security/nss/lib/freebl/freebl.rc,v <-- freebl.rc new revision: 1.3; previous revision: 1.2 done Checking in lib/freebl/freeblver.c; /cvsroot/mozilla/security/nss/lib/freebl/freeblver.c,v <-- freeblver.c new revision: 1.2; previous revision: 1.1 done Checking in lib/softoken/manifest.mn; /cvsroot/mozilla/security/nss/lib/softoken/manifest.mn,v <-- manifest.mn new revision: 1.27; previous revision: 1.26 done Checking in lib/softoken/pkcs11.c; /cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v <-- pkcs11.c new revision: 1.137; previous revision: 1.136 done Checking in lib/softoken/softkver.c; /cvsroot/mozilla/security/nss/lib/softoken/softkver.c,v <-- softkver.c new revision: 1.6; previous revision: 1.5 done RCS file: /cvsroot/mozilla/security/nss/lib/softoken/softkver.h,v done Checking in lib/softoken/softkver.h; /cvsroot/mozilla/security/nss/lib/softoken/softkver.h,v <-- softkver.h initial revision: 1.1 done Checking in lib/softoken/softokn.rc; /cvsroot/mozilla/security/nss/lib/softoken/softokn.rc,v <-- softokn.rc new revision: 1.11; previous revision: 1.10 done
Attachment #246995 - Attachment is obsolete: true
Created attachment 247495 [details] [diff] [review] Add NSS 3.11.4's nss.h to lib/freebl and lib/softoken (for NSS_3_11_BRANCH only) v2 Nelson, I'm proposing this patch for the NSS_3_11_BRANCH because it is much easier to explain what the patch is than "proposed patch v2". The hard part of this patch is to verify that all compilers use the local copy of nss.h, which I have done. I added the comments you suggested to the copies of nss.h in lib/freebl and lib/softoken: a big block comment at the beginning of the file, and a one-line comment at the place that someone is most likely to change. Please let me know if the comments are sufficient. Note that I didn't change the original nss.h. % diff -u ../nss/nss.h nss.h --- ../nss/nss.h 2006-12-04 16:49:45.437500000 -0800 +++ nss.h 2006-12-04 17:04:10.546875000 -0800 @@ -1,3 +1,15 @@ +/*********************************************************************** + * + * A copy of nss.h from NSS 3.11.4 for the directories that make up the + * NSS cryptographic module (lib/freebl and lib/softoken). + * + * When compiling in these directories, the compiler uses the local copy + * of nss.h, allowing the NSS cryptographic module to stay at version + * 3.11.4 (the version submitted to NIST for FIPS 140-2 validation). + * + * DO NOT CHANGE THIS FILE. + * + ***********************************************************************/ /* * NSS utility functions * @@ -52,6 +64,7 @@ * The format of the version string should be * "<major version>.<minor version>[.<patch level>] [<Beta>]" */ +/* ***** DO NOT CHANGE THIS FILE. ***** */ #ifdef NSS_ENABLE_ECC #ifdef NSS_ECC_MORE_THAN_SUITE_B #define NSS_VERSION "3.11.4 Extended ECC"
Comment on attachment 247495 [details] [diff] [review] Add NSS 3.11.4's nss.h to lib/freebl and lib/softoken (for NSS_3_11_BRANCH only) v2 r=nelsonb Thanks, Wan-Teh
Attachment #247495 - Flags: review?(nelson) → review+
I checked in the patch "add NSS 3.11.4's nss.h to lib/freebl and lib/softoken (for NSS_3_11_BRANCH only) v2". RCS file: /cvsroot/mozilla/security/nss/lib/freebl/Attic/nss.h,v done Checking in freebl/nss.h; /cvsroot/mozilla/security/nss/lib/freebl/Attic/nss.h,v <-- nss.h new revision: 126.96.36.199; previous revision: 1.1 done RCS file: /cvsroot/mozilla/security/nss/lib/softoken/Attic/nss.h,v done Checking in softoken/nss.h; /cvsroot/mozilla/security/nss/lib/softoken/Attic/nss.h,v <-- nss.h new revision: 188.8.131.52; previous revision: 1.1 done
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
I checked in the proposed patch v2 and backed out the workaround (lib/freebl/nss.h and lib/softoken/nss.h) on the NSS_3_11_BRANCH (NSS 3.11.6)
You need to log in before you can comment on or make changes to this bug.