190501 - Soft token returns wrong PKCS 11 error codes in many cases

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

Recently the need for accurate PKCS 11 error codes has become an issue.
Several third party PKCS 11 modules have been criticized for returning
the wrong error codes.  But NSS's own softoken does this a lot.
We should get our own house in order.

There are 104 places in softoken where the code sets/returns the error 
code CKR_HOST_MEMORY.  In some of these cases, this is appropriate because
a memory allocation function has just failed.  
But in MANY cases, this error is set merely because a function that is not 
(primarily) a memory allocation function returned a null pointer, without
any regard for WHY the null pointer was set.  

These places where CKR_HOST_MEMORY is erroneously set mostly fall into two
categories:

a) the called function was another softoken function that simply did not
return its ckr error value.  In many cases, the called function had the
correct ckr error value, but the function's signature did not provide any
way for it to return the ckr error to the caller.  For a documented example
of this, see http://bugzilla.mozilla.org/show_bug.cgi?id=188856#c6

b) the called function was a freebl function.  Freebl functions return 
SECStatus, and set error codes via PORT_SetError.  So, when a freebl 
function returns SECFailure, softoken should ALWAYS map the NSS error
code into the right ckr error, one appropriate for the operation being 
performed.  Instead, softoken just sets error CKR_HOST_MEMORY.

The error codes set by freebl functions include:
SEC_ERROR_BAD_DATA
SEC_ERROR_BAD_SIGNATURE
SEC_ERROR_INPUT_LEN
SEC_ERROR_INVALID_ALGORITHM
SEC_ERROR_INVALID_ARGS
SEC_ERROR_LIBRARY_FAILURE
SEC_ERROR_NEED_RANDOM
SEC_ERROR_NO_MEMORY
SEC_ERROR_OUTPUT_LEN

Only one of those should be mapped to CKR_HOST_MEMORY.  The rest should not.
There are appropriate CKR errors for each one of those.  To best map the 
error code, it may be necessary to know the PKCS 11 function that in being
performed and the PORT_ error code.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

For the functions in category a above, the function signature should be 
changed to enable the function to return the ckr error to the caller, so
the caller doesn't have to guess the proper ckr error.

Besides softoken, this problem also nss/lib/dev.

No NEW code should be written that blindly sets CKR_HOST_MEMORY for errors.
Reviewers of patches to softoken and devtoken should look for new code
that does this and review it for appropriateness.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

This can wait for 3.8.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Remove target milestone of 3.8, since these bugs didn't get into that release.