1582169 - firefox nss plugin tries to read fips_enabled flag on the machine

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[vineetha.hari.pai]

Attachment: firefox_nss_disable_fips_enabled_flag.patch

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.132 Safari/537.36

Steps to reproduce:

On a FIPS enabled system, i.e. a system running a FIPS enabled kernel, /proc/sys/crypto/fips_enabled is set to 1. The libraries that are FIPS certified reads this flag to decide if they have to operate in FIPS mode. Firefox's nss bundled code by default reads this flag. Firefox is not one of FIPS certified libraries and should not be reading this flag.

A bug has been filed against Ubuntu firefox package here - https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/1843044

Actual results:

On a FIPS enabled system. firefox crashes while starting up. An strace showed that it was repeatedly reading the flag before the crash.

Expected results:

Firefox and its associated nss bundled code are not FIPS certified and hence should not be reading the /proc/sys/crypto/fips_enabled flag. I propose to disable reading that flag.

Comment 1

[vineetha.hari.pai]

After applying the patch, no crash was observed on a FIPS enabled system.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 3

[J.C. Jones [:jcj] (he/him)]

Bob, as this is related to NSS and Firefox's FIPS mode, can you take this one?

Reporter: I will note that the patch as-is would need to be reworked to determine whether NSS was built in FIPS mode, rather than commenting out the reads.

Comment 4

[Robert Relyea]

Do not apply this patch as written. Firefox may not be FIPS validated, but NSS itself is. If you want a distribution free of NSS reading the flag, please create a new #define and build environment variable. Reading the FIPS flag on Linux should be default behavior (at least if the NSS FIPS value has been enabled).

This code was specifically added to NSS would automatically go into FIPS mode on systems that are FIPS enabled.

Comment 5

[J.C. Jones [:jcj] (he/him)]

Comment on attachment 9093608 [details] [diff] [review]
firefox_nss_disable_fips_enabled_flag.patch

As both above comments said, this would need to be rewritten to make use of our FIPS compile-time options, not unconditionally compile-out FIPS mode, as NSS is absolutely used in FIPS compliant ways regularly.

Comment 6

[Bryan Quigley]

Alternatively to patching this, what is the modern way to enable FIPS in Firefox? I found these instructions: https://support.mozilla.org/en-US/kb/Configuring%20Firefox%20for%20FIPS%20140-2 but no matter what I do I can't get FIPS enabled - nor will "Enable FIPS" not be grayed out in Security Devices.

Comment 7

[J.C. Jones [:jcj] (he/him)]

If NSS was built with the FIPS options enabled (./build.sh --enable-fips), and is then used with a database set to FIPS mode (modutil -fips true -dbdir dir), then Firefox should automatically also go into FIPS mode.

Comment 8

[Victor Tapia]

Attachment: nss-stop-fips-query-when-disabled.patch

I'm attaching a patch that uses NSS_FIPS_DISABLED so /proc/sys/crypto/fips_enabled won't be checked when NSS is not built in FIPS mode (without --enable-fips).

Comment 9

[Victor Tapia]

Attachment: nss-stop-fips-query-when-disabled.patch

Comment 10

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Victor, are you still interested in working on this bug? Note that we use phabricator to do code review: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html
Also note that you'll be making changes to nss (https://hg.mozilla.org/projects/nss/), not mozilla-central directly.

(it looks like fixing this bug will address at least some of the failures from bug 1544511)

Comment 11

[Victor Tapia]

Sure, I'm not familiar with the process but will give it a try. Sorry for the late response btw, I've been afk :)

Comment 12

[Victor Tapia]

Attachment: Bug 1582169 - Disable reading /proc/sys/crypto/fips_enabled if FIPS is not enabled on build

Comment 13

[J.C. Jones [:jcj] (he/him)]

Bob, can you take a look at this review when possible? It's pretty simple conditional compilation for FIPS.

Comment 14

[Robert Relyea]

The new patch looks fine, I've r+'ed it. since it's close to the end of the day, I'll push the change later.

bob

Comment 15

[J.C. Jones [:jcj] (he/him)]

https://hg.mozilla.org/projects/nss/rev/55ba54adfcaea2f984a999a511eec5047462eb57

Comment 16

[Olivier Tilloy]

Any chance this fix can be cherry-picked to the firefox 74 branch?

Comment 17

[J.C. Jones [:jcj] (he/him)]

(In reply to Olivier Tilloy from comment #16)

Any chance this fix can be cherry-picked to the firefox 74 branch?

It certainly can; I don't have any other current ride-along plans for a NSS 3.50 point release, but I'd be happy to add this to the to-do list if we make one. Since on Linux NSS is installed as a system library, we have to release it separately but in lock-step.

If you feel this is sufficient to warrant a point release on its own, could you give me a brief synopsis of why? Thanks!

Comment 18

[Olivier Tilloy]

https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/1843044 is a downstream (Ubuntu) bug report describing how firefox crashes with a FIPS-enabled kernel (and this is what prompted Victor to contribute this patch).

Given the nature of the problem (a crash), it would be good to have the patch in firefox as early as possible (but we can certainly cherry-pick it and apply it as a distro-patch if it's not making it to firefox 74).