12.74 KB, text/plain
NSS seems to have several functions that return error text strings for NSS's error codes. SECU_Strerror() in cmd/lib/secerror.c returns a pointer to an const error string. It returns strings for all known SEC_ SSL_ and NSPR error codes. It returns NULL for unknown error codes. In my opinion this function should be the ONLY source of error strings in NSS. cmd/lib/secutil.c also contains several other functions for returning error strings, including SECU_ErrorString(), SECU_ErrorStringRaw(), and SECU_GetString(), and others. These all use a comon static buffer to hold error messages, and so none of them is thread safe. SECU_ErrorStringRaw provides strings for a small subset of the SEC_ and SSL_ error codes, and none for the NSPR error codes. It's error messages are very terse and potentially ambiguous. These functions are presently used in the following source files in nss/cmd/... lib/secutil.c modutil/install.c modutil/instsec.c pk12util/pk12util.c signtool/sign.c signtool/util.c signtool/verify.c signver/signver.c sslstrength/sslstrength.c I thing that SECU_ErrorStringRaw should be deleted and functions that use it should be converted to use SEC_Strerror instead. Also, it would be good to make the functions in secutil be thread safe.
Assigned the bug to Kirk. Kirk, didn't you already add such a function (SECU_PrintPRandOSError) in bug #66244? If so, you just need to make sure that all the command line utilities on Nelson's list use this new function.
Changed the QA contact to Bishakha.
Set target milestone to NSS 3.5.
Set target milestone to NSS 3.6.
Moved to target milestone 3.8 because the original NSS 3.7 release has been renamed 3.8.
Remove target milestone of 3.8, since these bugs didn't get into that release.
Target Milestone: 3.9
Retargeted Future. Plan to address Sun issues first.
The right way to fix this is to have NSS_Init install error code translations into NSPR via PR_ErrorInstallTable, and then use the PR_ErrorToString() API for its intended purpose. An NSPR socket with NSS installed under it will cause NSPR APIs to return SSL errors, so PR_ErrorToString should have the strings for those errors installed. This is also important because when the error to string tables are not in the library that generates the error, then the numbers can get out of sync. For example, Mozilla libsslldap library has a copy of the errors and I just had to put another copy in the async I/O layer we use for Messaging Server at Sun. As NSS adds new errors, these tables won't be updated automatically, so it would be better if the tables were in libnss.so and not duplicated in everything that calls it.
Chris, thanks for your contribution! I don't think NSS_Init should install these values, but I agree the values should be available for NSS-based programs to use. Perhaps you would care to contribute a patch that shows the conversion of a small NSS utility command, such as pk12util/pk12util.c to a) install error code translations into NSPR via PR_ErrorInstallTable, (say, just before the call to NSS_Init) and then b) use the PR_ErrorToString() API for its intended purpose.
I'm very busy at work and NSS is not my direct responsibility. In order for me to make time to build a full patch, I would have to get something out of it. The benefit I want only happens if the error tables are folded into the NSS shared objects so I no longer have to maintain a separate error table in my layered code If there is agreement to do that, then I'm willing to do the integration work as a patch (including all tools and the core libraries). If we don't have NSS_Init install the table, then we need to export the table itself as a symbol so the caller can install it -- I'm fine with either approach.
Created attachment 141848 [details] [diff] [review] alternative patch - maintenance free Chris, This patch creates two tables, one for SEC errors and one for SSL errors. It also includes a little function that installs those tables into NSPR's error mechanism. It does NOT duplicate the contents of SECerrs.h or SSLerrs.h, but rather includes them, so as those tables grow, there is no need to perform maintenance on this source file. Is this what you want? If so, please contribute a small patch showing how any of the NSS utility commands might make use of this. Thanks.
Comment on attachment 141848 [details] [diff] [review] alternative patch - maintenance free I have added a large patch to bug 172051 that creates a function that registers the NSS error strings with NSPR. This new function becomes part of libNSS. I decided to include the SSL error strings in libNSS, and have one function that registers both NSS and SSL error codes, because NSS uses SSL error codes in places that are not necessarily SSL errors. The patch changes the common SECU_ error string functions to (a) call the new NSS function to register its error strings, and (b) use NSPR's set of registered error strings exclusively. The patch does not eliminate the above named functions that use a static error message buffer, but it does change them to use NSPR's strings rather than their own. After that patch is checked in and bug 172051 is closed, I can go to work on eliminating the calls to the error functions that use a static error buffer, replacing them with calls to the safe function(s). I believe that the patch for bug 172051 meets Chris Newman's needs.