453364 - Improve PK11_CipherOp error reporting (was: PK11_CreateContextBySymKey returns NULL, with zero error code).

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Graham Leggett]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.0.1) Gecko/2008070206 Firefox/3.0.1
Build Identifier: 3.11.9.0

When an attempt is made to call PK11_CreateContextBySymKey, the function returns NULL, without setting an error code.

To recreate this, follow the following gdb trace:

Breakpoint 1, crypto_block_encrypt_init (p=0x809018, f=0x809058, type=KEY_3DES_192, mode=MODE_ECB, key=0xbffff188 "", keyLen=24, iv=0xbffff184, doPad=1, ctx=0xbffff17c, ivSize=0xbffff180) at crypto/apr_crypto_nss.c:228
228		apr_crypto_block_t *block = *ctx;
(gdb) next
229	    if (!block) {
(gdb) 
230	        *ctx = block = apr_pcalloc(p, sizeof(apr_crypto_block_t));
(gdb) 
232	    if (!block) {
(gdb) 
235	    block->factory = f;
(gdb) 
236	    block->pool = p;
(gdb) 
238	    if (keyLen == 0 || !key) {
(gdb) 
242	    apr_pool_cleanup_register(p, f,
(gdb) 
247	    if (block->key != NULL) {
(gdb) 
252	    switch (type) {
(gdb) 
256	        cipherMech = CKM_DES3_CBC_PAD;
(gdb) 
257	        break;
(gdb) 
274	    keyItem.data = (unsigned char*)key;
(gdb) 
275	    keyItem.len = keyLen;
(gdb) 
277	    PK11SlotInfo * slot = PK11_GetBestSlot(cipherMech, NULL);
(gdb) print keyItem.len
$2 = 24
(gdb) next
280	    block->key = PK11_ImportSymKey(slot,
(gdb) 
288	    if (slot) {
(gdb) 
289	        PK11_FreeSlot(slot);
(gdb) 
292	    switch (type) {
(gdb) 
297	        if (mode == MODE_CBC) {
(gdb) 
329	            block->ctx = PK11_CreateContextBySymKey(CKM_DES3_ECB, CKA_ENCRYPT, block->key, NULL);
(gdb) 
330	            block->ivSize = 0;
(gdb) print block->ctx
$3 = (PK11Context *) 0x0
(gdb) next
389	    perr = PORT_GetError();
(gdb) 
390	    if (perr) {
(gdb) 
397	    if (!block->ctx) {
(gdb) 
398	    	return APR_EPADDING;
(gdb) print perr
$4 = 0


Reproducible: Always

Steps to Reproduce:
xxx

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

In a newsgroup posting, Graham wrote this supplemental info:

>> What was the value of keyItem.len ?
> 
> 24 (corresponding to the 192 bit cipher).
> 
>> Are you using any PKCS#11 modules besides NSS's own modules?
> 
> Not that I am aware of.
> 
>> Do you possibly have a module marked as preferred ("default") for 3DES
>> that doesn't actually do 3DES?
>>
>> If you were using only NSS's PKCS#11 modules, did you have the Softoken
>> module in "FIPS mode" ?
> 
> The module is initialised as follows:
> 
> s = NSS_NoDB_Init(NULL);
> 
> The code is a test case, and doesn't do anything else other than encrypt 
> a string, and then try and decrypt the same string.
> 
> Changing the encryption mode from MODE_ECB to MODE_CBC, causes the 
> PK11_CreateContextBySymKey function to succeed.

Oh!  There's a clue I missed before.  

I noticed that you were passing CKM_DES3_ECB instead of "cipherMech" to
PK11_CreateContextBySymKey.  What I didn't notice, until just now, is that
the mechanism that you passed to PK11_GetBestSlot is not the same as the 
mech that you passed to PK11_CreateContextBySymKey.  I don't know what 
mechanism was passed to PK11_ImportSymKey (Please add a comment here with
that information).

This bug is valid, but let me suggest that you change your code to pass the 
SAME mech value to all 3 functions.  That should get you farther.

Comment 2

[Graham Leggett]

I just went through the code to fix this, and it now uses the same cipherMech for all calls, based on a cipherMech chosen right at the start. It didn't make a difference though.

In the case of CKM_DES3_ECB, the call to PK11_CreateContextBySymKey still returns NULL with an error code of zero.

In the case of CKM_DES3_CBC_PAD, the call to PK11_CreateContextBySymKey succeeds, but the call to PK11_CipherOp fails with the unrecognised error code -8192.

I suspect that what is happening is that the PK11 code is expecting the lower level code to have set an error, and so didn't set one itself, but I have managed to find a way for the lower code to fail without setting an error, which is in turn not set by the higher code either, resulting in the zero error code.

Not sure what is happening in the error -8192 case, it could be that someone added an error constant, but then didn't add a human readable version of that constant in the relevant place.

Comment 3

[Wan-Teh Chang]

In comment 0, the PK11_CreateContextBySymKey call that returns NULL is:

  block->ctx = PK11_CreateContextBySymKey(CKM_DES3_ECB,
                                          CKA_ENCRYPT,
                                          block->key,
                                          NULL);

Note that the fourth argument 'param' is NULL.  Tracing the source code,
I found that a NULL 'param' causes PK11_CreateContextBySymKey to return
NULL.  In function pk11_CreateNewContextInSlot, we have:
http://mxr.mozilla.org/security/source/security/nss/lib/pk11wrap/pk11cxt.c#287

287     if (param) {
288         if (param->len > 0) {
289             context->param = SECITEM_DupItem(param);
290         } else {
291             context->param = (SECItem *)&pk11_null_params;
292         }
293     } else {
294         context->param = NULL;
295     }
296     context->init = PR_FALSE;
297     context->sessionLock = PZ_NewLock(nssILockPK11cxt);
298     if ((context->param == NULL) || (context->sessionLock == NULL)) {
299         PK11_DestroyContext(context,PR_TRUE);
300         return NULL;
301     }

I don't know why this function should fail if 'param' is NULL.
A fix is to add a PORT_SetError call between lines 294 and 295:

    PORT_SetError(SEC_ERROR_INVALID_ARGS);

I guess that in the case of CKM_DES3_CBC_PAD, you must pass a non-NULL 'param'
to specify the IV, which is one reason PK11_CreateContextBySymKey succeeds.

The code at lines 288-292 above shows that in the case of CKM_DES3_ECB,
you need to pass a non-NULL 'param' whose 'len' field is 0.

Comment 4

[Wan-Teh Chang]

To look up an NSS error code, you can do a web search for "NSS error codes"
to find http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslerr.html.
It shows that -8192 is SEC_ERROR_IO.  Unfortunately it is misnamed (or misused)
for an error in crypto operation, in which case our software crypto module
returns CKR_DEVICE_ERROR, for example at
http://mxr.mozilla.org/security/source/security/nss/lib/softoken/pkcs11c.c#800

In the NSS source tree, the SEC_ERROR_xxx constants are defined in
http://mxr.mozilla.org/security/source/security/nss/lib/util/secerr.h.

Comment 5

[Wan-Teh Chang]

Attachment: Solution 1: set error code (checked in)

Graham, could you test this patch?  It should fix the 0 error
code problem.

Comment 6

[Wan-Teh Chang]

Attachment: Solution 2: allow a NULL 'param'

Bob, which solution do you prefer?

Comment 7

[Wan-Teh Chang]

Comment on attachment 336986 [details] [diff] [review]
Solution 2: allow a NULL 'param'

Graham, I'd appreciate it if you could test this patch, too,
with CKM_DES3_ECB and a NULL 'param'.

Comment 8

[Graham Leggett]

The solution 2 patch fails like so:

(gdb) 
419	    block->ctx = PK11_CreateContextBySymKey(key->cipherMech, CKA_ENCRYPT, key->symKey, secParam);
(gdb) 

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000008
0x00261924 in pk11_CreateNewContextInSlot ()
(gdb) bt
#0  0x00261924 in pk11_CreateNewContextInSlot ()
#1  0x00261a18 in PK11_CreateContextBySymKey ()
#2  0x0001d46c in crypto_block_encrypt_init (p=0x809018, f=0x8090e8, key=0x809118, iv=0xbffff078, ctx=0xbfffefbc, ivSize=0xbffff07c) at crypto/apr_crypto_nss.c:419
#3  0x000408bc in apr_crypto_block_encrypt_init (driver=0x1e074, p=0x809018, f=0x8090e8, key=0x809118, iv=0xbffff078, ctx=0xbfffefbc, ivSize=0xbffff07c) at crypto/apr_crypto.c:270
#4  0x0000dc88 in encrypt_block (tc=0x100520, pool=0x809018, driver=0x1e074, f=0x8090e8, key=0x809118, in=0x11d50 "plain text string", inlen=18, cipherText=0xbffff068, cipherTextLen=0xbffff06c, iv=0xbffff078, ivSize=0xbffff07c) at testcrypto.c:130

Comment 9

[Graham Leggett]

The solution 1 patch returns invalid arguments, as expected.

Comment 10

[Wan-Teh Chang]

Attachment: Solution 2: allow a NULL 'param', v2

Sorry about that.  Please try this new "solution 2" patch.

Comment 11

[Wan-Teh Chang]

Comment on attachment 336985 [details] [diff] [review]
Solution 1: set error code (checked in)

Graham verified that this patch works.  I suggest we check
this patch in first to fix the 0 error code bug, while Bob
decides whether the function should allow a NULL 'param'.

Comment 12

[Graham Leggett]

The solution 2 patch worked this time.

It stops at PK11_CipherOp with SEC_ERROR_IO, which is what it does without the patch but when you specify a non-NULL secParam.

Comment 13

[Wan-Teh Chang]

Attachment: Improve PK11_CiperOp error reporting (checked in)

Graham, please test this patch and if it makes PK11_CipherOp
return a better error code.

Bob, is there a better CK_RV error code than CKR_DEVICE_ERROR
for indicating an encryption or decryption operation failed?
CKR_DEVICE_ERROR gets mapped to SEC_ERROR_IO, and this has
nothing to do with I/O.

Comment 14

[Graham Leggett]

Just applied the patch, and the error returned is now SEC_ERROR_OUTPUT_LEN, which made more sense.

Turned out that code that had been cut and pasted had set the maxlen to outlen, which was in this case zero, which in turn threw the error when CipherOp was called. For the first time, the code was able to encrypt and then decrypt a 16 byte block!

I confirmed that when padding is not enabled, and the block is not a multiple of the block size, the error returned is SEC_ERROR_INPUT_LEN.

Comment 15

[Wan-Teh Chang]

Thanks for verifying that the patch works.  The next thing I'll
do is to document these functions.

When you get everything working, I'll be happy to review your
code.

Comment 16

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 336985 [details] [diff] [review]
Solution 1: set error code (checked in)

Agreed.
Error reporting is PK11wrap's greatest weakness.  Every little bit we do to improve it helps.  The biggest single thing we could do would be to stop interperting the majority of PKCS11 errors as SEC_ERROR_IO.  I have always viewed that as an expedient that was done to get code working, with the intention of coming back and improving it, but the "coming back and improving part" was never done, to our shame.

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

SEC_ERROR_IO was originally intended to report an IO error that occurred
while trying to read a user's password.  It was intended to differentiate
between "bad password entered" and "we failed to get any password at all".

I have never understood why it is used as a default error code for PKCS#11
operation failures.

Comment 18

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 337210 [details] [diff] [review]
Improve PK11_CiperOp error reporting (checked in)

r=nelson.

Comment 19

[Wan-Teh Chang]

The misuse of SEC_ERROR_IO is also the Softoken's
fault, not just PK11wrap's.  PK11wrap maps only four CK_RV
codes to SEC_ERROR_IO: CKR_CANCEL, CKR_ATTRIBUTE_SENSITIVE,
CKR_DEVICE_ERROR, and CKR_TOKEN_NOT_RECOGNIZED.  But
Softoken ignores the error codes of freebl and blindly
returns CKR_DEVICE_ERROR (or CKR_SIGNATURE_INVALID for
the NSC_Verify* functions).  You can see this by searching
for "PORT_GetError" in lib/softoken/pkcs11c.c.

The fix requires four things:

1. Optional: Find a new default CK_RV code for crypto
operation failure.  I think CKR_DEVICE_ERROR isn't
appropriate.  CKR_GENERAL_ERROR and CKR_FUNCTION_FAILED
seem to be the only options, and CKR_FUNCTION_FAILED
seems better.

2. Optional: Decide exactly which CK_RV codes should
be mapped to SEC_ERROR_IO.  Perhaps we should add new
NSS SEC_ error codes for CKR_CANCEL, CKR_ATTRIBUTE_SENSITIVE,
CKR_DEVICE_ERROR, and CKR_TOKEN_NOT_RECOGNIZED.

3. Most important: Go through lib/softoken/pkcs11c.c
and properly map freebl's error codes to CK_RV codes.
My patch "Improve PK11_CiperOp error reporting" (attachment
337210 [review]) is just the first step towards this goal.

4. Document on the NSS and SSL Error Codes page what SEC_ERROR_IO
was intended to mean and what it means today (Nelson's comment 17).

Comment 20

[Wan-Teh Chang]

Comment on attachment 336985 [details] [diff] [review]
Solution 1: set error code (checked in)

I checked in this patch on the NSS trunk (NSS 3.12.2).

Checking in pk11cxt.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cxt.c,v  <--  pk11cxt.c
new revision: 1.6; previous revision: 1.5
done

Comment 21

[Wan-Teh Chang]

Comment on attachment 337210 [details] [diff] [review]
Improve PK11_CiperOp error reporting (checked in)

I checked in this patch on the NSS trunk (NSS 3.12.2).

Checking in pk11wrap/pk11err.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11err.c,v  <--  pk11err.c
new revision: 1.8; previous revision: 1.7
done
Checking in softoken/pkcs11c.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11c.c,v  <--  pkcs11c.c
new revision: 1.102; previous revision: 1.101
done

Comment 22

[Wan-Teh Chang]

Attachment: Map freebl errors to CK_RV (work in progress)

Mapping error codes properly is labor-intensive.
This is as much as I can do for three hours tonight.

Comment 23

[Wan-Teh Chang]

Attachment: Add new error codes for vague PKCS #11 errors

There are three PKCS #11 errors that are vague: CKR_GENERAL_ERROR,
CKR_FUNCTION_FAILED, and CKR_DEVICE_ERROR.  Rather than mapping them
to SEC_ERROR_IO and losing the little information they have, I think
it's better to map them to new error codes that indicate we get
these errors.

What do you think?

I'm working on a different computer now, which is why I created this
as a separate patch.

Comment 24

[Wan-Teh Chang]

Attachment: Add new error codes for vague PKCS #11 errors, v2

Add a succint summary of the PKCS #11 errors to the error strings.

Since NSS depends on PKCS #11 heavily, I think it's useful to
expose parts of PKCS #11 in our error reporting when we can't
map them properly.

Comment 25

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 339581 [details] [diff] [review]
Add new error codes for vague PKCS #11 errors, v2

I agree in principle with adding these new error codes.  
I would suggest some changes in wording of the messages.

1. Change "The PKCS #11 module" to "A PKCS #11 module" in all these
error strings.  There may be more than one module, and we're not 
telling the user which one it is.  Saying "The" creates the idea in
the user's mind that there is only one, which will cause confusion.

2. Drop "horrible, ".  Saying it's unrecoverable is enough.

3. Since users do make function calls, instead of saying 
"making the same function call", say "trying the same operation".

r+ with those changes.

Comment 26

[Wan-Teh Chang]

Attachment: Add new error codes for vague PKCS #11 errors, v3 (checked in)

Nelson, thanks a lot for improving my English in these
error messages.

The phrase "a horrible, unrecoverable error" comes from
PKCS #11.  Are you sure you don't want "horrible"?

I don't understand the distinction you're making between
"function" and "operation" in the error string for
CKR_FUNCTION_FAILED.

I checked in the patch on the NSS trunk (NSS 3.12.2).

Checking in cmd/lib/SECerrs.h;
/cvsroot/mozilla/security/nss/cmd/lib/SECerrs.h,v  <--  SECerrs.h
new revision: 1.17; previous revision: 1.16
done
Checking in lib/util/secerr.h;
/cvsroot/mozilla/security/nss/lib/util/secerr.h,v  <--  secerr.h
new revision: 1.23; previous revision: 1.22
done

I will add these error codes to
http://www.mozilla.org/projects/security/pki/nss/ref/ssl/sslerr.html

Comment 27

[Nelson Bolyard (seldom reads bugmail)]

Some thoughts in reply to comment 26.
- I laughed when I read that PKCS#11 itself uses "horrible".  Maybe we 
  should say "Terrible, horrible, no good, very bad" (from the title of a 
  popular children's book).  :)  I think "unrecoverable" is a statement of 
  fact, but "horrible" is a subjective value judgment, and I think those 
  don't belong in error dialogs.
- The phrase "Trying the same operation again might succeed." seems like it
  would be best presented to the user in the form of a "try again" button.
  Perhaps it's a hint to the UI designer.  
- I think the user will click some button or menu item, and will get this 
  error.  If the user reads "making the same function call might succeed",
  I doubt he would realize that it means "clicking the same button again
  might succeed."  That's why I suggested referring to an operation.
- Of course, despite this new error string, it's likely that the user will 
  actually see "for an unknown reason", IMO.  :(

Comment 28

[Wan-Teh Chang]

I agree.  Our current error code mapping consists of two
lossy stages.  We lose some when we map freebl's error
codes to the PKCS #11 CKR_ error codes, and we lose some
more when we map the PKCS #11 CKR_ error codes to pk11wrap
error codes.  I think part of the solution requires adding
more error codes, which is why I added these three error
codes to NSS, and proposed four new CKR_ error codes for
PKCS #11 v2.30.

Comment 29

[Wan-Teh Chang]

Attachment: Patch for lib/pk11wrap/pk11err.c

This patch maps CKR_DEVICE_ERROR, CKR_FUNCTION_FAILED, and
CKR_GENERAL_ERROR to the newly added NSS SEC_ERROR_ codes
for these PKCS #11 errors.

As a result, CKR_DEVICE_ERROR won't be mapped to SEC_ERROR_IO
any more.

Comment 30

[Wan-Teh Chang]

Attachment: Map freebl errors to CK_RV, v1

It was tedious for me to write this patch, so I suspect it'll
also be tedious for you to review this patch.  Sorry!  I think
the pain is worth it.

Summary of this patch:

1. Move the definition of sftk_MapCryptError to the beginning
of this file so that any function in this file can call it.

2. Enhance sftk_MapCryptError to map all the SEC_ERROR error
codes that freebl may return (found by code inspection).

3. Add sftk_MapDecryptError, which is only used by the Decrypt
(and Unwrap?) functions, because for these functions we need to
map SEC_ERROR_BAD_DATA to CKR_ENCRYPTED_DATA_INVALID instead of
CKR_ARGUMENTS_BAD.

4. Search for CKR_DEVICE_ERROR in this file and change them to
sftk_MapCryptError(PORT_GetError()) where appropriate.  We need
to make sure that a freebl function has just failed at those
places.

5. Do the same thing for CKR_SIGNATURE_INVALID, which (instead
of CKR_DEVICE_ERROR) was the default error for the signature
verify functions.

Comment 31

[Wan-Teh Chang]

Attachment: Improve error reporting in lib/freebl/ec.c

You may want to review this patch for lib/freebl first because it
needs to be checked in before we start the FIPS algorithm testing.

While reviewing the code, I found three places in lib/freebl/ec.c
where we were not setting the error code.

In ECDH_Derive, I assume that ec_points_mul sets the error code
when it fails (which is mostly true), so this requires me to separate
that if statement into two.  I found similar code in EECDSA_VerifyDigest.
There we set the error to SEC_ERROR_BAD_SIGNATURE, which we clearly
can't use here in ECDH_Derive.  I use SEC_ERROR_BAD_KEY, but am not sure
if that's the right error code.

Comment 32

[Wan-Teh Chang]

Attachment: Map freebl errors to CK_RV, v2

See comment 30 for a summary of this patch.

In this new version, I added some comments, and a new function
sftk_MapVerifyError so that I can preserve the default
CKR_SIGNATURE_INVALID value returned by the Verify functions now.

Comment 33

[Robert Relyea]

Comment on attachment 346581 [details] [diff] [review]
Patch for lib/pk11wrap/pk11err.c

r+ assuming the appropriate SEC_ERROR_'s have been defined in secerr.h

Comment 34

[Nelson Bolyard (seldom reads bugmail)]

On Solaris, during NSS_Init, if softoken is unable to dynamically load freebl, 
then NSS_Init fails and the error code from PR_GetError is SEC_ERROR_IO.  :(

I would GUESS that this is true on any platform: If softoken cannot load
freebl, then NSS_Init reports SEC_ERROR_IO.

That test was not conducted with the latest trunk, so it is possible that 
the improvements the Wan-Teh has been making to softoken error reporting 
have already had some impact on this.  

Wan-Teh, I think this symptom fits well within the scopr of this bug, 
but if you think I should file a separate bug on this, please say so.

Comment 35

[Wan-Teh Chang]

Please file a separate bug report on that.  The new summary of this bug
is overly general.  The scope of this bug is smaller.

To fix that bug, you need to add a vendor-defined CKR_xxx error code,
map PR_LOAD_LIBRARY_ERROR to that CKR_xxx error code, and map it back
to PR_LOAD_LIBRARY_ERROR (or some better SEC_ERROR_ error code).

Comment 36

[Nelson Bolyard (seldom reads bugmail)]

Thank you, Wan-Teh.
Please write a better summary for this bug, one that more narrowly defines
its scope.  Thanks.

Comment 37

[Wan-Teh Chang]

I wrote a new summary that describes what has been checked in.
My pending patches are random improvements for whatever I happened
to notice while investigating the original bug and had the time
to work on.

Comment 38

[Robert Relyea]

Comment on attachment 346715 [details] [diff] [review]
Improve error reporting in lib/freebl/ec.c

r+ rrelyea

Comment 39

[Robert Relyea]

Comment on attachment 346727 [details] [diff] [review]
Map freebl errors to CK_RV, v2

r+

I would also be OK with creating an NSS specific CKR_ value from Random_Number Failure.

bob

Comment 40

[Wan-Teh Chang]

Comment on attachment 346715 [details] [diff] [review]
Improve error reporting in lib/freebl/ec.c

I checked in this patch on the NSS trunk (NSS 3.12.3).

Checking in ec.c;
/cvsroot/mozilla/security/nss/lib/freebl/ec.c,v  <--  ec.c
new revision: 1.19; previous revision: 1.18
done

Comment 41

[Wan-Teh Chang]

Comment on attachment 346727 [details] [diff] [review]
Map freebl errors to CK_RV, v2

I checked in the patch on the NSS trunk (NSS 3.12.3).

Checking in pkcs11c.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11c.c,v  <--  pkcs11c.c
new revision: 1.104; previous revision: 1.103
done

Comment 42

[Wan-Teh Chang]

Comment on attachment 346581 [details] [diff] [review]
Patch for lib/pk11wrap/pk11err.c

I checked in the patch on the NSS trunk (NSS 3.12.3).

Checking in pk11err.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11err.c,v  <--  pk11err.c
new revision: 1.9; previous revision: 1.8
done

Comment 43

[Wan-Teh Chang]

Attachment: More error code mapping

For some reason, I missed these changes in my previous patches.
I just found them in one of my NSS source trees.

This patch consists of two independent changes.  It's fine
if you only approve one of them.  I suspect the second change
is more problematic.

1. Map SEC_ERROR_UNSUPPORTED_ELLIPTIC_CURVE
==>    CKR_DOMAIN_PARAMS_INVALID
==>    SEC_ERROR_INVALID_KEY

2. Map CKR_MECHANISM_INVALID to SEC_ERROR_INVALID_ALGORITHM
instead of SEC_ERROR_BAD_DATA.  I think "algorithm" is closer
to "mechanism" than "data" is.

Comment 44

[Robert Relyea]

Comment on attachment 369682 [details] [diff] [review]
More error code mapping

r+ rrelyea

Comment 45

[Wan-Teh Chang]

Comment on attachment 369682 [details] [diff] [review]
More error code mapping

Patch checked in on the NSS trunk (NSS 3.12.3).

Checking in softoken/pkcs11c.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11c.c,v  <--  pkcs11c.c
new revision: 1.109; previous revision: 1.108
done
Checking in pk11wrap/pk11err.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11err.c,v  <--  pk11err.c
new revision: 1.10; previous revision: 1.9
done