362278 - lib/util includes header files from other NSS directories

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P3)

Description

[Wan-Teh Chang]

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]

Attachment: 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]

Attachment: 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]

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.