106587 - FIPS enabled mode fails when Master PWD is not set.

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[Stephane Saux]

One of the requirement in FIPS mode is that the Master PWD be set. Thus in that
mode, when the db needs to be accessed, the user is prompted for the pwd.  If
the pwd is not set, the behavior should be to ask the user to set it.  Instead,
at least on some platform the client hangs.

Comment 1

[Kai Engert (:KaiE:)]

CC'ing relyea.

Bob, is this something that must be done within PSM or NSS ?

When there are many codepaths that access the DB, does it make more sense to add
the logic to trigger the "set password request" to NSS?

Should we create a new callback function, similar to PK11_PasswordPrompt, maybe
called PK11_PasswordSet ?

Comment 2

[Stephane Saux]

Also the set password functionality need to be able to tell the application
which token needs a password.  This way the PSM set password dialog can preset
the token drop-down list.  See bug 106939.

Comment 3

[Kai Engert (:KaiE:)]

Please help. I need answers to some questions.


My suggestion to fix this bug:

- If the application is already running, and the user requests to activate FIPS
mode, the application asks the user to set it first. If this fails, FIPS mode is
not activated.


Question 1: Is it ok for FIPS, if the password is the empty string? I ask,
because we currently allow the master password to be set, but empty.


Question 2: Will a newly created NSS database always default to non-FIPS mode?
Or is it possible, that a user start the browser for the first time, and FIPS is
already activated?

If that is possible, then we need to do something when PSM is loaded. It must
check whether FIPS is enabled and no password is set. I suggest, in this case we
require the user to set it during PSM init.

I wonder what we must do, when the user cancels the request to set the password.
Should the application abort? Or should it disable FIPS? Or should it loop
endlessly until the user has successfully set the password?

Thanks!

Comment 4

[Stephane Saux]

1) Relyea can correct me, but the password must not be empty in FIPS mode.
2) I think that's a good question. I could see someone want to customize the
browser distribution to make sure that FIPS mode is enabled by default, and then
there would be a need to ensure that the password be set.  It's probably a
thornier problem to solve in that one must detect at db init that fips mode is
set and prompt the user for a db pwd as soon as possible. If that is too hard to
do, we may not allow FIPS to be enabled by default.  Organizations that want
FIPS can enforce FIPS mode by only allowing connections from FIPS enabled clients.

If we decided to set the pwd during init, then it's acceptable in FIPS mode to
loop until the pwd is set.

Comment 5

[Robert Relyea]

Yes,

In FIPS mode you need to set the password.
Newly created profiles are currently and have always been non-FIPS by default, a
newly created profile will always be non-FIPS.

If we want to enable FIP's by default as an option, we would have to build a
design to do it. Yes Stephane we would have to detect the lack of DB at
start-up, but we could initialize the DB to a fixed password (other than '0') if
we are starting in FIPs mode. Do you have any requirements to have an enable
FIPs by default option?

Comment 6

[Stephane Saux]

At the moment there's no requirement. In fact I don't think that there is a FIPS
preference, so the only way for tools like mission control to create an install
package that would have FIPS enabled by default would be to use secmodutil if
that tools as the ability to toggle fips mode on the software security device.

If secmodutil has that capability, the browser won't support using it to create
such a special install.

Comment 7

[Kai Engert (:KaiE:)]

Looking at this again, I have another question.

Do we have to care for all tokens, including hardware tokens?

If a user has multiple tokens, and wants to enable FIPS mode, are we required to
iterate over all the tokens and request the user to change a non-empty password?

What will happen if the user has a hardware token, that is currently not plugged
in? He enables FIPS mode and afterwards inserts the token with an empty
password. Will this cause a problem?

Or can we assume that hardware tokens will always have a non-empty password set?

Comment 8

[Kai Engert (:KaiE:)]

Attachment: Suggested fix

Supposed we only have to care for the internal token, this patch implements it.


If we need to care for all tokens, the patch must be enhanced, and the check
extended to all available tokens.

Comment 9

[Robert Relyea]

FIPs is just the internal token. Other tokens independently decide if they are
FIPs complient or not on their own.

bob

Comment 10

[Kai Engert (:KaiE:)]

In that case the patch should do what is needed. Javi, can you please review?

Comment 11

[John Unruh]

nsbeta1

Comment 12

[Stephane Saux]

nsbeta1+

Comment 13

[Kai Engert (:KaiE:)]

Should we relnote this?

Comment 14

[Kai Engert (:KaiE:)]

Somehow we missed this, although it has a patch already.
I need to test whether my patch still applies, and if it does, will ask for
review again.

Comment 15

[Kai Engert (:KaiE:)]

The patch still applies.

Comment 16

[Javier Delgadillo]

Comment on attachment 69435 [details] [diff] [review]
Suggested fix

r=javi

Comment 17

[Kai Engert (:KaiE:)]

Sean, can you please review the new wording in this small patch?

Comment 18

[Sean Cotter]

> +pw_change2empty_in_fips_mode=You are currently in FIPS mode. It is not
allowed to have an empty password in FIPS mode.

Change to "You are currently in FIPS mode. FIPS requires a non-empty Master
Password.

> +fips_nonempty_password_required=FIPS mode requires that you have a password
set for your tokens. Please set the password before trying to enable FIPS mode.

Change to: "FIPS mode requires that you have a Master Password set for each
security device. Please set the password before trying to enable FIPS mode."


Per the discussion re the term "PKCS #11 Module" in bugzilla 96587, the
following changes to existing text should also be made:

> loadPK11TokenDialog=Choose a PKCS#11 device to load (title for open file dialog) 

Change to "Choose a PKCS #11 module to load"


This line (deviceManager.dtd#44) also needs to be changed:

> <!ENTITY loaddevice.title "Load PKCS#11 Device"> (title for dialog that
appears when you click Load)

Change to "Load PKCS #11 Module"

Comment 19

[Kai Engert (:KaiE:)]

Sean, thanks for your comments.

You had change requests for 4 strings. However, only 2 of the 4 are new strings
from me - only those lines that begin with a +.

For the other changes, you say they are being discussed in bug 96587. If you
think that change is happening in scope of bug 96587, I would prefer to not
include that change here.

Comment 20

[Kai Engert (:KaiE:)]

Attachment: Patch with updated wordings

Comment 21

[Kai Engert (:KaiE:)]

Comment on attachment 83056 [details] [diff] [review]
Patch with updated wordings

r=javi on code and r=cotter on new wordings

Comment 22

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 83056 [details] [diff] [review]
Patch with updated wordings

sr=shaver

Comment 23

[Kai Engert (:KaiE:)]

fixed on trunk

Comment 24

[John Unruh]

Verified on 8/26 trunk.