Closed Bug 1608480 Opened 4 years ago Closed 4 years ago

Upgrade Firefox 72 to use NSS 3.48.1

Categories

(Core :: Security: PSM, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
firefox72 + fixed

People

(Reporter: jcj, Assigned: jcj)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

Tracking NSS 3.48.1 for Firefox 72. Ultimate tag will be NSS_3_48_1_RTM.

[Tracking Requested - why for this release]:

It appears that Bug 1606992 might warrant a dot release to 72. This is just in case.

See Also: → 1608481

2020-01-13 J.C. Jones <jjones@mozilla.com>

* lib/softoken/lowpbe.c:
Bug 1606992 - Fixup for undefined PZ_Lock in lowpbe.c on
NSS_3_48_BRANCH r=kaie
[657afa63b6d8] [NSS_3_48_1_RTM] <NSS_3_48_BRANCH>

* .hgtags:
Added tag NSS_3_48_1_RTM for changeset e64f68a9b621
[e05ac7445946] <NSS_3_48_BRANCH>

* lib/nss/nss.h, lib/softoken/softkver.h, lib/util/nssutil.h:
Set version to 3.48.1 final
[e64f68a9b621] <NSS_3_48_BRANCH>

2020-01-11 Kai Engert <kaie@kuix.de>

* lib/softoken/lowpbe.c, lib/softoken/pkcs11.c:
Bug 1606992 - Cache the most recent PBKDF2 password hash, to speed
up repeated SDR operations. r=jcj
[e163d2097c74] <NSS_3_48_BRANCH>

2019-12-03 J.C. Jones <jjones@mozilla.com>

* .hgtags:
Added tag NSS_3_48_RTM for changeset 65d3150a258e
[2afcdf5a1352] <NSS_3_48_BRANCH>
Attachment #9120608 - Attachment description: Bug 1608480 - land NSS NSS_3_48_1_RTM UPGRADE_NSS_RELEASE, r=kjacobs → Bug 1608480 - Upgrade Firefox 72 to NSS_3_48_1_RTM UPGRADE_NSS_RELEASE, r=kjacobs

Comment on attachment 9120608 [details]
Bug 1608480 - Upgrade Firefox 72 to NSS_3_48_1_RTM UPGRADE_NSS_RELEASE, r=kjacobs

Beta/Release Uplift Approval Request

  • User impact if declined: Slowness in Lockwise (Bug 1606992)
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: https://bugzilla.mozilla.org/show_bug.cgi?id=1606992#c14 and https://bugzilla.mozilla.org/show_bug.cgi?id=1606992#c15 give the details.
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): It touches crypto code, so it can't be "low", but the additions are in-memory caches and easy to evaluate. I wouldn't recommend landing until we get a little bake-time in Nightly and Beta though.
  • String changes made/needed: None
Attachment #9120608 - Flags: approval-mozilla-release?
Flags: qe-verify+

Comment on attachment 9120608 [details]
Bug 1608480 - Upgrade Firefox 72 to NSS_3_48_1_RTM UPGRADE_NSS_RELEASE, r=kjacobs

nss update to fix regression on about:logins with master password, approved for 72.0.2, thanks

Attachment #9120608 - Flags: approval-mozilla-release? → approval-mozilla-release+
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
QA Whiteboard: [qa-triaged]

How exactly should this bug be verified?

The information found in the uplift request from comment 2 is related to a method of automatically generating/importing 1000 test logins with a code posted in Browser Console. Furthermore, this method works correctly in Nightly v74.0a1 from 2020-01-19, Beta v73.0b7. It also works in the (unfixed) Firefox Release v72.0.1 and on the (fixed) Release Candidate v72.0.2.

This being said, the information in comment 2 is not sufficient. How should it be verified correctly?

Flags: needinfo?(jjones)

(In reply to Bodea Daniel [:danibodea] from comment #5)

This being said, the information in comment 2 is not sufficient. How should it be verified correctly?

You should check for a delay when opening preferences, privacy & security, logins and passwords, then click "saved logins".

With the bug still present, you'd get a long, noticeable delay, before the list of saved logins appears.

With the bug fixed, the list should be shown very quickly.

Does this help?

Flags: needinfo?(jjones) → needinfo?(daniel.bodea)

I am sorry, but there is no noticeable difference between the affected and the fixed versions. There must be something more you can tell me to make sure that the fix is valid. The time until the mock data appears is the same (about 5 seconds) for every channel and both affected and fixed versions of the release build.

These are the steps I took:

  1. Open browser.
  2. Press F12 on keyboard.
  3. Go to DevTools Settings (click on the 3 horizontal dots button, and then on Settings).
  4. Check e optins at the end of the list:
    Enable browser chrome and add-on debugging
    Enable remote debugging
    Enable new performance recorder
  5. Press CRTL + SHIFT + J to open Browser Console.
  6. Input in the Browser Console input box:
    const { LoginHelper } = ChromeUtils.import(
      "resource://gre/modules/LoginHelper.jsm"
    );
  1. Input in the Browser Console input box:
    var logins = [];
    for (let i = 0; i < 1000; i++) {
      logins.push({origin: `https://${i}.example.com`, username: `user${i}`, password: `password${i}`});
    }
    await LoginHelper.maybeImportLogins(logins);
  1. Go to Browser options / Privacy & Security / Logins and Passwords / "Saved Logins..." button.

Affected: The list of mock data takes some time to be shown.
Fixed: The list of mock data is instantly shown.

What am I missing?

Edited by KaiE: added escape characters around the code blocks, to ensure the quotes are visible and can be copied.

Flags: needinfo?(kaie)
Flags: needinfo?(jjones)
Flags: needinfo?(daniel.bodea)

What am I missing?

You compared 71.0.1 and 71.0.2 ?

You say you get the same performance with both versions?
Are both fast (logins window opens instantly) or both slow (several seconds delay)?

Flags: needinfo?(kaie) → needinfo?(daniel.bodea)

sorry, of course I meant to say:

You compared 72.0.1 and 72.0.2 ?

Ah, we forgot to tell you "set a master password". Can be any length, even just one letter is fine for this comparison.

Yes, I did compare Firefox Release v72.0.1 (affected) and Firefox Release v72.0.2 (fixed). I still do not see a noticeable difference in time while the Firefox Lockwise (about:logins) page is loaded. Please check and correct the steps to reproduce I wrote in comment 7 and add the step with setting a master password where it belongs. Please explain the exact expected results for both the affected and fixed versions.

Thank you for your contribution!

Flags: needinfo?(daniel.bodea) → needinfo?(kaie)

While testing myself, I noticed that the distribution/policies.json file with

{
 "policies": {
   "DisableAppUpdate": true
 }
}

doesn't work to prevent an automatic update... So I'm adding the no update setting to prefs js.

I have the impression that 72.0.2 doesn't contain NSS 3.48.1

I've downloaded 72.0.2 for Linux 64 bit, and I confirm I still see an 8 second delay opening the password list with the test scenario.

Looking at the binary, it seems 72.0.2 still contains NSS 3.48. not 3.48.1

Please check the following with 72.0.2:
Enable menubar, open help troubleshooting, scroll to the bottom of the page, then a little up. Find section "Library versions".

For me, NSS is shown as 3.48 (not the expected 3.48.1)

Flags: needinfo?(kaie)

(In reply to Kai Engert (:KaiE:) from comment #13)

For me, NSS is shown as 3.48 (not the expected 3.48.1)

We just learned this display cannot be relied on, for some (related or unrelated) reason, the display string wasn't updated in the NSS 3.48.1 release, and still says 3.48

So we have to operate on behavior alone. I've tested on MacOS (to join Kai's Linux) and can confirm the issue is not fixed, though about:buildconfig shows the patch landed in-tree, and was otherwise identical to the one for Beta.

While there's a problem with the version string, inspection of the build's binaries indicates we changed the right things in NSS here, so this appears to be a case where somehow this patch isn't solving the problem on 72.

Un-marking fixed, though what the next steps are here, I do not yet know.

Flags: needinfo?(jjones)

Following up with comment 15, let's move the bug to reopen since the Resolved fixed is misleading since the original problem is not fixed.

Status: RESOLVED → REOPENED
QA Whiteboard: [qa-triaged]
Resolution: FIXED → ---

To clarify, the issue is not fixed. The patch from bug 1606992 (which was backported here) doesn't help, if a master password was created with version 72. The fix backported to 72.0.2 works only, if the MP was set with version 73.

The additional issue is described in bug 1606992 comment 70 and following.

However, bug 1606992 has meanwhile been set as wontfix for FF 72. So while we indeed upgraded FF 72.0.2 to NSS 3.48.1 (despite the misleading version display in about troubleshooting), the underlying issue isn't fixed.

Yeah, this bug as-filed was fixed (we did in fact update NSS for Firefox 72). The fact that doing so didn't fix the bug it was supposed to fix is a different problem and the status in bug 1606992 correctly reflects that.

Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED

Considering the fact that:

  • the steps to reproduce provided in comment 6 don't validate the fix
  • the fact that the NSS version displayed in the about:support page of the Firefox Release v72.0.2 build is 3.48, not 3.48.1
  • the fact that this bug has the "qe-verify+" tag
    => It means that in order to verify this fix correctly, we need other steps to verify.

@J.C. Jones: Any idea how I could verify it? or when?

Has STR: --- → no
Flags: needinfo?(jjones)
Keywords: steps-wanted

The fix was indeed not validated. There isn't really anything more to do for 72.

Flags: qe-verify+
Flags: needinfo?(jjones)
Keywords: steps-wanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: