Closed Bug 1541450 Opened 6 years ago Closed 6 years ago

"Forget about this site" should clear site certificate exceptions

Categories

(Toolkit :: Data Sanitization, enhancement, P2)

Product:
Toolkit
Component:
Data Sanitization
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox69 --- fixed

People

(Reporter: johnp, Assigned: carolina.jimenez.g)

References

(Blocks 1 open bug)

Details

(Keywords: privacy)

Attachments

(2 files)

Bug 1541450 - Add a Certs cleaner and defines that object in FLAGS_MAP. r=johannh
47 bytes, text/x-phabricator-request
Details | Review
Add c5d2d999a8ab again (bug 1541450)
47 bytes, text/x-phabricator-request
Details | Review

With the recent change to make certificate exceptions permanent by default users more easily accumulate certificate exceptions. Right now site certificate exceptions aren't cleared when "Forget this site" is used, leading to the possibility of leaking visited sites similar to bug 1540637.

Furthermore, "Clear Recent History" may need to get a "Certificate Exceptions" checkbox in the "Data" section, though I'm not sure if that's worth it (or if it should just be included when clearing "Site Preferences").

That's a good suggestion, I'm surprised that this isn't the case yet. Thanks for filing this bug.

Blocks: 1102808
Component: Bookmarks & History → Data Sanitization
Priority: -- → P2
Product: Firefox → Toolkit

Carolina, would you be interested in this? The basic idea is that you implement a new data cleaner in ClearDataService:

https://searchfox.org/mozilla-central/source/toolkit/components/cleardata/nsIClearDataService.idl
https://searchfox.org/mozilla-central/source/toolkit/components/cleardata/ClearDataService.jsm

which needs to implement a deleteByHost method that can internally use this function to clean certificates:

https://searchfox.org/mozilla-central/rev/9ee63566281365f26e7a4b06c9d4e2219e64c3e8/security/manager/ssl/nsICertOverrideService.idl#114

Let me know if you're interested in this bug and if you have any questions...

Flags: needinfo?(carolina.jimenez.g)

Hey, yes, thank you Johannh, I will work on it!

Flags: needinfo?(carolina.jimenez.g)
Assignee: nobody → carolina.jimenez.g

Hello Johannh, I'm working on this bug and I was trying to get the host and port of a certificate, I want to do something like:

let certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(Ci.nsIX509CertDB);

for (let cert of certdb.getCerts().getEnumerator()) {
  if (!cert.isBuiltInRoot) {
    Services.clearData.deleteByHost(cert.hostName, cert.portName);
  }
}

But cert does not have the attributes hostName nor portName, it has this attributes:

"isBuiltInRoot", "displayName", "emailAddress", "getEmailAddresses", "containsEmailAddress", 
"subjectName", "subjectAltNames", "commonName", "organization", "organizationalUnit", 
"sha256Fingerprint", "sha1Fingerprint", "tokenName", "issuerName", "serialNumber", 
"issuerCommonName", "issuerOrganization", "issuerOrganizationUnit", "validity", "dbKey", 
"certType", "isSelfSigned", "keyUsages", "ASN1Structure", "getRawDER", "equals", 
"sha256SubjectPublicKeyInfoDigest", "markForPermDeletion", "UNKNOWN_CERT", "CA_CERT", 
"USER_CERT", "EMAIL_CERT", "SERVER_CERT", "ANY_CERT"]

Do you know how to get the host name and port of a cert? I looked in some files through searchfox but found nothing yet.

Thank you

(In reply to Carolina Jimenez Gomez from comment #4)

Hello Johannh, I'm working on this bug and I was trying to get the host and port of a certificate, I want to do something like:

let certdb = Cc["@mozilla.org/security/x509certdb;1"].getService(Ci.nsIX509CertDB);

for (let cert of certdb.getCerts().getEnumerator()) {
  if (!cert.isBuiltInRoot) {
    Services.clearData.deleteByHost(cert.hostName, cert.portName);
  }
}

But cert does not have the attributes hostName nor portName, it has this attributes:

"isBuiltInRoot", "displayName", "emailAddress", "getEmailAddresses", "containsEmailAddress", 
"subjectName", "subjectAltNames", "commonName", "organization", "organizationalUnit", 
"sha256Fingerprint", "sha1Fingerprint", "tokenName", "issuerName", "serialNumber", 
"issuerCommonName", "issuerOrganization", "issuerOrganizationUnit", "validity", "dbKey", 
"certType", "isSelfSigned", "keyUsages", "ASN1Structure", "getRawDER", "equals", 
"sha256SubjectPublicKeyInfoDigest", "markForPermDeletion", "UNKNOWN_CERT", "CA_CERT", 
"USER_CERT", "EMAIL_CERT", "SERVER_CERT", "ANY_CERT"]

Do you know how to get the host name and port of a cert? I looked in some files through searchfox but found nothing yet.

Thank you

Flags: needinfo?(jhofmann)

I found that I can delete a cert using certdb.deleteCertificate(cert) I wonder if I should do this instead of calling Services.clearData.deleteByHost(cert.hostName, cert.portName).

I also found a class that has a int32_t mPort attribute (https://searchfox.org/mozilla-central/source/security/manager/ssl/nsCertTree.h#71), and nsICertTreeItem has a readonly attribute called hostPort (https://searchfox.org/mozilla-central/source/security/manager/ssl/nsICertTree.idl#14). However, I see here (https://searchfox.org/mozilla-central/source/security/manager/pki/resources/content/deletecert.js#91) that a certTreeItem is reachable from some window (I guess is the certificates window (https://searchfox.org/mozilla-central/source/security/manager/pki/resources/content/deletecert.xul#23)) but I wouldn't have that window in the file I'm modifying. I don't see any other way of getting the host and port of a cert, as I mentioned in the last comment, but I see no more examples of the use of certTreeItem, so, I feel like stuck here. Also, the clear history window has a drop-list to choose whether to remove something added last our, last 2 hours, last 4 hours, today or everything; with that in mind, I think I should check when was the certificate added to the data base, but, I don't know how to get that info, because a cert has the properties I mentioned in the last comment.

when a user clicks on the certificate window, a function viewCertificates() is called (https://searchfox.org/mozilla-central/source/security/manager/pki/resources/content/certManager.js#453 https://searchfox.org/mozilla-central/source/security/manager/pki/resources/content/certManager.xul#57) which checks the certificates that are selected and shows them using https://searchfox.org/mozilla-central/source/browser/base/content/pageinfo/security.js#324 but I don't see how they display the host and port of the certificate that is showed in the certificate window.

Can you help me clarifying this a little, please?

Hi Carolina, I think you've been going in the slightly wrong direction here. I'm sorry for not responding earlier.

Let me give you an overview of how ForgetAboutSite works.

The idea behind this feature is that you can right click on an item in e.g. the bookmarks or history menus and choose to "forget" anything about the particular site.

When you select the "Forget About this Site" item from the context menu, it will call ForgetAboutSite.removeDataFromDomain, passing the aDomain parameter which is the hostname/domain of the site that should be forgotten.

ForgetAboutSite.removeDataFromDomain then goes on to calling Services.perms.removeDataFromDomain which is a function that takes in (among other things) a number of flags that specify which data should be deleted. For example CLEAR_COOKIES or CLEAR_DOWNLOADS. "Forget About Site" has its own convenience flag that includes many other flags.

The implementation of removeDataFromDomain will then iterate over a number of "cleaners" that corresponds to the flags passed in. The cleaner for CLEAR_COOKIES for example is CookieCleaner. Cleaners can implement various methods for deletion depending on how they should be used. Ideally, of course, a cleaner would implement all methods, but sometimes that's not possible.

So, what does that mean for this bug? The idea is that we need to add a new cleaner (e.g. CLEAR_CERT_EXCEPTIONS) that will implement at least a deleteByHost method (because that's what Forget About Site needs). You can basically take what we do for CookieCleaner as a blueprint, but of course we should clear certificates, not cookies :)

(Here is a real life example of how to clear cert exceptions by host)

I hope that helps! Let me know if you have any questions.

Thanks!

Flags: needinfo?(jhofmann)
Status: NEW → ASSIGNED
Keywords: checkin-needed

Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c5d2d999a8ab
Add a Certs cleaner and defines that object in FLAGS_MAP. r=johannh

Keywords: checkin-needed

Typo: CLEAR_CERT_EXCEPTIONS collides with CLEAR_STORAGE_ACCESS

While looking at that I may have spotted another bug: Shouldn't CLEAR_STORAGE_ACCESS from bug 1524883 be included in CLEAR_FORGET_ABOUT_SITE as well?

Flags: needinfo?(jhofmann)

(In reply to Johannes Pfrang [:johnp] from comment #11)

Typo: CLEAR_CERT_EXCEPTIONS collides with CLEAR_STORAGE_ACCESS

Huh, I guess that happened during rebase. Carolina, can you update the constant there?

While looking at that I may have spotted another bug: Shouldn't CLEAR_STORAGE_ACCESS from bug 1524883 be included in CLEAR_FORGET_ABOUT_SITE as well?

Nope, that's covered by CLEAR_PERMISSIONS already

Thanks!

Flags: needinfo?(jhofmann)

(In reply to Johann Hofmann [:johannh] from comment #12)

Huh, I guess that happened during rebase. Carolina, can you update the constant there?

Sure!

Flags: needinfo?(carolina.jimenez.g)

NEW: Fixed collision with CLEAR_STORAGE_ACCESS flag.

(In reply to Carolina Jimenez Gomez from comment #14)

Created attachment 9061250 [details]
Add c5d2d999a8ab again (bug 1541450)

NEW: Fixed collision with CLEAR_STORAGE_ACCESS flag.

This is the result of the treeherder

Pushed by jhofmann@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6abefa3e063b Add a Certs cleaner and defines that object in FLAGS_MAP. r=johannh

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&fromchange=6abefa3e063b1ac132fa2e912055d0f184621eef&searchStr=xpc&tochange=f0566d1a9ab14fec0dc9e570fa138de14c535ccf&selectedJob=244766514

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244766324&repo=autoland&lineNumber=7827

Backout link: https://hg.mozilla.org/integration/autoland/rev/f0566d1a9ab14fec0dc9e570fa138de14c535ccf

23:31:43 INFO - TEST-START | extensions/pref/autoconfig/test/unit/test_autoconfig_nonascii.js
23:31:44 INFO - TEST-PASS | extensions/pref/autoconfig/test/unit/test_autoconfig_nonascii.js | took 501ms
23:31:44 INFO - TEST-START | toolkit/components/cleardata/tests/unit/test_certs.js
23:31:44 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/cleardata/tests/unit/test_certs.js | xpcshell return code: 0
23:31:44 INFO - TEST-INFO took 359ms
23:31:44 INFO - >>>>>>>
23:31:44 INFO - PID 7691 | [7691, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2528
23:31:44 INFO - SyntaxError: redeclaration of const Services at /Users/cltbld/tasks/task_1557096269/build/tests/xpcshell/tests/toolkit/components/cleardata/tests/unit/test_certs.js:1
23:31:44 INFO - @/Users/cltbld/tasks/task_1557096269/build/tests/xpcshell/tests/toolkit/components/cleardata/tests/unit/test_certs.js:1:1
23:31:44 INFO - load_file@/Users/cltbld/tasks/task_1557096269/build/tests/xpcshell/head.js:637:7
23:31:44 INFO - _load_files@/Users/cltbld/tasks/task_1557096269/build/tests/xpcshell/head.js:649:10
23:31:44 INFO - _execute_test@/Users/cltbld/tasks/task_1557096269/build/tests/xpcshell/head.js:503:3
23:31:44 INFO - @-e:1:1
23:31:44 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
23:31:44 INFO - PID 7691 | [7691, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, kKnownEsrVersion) failed with result 0x80004002: file /builds/worker/workspace/build/src/toolkit/components/resistfingerprinting/nsRFPService.cpp, line 662
23:31:44 INFO - PID 7691 | [7691, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /builds/worker/workspace/build/src/extensions/permissions/nsPermissionManager.cpp, line 2956
23:31:44 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
23:31:44 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
23:31:44 INFO - running event loop
23:31:44 INFO - (xpcshell/head.js) | test run_next_test 0 finished (1)
23:31:44 INFO - exiting test
23:31:44 INFO - PID 7691 | [7691, Main Thread] WARNING: OOPDeinit() without successful OOPInit(): file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 3104
23:31:44 INFO - PID 7691 | [7691, Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/workspace/build/src/xpcom/base/nsTraceRefcnt.cpp, line 194
23:31:44 INFO - PID 7691 | [7691, Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/workspace/build/src/xpcom/base/nsTraceRefcnt.cpp, line 194
23:31:44 INFO - PID 7691 | nsStringStats
23:31:44 INFO - PID 7691 | => mAllocCount: 10822
23:31:44 INFO - PID 7691 | => mReallocCount: 0
23:31:44 INFO - PID 7691 | => mFreeCount: 10822
23:31:44 INFO - PID 7691 | => mShareCount: 10000
23:31:44 INFO - PID 7691 | => mAdoptCount: 162
23:31:44 INFO - PID 7691 | => mAdoptFreeCount: 162
23:31:44 INFO - PID 7691 | => Process ID: 7691, Thread ID: 140735306535680

Flags: needinfo?(carolina.jimenez.g)

The important part here is

SyntaxError: redeclaration of const Services

which probably means you just have to remove the Services declaration from your test because it's already in a shared head file :)

Hello, sorry for the delay, I was traveling to another country and I didn't bring my computer where I have the repo installed. I will fix it as soon as I can. Also sorry for all the mistakes in this bug :/

Flags: needinfo?(carolina.jimenez.g)

No worries at all, I appreciate that you're helping out!

Pushed by jhofmann@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/72078dd30a1a Add a Certs cleaner and defines that object in FLAGS_MAP. r=johannh

https://hg.mozilla.org/mozilla-central/rev/72078dd30a1a

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox69: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: