Bug 2051904 - Report disk encryption in device posture and about:support - #1375
Bug 2051904 - Report disk encryption in device posture and about:support#1375gcp wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds cross-platform disk-encryption detection to enterprise builds and surfaces the aggregated status both in enterprise device posture reporting and in about:support’s Security Software section.
Changes:
- Introduce a new
DiskEncryptionJS wrapper and XPCOM-backed Rust implementation to detect encryption state (Windows BitLocker, macOS FileVault, Linux dm-crypt/ZFS). - Extend Troubleshoot snapshot +
about:supportUI and tests to display disk-encryption status/method. - Extend enterprise device posture payload and enterprise tests to include
diskEncryption.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| toolkit/modules/Troubleshoot.sys.mjs | Adds disk-encryption status to Troubleshoot security software snapshot on enterprise builds. |
| toolkit/modules/tests/xpcshell/xpcshell.toml | Registers new xpcshell coverage for DiskEncryption module. |
| toolkit/modules/tests/xpcshell/test_DiskEncryption.js | Adds xpcshell tests for JS wrapper timeout/normalization/error handling. |
| toolkit/modules/tests/browser/browser_Troubleshoot.js | Extends Troubleshoot snapshot schema to include diskEncryption object. |
| toolkit/modules/moz.build | Installs DiskEncryption enterprise JS module on enterprise builds. |
| toolkit/modules/DiskEncryption.sys.mjs | New JS wrapper for the XPCOM disk-encryption checker with timeout handling. |
| toolkit/locales/en-US/toolkit/enterprise/enterprise.ftl | Adds localized strings for disk-encryption row values in about:support. |
| toolkit/content/aboutSupport.xhtml | Adds the Disk Encryption row to the Security Software table. |
| toolkit/content/aboutSupport.js | Renders disk-encryption status into about:support via Fluent and toggles row visibility. |
| toolkit/components/felt/rust/src/process.rs | Factors bounded subprocess execution + budget helper into a shared module. |
| toolkit/components/felt/rust/src/lib.rs | Wires in new disk-encryption modules and shared process helper. |
| toolkit/components/felt/rust/src/edr_checker.rs | Removes inlined process-spawn helper now shared in process.rs. |
| toolkit/components/felt/rust/src/edr_checker_win.rs | Switches to shared run_command_bounded helper. |
| toolkit/components/felt/rust/src/edr_checker_macos.rs | Switches to shared run_command_bounded helper. |
| toolkit/components/felt/rust/src/edr_checker_linux.rs | Switches to shared run_command_bounded helper. |
| toolkit/components/felt/rust/src/disk_encryption.rs | New XPCOM component coordinating async detection, caching, and concurrent callers. |
| toolkit/components/felt/rust/src/disk_encryption_win.rs | Windows BitLocker-based detection across fixed volumes. |
| toolkit/components/felt/rust/src/disk_encryption_macos.rs | macOS FileVault + internal fixed-volume encryption detection. |
| toolkit/components/felt/rust/src/disk_encryption_linux.rs | Linux dm-crypt stack walking plus Btrfs/ZFS/swap handling. |
| toolkit/components/felt/rust/nsIDiskEncryptionChecker.idl | New scriptable XPCOM interfaces for disk-encryption queries. |
| toolkit/components/felt/rust/moz.build | Adds IDL and Windows helper source to felt build. |
| toolkit/components/felt/rust/FeltDiskEncryptionWin.cpp | Implements unprivileged BitLocker property read via Windows shell property store. |
| toolkit/components/felt/rust/felt.h | Exposes disk-encryption checker constructor symbol. |
| toolkit/components/felt/rust/components.conf | Registers new disk-encryption checker XPCOM component/contract. |
| toolkit/components/felt/rust/Cargo.toml | Adds plist (macOS) and tempfile (tests) dependencies; extends winapi features. |
| toolkit/components/enterprise/modules/DevicePosture.sys.mjs | Adds diskEncryption field to device posture payload assembly. |
| testing/enterprise/test_felt_device_posture.py | Validates diskEncryption shape/values in enterprise device posture tests. |
| Cargo.lock | Locks new Rust dependency entries for plist and tempfile. |
| browser/base/content/test/about/browser_aboutSupport.js | Adds browser test asserting the disk-encryption row renders correctly for status/method cases. |
Suppressed comments (1)
toolkit/locales/en-US/toolkit/enterprise/enterprise.ftl:38
- Like the “enabled” message, this defaults to the FileVault variant (
*[filevault]), which would mislabel other platforms if$methodis missing/unexpected. Consider adding an explicit[filevault]branch and an*[other]fallback with generic text.
[bitlocker] Partial (BitLocker); some mounted fixed volumes are not encrypted
[dm-crypt] Partial (dm-crypt); some mounted fixed volumes are not encrypted
[zfs] Partial (ZFS); some mounted fixed volumes are not encrypted
*[filevault] Partial (FileVault); some mounted fixed volumes are not encrypted
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
toolkit/modules/DiskEncryption.sys.mjs:63
statusandmethodcome straight from the native component and are used to build UI l10n IDs and posture payloads. If the component ever returns an unexpected status (or a non-empty method with status "unknown"), callers can end up referencing missing Fluent IDs or reporting inconsistent shapes. Consider normalizing to the documented set here (fallback to {status:"unknown", method:null} when invalid).
onComplete(status, method) {
finish({ status, method: method || null });
},
toolkit/components/felt/rust/src/disk_encryption.rs:81
- The PR description defines
status: "in-progress"as "A volume is being encrypted or decrypted". Inaggregate(), a converting secondary volume is currently ignored if any other secondary volume is unencrypted (it reportspartialinstead). That seems to contradict the documented meaning when both conditions are true. Either update the aggregation precedence to preferInProgresswhen any inspected volume is converting, or adjust the documented meaning/tests to match the intended precedence.
VolumeState::Encrypted => match others {
None => EncryptionStatus::Unknown,
Some(states) if states.contains(&VolumeState::Unencrypted) => EncryptionStatus::Partial,
Some(states) if states.contains(&VolumeState::Converting) => {
EncryptionStatus::InProgress
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
toolkit/components/felt/rust/src/disk_encryption_win.rs:106
fixed_volumes()decides whether a volume is fixed by callingGetDriveTypeWon the mount path returned byGetVolumePathNamesForVolumeNameW. If the first mount point is a directory mount (e.g.C:\mounts\vol\),GetDriveTypeWwill report the drive type ofC:\, not the mounted volume, which can misclassify removable volumes as fixed (and affect the aggregated encryption status). Use the volume GUID path (\\?\Volume{...}\) when checking the drive type instead.
let guid_path = from_wide(&name);
match first_mount_path(&guid_path) {
Ok(Some(mount_path)) => {
if is_fixed_drive(&mount_path) {
volumes.push(Volume {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 31 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
toolkit/components/felt/rust/src/disk_encryption.rs:83
- When the boot volume is encrypted, secondary volumes with
VolumeState::Unknownare currently treated asEnabled. That can incorrectly report "enabled" even though one or more fixed volumes could not be inspected; per the documented posture values, inability to determine should surface asunknown(unless a known unencrypted or converting volume already determinespartial/in-progress).
Some(states) if states.contains(&VolumeState::Unencrypted) => EncryptionStatus::Partial,
Some(states) if states.contains(&VolumeState::Converting) => {
EncryptionStatus::InProgress
}
Some(_) => EncryptionStatus::Enabled,
toolkit/components/felt/rust/src/process.rs:101
- This timeout test currently allows up to
budget + PROBE_TIMEOUT(~5s) of runtime, which would still pass even ifrun_command_withinaccidentally waited the full probe timeout instead of the provided budget. Tightening the assertion will make the test actually catch regressions in budget handling.
let budget = Duration::from_millis(200);
let start = Instant::now();
assert!(run_command_within("/bin/sleep", &["300"], budget).is_none());
assert!(start.elapsed() < budget + PROBE_TIMEOUT);
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 34 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
browser/base/content/test/about/browser_aboutSupport.js:112
- This test case also uses an unrecognized
method: "unexpected", but the JS wrapper rejects unknown methods and will reportstatus: "unknown"instead. That means this case will never render the expected "Partial …" string.
Remove this case so the test matches DiskEncryption.getStatus() normalization.
{
status: "in-progress",
method: "bitlocker",
text: "Encryption or decryption in progress",
},
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 34 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
browser/base/content/test/about/browser_aboutSupport.js:154
- This test calls
Troubleshoot.snapshot()once per CASE, which also triggers the enterprise EDR probe on enterprise builds. That adds unnecessary external probing (and potential 30s timeouts) for a test that only validates disk-encryption rendering, making it slower/flakier than needed.
Consider taking the snapshot with includeEnterpriseSecurity: false and explicitly populating snapshot.securitySoftware.diskEncryption via DiskEncryption.getStatus() (which will use the mocked XPCOM component), so the test stays focused and avoids EDR detection entirely.
const snapshot = await Troubleshoot.snapshot();
content.wrappedJSObject.snapshotFormatters.securitySoftware(
Cu.cloneInto(snapshot.securitySoftware, content)
);
… r?#firefox-desktop-core-reviewers Move `run_command_bounded` into a shared process module.
6f99c80 to
1bb1448
Compare
There was a problem hiding this comment.
🟡 Changes recommended
A new browser test references AppConstants without importing it, which will cause a ReferenceError and break the test run.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 31/32 changed files
- Comments generated: 1
- Review effort level: Lite
…eviewers r?#supply-chain-reviewers Inspect the operating-system volume and other mounted fixed volumes, including active Linux swap. Use unprivileged platform APIs and expose full, enabled, partial, disabled, in-progress, and unknown through an asynchronous XPCOM service. Run detection off the main thread, coalesce concurrent callers, and cache results. A volume that cannot be inspected keeps the result at enabled rather than full. Resolve btrfs and ZFS through the mount source, since a pooled filesystem reports an anonymous device number rather than the storage beneath it, and report ZFS native encryption as a method of its own. Trust LUKS mappings without reading the mapping table, which the kernel only discloses to root, and attest plain dm-crypt mappings when possible. A loop-backed root, as seen by a snap-confined browser, is unknown. The plist crate only parses diskutil output on macOS and is vetted safe-to-run for felt.
…ore-reviewers Wrap the callback-based disk-encryption checker in a Promise with a 30-second timeout. Resolve failures and timeouts as an unknown status.
…fox-desktop-core-reviewers Collect disk-encryption status with the other slow probes and include it in every device-posture payload.
…-desktop-core-reviewers r?#webcompat-reviewers Expose disk-encryption status through Troubleshoot and show it in about:support. Keep inconclusive results visible. Remote troubleshooting and Report Broken Site opt out of the enterprise probes.
1bb1448 to
4bf7d47
Compare
|
Accepted limitations
|
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces substantial new cross-platform native detection logic (Rust + Windows C++) and a new XPCOM surface area that warrants final human review for correctness and platform-specific edge cases.
Review details
- Files reviewed: 31/32 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Verified on macOS and Linux, need to test on Windows still. |
Description
Bugzilla: Bug 2051904
Add disk-encryption status to enterprise device posture:
{ "diskEncryption": { "status": "full", "method": "dm-crypt" } }The first patch moves
run_command_boundedout of the EDR checker and into a shared process module. Disk-encryption detection reuses this machinery to run platform probes with bounded execution time and output.Firefox checks the operating-system volume, other mounted fixed volumes, and active Linux swap. This is not a root-only or remotely attested signal: an encrypted
/with an unencrypted, separately mounted/homeis reported aspartial.The same result is shown in the Security Software section of
about:supportfor troubleshooting.Posture values
statusfullstatusenabledstatuspartialstatusdisabledstatusin-progressstatusunknownmethodfilevaultmethodbitlockermethoddm-cryptmethodzfsmethodnullPlatform coverage
Btrfs is not reported as an encryption method because Btrfs does not provide native filesystem encryption. An encrypted Btrfs filesystem is reported as
dm-cryptwhen its member devices are protected by dm-crypt.Detection runs off the main thread. Concurrent callers share a scan, and results are cached to limit repeated platform probes.
Testing
Tests cover status aggregation, dm-crypt device stacks, multi-device Btrfs, native and dm-crypt-backed ZFS, Linux swap, FileVault, BitLocker, the JavaScript API, device-posture reporting, and
about:support.