Closed
Bug 1329360
Opened 6 years ago
Closed 6 years ago
avoid some NSS functions that internally use PK11_GetInternalKeySlot
Categories
(Core :: Security: PSM, defect, P1)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: keeler, Assigned: keeler)
Details
(Whiteboard: [psm-assigned])
Attachments
(1 file)
CERT_AddTempCertToPerm and CERT_ImportCerts (when called with keepCerts=true) internally use PK11_GetInternalKeySlot. The current plan for making NSS always available involves initializing it in memory-only mode and later opening the user's certificate and key databases. Doing so means that PK11_GetInternalKeySlot will not return the right token, so we can't rely on functions that make use of it internally. For now we'll simply use equivalent functions that take an explicit PK11SlotInfo argument and pass in the current internal token. A later patch will change all places where PSM and Gecko use the internal token to use the correct token. (See https://wiki.mozilla.org/Security/CryptoEngineering/Platform_Use_of_NSS )
Comment hidden (mozreview-request) |
![]() |
||
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8826660 [details] bug 1329360 - avoid some NSS functions that internally use PK11_GetInternalKeySlot https://reviewboard.mozilla.org/r/104566/#review105426 Cool - glad to see us moving towards something simpler. ::: security/manager/ssl/nsNSSCertificateDB.cpp:537 (Diff revision 1) > - int i = 0; > - for (CERTCertListNode *chainNode = CERT_LIST_HEAD(certChain); > - !CERT_LIST_END(chainNode, certChain); > + if (encounteredFailure) { > + PR_SetError(savedErrorCode, 0); > + return SECFailure; > - chainNode = CERT_LIST_NEXT(chainNode), i++) { > - rawArray[i] = &chainNode->cert->derCert; > } > - SECStatus srv = CERT_ImportCerts(CERT_GetDefaultCertDB(), usage, chainLen, > - rawArray, nullptr, true, caOnly, nullptr); > > - PORT_Free(rawArray); > + return SECSuccess; Nit: I think we can just return `savedErrorCode` instead, and maybe update the callers to propagate the return value via `GetXPCOMFromNSSError()`.
Attachment #8826660 -
Flags: review?(cykesiopka.bmo) → review+
![]() |
Assignee | |
Comment 3•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8826660 [details] bug 1329360 - avoid some NSS functions that internally use PK11_GetInternalKeySlot https://reviewboard.mozilla.org/r/104566/#review105426 Thanks! > Nit: I think we can just return `savedErrorCode` instead, and maybe update the callers to propagate the return value via `GetXPCOMFromNSSError()`. I don't know if this is what you meant, but I ended up having ImportCertsIntoPermanentStorage return an nsresult (obtained via GetXPCOMFromNSSError) since its callers all return nsresult anyway.
Comment hidden (mozreview-request) |
Pushed by dkeeler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e6d89beea427 avoid some NSS functions that internally use PK11_GetInternalKeySlot r=Cykesiopka
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e6d89beea427
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•