Last Comment Bug 362278 - lib/util includes header files from other NSS directories
: lib/util includes header files from other NSS directories
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.11.4
: All All
: P3 enhancement (vote)
: 3.12
Assigned To: Wan-Teh Chang
:
Mentors:
Depends on:
Blocks: 360426
  Show dependency treegraph
 
Reported: 2006-11-29 13:38 PST by Wan-Teh Chang
Modified: 2008-03-27 16:24 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Diffs that show the problems (3.81 KB, patch)
2006-11-29 14:34 PST, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Move CERT_TimeChoiceTemplate to lib/certdb (4.31 KB, patch)
2007-11-20 18:46 PST, Wan-Teh Chang
julien.pierre: review+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2006-11-29 13:38:22 PST
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.
Comment 1 Wan-Teh Chang 2006-11-29 14:34:01 PST
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.
Comment 2 Wan-Teh Chang 2007-11-20 18:46:01 PST
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
Comment 3 Wan-Teh Chang 2007-11-21 13:24:15 PST
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.

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