Closed Bug 273444 Opened 20 years ago Closed 20 years ago

Not all generic hash api functions are exported e.g. HASH_HashBuf

Categories

(NSS :: Libraries, defect, P3)

3.9.3
Sun
Solaris
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jason.m.reid, Assigned: nelson)

Details

Attachments

(1 file)

Not all of the generic hash (sechash.h) api functions are exported for usage.
(I particulary think that HASH_HashBuf should be exported.)

HASH_ResultLen              exported
HASH_ResultLenContext       Not exported
HASH_ResultLenByOidTag      Not exported
HASH_HashBuf                Not exported
HASH_Create                 Exported
HASH_Clone                  Not exported
HASH_Destroy                Exported
HASH_Begin                  Exported
HASH_Update                 Exported
HASH_End                    Exported
HASH_GetHashObject          Exported
HASH_GetHashObjectByOidTag  Exported
HASH_GetHashTypeByOidTag    Exported

From Solaris nm(1) of libnss3.so
[7516]  |        219272|         108|FUNC |GLOB |0    |13     |HASH_Begin
[3101]  |        218808|         284|FUNC |LOCL |2    |13     |HASH_Clone
[7246]  |        218392|         388|FUNC |GLOB |0    |13     |HASH_Create
[7176]  |        219120|         128|FUNC |GLOB |0    |13     |HASH_Destroy
[7650]  |        219568|         144|FUNC |GLOB |0    |13     |HASH_End
[7586]  |        216856|         120|FUNC |GLOB |0    |13     |HASH_GetHashObject
[7095]  |        217392|         168|FUNC |GLOB |0    |13    
|HASH_GetHashObjectByOidTag
[7378]  |        217000|         340|FUNC |GLOB |0    |13    
|HASH_GetHashTypeByOidTag
[3730]  |        218080|         284|FUNC |LOCL |2    |13     |HASH_HashBuf
[7795]  |        217752|         176|FUNC |GLOB |0    |13     |HASH_ResultLen
[3106]  |        217584|         140|FUNC |LOCL |2    |13    
|HASH_ResultLenByOidTag
[808]   |        217952|         100|FUNC |LOCL |2    |13     |HASH_ResultLenContext
[7528]  |        219408|         132|FUNC |GLOB |0    |13     |HASH_Update
[1308]  |       1094132|           4|OBJT |LOCL |2    |17    
|NSS_ERROR_HASH_COLLISION

From Linux nm of libnss3.so:
nm libnss3.so | grep HASH
00019667 T HASH_Begin
0001958a t HASH_Clone
000194b3 T HASH_Create
00019629 T HASH_Destroy
000196a9 T HASH_End
00019238 T HASH_GetHashObject
0001931a T HASH_GetHashObjectByOidTag
00019263 T HASH_GetHashTypeByOidTag
00019411 t HASH_HashBuf
000193b2 T HASH_ResultLen
00019371 t HASH_ResultLenByOidTag
00019405 t HASH_ResultLenContext
00019685 T HASH_Update
00094c68 r NSS_ERROR_HASH_COLLISION
Jason, I believe the reason is that we want people to
create a PK11Context and use the PK11_* functions on
the PK11Context to perform hash operations.  Bob?
Assignee: wtchang → rrelyea0264
Well, yes and no. The HASH_ interface is a 'compatibility' interface which
predates the PK11 interface and was maintained because a lot of code inside NSS
used it internally. (including SSL and S/MIME).

We can debate whether or not the interface as a whole should be marked
depricated or not. Not only are there identical PK11 functions for doing the
same thing, the HASH_ functions map pretty much one for one with those PK11
functions.

The main difference between the HASH_ functions and the PK11_ functions is the
HASH_ functions take a hash enum to identify the hashing algorithm and the
PK11_Digest* functions take a SECOIDTag

bob
Let me describe the HASH_ API as if it were implemented in C++.
I believe these statements are true, even though it is not implemented in C++.

The HASH_* API  provides the caller with hash objects (contexts), and 
virtual methods that work on those objects.  There are public constructors 
and there are private constructors.  

The public constructors can be used outside of softoken, and return 
objects that point to PK11 contexts and point to PK11_ methods.  
A user who gets a HASH_ object from one of the public constructors,
and uses the HASH_ methods to access it will be using a PK11Context and the 
PK11_ methods, as suggested in comment 1 above, perhaps without knowing it.

The vtables for these public HASH_ objects may be seen at 
http://lxr.mozilla.org/security/source/security/nss/lib/cryptohi/sechash.c#109 
The table of vtables is followed by the public constructors.  Many of our
utility command programs use this public HASH_ API, and do so properly, IMO.  

This HASH_ API is also used within softoken itself.  When used within softoken
the HASH_ objects point at internal context types, not PK11Context types,
and point at internal softoken (or blapi) methods, not at PK11_ methods,
but other than the method of construction, the use of the HASH_ API within
softoken is the same as the use outside of softoken - simple objects and
methods.  Softoken's private set of vtables this API may be seen at 
http://lxr.mozilla.org/security/source/security/nss/lib/softoken/rawhash.c#79

Jason observes that a few of the methods defined for HASH_ objects, including
the "HashBuf" method, are not exported from libNSS3.so.  They are publicly
declared, but not exported.  These unexported methods call other public 
exported functions, so I see no reason to keep these methods unexported,
especially the HashBuf methods.

The omission of HashBuf and the other methods from the table of exported
functions looks like an oversight, rather than intentional.  I'm willing 
to add them to the appropriate .def file, if no-one strongly objects.
A problem with using PK11_HashBuf is that you are limited to hashing
what can fit into memory. Using the generic hash api, you can hash
an item larger than total memory in chunks small enough to fit.In addition, both
PK11_HashBuf and HASH_HashBuf are limited to objects of four gigabytes. Using
the other HASH functions, you can hash larger objects.
The PK11_interfaces are exactly the same... there is a constructor that takes an
oid rather than and enum. The vtable in the PK11 interface happens to physically
live in the particular PKCS #11 module the hash algorithms are operating against.

Translation of the HASH_ calls into PK11_ calls would be nothing more than
renaming functions and structure pointers for the same functionality. HASH_ made
more sense when there is a difference between a SHA_Context and an MD5_Context
(like there is under the PKCS11 boundary in softoken). At the PK11_ layer they
are all PK11_Contexts.

That being said, I believe the primary reason for not exporting the HASH_ is the
'oversight' argument (or more exactly the 'until we have time to evaluate every
interface, only export what we need to get SSL and S/MIME working).

There is nothing wrong with the HASH_ interfaces. I just want to make sure we
understand in doing so (and using those interfaces) we are basically mantaining
two identical interfaces (verying only in function names and structure names),
one of which calls the other (the HASH_ functions all call the PK11_ functions).
(perhaps the reason is so the softoken code looks like code outside softoken?
though that would only be true of hash functions, and not any other crypto
functions).

Anyway if you go in knowing that there already is a one-for-one mapping between
HASH_ and PK11_Digest calls and your OK with that, then I have no objections to
exporting the HASH_ functions.

bob
Jason, I don't understand your comment in the context of this bug?

PK11_HashBuf and HASH_HashBuf have exactly the same signature. The only
difference between the two is HASH_HashBuf takes HASH_HashType which is an enum
of hashes and PK11_HashBuf takes a SECOidTag which is an enum of OIDS.

The PK11_ interface maps one for one with the HASH_interface...

PK11_CreateDigestContext(SECOidTag) <----> HASH_Create(HASH_Type)
PK11_CloneContext     <---->    HASH_Clone
PK11_DestroyContext   <---->    HASH_Destroy
PK11_DigestBegin      <---->    HASH_Begin
PK11_DigestOp         <---->    HASH_Update
PK11_DigestFinal      <---->    HASH_End

This is extremely clear since all the HASH_ functions simply turn around and
call the PK11_functions (dropping any error codes that may have been returned).

------------

All that being said, there are two areas where the HASH_ interface adds
functionality: 1) the HASH_ interface provides functions which return the hash
length, and 2) the SECHashObject type and functions to get it are exported. This
means some interfaces may pass a SECHashObject, expecting the caller to
dereferece the functions pointers on it's own.

Anyway that should be enough information to determine if it's necessary to
export the entire HASH_ function list or not.

bob
My comment was erronous. When looking at the pk11 header, I did not see
how it mapped to the HASH api.
We have been recommending the PK11_ interfaces as the official, supported API .
I would suggest we don't expose multiple interfaces for the same thing unless
there is a compelling reason. There is a functional difference, but it appears
small enough, and I'm not sure it's worth the trouble of adding this to our
supported set of APIs .
The HASH_ API is already exposed, all except for one function that appears
to have been omitted by mistake.  I don't understand why there is ANY 
pushback whatsoever on the suggestion to export the missing piece.  
I don't recall there being such pushback to previous requests of this nature.
I was just asking whether the HASH_ interface
was exported just to get the SSL and SMIME
shared libraries working quickly, or it was
intended to be a second API for hashing.

In general one strives to have only one way
of getting a task done in an API.

In any case, given the information provided
by Bob and Nelson I don't object to exporting
the full HASH_ interface, and I reiterate what
Bob said in comment 5: in doing so we are
basically mantaining two identical interfaces.
Attached patch patch v1Splinter Review
One of the requested symbols was already added in 3.10.  
This patch adds the rest.
Julien, please review.
Attachment #174005 - Flags: review?(julien.pierre.bugs)
Attachment #174005 - Flags: review?(julien.pierre.bugs) → review+
Taking bug.  (Hope Bob won't mind :-)
Assignee: rrelyea → nelson
Priority: -- → P3
Target Milestone: --- → 3.10
Checking in nss.def;
/cvsroot/mozilla/security/nss/lib/nss/nss.def,v  <--  nss.def
new revision: 1.137; previous revision: 1.136
done
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
go for it... oh you did already;).

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

Attachment

General

Created:
Updated:
Size: