Closed Bug 428421 Opened 17 years ago Closed 8 years ago

FIPS mode: Master password dialog truncates security device name

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: joejr, Assigned: u60234)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [psm-assigned])

Attachments

(2 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/20080311 Firefox/2.0.0.13 Build Identifier: 2008041005 When FIPS mode is enabled in Firefox or Thunderbird and the user is prompted for the master password, the security device name is truncated: "Please enter the master password for the FIPS 140 Cryptographic, Key and." I think I saw the same truncation in the Security Devices dialog (where the Enable FIPS button lives). I might have a fix. WARNING: I know nothing about NSS; this is just what I found by poking around LXR. mozilla/security/manager/locales/en-US/chrome/pipnss/pipnss.properties line 71 seems to indicate that Fips140TokenDescription gets 32 bytes and Fips140SlotDescription gets 64. The slot description is what's used in the master password dialog box, but it's getting truncated at 32 characters (or possibly 31). The method nsNSSComponent::ConfigureInternalPKCS11Token() (mozilla/security/manager/ssl/src/nsNSSComponent.cpp line 857) loads those two strings and passes them to PK11_ConfigurePKCS11() as arguments 7 and 8 (fslotdes and fpslotdes), respectively. PK11_ConfigurePKCS11() then transposes them (mozilla/security/nss/lib/nss/nssinit.c line 202), setting "FIPSSlotDescription='<fslotdes>' FIPSTokenDescription='<fpslotdes>'" in pk11_config_strings. I think that's where the problem occurs. So it would seem to the untrained eye (i.e. my eye) that fixing the problem is as easy as swapping lines 203 and 211 in mozilla/security/nss/lib/nss/nssinit.c. However, I'm not sure I've understood the code correctly, and there's always the chance that the fix would have unintended consequences. Reproducible: Always Steps to Reproduce: Using Firefox: 1. Enable FIPS mode (Tools > Options > Advanced > Encryption > Security Devices > Enable FIPS), ensure a master password is set (Tools > Options > Security > Use a master password), and restart Firefox. 2. Do something to require Firefox to ask for the master password. Actual Results: Prompt reads, "Please enter the master password for the FIPS 140 Cryptographic, Key and." Expected Results: Prompt should read, "Please enter the master password for the FIPS 140 Cryptographic, Key and Certificate Services." Happens in Firefox 3b5 (build ID 2008031114) and also in the latest nightly I downloaded (2008041005) with a fresh profile. Similar behavior occurs in Thunderbird 2.0.0.12 (build ID 20080213).
PKCS11 defines numerous string types. In each case, the strings are fixed length arrays of bytes, not NUL terminated, blank padded. That is, all the bytes in the array contain characters. NSS's APIs allow callers to pass NUL-terminated strings. NSS then converts them to the PKCS#11 standard form by adding blank padding, or by truncating, as necessary. Among the strings of these forms are: module: libraryDescription[32] Slot: slotDescription[64] This is (obviously) the slot description manufacturerID[32] Token: label[32] This is the token name or token description manufacturerID[32] model[16] serialNumber[16] NSS provides function PK11_ConfigurePKCS11 by which applications can set these names for the pure-software virtual slots and virtual tokens implemented in NSS. Sadly there is no documentation for that function, and the names of the function parameters ("fslotdes" and "fpslotdes") offer no meaningful clues about which which PKCS#11 strings are intended to correspond to them. The code at lines 203 and 211 (cited above) make it pretty clear that fslotdes is intended to be the slot description and fpslotdes is intended to be the token description. Elsewhere, these string variables are named fslotdes and ftokdes, which is less ambiguous. The cited code hasn't changed since it was written in 2001. But we see that the caller of this function in PSM was changed in February 2007, and that change switched the order of those two arguments. See <http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/manager/ssl/src/nsNSSComponent.cpp&rev=1.164#884> and <http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/security/manager/ssl/src&command=DIFF_FRAMESET&file=nsNSSComponent.cpp&rev2=1.146&rev1=1.145> So, I conclude that the truncation problem being reported here is not the fault of NSS, but of its caller. Consequently, I am changing this to be a PSM bug.
Assignee: nobody → kengert
Status: UNCONFIRMED → NEW
Component: Libraries → Security: PSM
Ever confirmed: true
Product: NSS → Core
QA Contact: libraries → psm
Version: unspecified → 1.8 Branch
The PSM change in February 2007, to which I referred in comment 1, was made on the trunk, but this bug is about the 1.8 branch. So, maybe the fix is to backport that trunk change to the branch?
Joe, Let me suggest that you try the latest Firefox 3 Beta build (or release candidate build) and see if the problem persists there. If not, then this gives us a very good idea about the fix.
Thanks for your investigation and explanation! It still happens in Firefox 3b5 and in the latest nightly I tried (2008041005). I agree with your assessment and that it sounds like a PSM issue (though my opinion shouldn't count for much). This may be a duplicate of bug 317630, which is closed, but applies to the trunk, not 1.8. I'll let someone else judge whether this should be marked as a duplicate bug. (I think "fslotdes" is "FIPS slot description", and "fpslotdes" is "FIPS private slot description," but I don't know what either of those is.)
Cp. bug 426051 for a FIPS label/description related regression on trunk.
Thanks, Lars. Bug 426051 says: > This regressed between the builds from > 2008-03-16-05 and 2008-03-17-05. > <http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2008-03-16+04&maxdate=2008-03-17+06> Among the checkins shown on that page are: Bug 420151, FF3Beta5 should use updated NSS tag NSS_3_12_BETA3 Also updating NSPR tag to NSPR_4_7_1_BETA2 r=rrelyea, r=wtc, blocking1.9=dsicore Bug 406755, EV certs not recognized as EV with some cross-certification scenarios r=rrelyea, blocking1.9=dsicore
Severity: trivial → normal
Keywords: regression
I believe this is a trunk-only regression. The similar behaviour the reporter saw in Thunderbird 2.0.0.12 is most likely bug 317630, which was never fixed on 1.8 branch. So it show the truncated label "PSM Internal FIPS-140-1 Cryptogr". This new bug on trunk is that the description string "FIPS 140 Cryptographic, Key and Certificate Services" is used as a truncated label, and the label "Software Security Device (FIPS)" is used as description. I see this also on Linux, so changing Hardware/OS to All/All.
Severity: normal → trivial
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: PC → All
Version: 1.8 Branch → Trunk
Sorry, I didn't mean to change the severity or status. Setting back to "Normal" and "New".
Severity: trivial → normal
Status: ASSIGNED → NEW
Bob Relyea, Can you think of any change made to NSS between the tags of NSS_3_12_BETA2 and NSS_3_12_BETA3 that could have affected this behavior?
Not that I can think of. If there was something that let a longer label through, that would have been a bug. Regression or not, PSM needs to have Token Descriptions of 31 or less bytes. bob
So, we all know that description and label of the FIPS-security device got, well, "interchanged" - or at least the label is cut because of a 32-char-limit.. IMHO this is _very_ trivial to fix, so why doesn't a developer does it? :(
Attached patch patch (obsolete) (deleted) — Splinter Review
From my limited understanding of this code, I would guess that this was caused by the main patch in bug 391296. The "fpslotdes" variable was renamed to "ftokdes" and then switched place with "fslotdes" at http://bonsai.mozilla.org/cvsview2.cgi?command=DIFF&subdir=mozilla%2Fsecurity%2Fnss%2Flib%2Fsoftoken&file=sftkpars.c&rev1=1.3&rev2=1.4&whitespace_mode=show&diff_mode=context#7 That looks like a reasonable change, so I guess Nelson's idea of changing the caller in nsNSSComponent.cpp is a good way to fix this bug.
Attachment #331986 - Flags: review?(kaie)
Thanks a lot for providing a patch! :) Since I'm not that deep into C(++) and CVS/SVN in general, I as a Nightly user would like to know when this appears within the builds.. ;)
(In reply to comment #14) > Thanks a lot for providing a patch! :) > > Since I'm not that deep into C(++) and CVS/SVN in general, I as a Nightly user > would like to know when this appears within the builds.. ;) > It will appear in the nightlies once the patch has been reviewed and landed (in other words, when this bug have been resolved as fixed).
Attachment #331986 - Flags: review?(kaie) → review-
Comment on attachment 331986 [details] [diff] [review] patch >+ nsAutoString fips140SlotDescription; > nsAutoString fips140TokenDescription; >- nsAutoString fips140SlotDescription; This code has no effect, you are simply changing the order. >- rv = GetPIPNSSBundleString("Fips140TokenDescription", fips140TokenDescription); >+ rv = GetPIPNSSBundleString("Fips140SlotDescription", fips140SlotDescription); > if (NS_FAILED(rv)) return rv; > >- rv = GetPIPNSSBundleString("Fips140SlotDescription", fips140SlotDescription); >+ rv = GetPIPNSSBundleString("Fips140TokenDescription", fips140TokenDescription); > if (NS_FAILED(rv)) return rv; This code has no effect, you are simply changing the order. when calling function PK11_ConfigurePKCS11 > NS_ConvertUTF16toUTF8(privateSlotDescription).get(), >+ NS_ConvertUTF16toUTF8(fips140SlotDescription).get(), > NS_ConvertUTF16toUTF8(fips140TokenDescription).get(), >- NS_ConvertUTF16toUTF8(fips140SlotDescription).get(), > 0, 0); Clearly Bob Relyea should not have made a change that requires a change of parameters to this stable interface. If this change helps to fix the regression, then I believe Bob's change in bug 391296 had an undesired side effect. I have sent email to Bob asking him to double check his change.
It's been a year since this bug has been touched, but it's still present in Firefox 3.5.2 and Thunderbird 3.0b3. What needs to be done to get this fixed? Thanks.
Attached image bizarre password dialog text (deleted) —
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7 GTB6 I'm attaching a screenshot of the dialog containing the strange wording.
Assignee: kaie → nobody
We tried to fix this in the past (bug 317630), but it seems it wasn't sufficient.
Depends on: 317630
Any progress on this bug? Seemed to not require a lot of code and should definitely get fixed before 4.0 is released, because such a small UI-glitch isn't a good example for an otherwise very good product.
Is anyone looking at this? Yes, it's a minor irritation, but it's been 5 years now!
Attached patch 428421-fips-token-name.diff (deleted) — Splinter Review
Hasse's patch is correct. The order in which PSM supplied the parameters in question to PK11_ConfigurePKCS11 was switched in bug 317630. I've rebased it (the location of nsNSSComponent.cpp changed) and I'm going to check it in.
Attachment #331986 - Attachment is obsolete: true
Attachment #8751968 - Flags: review+
Assignee: nobody → hasse
Whiteboard: [psm-assigned]
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: