lib/util is in the lowest layer of NSS, but three files in lib/util -- pqgutil.c, secoid.c, and sectime.c -- includes header files from other NSS directories. This is "layering violation" if we need to package lib/util separately from the rest of NSS.
Created attachment 246992 [details] [diff] [review] Diffs that show the problems This diffs file shows exactly what secoid.c, sectime.c, and pqgutil.c (pqgutil.h) need from the headers from the other NSS directories. This is not a proposed patch. 1. pqgutil.c/pqgutil.h The problem with pqgutil.h is that it includes blapi.h from lib/freebl. I found that the functions in pqgutil.c are only used in lib/pk11wrap/pk11pqg.c, and cmd/makepqg, except PQG_DestroyParams and PQG_DestroyVerify, which are also used in lib/softoken/pkcs11c.c and cmd/fipstest. So the solution may be to move pqgutil.c to lib/freebl/pqg.c and lib/pk11wrap/pk11pqg.c. 2. secoid.c This file clearly should stay in lib/util. We need to make the pkcs11t.h header (and the pkcs11*.h headers it includes) available to lib/util somehow. We also need the definition of the macro CKM_INVALID_MECHANISM. 3. sectime.c The only function in sectime.c used by the softoken is DER_DecodeTimeChoice, which is used in lib/softoken/lowcert.c. So solution may be to move DER_DecodeTimeChoice (probably all those that are declared in secder.h) to lib/util/dertime.c, and move the rest of sectime.c (especially those that are declared in cert.h or certt.h) to lib/certdb. These are declared in cert.h or certt.h: CERT_TimeChoiceTemplate CERT_ValidityTemplate CERT_CreateValidity CERT_CopyValidity CERT_DestroyValidity These functions have the CERT_ prefix but are declared in secder.h: CERT_UTCTime2FormattedAscii CERT_GenTime2FormattedAscii CERT_GeneralizedTime2FormattedAscii They probably should stay in lib/util in spite of their CERT_ prefix. Note that January1st2050 should be made static.
Created attachment 289598 [details] [diff] [review] Move CERT_TimeChoiceTemplate to lib/certdb Issues 1 and 2 and most of issue 3 have been addressed in bug 402777, which is really a duplicate of this bug. This patch completely fixes issue 3. CERT_TimeChoiceTemplate is declared in certt.h but defined in lib/util. Since lib/util, lib/freebl, and lib/softoken don't use this template, I suggest that we move it to lib/certdb. (All the TimeChoice functions in lib/util encode or decode manually, without using this template.) http://lxr.mozilla.org/security/ident?i=CERT_TimeChoiceTemplate
I checked in the patch on the NSS trunk for NSS 3.12. lib/util still includes pkcs11*.h and nss.h as I described in bug 402777 comment 19, but we can easily fix them without changing the API when we really need to build lib/util as a separate component. So I'm marking this bug fixed.