Closed
Bug 453364
Opened 16 years ago
Closed 16 years ago
Improve PK11_CipherOp error reporting (was: PK11_CreateContextBySymKey returns NULL, with zero error code).
Categories
(NSS :: Libraries, defect)
NSS
Libraries
Tracking
(Not tracked)
RESOLVED
FIXED
3.12.3
People
(Reporter: minfrin, Assigned: wtc)
References
Details
(Whiteboard: FIPS)
Attachments
(8 files, 5 obsolete files)
945 bytes,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
1.76 KB,
patch
|
Details | Diff | Splinter Review | |
4.08 KB,
patch
|
rrelyea
:
review+
nelson
:
review+
|
Details | Diff | Splinter Review |
2.42 KB,
patch
|
Details | Diff | Splinter Review | |
2.37 KB,
patch
|
rrelyea
:
review+
|
Details | Diff | Splinter Review |
2.08 KB,
patch
|
rrelyea
:
review+
|
Details | Diff | Splinter Review |
18.50 KB,
patch
|
rrelyea
:
review+
|
Details | Diff | Splinter Review |
3.20 KB,
patch
|
rrelyea
:
review+
|
Details | Diff | Splinter Review |
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•16 years ago
|
||
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.
Reporter | ||
Comment 2•16 years ago
|
||
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.
Assignee | ||
Comment 3•16 years ago
|
||
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.
Assignee | ||
Comment 4•16 years ago
|
||
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.
Assignee | ||
Comment 5•16 years ago
|
||
Graham, could you test this patch? It should fix the 0 error code problem.
Assignee | ||
Comment 6•16 years ago
|
||
Bob, which solution do you prefer?
Assignee | ||
Comment 7•16 years ago
|
||
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'.
Reporter | ||
Comment 8•16 years ago
|
||
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
Reporter | ||
Comment 9•16 years ago
|
||
The solution 1 patch returns invalid arguments, as expected.
Assignee | ||
Comment 10•16 years ago
|
||
Sorry about that. Please try this new "solution 2" patch.
Assignee: nobody → wtc
Attachment #336986 -
Attachment is obsolete: true
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Updated•16 years ago
|
OS: Mac OS X → All
Hardware: Macintosh → All
Assignee | ||
Updated•16 years ago
|
Attachment #336985 -
Flags: review?(nelson)
Assignee | ||
Comment 11•16 years ago
|
||
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'.
Reporter | ||
Comment 12•16 years ago
|
||
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.
Assignee | ||
Comment 13•16 years ago
|
||
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.
Attachment #337210 -
Flags: review?(rrelyea)
Reporter | ||
Comment 14•16 years ago
|
||
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.
Assignee | ||
Comment 15•16 years ago
|
||
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.
Updated•16 years ago
|
Attachment #336985 -
Flags: review?(nelson) → review+
Comment 16•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
Comment on attachment 337210 [details] [diff] [review] Improve PK11_CiperOp error reporting (checked in) r=nelson.
Attachment #337210 -
Flags: review+
Assignee | ||
Comment 19•16 years ago
|
||
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).
Assignee | ||
Updated•16 years ago
|
Attachment #336985 -
Attachment description: Solution 1: set error code → Solution 1: set error code (checked in)
Assignee | ||
Comment 20•16 years ago
|
||
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
Assignee | ||
Comment 21•16 years ago
|
||
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
Attachment #337210 -
Attachment description: Improve PK11_CiperOp error reporting → Improve PK11_CiperOp error reporting (checked in)
Assignee | ||
Comment 22•16 years ago
|
||
Mapping error codes properly is labor-intensive. This is as much as I can do for three hours tonight.
Assignee | ||
Comment 23•16 years ago
|
||
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.
Attachment #339562 -
Flags: review?(nelson)
Assignee | ||
Comment 24•16 years ago
|
||
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.
Attachment #339562 -
Attachment is obsolete: true
Attachment #339581 -
Flags: review?(nelson)
Attachment #339562 -
Flags: review?(nelson)
Comment 25•16 years ago
|
||
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.
Attachment #339581 -
Flags: review?(nelson) → review+
Assignee | ||
Comment 26•16 years ago
|
||
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
Attachment #339581 -
Attachment is obsolete: true
Comment 27•16 years ago
|
||
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. :(
Assignee | ||
Comment 28•16 years ago
|
||
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.
Updated•16 years ago
|
Attachment #337210 -
Flags: review?(rrelyea) → review+
Updated•16 years ago
|
Summary: PK11_CreateContextBySymKey returns NULL, with zero error code → Improve softoken error reporting.
Updated•16 years ago
|
Whiteboard: FIPS
Assignee | ||
Comment 29•16 years ago
|
||
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.
Attachment #346581 -
Flags: review?(rrelyea)
Assignee | ||
Comment 30•16 years ago
|
||
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.
Attachment #339557 -
Attachment is obsolete: true
Attachment #346593 -
Flags: review?(rrelyea)
Assignee | ||
Comment 31•16 years ago
|
||
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.
Attachment #346715 -
Flags: review?(rrelyea)
Assignee | ||
Comment 32•16 years ago
|
||
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.
Attachment #346593 -
Attachment is obsolete: true
Attachment #346727 -
Flags: review?(rrelyea)
Attachment #346593 -
Flags: review?(rrelyea)
Comment 33•16 years ago
|
||
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
Attachment #346581 -
Flags: review?(rrelyea) → review+
Comment 34•16 years ago
|
||
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.
Assignee | ||
Comment 35•16 years ago
|
||
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•16 years ago
|
||
Thank you, Wan-Teh. Please write a better summary for this bug, one that more narrowly defines its scope. Thanks.
Assignee | ||
Comment 37•16 years ago
|
||
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.
Summary: Improve softoken error reporting. → Improve PK11_CipherOp error reporting (was: PK11_CreateContextBySymKey returns NULL, with zero error code).
Comment 38•16 years ago
|
||
Comment on attachment 346715 [details] [diff] [review] Improve error reporting in lib/freebl/ec.c r+ rrelyea
Attachment #346715 -
Flags: review?(rrelyea) → review+
Comment 39•16 years ago
|
||
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
Attachment #346727 -
Flags: review?(rrelyea) → review+
Assignee | ||
Comment 40•16 years ago
|
||
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
Assignee | ||
Comment 41•16 years ago
|
||
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
Assignee | ||
Comment 42•16 years ago
|
||
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
Assignee | ||
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.12.3
Assignee | ||
Comment 43•15 years ago
|
||
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.
Attachment #369682 -
Flags: review?(rrelyea)
Updated•15 years ago
|
Attachment #369682 -
Flags: review?(rrelyea) → review+
Comment 44•15 years ago
|
||
Comment on attachment 369682 [details] [diff] [review] More error code mapping r+ rrelyea
Assignee | ||
Comment 45•15 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•