Closed Bug 1888751 Opened 1 month ago Closed 14 days ago

Client Certificate prompt no longer pops up for extension background script

Categories

(Core :: Security: PSM, defect)

Product:
Core
Component:
Security: PSM
Version:
Firefox 125
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
127 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox124 --- wontfix
firefox125 --- wontfix
firefox126 --- verified
firefox127 --- verified

People

(Reporter: yc, Assigned: kershaw)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

cc-bug.png
259.52 KB, image/png
Details
Bug 1888751 - Use getMostRecentBrowserWindow when where is no gDialogBox, r=Gijs
48 bytes, text/x-phabricator-request
Details | Review
Bug 1888751 - Use getMostRecentBrowserWindow when where is no gDialogBox, r=Gijs
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-beta+
Details | Review
Attached image cc-bug.png —

Steps to reproduce:

  1. Have self-hosted web server configured with client certificate authentication.
  2. Import client certificate into Firefox.
  3. Load below extension from "about:debugging".

background.html

<html>
  <head>
    <meta charset="UTF-8" />
  </head>
  <body></body>
  <script src="test.js"></script>
</html>

manifest.json

{
  "manifest_version": 2,
  "name": "Client Cert test",
  "version": "1.0",

  "description": "Test that client cert works properly in extension.",

  "background": {
    "page": "background.html",
    "persistent": true
  },

  "content_scripts": [
    {
      "matches": ["https://example.com/*"],
      "js": ["test.js"]
    }
  ],
  "permissions": [
    "<all_urls>"
  ]
}

test.js

document.body.style.border = "5px solid red";

async function test() {
    await fetch("https://<redacted-host-which-has-client-auth>");
    const x = await fetch("https://<redacted-host-which-has-client-auth>", { credentials: 'include' });
    console.log(x);
}
test();

Actual results:

There was no "User Identification Request" prompt, with "This site has requested that you identify yourself with a certificate". The fetch requests fails with 403.

Expected results:

There is a User Identification Request prompt, where user can confirm certificate.

Workaround: Downgrade to old Firefox, perform login, upgrade Firefox. Do not clear Site Settings.

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Product: Firefox → WebExtensions

Hello,

Would you be able to provide the steps required to self-host a web server with client certificate authentication as well as the actual client certificate you used?

Thank you !

Flags: needinfo?(yc)

I've attached a script which can reproduce the issue locally.
Steps:

  1. Run script (rootless). This should generate certificates and start up nginx.
  2. In Firefox Certificate Manager, Add Security Exception, Location: https://127.0.0.1:8443, Permanently store, Confirm.
    (In a real setup, this is not necessary since the root CA is in the trust store)
  3. In Firefox Certificate Manager, Your Certificates, Import client.pfx, Type in password "1888751".
  4. Modify fetch URL above to https://127.0.0.1:8443.
  5. In about:debugging, import extension.
#!/bin/bash

rm -f nginx.conf *.crt *.key *.csr *.srl *.log

cat > nginx.conf << EOL
worker_processes 1;
daemon off;
error_log stderr;
pid nginx.pid;

events {
    worker_connections 1024;
}

http {
    server {
        access_log access.log;

        listen 8443 ssl default_server;
        listen [::]:8443 ssl default_server;

        ssl_certificate ./server.crt;
        ssl_certificate_key ./server.key;

        server_name _;
        ssl_client_certificate ca.crt;
        ssl_verify_client optional;

        location / {
            if (\$ssl_client_verify != SUCCESS) { return 403; }
            add_header Content-Type text/plain;
            return 200 'hello\n';
        }
    }
}
EOL

# CA
openssl genrsa -out ca.key 4096
openssl req -x509 -new -nodes -key ca.key -sha256 -days 30 -out ca.crt  -subj '/CN=1888751/O=1888751'

# Server
openssl genrsa -out server.key 4096
openssl req -new -key server.key -out server.csr -subj '/CN=127.0.0.1/O=1888751'
openssl x509 -req -in server.csr -CA ca.crt -CAkey ca.key -CAcreateserial -out server.crt -days 30 -sha256

# Client
openssl genrsa -out client.key 4096
openssl req -new -key client.key -out client.csr -subj '/CN=127.0.0.1/O=188751'
openssl x509 -req -in client.csr -CA ca.crt -CAkey ca.key -CAcreateserial -out client.crt -days 30 -sha256
openssl pkcs12 -export -out client.pfx -inkey client.key -in client.crt -certfile ca.crt -password pass:1888751

/usr/sbin/nginx -c nginx.conf -p $PWD &
nginx_pid=$!

echo "--- No client cert ---"
curl --cacert ca.crt https://127.0.0.1:8443
echo "--- With client cert ---"
curl --cacert ca.crt --key client.key --cert client.crt https://127.0.0.1:8443

wait $nginx_pid
Flags: needinfo?(yc)

Hello and thank you for the additional info !

I reproduced the issue on the latest Nightly (126.0a1/20240401213149), Beta (125.0b7/20240401091600) and Release (124.0.1/20240321230221) under Ubuntu 22.04 LTS.

After loading the extension from Comment 0, there is no prompt and checking the browser toolbox network tab also shows that the fetch requests fail with 403.

Status: UNCONFIRMED → NEW
status-firefox124: --- → affected
status-firefox125: --- → affected
status-firefox126: --- → affected
Ever confirmed: true

DEBUG : Found commit message:
Bug 1401466 - make the client auth certificate selection dialog tab modal r=jschanck,necko-reviewers,bolsson,kershaw,valentin

Differential Revision: https://phabricator.services.mozilla.com/D183775

Corresponding pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=34dcd8f28e9a642600425b3383df225859ea91cd&tochange=9d1733984f18d8faafeeb405e1519bd8f2f239d0

Keywords: regressionwindow-wanted → regression
Regressed by: 1401466

Set release status flags based on info from the regressing bug 1401466

status-firefox-esr115: --- → unaffected

Can you help with adding severity to this ticket.

Flags: needinfo?(tomica)

Hey Kershaw, can you please help with confirming if this is intended behavior after bug 1401466 (Dana seems OoO, but you reviewed the patch in question -- if you're not the best person to answer, please forward to someone who might).

The commit message makes it sound like there should be a fallback to a last used window, but I couldn't figure out if that's still in the landed version of the patch.

Flags: needinfo?(tomica) → needinfo?(kershaw)

(In reply to Tomislav Jovanovic :zombie from comment #8)

Hey Kershaw, can you please help with confirming if this is intended behavior after bug 1401466 (Dana seems OoO, but you reviewed the patch in question -- if you're not the best person to answer, please forward to someone who might).

The commit message makes it sound like there should be a fallback to a last used window, but I couldn't figure out if that's still in the landed version of the patch.

It's not about fallback to the last used window, because in this case, there is a loadContext here.
The problem seems to be at this line, since I got the error below:

JavaScript error: resource://gre/modules/psm/ClientAuthDialogService.sys.mjs, line 58: TypeError: can't access property "open", browserWindow.gDialogBox is undefined

I am not sure why gDialogBox is undefined, but looking at other places where gDialogBox is used, it can be null.

When gDialogBox is null, using Services.wm.getMostRecentWindow("").gDialogBox like the code below seems to work.

    if (browserWindow) {
      let dialogBox = browserWindow.gDialogBox
        ? browserWindow.gDialogBox
        : Services.wm.getMostRecentWindow("").gDialogBox;
      dialogBox
        .open(clientAuthAskURI, { hostname, certArray, retVals })
        .then(() => {
          callback.certificateChosen(retVals.cert, retVals.rememberDecision);
        });
      return;
    }

However, I am not sure if this is a correct fix, since I don't know front end code really well.
Gijs, do you know a better way to fix this?
Thanks.

Flags: needinfo?(kershaw) → needinfo?(gijskruitbosch+bugs)

The variable name is browserWindow, but is this actually a browser window in the case where this fails?

I expect not - comment 0 suggests the fetch call is happening in an extension background script, which I expect isn't associated to a given browser window at all.

The code at https://searchfox.org/mozilla-central/rev/98a54cbdc5964e778af0e6b325f9e7831f00c05b/security/manager/ssl/ClientAuthDialogService.sys.mjs#58 is kind of surprising to me. It looks like we ship this as part of toolkit/gecko, so presumably it also ships on Android (and potentially Thunderbird etc.). But it assumes a lot of things about where connections might come from (ie browser windows).

The fix described in comment #9 will fall back to trying to create a window-modal prompt in "some other window", which might also not be a browser window (either because we're not running in desktop Firefox or because it so happens that the user last selected a different type of window, like the library window or a separate devtools window, the about dialog, ...).

The right fix would probably look more like:

    // Otherwise, attempt to open a window-modal dialog on the window that at
    // least has the tab the load is occurring in.
    let browserWindow = loadContext?.topFrameElement?.ownerGlobal;
    // Failing that, open a window-modal dialog on the most recent "main" window.
    if (!browserWindow?.gDialogBox) {
      // XXXgijs not sure how 'good' this fallback is. This would mean that instead
      // of opening a separate, OS-level modal window for the dialog on top of a
      // window from which the client cert originated, we open it on a "main"
      // window. This could be disorientating if the request is from some non-main
      // window, and the prompt shows up elsewhere.
      // But this fallback was already here...
      browserWindow = Services.wm.getMostRecentBrowserWindow();
    }
    if (browserWindow?.gDialogBox) {
      browserWindow.gDialogBox
        .open(clientAuthAskURI, { hostname, certArray, retVals })
        .then(() => {
          callback.certificateChosen(retVals.cert, retVals.rememberDecision);
        });
      return;
    }
    // If we get here, we couldn't find a window with a `gDialogBox` property.
    // Fall back to opening the window using the window watcher as a separate
    // modal on top of whatever window we can find.
    let mostRecentWindow = Services.wm.getMostRecentWindow("");
    let response = Services.ww.openWindow(mostRecentWindow, clientAuthAskURI, {hostname, certArray, retVals});
    callback.certificateChosen(retVals.cert, retVals.rememberDecision);

(completely untested, probably doesn't work, please check the actual docs for openWindow and/or other callsites)

And then on top of that we probably need a bug to do something else on android, if we haven't already?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(kershaw)

Thanks for the sample code, Gijs. I'll try to write a patch based on it.

Note that I am not sure how to make the case that there is no gDialogBox work, because of the error caused by the line below.

let response = Services.ww.openWindow(mostRecentWindow, clientAuthAskURI, {hostname, certArray, retVals});
Flags: needinfo?(kershaw)
Assignee: nobody → kershaw
Status: NEW → ASSIGNED

Set release status flags based on info from the regressing bug 1401466

status-firefox127: --- → affected

:willdurand tagging as Triage Owner. Could you set the severity on this?
This has a patch, wondering if we should keep an eye for uplift or it should ride the train with Fx127

Flags: needinfo?(wdurand)
Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d35fa1190f2a
Use getMostRecentBrowserWindow when where is no gDialogBox, r=Gijs
Component: Untriaged → Security: PSM
Flags: needinfo?(wdurand)
Product: WebExtensions → Core

https://hg.mozilla.org/mozilla-central/rev/d35fa1190f2a

Status: ASSIGNED → RESOLVED
Closed: 14 days ago
status-firefox127: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch

Verified as Fixed. Tested on the latest Nightly (127.0a1/20240418214121) under Ubuntu 22.04 LTS.

After loading the extension from Comment 0, the "User Identification Request" prompt is shown, confirming the fix.

Status: RESOLVED → VERIFIED
status-firefox127: fixed → verified

The patch landed in nightly and beta is affected.
:kershaw, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(kershaw)
Attachment #9397589 - Flags: approval-mozilla-beta?

beta Uplift Approval Request

  • User impact if declined: Client certificate dialog could not show when the HTTP request is from an extension
  • Code covered by automated testing: yes
  • Fix verified in Nightly: yes
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: N/A
  • Risk associated with taking this patch: Low
  • Explanation of risk level: Shoule be low risk, since the patch is straightforward.
  • String changes made/needed: N/A
  • Is Android affected?: yes
Flags: needinfo?(kershaw)
Attachment #9397589 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Verified as Fixed. Tested on the latest Beta (126.0b4/20240422091940) under Ubuntu 22.04 LTS.

After loading the extension from Comment 0, the "User Identification Request" prompt is shown, confirming the fix.

status-firefox126: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: