Closed Bug 523336 Opened 15 years ago Closed 7 years ago

User Identification Request-Dialog's "remember this decision" remembers the wrong certificate

Categories

(Core :: Security: PSM, defect)

All
Linux
defect
Not set
major

Tracking

()

RESOLVED DUPLICATE of bug 307081

People

(Reporter: mozilla.org, Unassigned)

Details

(Keywords: privacy, Whiteboard: [psm-auth][psm-backlog])

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.12) Gecko/2009071200 SUSE/3.0.12-0.1.2 Firefox/3.0.12
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.12) Gecko/2009071200 SUSE/3.0.12-0.1.2 Firefox/3.0.12

If you have installed more than one user certificate, there is a chance that a checked rememberBox (security/manager/pki/resources/content/clientauthask.xul) leads the browser to send the wrong certificate the next time. Therefore, the browser shows error "-12225".

Reproducible: Always

Steps to Reproduce:
The order of step 2 and 3 is important.

1. Remove all your certificates.
2. Install a pkcs#12-file, which you don't need
3. Install a pkcs#12-file, which you need to authenticate yourself
4. Go to a website which expects the certificate installed in step 3 (and needs the certificate more than once)
5. Select the right certificate in the clientauthask.xul-Dialog and check the rememberBox

Actual Results:  
When the webserver needs the certificate one more time, firefox will send the certificate installed in step 2 and not the right one.

Expected Results:  
The selected certificate should used.

I guess this is the reason: security/manager/ssl/src/nsNSSIOLayer.cpp, Lines 2926-3003 (Firefox 3.5.3 source code):

    for (CertsToUse = 0, node = CERT_LIST_HEAD(certList);
         !CERT_LIST_END(node, certList) && CertsToUse < nicknames->numnicknames;
         node = CERT_LIST_NEXT(node)
        )
    {
      nsRefPtr<nsNSSCertificate> tempCert = new nsNSSCertificate(node->cert);

      if (!tempCert)
        continue;

      NS_ConvertUTF8toUTF16 i_nickname(nicknames->nicknames[CertsToUse]);
      nsAutoString nickWithSerial, details;

      if (NS_FAILED(tempCert->FormatUIStrings(i_nickname, nickWithSerial, details)))
        continue;

      certNicknameList[CertsToUse] = ToNewUnicode(nickWithSerial);
      if (!certNicknameList[CertsToUse])
        continue;
      certDetailsList[CertsToUse] = ToNewUnicode(details);
      if (!certDetailsList[CertsToUse]) {
        nsMemory::Free(certNicknameList[CertsToUse]);
        continue;
      }

      ++CertsToUse;
    }

    /* Throw up the client auth dialog and get back the index of the selected cert */
    rv = getNSSDialogs((void**)&dialogs,
                       NS_GET_IID(nsIClientAuthDialogs),
                       NS_CLIENTAUTHDIALOGS_CONTRACTID);

    if (NS_FAILED(rv)) {
      NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(CertsToUse, certNicknameList);
      NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(CertsToUse, certDetailsList);
      goto loser;
    }

    {
      nsPSMUITracker tracker;
      if (tracker.isUIForbidden()) {
        rv = NS_ERROR_NOT_AVAILABLE;
      }
      else {
        rv = dialogs->ChooseCertificate(info, cn_host_port.get(), org.get(), issuer.get(),
          (const PRUnichar**)certNicknameList, (const PRUnichar**)certDetailsList,
          CertsToUse, &selectedIndex, &canceled);
      }
      }
    }

    NS_RELEASE(dialogs);
    NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(CertsToUse, certNicknameList);
    NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(CertsToUse, certDetailsList);

    if (NS_FAILED(rv)) goto loser;

    // even if the user has canceled, we want to remember that, to avoid repeating prompts
    PRBool wantRemember = PR_FALSE;
    info->GetRememberClientAuthCertificate(&wantRemember);

    int i;
    if (!canceled)
    for (i = 0, node = CERT_LIST_HEAD(certList);
         !CERT_LIST_END(node, certList);
         ++i, node = CERT_LIST_NEXT(node)) {

      if (i == selectedIndex) {
        cert = CERT_DupCertificate(node->cert);
        break;
      }
    }

    if (cars && wantRemember) {
      cars->RememberDecision(hostname,
                             serverCert,
                             canceled ? 0 : cert);
    }
--------------------------

The first for-loop builds a list of certificates which are shown to the user, but the loop skips some certificates. The index of the selected item from the dropdown box is used to identify the selected certificate. But the second for-loop doesn't skip any certificates. This is, why the wrong certificate is saved by "cert = CERT_DupCertificate(node->cert);".

This Bug also happens in Firefox 2. In Firefox 3, the rememberBox was removed. In Firefox 3.5, the checkbox is back. In seamonkey-2.0rc2, there is the same code, too.

I check the "Security" checkbox below, because transmitting an unwanted certificate is wicked!
Assignee: nobody → kaie
Component: Security → Security: PSM
Product: Firefox → Core
QA Contact: firefox → psm
Summary: User Identification Request-Dialog's "remeber this decision" remembers the wrong certificate → User Identification Request-Dialog's "remember this decision" remembers the wrong certificate
nsCertPicker::PickByUsage() may have the same problem from a quick read of http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsCertPicker.cpp#66.
Is this a "vulnerability" that needs to be hidden? Broken for sure, but I think the worst that can happen is that you offer up the wrong credentials to a web site that might now know stuff about you you hadn't intended to share. A privacy issue, but not one an evil site can force on you.

Better to open this up and potentially warn users against the "Remember" setting if they have multiple certs.
Group: core-security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: privacy
@Daniel: You are right, of course. It wasn't my intention to hide this bug... I even doesn't know how I activated this (this is my first usage of bugzilla).
-> me

Should be easy to fix.
Assignee: kaie → honzab.moz
Status: NEW → ASSIGNED
Whiteboard: [psm-clientauth]
Whiteboard: [psm-clientauth] → [psm-auth]
Anyone feel free to take this bug.  I can get to this later.
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
www-client/firefox-17.0.4 on amd64 GNU/Linux OS:
The same issue.
The only difference is that auto-suggested is the last imported certificate:
I.e.:

On start point I have two client certificates, the second (last inported) is the normally used one.
And it is suggested to web-server by default.
But later I needed for some test purposes imports additional (3-rd) client certificate.
After that find, that by default FF suggests last imported (now --- temporary test) client certificate.

Other questions about this dialog window I'll list in separate bug.
Whiteboard: [psm-auth] → [psm-auth][psm-backlog]
FWIW, I might've fixed this as part of Bug 307081:
https://hg.mozilla.org/mozilla-central/rev/34398aad1f9a

If anyone can confirm or refute this, please leave a comment or resolve the bug.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.