Last Comment Bug 391183 - rename libPKIX error string number type to pkix error number types
: rename libPKIX error string number type to pkix error number types
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: All All
P1 normal (vote)
: 3.12
Assigned To: Nelson Bolyard (seldom reads bugmail)
Depends on: 391560
  Show dependency treegraph
Reported: 2007-08-06 21:59 PDT by Alexei Volkov
Modified: 2007-10-02 18:15 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

big renaming patch, v1 (62.08 KB, patch)
2007-09-20 22:13 PDT, Nelson Bolyard (seldom reads bugmail)
alvolkov.bgs: review+
Details | Diff | Splinter Review

Description User image Alexei Volkov 2007-08-06 21:59:14 PDT
libpkix returns PKIX error message in case when it fails to build certificate chain. It is essential to convert these error messages to nss error codes to preserve backward compatibility. 

libpkix has 1200+ different error messages. Error messages(currently as char*) delivered to callers of PKIX_BuildChian/PKIX_ValidateChain as a member of PKIX_Error structure.

 53 struct PKIX_ErrorStruct {
 54         PKIX_UInt32 code;
 55         PKIX_Error *cause;
 56         PKIX_PL_Object *info;
 57         PKIX_PL_String *desc;
 58 };

The fix for this bug should change the way an error info is stored. We should abandon the use of string to store error codes. So *desc from PKIX_Error should be replace to something for suitable for the library. Instead of PKIX_PL_String* an error code (or better a pointer to PL layer error object allocated when a particular error has occurred) should be used.

Also, if possible extend and use nss error codes instead of pkix error codes. If the last one is not achievable, come up with a function that will converter libpkix codes to nss codes.
Comment 1 User image Nelson Bolyard (seldom reads bugmail) 2007-08-07 09:46:21 PDT
I suggest:

 struct PKIX_ErrorStruct {
         PKIX_ERRORNUM code;
         PKIX_Error *cause;
         PKIX_PL_Object *info;
         PKIX_ERRSTRINGNUM desc;

I further suggest that we do a global replacement of 
and change the names of the structure elements accordingly, to get 
something like

 struct PKIX_ErrorStruct {
         PKIX_ERRORCODE  errcode;
         PKIX_ERRORCLASS errclass;
         PKIX_Error     *cause;
         PKIX_PL_Object *info;
Comment 2 User image Nelson Bolyard (seldom reads bugmail) 2007-09-18 16:45:34 PDT
We need a separate RFE to map from libPKIX error numbers to NSS error numbers
Comment 3 User image Alexei Volkov 2007-09-20 14:28:29 PDT
Assign to Nelson, since he has a script that can nicely do global replacements in the code.
Comment 4 User image Nelson Bolyard (seldom reads bugmail) 2007-09-20 22:13:07 PDT
Created attachment 281784 [details] [diff] [review]
big renaming patch, v1

This is not yet tested.
Comment 5 User image Alexei Volkov 2007-09-24 16:51:44 PDT
Comment on attachment 281784 [details] [diff] [review]
big renaming patch, v1

r+. good patch. It solves  lot confusions related to error codes. 
I've tested it with libpkix tests.
Comment 6 User image Nelson Bolyard (seldom reads bugmail) 2007-09-24 17:55:23 PDT
nss/lib/certhigh/certvfypkix.c                      new: 1.5 previous: 1.4
nss/lib/nss/pkixpriv.def                            new: 1.2 previous: 1.1
nss/cmd/libpkix/perf/libpkix_buildthreads.c         new: 1.6 previous: 1.5
nss/cmd/libpkix/pkix/top/test_validatechain_NB.c    new: 1.6 previous: 1.5
nss/cmd/libpkix/pkix/util/test_error.c              new: 1.6 previous: 1.5
nss/cmd/libpkix/pkix/util/test_logger.c             new: 1.5 previous: 1.4
nss/cmd/libpkix/pkix_pl/module/test_httpcertstore.c new: 1.4 previous: 1.3
nss/cmd/libpkix/pkix_pl/module/test_socket.c        new: 1.4 previous: 1.3
nss/lib/libpkix/include/pkix_util.h                 new: 1.5 previous: 1.4
nss/lib/libpkix/include/pkixt.h                     new: 1.7 previous: 1.6
nss/lib/libpkix/pkix/top/pkix_build.c               new: 1.9 previous: 1.8
nss/lib/libpkix/pkix/top/pkix_policychecker.c       new: 1.5 previous: 1.4
nss/lib/libpkix/pkix/top/pkix_validate.c            new: 1.5 previous: 1.4
nss/lib/libpkix/pkix/util/pkix_error.c              new: 1.6 previous: 1.5
nss/lib/libpkix/pkix/util/pkix_error.h              new: 1.6 previous: 1.5
nss/lib/libpkix/pkix/util/pkix_errpaths.c           new: 1.4 previous: 1.3
nss/lib/libpkix/pkix/util/pkix_logger.c             new: 1.5 previous: 1.4
nss/lib/libpkix/pkix/util/pkix_logger.h             new: 1.5 previous: 1.4
nss/lib/libpkix/pkix/util/pkix_tools.c              new: 1.7 previous: 1.6
nss/lib/libpkix/pkix/util/pkix_tools.h              new: 1.8 previous: 1.7
nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_lifecycle.c new: 1.9 previous: 1.8
Comment 7 User image Robert Relyea 2007-09-26 11:54:32 PDT
Alexei, you said you tested this patch with the libpkix tests, but the tests in nss/cmd/libpkix in CVS do not seem to be updated with the new error names. Are the tests in your own tree, or should I look elsewhere for the libpkix tests.

Comment 8 User image Nelson Bolyard (seldom reads bugmail) 2007-09-26 14:01:35 PDT
Bob, Your comment 7 implies that something seems to not be working. 
Please be specific.  What problems do you see?
Comment 9 User image Robert Relyea 2007-09-26 15:14:56 PDT
Sorry about the alarm.

It turns out you have to build all of NSS with BUILD_LIBPKIX_TESTS set from the beginning.

Going into cmd/libpkix and doing a 'make' doesn't work.

Comment 10 User image Julien Pierre 2007-10-02 18:15:38 PDT

Correct. libnss needs to be rebuilt with BUILD_LIBPKIX_TESTS to export the libpkix symbols needed by the libpkix tests.

Note You need to log in before you can comment on or make changes to this bug.