Closed Bug 273444 Opened 21 years ago Closed 20 years ago

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

Categories

(NSS :: Libraries, defect, P3)

Product:
NSS
Component:
Libraries
Version:
3.9.3
Platform:
Sun
Solaris
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

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

Details

Attachments

(1 file)

patch v1
585 bytes, patch
julien.pierre
: review+
Details | Diff | Splinter Review
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 v1 — — Splinter 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

Creator:
Created:
Updated:
Size: