From 4d56d25f111f87998ee223032a2d7c936ed471ec Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Mon, 27 Nov 2023 18:23:01 -0300 Subject: [PATCH] show FV banner if the disk is encrypted but we don't get a key (#15317) for #15068 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/` or `orbit/changes/`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/15068-host-disk-encryption | 2 ++ server/datastore/mysql/hosts.go | 6 ++++- server/datastore/mysql/hosts_test.go | 10 +++++++ server/service/osquery_utils/queries.go | 28 +++++++++++++++----- server/service/osquery_utils/queries_test.go | 3 +++ 5 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 changes/15068-host-disk-encryption diff --git a/changes/15068-host-disk-encryption b/changes/15068-host-disk-encryption new file mode 100644 index 0000000000..7768ef6e5c --- /dev/null +++ b/changes/15068-host-disk-encryption @@ -0,0 +1,2 @@ +* Fixed a bug causing the disk encryption key banner to not appear if the host + had disk encryption turned on manually without FV escrow. diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 425c554464..a52be1f91c 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -3294,7 +3294,11 @@ VALUES (?, ?, ?, ?) ON DUPLICATE KEY UPDATE /* if the key has changed, set decrypted to its initial value so it can be calculated again if necessary (if null) */ - decryptable = IF(base64_encrypted = VALUES(base64_encrypted), decryptable, VALUES(decryptable)), + decryptable = IF( + base64_encrypted = VALUES(base64_encrypted) AND base64_encrypted != '', + decryptable, + VALUES(decryptable) + ), base64_encrypted = VALUES(base64_encrypted), client_error = VALUES(client_error) `, hostID, encryptedBase64Key, clientError, decryptable) diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index aebcfe61f3..2be76f30ce 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -6798,6 +6798,16 @@ func testHostsSetOrUpdateHostDisksEncryptionKey(t *testing.T, ds *Datastore) { err = ds.SetOrUpdateHostDiskEncryptionKey(context.Background(), host3.ID, "ghi", "", ptr.Bool(false)) require.NoError(t, err) checkEncryptionKeyStatus(t, ds, host3.ID, "ghi", ptr.Bool(false)) + + // set an empty key (backfill for issue #15068) + err = ds.SetOrUpdateHostDiskEncryptionKey(context.Background(), host3.ID, "", "", nil) + require.NoError(t, err) + checkEncryptionKeyStatus(t, ds, host3.ID, "", nil) + + // setting the decryptable value works even if the key is still empty + err = ds.SetOrUpdateHostDiskEncryptionKey(context.Background(), host3.ID, "", "", ptr.Bool(false)) + require.NoError(t, err) + checkEncryptionKeyStatus(t, ds, host3.ID, "", ptr.Bool(false)) } func testHostsSetDiskEncryptionKeyStatus(t *testing.T, ds *Datastore) { diff --git a/server/service/osquery_utils/queries.go b/server/service/osquery_utils/queries.go index 4eaf4ce2a7..a82af9e931 100644 --- a/server/service/osquery_utils/queries.go +++ b/server/service/osquery_utils/queries.go @@ -19,6 +19,7 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/publicip" "github.com/fleetdm/fleet/v4/server/fleet" apple_mdm "github.com/fleetdm/fleet/v4/server/mdm/apple" + "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/service/async" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" @@ -1505,9 +1506,16 @@ func directIngestDiskEncryptionKeyFileDarwin( return nil } - // it's okay if the key comes empty, this can happen and if the disk is - // encrypted it means we need to reset the encryption key - return ds.SetOrUpdateHostDiskEncryptionKey(ctx, host.ID, rows[0]["filevault_key"], "", nil) + // at this point we know that the disk is encrypted, if the key is + // empty then the disk is not decryptable. For example an user might + // have removed the `/var/db/FileVaultPRK.dat` or the computer might + // have been encrypted without FV escrow enabled. + var decryptable *bool + base64Key := rows[0]["filevault_key"] + if base64Key == "" { + decryptable = ptr.Bool(false) + } + return ds.SetOrUpdateHostDiskEncryptionKey(ctx, host.ID, base64Key, "", decryptable) } // directIngestDiskEncryptionKeyFileLinesDarwin ingests the FileVault key from the `file_lines` @@ -1556,9 +1564,17 @@ func directIngestDiskEncryptionKeyFileLinesDarwin( return ctxerr.Wrap(ctx, err, "decoding hex string") } - // it's okay if the key comes empty, this can happen and if the disk is - // encrypted it means we need to reset the encryption key - return ds.SetOrUpdateHostDiskEncryptionKey(ctx, host.ID, base64.StdEncoding.EncodeToString(b), "", nil) + // at this point we know that the disk is encrypted, if the key is + // empty then the disk is not decryptable. For example an user might + // have removed the `/var/db/FileVaultPRK.dat` or the computer might + // have been encrypted without FV escrow enabled. + var decryptable *bool + base64Key := base64.StdEncoding.EncodeToString(b) + if base64Key == "" { + decryptable = ptr.Bool(false) + } + + return ds.SetOrUpdateHostDiskEncryptionKey(ctx, host.ID, base64Key, "", decryptable) } func directIngestMacOSProfiles( diff --git a/server/service/osquery_utils/queries_test.go b/server/service/osquery_utils/queries_test.go index 6be0eb1732..ce447175ee 100644 --- a/server/service/osquery_utils/queries_test.go +++ b/server/service/osquery_utils/queries_test.go @@ -1184,6 +1184,9 @@ func TestDirectIngestDiskEncryptionKeyDarwin(t *testing.T) { if host.ID != hostID { return errors.New("host ID mismatch") } + if encryptedBase64Key == "" && (decryptable == nil || *decryptable == true) { + return errors.New("decryptable should be false if the key is empty") + } return nil }