Note: There are a few cases of duplicates in user autocompletion which are being worked on.

lib/util includes header files from other NSS directories

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P3
enhancement
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Wan-Teh Chang)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
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.
(Assignee)

Comment 2

10 years ago
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
Attachment #289598 - Flags: review?(julien.pierre.boogz)
(Assignee)

Updated

10 years ago
Assignee: nobody → wtc
Priority: -- → P3
Target Milestone: --- → 3.12

Updated

10 years ago
Attachment #289598 - Flags: review?(julien.pierre.boogz) → review+
(Assignee)

Comment 3

10 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.