Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filevault_status to disk_encryption table #6823

Merged
merged 2 commits into from
Dec 20, 2020

Conversation

terracatta
Copy link
Contributor

@terracatta terracatta commented Dec 19, 2020

What does the PR do?

This PR adds a new darwin-only column to the disk_encryption table called filevault_status which on Macs will return either on, off, or unknown.

Why do we need filevault_status?

Some Mac Admins often believe the terms "Filevault On" and "Encryption Enabled" are equivalent. While historically this is true, the introduction of the T2 Secure Enclave has made these terms diverge in meaning in very important ways.

On T2 Macs, even when Filevault is off, the Secure Enclave by default will encrypt the contents of the drive at line speed. This means, at rest, the drive is encrypted. In these cases the disk_encryption table in osquery correctly indicates the volume is encrypted.

However, this does not mean the data of the drive is inaccessible to someone with physical access to the Mac. In fact, in Kolide's testing, if a Mac that has FileVault off, any volume can be easily mounted using the macOS recovery OS in target disk mode, without the user's password EVEN IF THE DRIVE IS ENCRYPTED AT REST.

This is a really big deal because many people use Osquery to determine if their devices are encrypted using a methodology that makes them resistant to exfil attempts that involve physical access to the device (ex: theft, accidental loss, evil maid attacks). Right now Osquery cannot accurately report this status on all Macs with a T2 chip (which have been on the market since the introduction of the iMac Pro in Jan 2018).

When FileVault is correctly enabled, the drive contents will be inaccessible to bad-actors because they must know the user's Filevault password to unlock the drive, even for target disk mode. See this Apple Support article for more details.

Based on the above, I recommend immediately cutting a release after this PR is merged so that this crucial distinction is available to people who need it.

Other ways I could have written this PR

Not adding a new column

One thing I considered was instead of adding a new column, was to instead to change the definition of encrypted on Macs to mean any drive that didn't have Filevault enabled. While this was tempting, I think it's better for us to expose the nuance here instead of hiding it. That being said, I am still open to this option of the core team thinks its a better choice.

Making filevault_status a platform-specific column

While this was my first instinct, I opted to instead copy the established pattern of this table to simply mark (Apple) in the description along with uid and the user_uuid columns.

Edit: The platform-specific columns have been correctly documented as an extended DARWIN schema.

A Note On Testing

I have tested this table on the following Macs and am confident in its compatibility.

  • An M1 Macbook running Big Sur
  • An Intel MacBook running Big Sur
  • An Intel Macbook running High Sierra

A Note On HFS+ and other volumes

For backwards compatibility I've included a assumption that on a Mac, if non-APFS drive is encrypted, filevault is also "on". This is mostly done for ease-of-querying purposes. My basic understanding is that before the T2 chip made this complicated, FileVault and encryption basically meant the same thing. That assumption is documented in a comment.

@terracatta terracatta force-pushed the jem_add_filevault_status branch from 107ac62 to 604de42 Compare December 19, 2020 22:45
@theopolis
Copy link
Member

Making filevault_status a platform-specific column

I agree this is the right approach. Do you mind fixing our bad precedent and pulling these three columns into a platform-specific definition?

@terracatta
Copy link
Contributor Author

terracatta commented Dec 20, 2020

I agree this is the right approach. Do you mind fixing our bad precedent and pulling these three columns into a platform-specific definition?

I do not mind. Edit: All done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants