Closed
Bug 1338897
Opened 7 years ago
Closed 7 years ago
Avoid using NSS Base64 functions in PSM
Categories
(Core :: Security: PSM, defect, P1)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: Cykesiopka, Assigned: Cykesiopka)
References
Details
(Keywords: addon-compat, Whiteboard: [psm-assigned])
Attachments
(1 file)
The NSS Base64 functions are less safe and convenient to use than the XPCOM ones. They're also an unnecessary dependency on NSS.
Comment hidden (mozreview-request) |
![]() |
||
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8847155 [details] Bug 1338897 - Avoid using NSS Base64 functions in PSM. https://reviewboard.mozilla.org/r/120172/#review123060 Looks great. Sorry this took a while to review. ::: security/apps/AppSignatureVerification.cpp:50 (Diff revision 1) > extern mozilla::LazyLogModule gPIPNSSLog; > > namespace { > > +inline nsDependentCSubstring > +DigestToString(const Digest& digest) It might be nice to call this "DigestToDependentString" to be a bit more clear that the result depends on the input living longer than it (also might be nice to document this). ::: security/nss.symbols:19 (Diff revision 1) > #endif > #include ../db/sqlite3/src/sqlite.symbols > ATOB_AsciiToData > ATOB_AsciiToData_Util > -ATOB_ConvertAsciiToItem > ATOB_ConvertAsciiToItem_Util I'd imagine we can get rid of this line as well now, right? ::: security/nss.symbols:510 (Diff revision 1) > SECITEM_AllocArray > SECITEM_AllocItem > SECITEM_AllocItem_Util > SECITEM_ArenaDupItem_Util > -SECITEM_CompareItem > SECITEM_CompareItem_Util Same here.
Attachment #8847155 -
Flags: review?(dkeeler) → review+
![]() |
Assignee | |
Comment 3•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8847155 [details] Bug 1338897 - Avoid using NSS Base64 functions in PSM. https://reviewboard.mozilla.org/r/120172/#review123060 No worries, thanks for the review! > I'd imagine we can get rid of this line as well now, right? Sadly, getting rid of the _Util symbols broke my try builds the last time I tried. Looks like we still need to keep this here as long as NSS itself uses the function.
Comment hidden (mozreview-request) |
![]() |
Assignee | |
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3addf5adadbd72dec2ee8a72fb556754684cc19
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/312bdabd35b8 Avoid using NSS Base64 functions in PSM. r=keeler
Keywords: checkin-needed
![]() |
||
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/312bdabd35b8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
![]() |
Assignee | |
Comment 8•7 years ago
|
||
The changes here may impact add-ons that fit the following criteria: 1. Use nsICryptoHash or nsICryptoHMAC directly or indirectly. 2. Init either of the above with SHA-512 as the choice of hash algorithm. 3. Call `finish(true)` to get back base 64 output. If these add-ons previously depended on the base 64 output having CRLF characters inserted every 64 characters, they need to stop doing that - the output will no longer have such characters.
Keywords: addon-compat
You need to log in
before you can comment on or make changes to this bug.
Description
•