391183 - rename libPKIX error string number type to pkix error number types

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Alexei Volkov]

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

[Nelson Bolyard (seldom reads bugmail)]

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 
s/PKIX_ERRORNUM/PKIX_ERRORCLASS/g
s/PKIX_ERRSTRINGNUM/s//PKIX_ERRORCODE/
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

[Nelson Bolyard (seldom reads bugmail)]

We need a separate RFE to map from libPKIX error numbers to NSS error numbers

Comment 3

[Alexei Volkov]

Assign to Nelson, since he has a script that can nicely do global replacements in the code.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Attachment: big renaming patch, v1

This is not yet tested.

Comment 5

[Alexei Volkov]

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

[Nelson Bolyard (seldom reads bugmail)]

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

[Robert Relyea]

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.

bob

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Bob, Your comment 7 implies that something seems to not be working. 
Please be specific.  What problems do you see?

Comment 9

[Robert Relyea]

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.

bob

Comment 10

[Julien Pierre]

Bob,

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