1620110 - Client certs: "Remember this decision" doesn't actually remember the decision

Status: VERIFIED FIXED

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

Description

[Daniel Lo Nigro (daniel15)]

Attachment: dialog.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:73.0) Gecko/20100101 Firefox/73.0

Steps to reproduce:

At Facebook we have many internal services that use client certificates for validation. I attempted to access one of these pages using Firefox. The cert is deployed to the OS cert storage, so I enabled the "security.osclientcerts.autoload" option.

Actual results:

Even though I have "remember this decision" selected in the attached dialog, the dialog keeps popping up, even within the same session!

Expected results:

"Remember this decision" should actually remember the decision. This works fine in Chrome.

Comment 1

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

It turns out CERT_INFO.SerialNumber is little-endian whereas in PKCS#11 it's essentially expected to be big-endian. More accurately, the serial number is supposed to be DER-encoded (and hence big-endian), which is another thing osclientcerts gets wrong (it just so happens that NSS has a workaround for this when looking for a certificate by issuer and serial-number).

Comment 2

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

Attachment: bug 1620110 - osclientcerts: properly store serial numbers as DER-encoded integers r?kjacobs

PKCS#11 requires that serial numbers be DER-encoded (essentially, the bytes of
the serialNumber component of TBSCertificate). On macOS,
SecCertificateCopySerialNumberData gives the contents of this component (so it
lacks the tag and length fields, and may or may not have leading 00 or FF bytes
to indicate sign). On Windows, CERT_INFO.SerialNumber is the value of the
integer with the least significant byte first, which is the opposite of DER
(which has the most significant byte first). It also lacks any leading 00 or FF
sign bytes. Since the OS APIs can't be used here, this patch introduces a
utility function to grab the value of the serialNumber component of a
DER-encoded certificate.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4dfd0fad1445
osclientcerts: properly store serial numbers as DER-encoded integers r=kjacobs

Comment 4

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/4dfd0fad1445

Comment 5

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

Daniel, can you try out the latest Nightly to see if this is fixed for you?

Comment 6

[Daniel Lo Nigro (daniel15)]

How long is it supposed to remember the decision for? Is it just per session, or should it persist between sessions too? From my limited testing so far it seems okay within the same session, but once I close and reopen the browser window, it shows the dialog again.

Comment 7

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

Right now it only remembers decisions for a given session, so that sounds like the expected behavior (see also bug 634697). Thanks!

Comment 8

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

Comment on attachment 9132425 [details]
bug 1620110 - osclientcerts: properly store serial numbers as DER-encoded integers r?kjacobs

Beta/Release Uplift Approval Request

• User impact if declined: osclientcerts is shipping in 75. Without this, the user experience is even more annoying than normal ("remember this decision" won't work at all with client certificates provided by the new module)
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Follow the steps to test osclientcerts on Windows: https://docs.google.com/document/d/1QdygRHFS973gegmzcnLLl2wE3uhqdEhjfHrfM3dXj9A
  When connecting to the test server, without this patch, Firefox will ask to select a client certificate twice. With the patch, it should only ask once (on macOS it won't ever ask twice, even without this patch).
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is for a feature that is disabled by default. The changes are self-contained and the fix has been verified by the reporter. It should be low risk.
• String changes made/needed: none

Comment 9

[Daniel Bodea [:danibodea]]

@ assignee: I am attempting to verify this fix, but I need help reproducing it or understanding the steps. Can you, please, help?
I tried using the steps in the google doc from comment 8 on Windows 10 and these are my steps:

1. I have created text files with the "certificate" information from the files from point 1 and gave them their names and format as requested. Correct?
2. What's the difference between the ones "For RSA" and the ones "For EC"? Which one do I use and in which case?
3. Do I run commands from point 3 in the System terminal? What do I need to know to run them correctly? (I get the error "The system cannot find the file specified.")
4. How do I "Import client certificates into the Windows certificate store"? I do not understand what I must do with point 4.
5. The fix for bug 1597525 has landed 4 months ago in firefox72. Pref "security.osclientcerts.autoload" IS visible since then in all OSes. Do I only need to flip this pref to "true" in order to "Load the os client certs module in Firefox"?
6. How do I "Open a terminal and run a TLS server that requests a client certificate"? Do I simply copy-paste both the commands from point 6 in the system terminal? (It gives out the error "'openssl' is not recognized as an internal or external command, operable program or batch file.")
7. Do I only need to verify it on Windows?
8. Is it recommended to run these in a VM to avoid damage to system certificates?

Thank you!

@Daniel Lo Nigro: Is this issue fixed for you in the latest version of Nightly?

Comment 10

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

:danibodea, it might be fastest if you get in touch with either :mcurtean or :romartin - they've already done work on verifying osclientcerts and they can either tell you what you need to know or take care of verifying this themselves.

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9132425 [details]
bug 1620110 - osclientcerts: properly store serial numbers as DER-encoded integers r?kjacobs

fix an issue when using client certs imported from the OS, approved for 75.0b7

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7f3dfa5b4afc

Comment 13

[Miruna Curtean]

Marking this as Verified fixed as the "User identification Request" dialogue does no longer popup when re-opening closed tab containing OS client certificate, when opening a new tab in browser and entering the same address, opening a duplicate tab or after the browser has been left idle
When browser is restarted, after trying to access the same address, the user dialogue window shows up again, as expected. Same when erasing browser history (CTRL+SHIFT+DEl).
Tested using both RSA and EC certificates across TLS 1.2 and TLS 1.3.
Testes on Windows 10 64-bit and macOS 10.15 Catalina on FF 75.0b7 (20200322132212) and FF 76.0a1 (20200324093140).

Comment 14

[Daniel Bodea [:danibodea]]

Thank you, Miruna!