Starting with Windows 8, we can use the IWSCProductList interface to enumerate registered AV/anti-spyware/firewall products that are installed on the system. This would be useful into to add to about:support, crash report annotations, telemetry environment, etc. CoCreateInstance using CLSID_WSCProductList.

I have some patches for this.

Comment on attachment 8935922 [details] Bug 1418131: Part 1 - Add Windows Security Center info to nsSystemInfo; https://reviewboard.mozilla.org/r/206782/#review212602 Looks good, I added some notes but I'll leave it up to you if you want to change anything. ::: xpcom/base/nsSystemInfo.cpp:261 (Diff revision 1) > + if (FAILED(hr)) { > + return hr; > + } > + > + _bstr_t bName; > + hr = product->get_ProductName(bName.GetAddress()); Does the produce name include the version, or do we need to get the timestamp as well (the docs are pretty vague on this). ::: xpcom/base/nsSystemInfo.cpp:272 (Diff revision 1) > + hr = product->get_ProductState(&state); > + if (FAILED(hr)) { > + return hr; > + } > + > + // We only care about products that are active Do we? Maybe we should include it with an attribute. "foo av (snoozed)". On the other hand if we don't want it we could check this before getting the name. ::: xpcom/base/nsSystemInfo.cpp:282 (Diff revision 1) > + > + if (!aOutput.IsEmpty()) { > + aOutput.AppendLiteral(u";"); > + } > + > + nsDependentString name((wchar_t*)bName, bName.length()); I *think* you can avoid the intermediary here and just use `Append((wchar_t*)bName, bName.length())`. ::: xpcom/base/nsSystemInfo.cpp:313 (Diff revision 1) > + > + // 1. Antivirus > + RefPtr<IWSCProductList> prodList; > + HRESULT hr = ::CoCreateInstance(clsid, nullptr, CLSCTX_INPROC_SERVER, iid, > + getter_AddRefs(prodList)); > + if (FAILED(hr)) { Okay, so does this safely fail on Win7? I assume it does but just checking. ::: xpcom/base/nsSystemInfo.cpp:327 (Diff revision 1) > + hr = EnumWSCProductList(aAVInfo, WrapNotNull(prodList.get())); > + if (FAILED(hr)) { > + return NS_ERROR_UNEXPECTED; > + } > + > + // 2. Anti-spyware I suppose you could fold this into a helper or just a loop over the provider types. ::: xpcom/base/nsSystemInfo.cpp:822 (Diff revision 1) > if (NS_WARN_IF(NS_FAILED(rv))) { > return rv; > } > } > + > + nsAutoString avInfo, antiSpyInfo, firewallInfo; Differentiating b/w AV, anti-spyware, and firewalls seems kind of pointless to me. Would it make more sense to just get an array of security things, then munge them together here? ie: ``` nsTArray<nsString> secItems; GetWindowsSecurityCenterInfo(secItems); nsString registeredSecItems = secItems.join(";"); // okay that's not real but you get the idea ``` It's fine if you see merit in differentiating, I don't feel too strongly about this.

Comment on attachment 8935926 [details] Data Review Request Form 1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate? Only in the data review request form attached to this bug (I think). 2) Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available. Yes, telemetry setting. 3) If the request is for permanent data collection, is there someone who will monitor the data over time?** Yes, Aaron Klotz. 4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under? ** Category 1. 5) Is the data collection request for default-on or default-off? Default-on. 6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)? No. 7) Is the data collection covered by the existing Firefox privacy notice? Yes. 8) Does there need to be a check-in in the future to determine whether to renew the data? No, permanent.

Comment on attachment 8935924 [details] Bug 1418131: Part 3 - Add Windows Security Center info to telemetry environment; https://reviewboard.mozilla.org/r/206786/#review212670 ::: toolkit/components/telemetry/TelemetryEnvironment.jsm:1704 (Diff revision 1) > memoryMB, > virtualMaxMB: virtualMB, > cpu: this._getCpuData(), > os: this._getOSData(), > hdd: this._getHDDData(), > + sec: this._getSecurityAppData(), This is Windows only? Let's limit adding this to `platform === "win"`. This addition will need: - checking the tests: `mach test toolkit/components/telemetry` - at least updating `test_TelemetryEnvironment.js` - updating the docs: `toolkit/components/telemetry/docs/data/environment.rst` For pipeline dependencies, we can flag :mreid after the client code is settled.

Comment on attachment 8935922 [details] Bug 1418131: Part 1 - Add Windows Security Center info to nsSystemInfo; https://reviewboard.mozilla.org/r/206782/#review214178

Comment on attachment 8935923 [details] Bug 1418131: Part 2 - Add security software section to about:support; https://reviewboard.mozilla.org/r/206784/#review214180

Comment on attachment 8935924 [details] Bug 1418131: Part 3 - Add Windows Security Center info to telemetry environment; https://reviewboard.mozilla.org/r/206786/#review217458 ::: toolkit/components/telemetry/TelemetryEnvironment.jsm:1604 (Diff revision 2) > > /** > + * Get registered security product information. > + * @return Object containing the security product data > + */ > + _getSecurityAppData() { It looks like the data size here can be unbounded (array lengths, string sizes). Can you - put explicit bounds on it or - document any existing bounds? I missed that in the last pass.

(In reply to Georg Fritzsche [:gfritzsche] from comment #14) > Comment on attachment 8935924 [details] > Bug 1418131: Part 3 - Add Windows Security Center info to telemetry > environment; > > https://reviewboard.mozilla.org/r/206786/#review217458 > > ::: toolkit/components/telemetry/TelemetryEnvironment.jsm:1604 > (Diff revision 2) > > > > /** > > + * Get registered security product information. > > + * @return Object containing the security product data > > + */ > > + _getSecurityAppData() { > > It looks like the data size here can be unbounded (array lengths, string > sizes). > Can you > - put explicit bounds on it or > - document any existing bounds? > > I missed that in the last pass. There are not any explicit bounds. Can you clarify what you are looking for with respect to bounds? We don't do that for various other string properties pulled from nsSystemInfo (eg. binHDDModel)

We could generally be better about limits, yes. We started on some of the data but haven't completed this yet. Here we pull in a list of entries, so i'd like us to be on the safe side: - Limiting the number of entries to an arbitrary higher number. - Use limitStringToLength() to be safe on the individual items.

Here are the limits we use in the modules ping: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryModules.jsm#34.

Comment on attachment 8935924 [details] Bug 1418131: Part 3 - Add Windows Security Center info to telemetry environment; https://reviewboard.mozilla.org/r/206786/#review217458 > It looks like the data size here can be unbounded (array lengths, string sizes). > Can you > - put explicit bounds on it or > - document any existing bounds? > > I missed that in the last pass. I decided to place a 256 character bound on the string *before* the split. In the unlikely event that something exceeds that bound, I think that will produce the cleanest result.

Comment on attachment 8935924 [details] Bug 1418131: Part 3 - Add Windows Security Center info to telemetry environment; https://reviewboard.mozilla.org/r/206786/#review218522 Thanks, this looks good to me. If you need this available in Redash / sql.tmo / some dataset, please file a bug with the details: https://bugzilla.mozilla.org/enter_bug.cgi?product=Data%20Platform%20and%20Tools&component=Datasets%3A%20General If you are planning to access this through custom analysis, the data will available through ATMO as it comes in. ::: toolkit/components/telemetry/docs/data/environment.rst:205 (Diff revision 3) > status: <string>, // See the status codes above. > }, > }, > }, > appleModelId: <string>, // Mac only or null on failure > + sec: { // This feature is Windows-only "Windows 8+ only."

Pushed by aklotz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5dd6963e2a12 Part 1 - Add Windows Security Center info to nsSystemInfo; r=erahm,jimm https://hg.mozilla.org/integration/autoland/rev/73dca2043525 Part 2 - Add security software section to about:support; r=jimm https://hg.mozilla.org/integration/autoland/rev/75591ea83b3c Part 3 - Add Windows Security Center info to telemetry environment; r=gfritzsche

Backed out 3 changesets (bug 1418131) for Browser-chrome failure, mochitest failre and build bustage on Windows mingw https://treeherder.mozilla.org/logviewer.html#?job_id=156422382&repo=autoland&lineNumber=8622 https://hg.mozilla.org/integration/autoland/rev/c4dd70f035f664022212b841862ffe4d9cd1e96e https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=75591ea83b3c104643603a8992f2e20b1abd2bdf&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

This should be fixed now. Relanding.

Pushed by aklotz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d66bae3d5be1 Part 1 - Add Windows Security Center info to nsSystemInfo; r=erahm,jimm https://hg.mozilla.org/integration/autoland/rev/625b2d120fd9 Part 2 - Add security software section to about:support; r=jimm https://hg.mozilla.org/integration/autoland/rev/073eb94e1d21 Part 3 - Add Windows Security Center info to telemetry environment; r=gfritzsche

Requesting another backout -- mingw doesn't include the required headers, so I need to go back and disable this code for mingw.

Backed out 3 changesets (bug 1418131) for build bustages on Windows mingw https://treeherder.mozilla.org/logviewer.html#?job_id=156437827&repo=autoland&lineNumber=9463 https://hg.mozilla.org/integration/autoland/rev/412cb15c620265f7545812b2de1714e7b7ad1485 https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=073eb94e1d21ce20fa1452d65be4f4fc420fe0d8&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Pushed by aklotz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a0cf39ae0b3d Part 1 - Add Windows Security Center info to nsSystemInfo; r=erahm,jimm https://hg.mozilla.org/integration/autoland/rev/a287c5db8be6 Part 2 - Add security software section to about:support; r=jimm https://hg.mozilla.org/integration/autoland/rev/8962ed624e0c Part 3 - Add Windows Security Center info to telemetry environment; r=gfritzsche

Backed out 3 changesets (bug 1418131) for failing Browser-chrome on toolkit/modules/tests/browser/browser_Troubleshoot.js on a CLOSED TREE https://treeherder.mozilla.org/logviewer.html#?job_id=156465205&repo=autoland&lineNumber=13329 https://hg.mozilla.org/integration/autoland/rev/4ad0761df5db065e46f0716f966076c0ad00b62e https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=8962ed624e0c5e775ae46990563de641f6c6a861

OK, this should be the final push. I've run a full try unit test suite.

Pushed by aklotz@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fcad66c3d2e2 Part 1 - Add Windows Security Center info to nsSystemInfo; r=erahm,jimm https://hg.mozilla.org/integration/autoland/rev/e825d7b57a4f Part 2 - Add security software section to about:support; r=jimm https://hg.mozilla.org/integration/autoland/rev/d47ec9fc2586 Part 3 - Add Windows Security Center info to telemetry environment; r=gfritzsche

https://hg.mozilla.org/mozilla-central/rev/fcad66c3d2e2 https://hg.mozilla.org/mozilla-central/rev/e825d7b57a4f https://hg.mozilla.org/mozilla-central/rev/d47ec9fc2586