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

Windows careful bitlocker selection #18189

Merged
merged 7 commits into from Apr 12, 2024

Conversation

dantecatalfamo
Copy link
Member

@dantecatalfamo dantecatalfamo commented Apr 10, 2024

#17796

Checklist for submitter

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
  • Added/updated tests No updates needed
  • Manual QA for all new/changed functionality

Fixes an issue in windows server where selecting from `bitlocker_info`
will cause the query to abort. Bitlocker is not available by default
on some version of windows server, so we first check if the optional
component is enabled before making our query
@dantecatalfamo dantecatalfamo marked this pull request as ready for review April 10, 2024 19:55
@dantecatalfamo dantecatalfamo requested a review from a team as a code owner April 10, 2024 19:55
-- Bitlocker is not optional, and should be on the device
SELECT 1 FROM bitlocker_info WHERE drive_letter = 'C:' AND protection_status = 1
) END
) SELECT enabled FROM encrypted WHERE enabled IS NOT NULL;`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome query! have you tried using a discovery query?

A distributed discovery query controls whether or not a distributed query will be executed on a host.

  • If a distributed query has no corresponding discovery query, then it is always executed on the host.
  • If a discovery query returns one or more results, then its corresponding distributed query will be executed on the host.
  • If a discovery query returns no results, then its corresponding distributed query will not be executed on the host.

In our system, you can define a DiscoveryQuery as part of the struct:

// Discovery is the SQL query that defines whether the query will run on the host or not.
// If not set, Fleet makes sure the query will always run.
Discovery string

sorry if you already tried this! if it didn't work, could you add a comment or comment here why for future reference?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can rewrite it to use two discovery queries if you think it makes more sense!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary! I'm just curious if you considered it and the rationale for not using it (as that was my expectation based on the other queries we have)

I don't think you can do two discovery queries, but I was thinking a single query that does all the conditional logic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would need two queries then, each with their own discovery queries, one for systems where bitlocker is optional and one for systems where bitlocker is built-in. I actually hadn't considered using a discovery query when I was writing this 😅. I'll try doing a quick rewrite and see if it looks any cleaner!

Copy link
Member

@roperzh roperzh Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dantecatalfamo two queries would be super confusing, so if we can't do it in a single query please don't bother. This is fine as-is.

Just for fun, I was thinking something on the lines of (quick and dirty, haven't tested) for the discovery query

SELECT 1
WHERE NOT EXISTS (SELECT 1 FROM windows_optional_features WHERE name = 'BitLocker')
AND EXISTS (SELECT 1 FROM windows_optional_features WHERE state = 1);

roperzh
roperzh previously approved these changes Apr 11, 2024
roperzh
roperzh previously approved these changes Apr 11, 2024
Copy link
Member

@roperzh roperzh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sweet!

@dantecatalfamo dantecatalfamo merged commit 80c906a into main Apr 12, 2024
16 checks passed
@dantecatalfamo dantecatalfamo deleted the windows-careful-bitlocker-selection branch April 12, 2024 14:00
sharon-fdm pushed a commit that referenced this pull request Apr 15, 2024
#17796

Fixes an issue in windows server where selecting from `bitlocker_info`
will cause the query to abort. Bitlocker is not available by default
on some version of windows server, so we first check if the optional
component is enabled before making our query
sharon-fdm pushed a commit that referenced this pull request Apr 16, 2024
#17796

Fixes an issue in windows server where selecting from `bitlocker_info`
will cause the query to abort. Bitlocker is not available by default
on some version of windows server, so we first check if the optional
component is enabled before making our query
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants