Closed
Bug 1343609
Opened 7 years ago
Closed 7 years ago
adapt the telemetry experiment from bug 1311479 to identify misissued certificates for mozilla properties
Categories
(Core :: Security: PSM, enhancement, P1)
Core
Security: PSM
Tracking
()
RESOLVED
DUPLICATE
of bug 1346017
People
(Reporter: keeler, Assigned: keeler)
Details
(Keywords: sec-other, Whiteboard: [psm-assigned])
Attachments
(3 files, 2 obsolete files)
In the process of disabling sha-1, we have been gathering telemetry data to answer the question of if the policy of "disable sha-1 except if the root certificate is not part of the web pki" would cause compatibility issues for users behind TLS intercepting proxies. Our assumption was that it would not, because if an intercepting proxy were issuing certificates that chained to a root in the web pki, such certificates would almost certainly be misissuances. To gather the data, the telemetry experiment (and later the sha-1 disabling add-on) would attempt to contact telemetry.mozilla.org and see what certificate chain it was using. For each certificate in the chain, it would send back the sha256 hash and whether or not the certificate was a built-in root (i.e. part of the web pki), as well as whether or not the chain validated correctly for the host telemetry.mozilla.org. Given this data, we have found a handful of cases where a user encountered a certificate chain purportedly sent by telemetry.mozilla.org that validated correctly from a root in the web pki yet differed from the real telemetry.mozilla.org. Given only the hashes of the certificates, we have been unable to identify the corresponding certificates in public data sets (e.g. CT/censys, etc.). Additionally, the issuing CA has indicated that they have no record of at least one of these certificates (as of this writing, we haven't asked about the others yet). In order to determine if these certificates were misissued, we need to know what they are. Furthermore, we should attempt to identify other similar situations for other mozilla properties. To that end, we should extend the telemetry experiment from bug 1311479 to have it check a list of mozilla properties and send back the entire certificate chain if it identifies a possible misissuance.
![]() |
Assignee | |
Comment 1•7 years ago
|
||
Here's an initial approach - let me know what you think.
Attachment #8842638 -
Flags: feedback?(jjones)
Comment 2•7 years ago
|
||
Comment on attachment 8842638 [details] [diff] [review] 1343609-certificate-checker.diff Review of attachment 8842638 [details] [diff] [review]: ----------------------------------------------------------------- ::: experiments/certificate-checker/code/bootstrap.js @@ +56,5 @@ > .createInstance(Ci.nsIXMLHttpRequest); > req.open("GET", "https://" + hostname); > req.timeout = 30000; > req.addEventListener("error", (evt) => { > + let result = { hostname, error: "connection error" }; Might as well toss this object into the resolve() directly, like we do for timeout @@ +101,5 @@ > + // what we sould send if we encountered something we thought was wrong. > + "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", > + "154c433c491929c5ef686e838e323664a00e6a0d822ccc958fb4dab03e49a08f", > + "4348a0e9444c78cb265e058d5e8944b4d84f9662bd26db257f8934a443c70161" > + ], Yup, this looks about right for me. We should pow-wow about what hosts to include. I'll start a list. ::: experiments/certificate-checker/code/install.rdf @@ +11,5 @@ > <em:targetApplication> > <Description> > <em:id>{5428e386-68cf-4283-8ee3-04c12d3f4f4e}</em:id> > <em:minVersion>50.0</em:minVersion> > <em:maxVersion>51.0</em:maxVersion> We'll need to crank this up to 54.0 methinks
Attachment #8842638 -
Flags: feedback?(jjones) → feedback+
![]() |
Assignee | |
Comment 3•7 years ago
|
||
I set the versions to 51-55 since Nightly will almost certainly be 55 before we ship this.
Attachment #8842638 -
Attachment is obsolete: true
Attachment #8843354 -
Flags: review?(jjones)
![]() |
Assignee | |
Comment 4•7 years ago
|
||
Comment on attachment 8843354 [details] [diff] [review] patch data-review? to :bsmedberg.
Attachment #8843354 -
Flags: review?(benjamin)
Comment 5•7 years ago
|
||
Comment on attachment 8843354 [details] [diff] [review] patch data-review=me. I did not review the code.
Attachment #8843354 -
Flags: review?(benjamin) → review+
Comment 6•7 years ago
|
||
Comment on attachment 8843354 [details] [diff] [review] patch Review of attachment 8843354 [details] [diff] [review]: ----------------------------------------------------------------- Please check that error case I noted. Thanks! ::: experiments/certificate-checker/code/bootstrap.js @@ +191,5 @@ > + return Promise.all(promises); > +} > + > +function analyzeAndReport(results) { > + let payload = { mismatches: [] }; Maybe we should stick a version marker in here? It would have been nice to know v1.2 vs 1.3 of the last addon. @@ +195,5 @@ > + let payload = { mismatches: [] }; > + for (let result of results) { > + // Skip if the connection resulted in any kind of error. > + if ("error" in result) { > + console.log(`${result.hostname}: ${result.error} - skipping`); This looks like it won't work; the objects constructed in the error cases don't define `hostname: `... on line 61/64/etc.
![]() |
Assignee | |
Comment 7•7 years ago
|
||
(In reply to J.C. Jones [:jcj] from comment #6) Thanks! > ::: experiments/certificate-checker/code/bootstrap.js > @@ +191,5 @@ > > + return Promise.all(promises); > > +} > > + > > +function analyzeAndReport(results) { > > + let payload = { mismatches: [] }; > > Maybe we should stick a version marker in here? It would have been nice to > know v1.2 vs 1.3 of the last addon. Sure - I added a version field and set it to "1.0" (I'm carrying over data-review on this, since I'm assuming that adding a field that is the same across all responses doesn't have any privacy implications). > @@ +195,5 @@ > > + let payload = { mismatches: [] }; > > + for (let result of results) { > > + // Skip if the connection resulted in any kind of error. > > + if ("error" in result) { > > + console.log(`${result.hostname}: ${result.error} - skipping`); > > This looks like it won't work; the objects constructed in the error cases > don't define `hostname: `... on line 61/64/etc. Constructing an object like { hostname } is shorthand for { hostname: hostname }, so this should work (and I verified that it does in the error case).
Attachment #8843414 -
Flags: review+
![]() |
Assignee | |
Updated•7 years ago
|
Attachment #8843354 -
Attachment is obsolete: true
Attachment #8843354 -
Flags: review?(jjones)
![]() |
Assignee | |
Comment 8•7 years ago
|
||
Comment on attachment 8843414 [details] [diff] [review] patch v1.1 Review of attachment 8843414 [details] [diff] [review]: ----------------------------------------------------------------- Whoops I guess you didn't actually r+ that.
Attachment #8843414 -
Flags: review+ → review?(jjones)
Comment 9•7 years ago
|
||
Comment on attachment 8843414 [details] [diff] [review] patch v1.1 Review of attachment 8843414 [details] [diff] [review]: ----------------------------------------------------------------- Cool, LGTM. Also, that kind of JS initialization seems like cheating. :)
Attachment #8843414 -
Flags: review?(jjones) → review+
![]() |
Assignee | |
Comment 10•7 years ago
|
||
Jason, if you could sign this experiment, that would be great - thanks!
Flags: needinfo?(jthomas)
Comment 12•7 years ago
|
||
Historical note, the SHA256-sums of the end-entity certificates we observed were: d8cdca300ff9920c79335936c89f6947cfdb8cbeede604ed77d15fa3bf91a04b 7f8b25ddb928cfd0c65755ce1a60a73d7e0d81f2753f21e567d3f27acee8841b b630beb62bbdf7cac7185a843140a92b8ba9959e40bb1ece4dbdfbc099a53c1f 7e0cc1c93c357ee17ff968ae5f6485c5a9c4981d0953ecd452775666d2f70983 c5e0aecdd348b9f34bd3b3bdecc21846ab2df9865c939a1fd0bf37f2a6bd887e 9ec14a5506b68c201bfb9afe077099457c6c5d3332640ed31a6ef64836d87d69 9735baed4c71b963eaaf3516b6c38b6c354d66150018af3a4d2ca3aa00a1a9dc 26e5d7a18caa7bab6c9ae7e69d03818231146d0e559c300a30851ba90e4d8f2b 65f924b4ca63aba96e433d916e2133bde502a855e41eaac9f0cb0746e941c486 71f15da582c2bc81b019cf5a2cc03a5cfefd61d58891c62940371b0b9b24e1b6 All of these chained up through this intermediate: 154c433c491929c5ef686e838e323664a00e6a0d822ccc958fb4dab03e49a08f To this root: 4348a0e9444c78cb265e058d5e8944b4d84f9662bd26db257f8934a443c70161 None of the above end-entity certificates have been found yet; the issuer doesn't believe they exist, nor do they appear in Censys or CT.
![]() |
Assignee | |
Comment 13•7 years ago
|
||
This ended up being developed in bug 1346017. Analysis here: https://people-mozilla.org/~dkeeler/deployment-checker-analysis.html Long story short, the most plausible explanation we came up with is that bad RAM caused flipped bits and resulted in mismatched hashes. There never were any misissued certificates.
Group: crypto-core-security
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.
Description
•