226861 - NSS_CMSSignedData_GetDigestValue and NSS_CMSSignedData_GetDigestByAlgTag are duplicates

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

The functions NSS_CMSSignedData_GetDigestValue and
NSS_CMSSignedData_GetDigestByAlgTag are duplicates.
Neither is exported from the NSS S/MIME library, so
we can simply delete one.

I think it is better to delete the function
NSS_CMSSignedData_GetDigestValue because it is not
being used, but use its code because it has better
error checking (it tests for a null sigd->digestAlgorithms
and a -1 return value from NSS_CMSAlgArray_GetIndexByAlgTag).

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

Upon closer inspection, I think it is better to make
NSS_CMSSignedData_GetDigestValue the surviving function
because it has a corresponding NSS_CMSSignedData_SetDigestValue,
which is exported.

This patch deletes NSS_CMSSignedData_GetDigestByAlgTag
and replaces it by NSS_CMSSignedData_GetDigestValue.
Note that NSS_CMSSignedData_GetDigestValue already handles
errors correctly.  Its only problem is that it doesn't set
error code, which I noted with a comment.

Comment 2

[Robert Relyea]

Comment on attachment 136376 [details] [diff] [review]
Proposed patch

The fix isn't quite complete. The caller of NSS_CMSSignedDAta_GetDigestValue is
not expecting a failure.

Either NSS_CMSSignedData_GetDigestValue
 or its caller should set
 SEC_ERROR_DIGEST_NOT_FOUND

Other than these two issues, the patch is fine.

Comment 3

[Wan-Teh Chang]

Attachment: Proposed patch v1.1

This is the same patch, but the code it patches has
just changed.

It shows that the caller of NSS_CMSSignedData_GetDigestValue
is expecting a failure and sets the error code
SEC_ERROR_PKCS7_BAD_SIGNATURE.	The error code is not what
Bob suggested (SEC_ERROR_DIGEST_NOT_FOUND), but that's the
subject of Bugscape bug 54057 comments 3 and 4.

Comment 4

[Wan-Teh Chang]

Fix checked in on the NSS trunk (NSS 3.9).

Checking in cms.h;
/cvsroot/mozilla/security/nss/lib/smime/cms.h,v  <--  cms.h
new revision: 1.18; previous revision: 1.17
done
Checking in cmssigdata.c;
/cvsroot/mozilla/security/nss/lib/smime/cmssigdata.c,v  <--  cmssigdata.c
new revision: 1.20; previous revision: 1.19
done

Comment 5

[Robert Relyea]

Comment on attachment 136385 [details] [diff] [review]
Proposed patch v1.1

r=relyea