Closed Bug 488067 Opened 15 years ago Closed 15 years ago

PK11_ImportCRL reports SEC_ERROR_CRL_NOT_FOUND when it fails to import a CRL

Categories

(NSS :: Libraries, defect, P2)

3.12
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.12.4

People

(Reporter: nelson, Assigned: julien.pierre)

Details

Attachments

(1 file, 1 obsolete file)

I tried to import a CRL into cert8 with the command

 crlutil -I -u "https://bugzilla.mozilla.org/attachment.cgi?id=372271" \
 -i crl.der -d DBDIR -B 

It failed to import because the CRL's issuer name was too big (> 64KB)
which is correct (that is, it is correct for cert8 to reject the import 
in that case).  But the error code that was left set by PK11_ImportCRL 
was SEC_ERROR_CRL_NOT_FOUND.  

That error code was left over from PK11_ImportCRL's attempt to see if there
was already a CRL for that issuer.  There wasn't, which set that error.
but then later, when the import attempt failed, no newer error code was set.

We need to do better.  Otherwise, we'll have all sorts of support headaches.
Nelson,

Here is the stack of what happens in this case :

current thread: t@1
=>[1] nss_SetError(error = 38U), line 270 in "error.c"
  [2] import_object(tok = 0x80b5fd8, sessionOpt = (nil), objectTemplate = 0x80464b0, otsize = 6U), line 256 in "devtoken.c"
  [3] nssToken_ImportCRL(token = 0x80b5fd8, sessionOpt = (nil), subject = 0x8046548, encoding = 0x8046550, isKRL = 0, url = (nil), asTokenObject = 1), line 1213 in "devtoken.c"
  [4] PK11_PutCrl(slot = 0x80b30a0, crl = 0x804664c, name = 0x80fb704, url = (nil), type = 1), line 523 in "pk11nobj.c"
  [5] crl_storeCRL(slot = 0x80b30a0, url = (nil), newCrl = 0x80fb6d8, derCrl = 0x804664c, type = 1), line 773 in "crl.c"
  [6] PK11_ImportCRL(slot = 0x80b30a0, derCRL = 0x804664c, url = (nil), type = 1, wincx = (nil), importOptions = 1, arena = (nil), decodeoptions = 1), line 820 in "pk11nobj.c"
  [7] ImportCRL(certHandle = 0x80b3790, url = (nil), type = 1, inFile = 0x8087710, importOptions = 1, decodeOptions = 1), line 280 in "crlutil.c"
  [8] main(argc = 7, argv = 0x804675c), line 1053 in "crlutil.c"
(dbx)

The problem is that the specific error is being lost between nssToken_ImportCRL in the Stan layer and PK11_PutCrl in pk11wrap .

The former only returns an object with a PKCS#11 object ID and never calls PORT_SetError. The specific ckrv from C_CreateObject is added to the Stan "error stack" via nss_SetError . I'm not really sure how PK11_PutCrl could figure out where that specific relevant error code is in the Stan error stack. I propose that PK11_PutCrl just map this to a generic token import error code. I don't see one defined for a CRL import error. We could define something like

SEC_ERROR_CRL_PKCS11_IMPORT_ERROR

and set it in PK11_PutCrl in the "else" statement at 
http://mxr.mozilla.org/security/source/security/nss/lib/pk11wrap/pk11nobj.c#529 .

I will attach a patch that does that.
OS: Windows XP → All
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → 3.12.4
OK, Please make that error code be 

SEC_ERROR_CRL_IMPORT_FAILED
Attached patch Set error code (obsolete) — Splinter Review
Attachment #372508 - Flags: review?(nelson)
Attached patch Set error codeSplinter Review
One file was missing from the previous diff.
Attachment #372508 - Attachment is obsolete: true
Attachment #372509 - Flags: review?(nelson)
Attachment #372508 - Flags: review?(nelson)
Comment on attachment 372509 [details] [diff] [review]
Set error code

r=nelson, AFTER you resolve the merge conflicts.  :)

Earlier today, I gave r+ to Alexei for a patch that adds another new error code to the same files that you've modified in this patch. 
So, please let him check in that patch first, then resolve the merge conflicts (and bump the error code number), then commit.  

Thanks.
Attachment #372509 - Flags: review?(nelson) → review+
I thought the tree was frozen as of last night ?
No, plans are fluid.  Current NSS tree state is shown at 
http://bonsai.mozilla.org/toplevel.cgi?treeid=NSS
(Contact me elsewhere for details.)
Checking in lib/util/secerr.h;
/cvsroot/mozilla/security/nss/lib/util/secerr.h,v  <--  secerr.h
new revision: 1.25; previous revision: 1.24
done
Checking in lib/pk11wrap/pk11nobj.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11nobj.c,v  <--  pk11nobj.c
new revision: 1.11; previous revision: 1.10
done
Checking in cmd/lib/SECerrs.h;
/cvsroot/mozilla/security/nss/cmd/lib/SECerrs.h,v  <--  SECerrs.h
new revision: 1.19; previous revision: 1.18
done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: