"Forget about this site" should clear site certificate exceptions
Categories
(Toolkit :: Data Sanitization, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: johnp, Assigned: carolina.jimenez.g)
References
(Blocks 1 open bug)
Details
(Keywords: privacy)
Attachments
(2 files)
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").
Comment 1•6 years ago
|
||
That's a good suggestion, I'm surprised that this isn't the case yet. Thanks for filing this bug.
Comment 2•6 years ago
|
||
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:
Let me know if you're interested in this bug and if you have any questions...
Assignee | ||
Comment 3•6 years ago
|
||
Hey, yes, thank you Johannh, I will work on it!
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 4•6 years ago
|
||
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
Assignee | ||
Comment 5•6 years ago
|
||
(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 attributeshostName
norportName
, 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
Assignee | ||
Comment 6•6 years ago
|
||
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?
Comment 7•6 years ago
|
||
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!
Assignee | ||
Comment 8•6 years ago
|
||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
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
Comment 10•6 years ago
|
||
Backed out changeset c5d2d999a8ab (bug 1541450) for XPCShell failures in toolkit/components/cleardata/tests/unit/test_certs.js
Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=242790134&repo=autoland&lineNumber=1888
Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=242752346&revision=c5d2d999a8abf0f920a2d9166e6ab5a325c29876
Backout:
https://hg.mozilla.org/integration/autoland/rev/72d674a9bac7739024cbff7e4e2be4aaf4c635a5
Reporter | ||
Comment 11•6 years ago
|
||
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?
Comment 12•6 years ago
|
||
(In reply to Johannes Pfrang [:johnp] from comment #11)
Typo:
CLEAR_CERT_EXCEPTIONS
collides withCLEAR_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 inCLEAR_FORGET_ABOUT_SITE
as well?
Nope, that's covered by CLEAR_PERMISSIONS already
Thanks!
Assignee | ||
Comment 13•6 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #12)
Huh, I guess that happened during rebase. Carolina, can you update the constant there?
Sure!
Assignee | ||
Comment 14•6 years ago
|
||
NEW: Fixed collision with CLEAR_STORAGE_ACCESS flag.
Assignee | ||
Comment 15•6 years ago
|
||
(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
Comment 16•6 years ago
|
||
Comment 17•6 years ago
|
||
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
Comment 18•6 years ago
|
||
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 :)
Assignee | ||
Comment 19•6 years ago
|
||
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 :/
Comment 20•6 years ago
|
||
No worries at all, I appreciate that you're helping out!
Comment 21•6 years ago
|
||
Comment 22•6 years ago
|
||
bugherder |
Description
•