Add support for basic chassis information #5282

jjerger · 2018-11-06T18:32:52Z

This will query basic chassis_information such as equipped alarms and breach status/description should there be one.

I have not yet included an integration test because I am not sure which fields to verify as not null - this WMI class can return alot of null values depending on what the chassis is equipped with. I was hoping for some input on what should be verified for this table to be considered sane as well as my approach to handling the boolean values (resetting a value each time for example).

osquery/tables/system/windows/chassis_info.cpp

obelisk · 2018-11-13T22:43:51Z

osquery/tables/system/windows/chassis_info.cpp

+    if (!wmiResults.empty()) {
+      for (const auto& data : wmiResults) {
+        Row r;
+        bool boolean = false;


Rename this :p Also auto

I struggled to find a good name for this since it's so generic. Any ideas here? What if I just said "condition"? Or maybe "isPresent" ? That sounds very c# though

Perhaps an endrun, these seem to be more commonly represented as INTs

You know, revisiting this after a long hiatus made me realize "boolean" in this case relates to items in wmiResults, not a variable that is tracking if I want to bail out.

I'm renaming this to isPresent, because it will represent if a certain feature exists on the machine (wmiResults will have true/false for this field). If there's any thought on a better variable name, let me know.

Consequently, I'm going to add a bail condition to the for loop.

osquery/tables/system/windows/chassis_info.cpp

obelisk · 2018-11-13T22:44:22Z

specs/windows/chassis_info.table

+implementation("chassis_info@genChassisInfo")
+examples([
+  "select * from chassis_info",
+])


specs/windows/chassis_info.table

jjerger · 2018-12-13T22:08:39Z

Anything else I need to change? I'd like to upstream this by sometime early January if possible

jjerger · 2019-02-21T17:47:40Z

Oops, I forgot I let this one slip. Is there anything else I needed to change and any response wrt to the questions re: flipping the logic and returning early?

muffins · 2019-09-15T03:35:52Z

@jjerger sorry we let this one slip! Any chance you could rebase this off of the latest master, make the "return early" changes suggested and we'll get this merged? Otherwise the code looked good to me.

jjerger · 2019-09-23T22:20:48Z

Oh man I totally forgot about this one. I can probably do that, just gotta find the time. It's been busy on my end the last few months.

…

On Sat, Sep 14, 2019 at 8:36 PM Nick Anderson ***@***.***> wrote: @jjerger <https://github.com/jjerger> sorry we let this one slip! Any chance you could rebase this off of the latest master, make the "return early" changes suggested and we'll get this merged? Otherwise the code looked good to me. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5282?email_source=notifications&email_token=AISPRFYHSF6R6WAD66HY72DQJWUT7A5CNFSM4GCED4I2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6XIOHQ#issuecomment-531531550>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AISPRF27PTJ53FMFRNJG2P3QJWUT7ANCNFSM4GCED4IQ> .

-- * • **James Jerger* * • **Technical Solutions Engineer* * • *Google LLC * • *[email protected] • +1 425 655 4867

jjerger · 2020-05-07T23:32:00Z

Holy cow I really need to get on this.

Are we still okay to rebase off the master, flip the return early changes and get this merged? I can get moving on that.

directionless · 2020-05-07T23:52:42Z

Yes. Rebasing on master is correct. Early returns are preferred. Happy to re-review with those.

linux-foundation-easycla · 2020-05-27T23:12:44Z

✅ James Jerger (6de917d, cac2eb7, ea63fbd, a5d5037, 79cf990, 0627839, a981308)
✅ Ivan Tse (5d83002, 4a4d52d)
✅ Jason Meller (a7655d3, 309ca4a)
✅ Oleg Dolgov (d3a10a3, 2bf31f3)
✅ Ateeq Sharfuddin (b63dc57, 8ca769c)
✅ seph (85ac165, 6a6d08a, ae66d8f, 1374238, f54d904)
✅ Teddy Reed (036ef01, 983d2a3, bb861fb, 46a46da, 7920c2e, ab8c1d8, a977045, 702a6e0, 34c8ac3, da7c95b, dec9a17, d8e4d5e, 247f8d6, 25e0088)
✅ flappy-sh (d9faba2)
✅ Alessandro Gario (7560e3c)
✅ Exane Server Team (f61c0ca)
✅ Breakwell (aeaf624, b35d968, cd6ebcd, 2993321, 0e5b048, 8937442, 63a52f2, fb6b514, bae089e, 089becf, a154e45)
✅ scoders-tob (4e5dcd9)
✅ Nick Collier (09c5246)
✅ Stefano Bonicatti (58be58c, 17aa6f2, 0409360, 2e84e8c, 7ff9cf5, 29697f6, 8b8462d, e70de5b, ef2763a, 296c6d3, c22ab5c, 5a2ad26, 10e6938, dcca9f1, 52f310a, e5f9a8d, 62f9938, 46c1e6d, d2c9300, fd7cfbd, 0855a5f, 69f8877, c84d288, 3d4102c, a07140a, 4d4957f, 6e08fdb, a5104a4, 9373b30, baa8ebe, b64a63f)
✅ puffyCid (d974bb3)
✅ outSH (b428e4e)
✅ Robbie Ostrow (3486bf3)
✅ kumarak (58d0671)
✅ Christopher Bennett (c3e052c)
✅ Zachary Wasserman (4e17ede, 499d775)
✅ Gaëtan (a9b5df4)
✅ Rich (a851d85)
✅ Reed Loden (9e116e5)
✅ srikwit (3bdbdaf, ff1227f)
✅ Chris Broome (26d94ce)
✅ Scott Lundgren (e237619)
✅ Ryan Mack (c6f811b)
✅ uptycs-nishant (6c75b04)
✅ Mike Myers (c75ae5b, 8a9fb05)
✅ Nick Anderson (d1feab7)

❌The commit (8a9896a ,a8956a751ac03e684ce1f21a670496b655c553c6 ,8afbf249406d26eaddfcfdc8115837e5ec824257) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve. For further assistance with EasyCLA, please submit a support request ticket.

jjerger · 2020-05-27T23:17:42Z

Ah, regarding CLAs, I left and rejoined Google in the time since I opened this, so I will need to re-link my github account to the org. I should have that handled in the next day or so.

osquery/tables/system/windows/chassis_info.cpp

theopolis · 2020-05-31T21:07:32Z

Hopefully two more steps and this is ready to go!

The Linux builds are failing due to code formatting, apply the patch:

diff --git a/osquery/tables/system/windows/chassis_info.cpp b/osquery/tables/system/windows/chassis_info.cpp
index 3682cd80..c0e2e975 100644
--- a/osquery/tables/system/windows/chassis_info.cpp
+++ b/osquery/tables/system/windows/chassis_info.cpp
@@ -24,7 +24,7 @@ QueryData genChassisInfo(QueryContext& context) {
   WmiRequest wmiSystemReq("select * from Win32_SystemEnclosure");
   const auto& wmiResults = wmiSystemReq.results();
 
-  //check if the results are empty and return a warning if so
+  // check if the results are empty and return a warning if so
   if (wmiResults.empty()) {
     LOG(WARNING) << wmiSystemReq.getStatus().getMessage();
     return results;
@@ -41,7 +41,7 @@ QueryData genChassisInfo(QueryContext& context) {
     r["chassis_types"] = INTEGER(number);
     data.GetString("Description", r["description"]);
 
-    //reset boolean to make sure there is no interference from the last call
+    // reset boolean to make sure there is no interference from the last call
     boolean = false;
 
     data.GetBool("LockPresent", boolean);
@@ -55,7 +55,7 @@ QueryData genChassisInfo(QueryContext& context) {
     data.GetString("SKU", r["sku"]);
     data.GetString("Status", r["status"]);
 
-    //reset boolean to make sure there is no interference from the last call
+    // reset boolean to make sure there is no interference from the last call
     boolean = false;
 
     data.GetBool("VisibleAlarm", boolean);

Or run the build target format_check.

Add an integration test for the new table. Take a look here for a recent example: https://github.com/osquery/osquery/pull/5539/files but ignore all of the Buck stuff.

osquery/tables/system/windows/chassis_info.cpp

jjerger · 2020-06-01T22:56:28Z

Uh oh, I think i botched the rebase from your master (this feels familiar, thought I'd learned).

Perhaps I'm best nuking the current branch and opening a PR under a fresh one properly rebased?

Smjert · 2020-06-02T01:09:43Z

Uh oh, I think i botched the rebase from your master (this feels familiar, thought I'd learned).

Perhaps I'm best nuking the current branch and opening a PR under a fresh one properly rebased?

You can push force, something like:

// Being checked out on the PR branch, rename it
git branch -M chassis_info-backup
// Switch to master and "recreate" the branch
git checkout master
git checkout -b chassis_info
// Might not be necessary, but tell the branch which remote to track (substitute origin with the PR remote)
git branch -u origin/chassis_info
// Get the top commit with the PR implementation from the old branch
git cherry-pick chassis_info-backup
// Push force the PR branch
git push --force

…o chassis_info

jjerger · 2020-06-02T23:54:13Z

Ok after that mess of rebasing I think I have it current again and added an integration test. I haven't added one of those in ages, so please sanity check it to make sure I made the right assertions.

Smjert · 2020-06-03T21:18:52Z

@jjerger You can verify the formatting of the code via cmake --build . --target format_check from the build folder (it doesn't require anything to be built, but the CMake configure has to have been run).

We also have cmake --build . --target format to actually format the code how we would like, though keep in mind that this is meant to be run before committing, which means that it doesn't work on already committed code.
Although you can go back in time via interactive rebase + reset soft to format and then recommit the commits that needed formatting.

EDIT: Check https://osquery.readthedocs.io/en/latest/development/building/ Formatting the code for more info, also being on Windows you would have to install C++ Clang tools for Windows, as stated in the Windows pre-requisites for building.

jjerger · 2020-06-03T21:25:04Z

I set up a new Windows instance following those prereqs so it should have them all. I'll verify with that.

jjerger · 2020-06-03T22:27:14Z

Oddly enough clang format was formatting in a way that caused it to fail check, so that's interesting.

I see Wix errors - is there a formatting command that needs to be done through Wix? I have no experience with that toolset and I didn't see much mention of it in the build doc (it's only used on Release build by the look of it?)

jjerger · 2020-06-03T23:56:22Z

Actually, checking the outupt it looks like Wix is missing from the test container?

mike-myers-tob · 2020-06-04T01:27:12Z

Actually, checking the outupt it looks like Wix is missing from the test container?

Is it? I see passing Wix tests:

theopolis · 2020-06-04T01:49:21Z

Heads up I added a quick-fix since the integration test code was not being built.

…o chassis_info

…s configs recently got reset

jjerger · 2020-06-05T01:02:55Z

It seems to be failing on WmiRequest but I am not quite sure why.. I used the same logic as I had used in previous tables but chassis_info is returning false for wmiSystemReq.getStatus().ok() resulting in an empty result set.

This isn't being caught in the build process but surfacing in the table integration check. Am I missing something here?

theopolis · 2020-06-05T01:08:32Z

Does the WMI query not work on a VM? If that is the case then the integration test should only apply the validation if there is a non-empty result set.

jjerger · 2020-06-05T01:13:50Z

I suppose it depends on the hypervisor, since GCP instances do return some info, although almost nothing.

Maybe Azure doesn't? The part that got me thinking about this was The Windows Release Output indicating that the table didn't exist and that status.ok() was false. I figured this was referring to the query that built the table?

When I moved it to a physical desktop and rebuilt it, I saw the same behavior. No error during build, but if I check osqueryi, there is no chassis_info table at all. Win32_SystemEnclosure is in the Root\CIMv2 namespace so we shouldn't have to specify it, right?

tests/integration/tables/chassis_info.cpp

…s_info cause I think it was also failing to retrieve the status

…o chassis_info

jjerger · 2020-06-09T00:50:44Z

Looks like LGTM timed out running tests - not sure if they all pass.

As it stands, let's hold off on merging too. I found an error with ChassisTypes that I'm addressing.

jjerger · 2020-06-09T01:22:06Z

Related to my previous comment: I see that WmiResultItem has a way to retrieve an array of strings with GetVectorOfStrings, but is there a similar way to retrieve integers?

I'm having difficulty retrieving ChassisTypes as it is an array of UInt16 values, although I have not been able to find a device with multiple values contained within. Perhaps Surface Pros might have two with the detachable chassis? No idea.

mike-myers-tob · 2020-06-09T01:51:38Z

Looks like LGTM timed out running tests - not sure if they all pass.

The LGTM test runner doesn't run long enough to compile osquery and all of its dependencies, so right now LGTM will always fail.

outdated

jjerger · 2020-06-10T21:40:18Z

Eek! I had not yet fixed an error with ChassisTypes. It'll run as is, but the value it's returning is less than helpful.

Want me to open a separate PR with the fix to that?

directionless · 2020-06-10T22:49:49Z

Want me to open a separate PR with the fix to that?

Yes please!

…0 to master * commit 'eeee0fb0957f5af983f817c2e6f19c53108d9e09': (83 commits) Add additional changelog items (osquery#6523) Changelog for 4.4.0 (osquery#6492) build: Add Azure tables to specs CMakeLists (osquery#6507) CMake: Correct macOS framework linking (osquery#6522) tables: Only populate table cache with star-like selects (osquery#6513) CMake: Fix and cleanup compile flags (osquery#6521) docs: Add note to bump the Homebrew cask (osquery#6519) tests: Fix atom_packages, processes, rpm_packages flakiness (osquery#6518) bug: Do not use system proxy for AWS local authority (osquery#6512) packaging: updating docs on cpack usage to include Chocolatey (osquery#6022) bug: Fix typed_row table caching (osquery#6508) Implement event batching support for Windows tables (osquery#6280) http: Use sync resolve (osquery#6490) Add support for basic chassis information (osquery#5282) Only emit 'denylist' warning once (osquery#6493) docs: Remove references to brew in macOS install (osquery#6494) Fix for osquery#5890: Event Format Results and the Kafka Logger (osquery#6449) make apt_sources table parsing much more resilient (osquery#6482) Make file and hash container columns hidden (osquery#6486) Update documentation to use 'allow list' and 'deny list' diction (osquery#6489) ...

facebook-github-bot added the cla signed Automated label: Pull Request author has signed the osquery CLA label Nov 6, 2018

obelisk added the Windows label Nov 13, 2018

obelisk previously requested changes Nov 13, 2018

View reviewed changes

obelisk added the virtual tables label Nov 13, 2018

directionless reviewed May 28, 2020

View reviewed changes

osquery/tables/system/windows/chassis_info.cpp Outdated Show resolved Hide resolved

osquery/tables/system/windows/chassis_info.cpp Show resolved Hide resolved

osquery/tables/system/windows/chassis_info.cpp Outdated Show resolved Hide resolved

theopolis reviewed May 31, 2020

View reviewed changes

osquery/tables/system/windows/chassis_info.cpp Show resolved Hide resolved

theopolis requested changes May 31, 2020

View reviewed changes

osquery/tables/system/windows/chassis_info.cpp Show resolved Hide resolved

Merge branch 'chassis_info' of https://github.com/jjerger/osquery int…

e6e52b1

…o chassis_info

jjerger force-pushed the chassis_info branch from a981308 to e6e52b1 Compare June 2, 2020 23:06

jjerger added 4 commits June 2, 2020 16:12

Re-adding chassis_info.cpp after rebasing mess

efd29cf

Add newline at EOF

5143da1

Formatting fixes and reinserting isPresent in boolean place

0c30021

Add integration test for chassis_info

3cd2920

Formatting fixes

2891768

jjerger force-pushed the chassis_info branch from 391add4 to 2891768 Compare June 3, 2020 23:50

Update integration test code

81cd894

jjerger added 3 commits June 4, 2020 12:47

Move final return of results, which was likely breaking table generation

4d0b8ed

Merge branch 'chassis_info' of https://github.com/jjerger/osquery int…

32f4190

…o chassis_info

Formatting - Turns out IDE was adding blank space on newline cause it…

07b732f

…s configs recently got reset

theopolis previously approved these changes Jun 4, 2020

View reviewed changes

theopolis reviewed Jun 5, 2020

View reviewed changes

tests/integration/tables/chassis_info.cpp Show resolved Hide resolved

Add windows/chassis_info.cpp to system CMakeLists.txt

e5a55f9

theopolis dismissed their stale review via e5a55f9 June 7, 2020 01:32

jjerger added 2 commits June 8, 2020 13:25

Add chassis_info to spec cmakelist; simplify error message for chassi…

eb1dfbe

…s_info cause I think it was also failing to retrieve the status

Merge branch 'chassis_info' of https://github.com/jjerger/osquery int…

2592398

…o chassis_info

theopolis approved these changes Jun 10, 2020

View reviewed changes

theopolis merged commit d09ad2b into osquery:master Jun 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for basic chassis information #5282

Add support for basic chassis information #5282

jjerger commented Nov 6, 2018

obelisk Nov 13, 2018

jjerger Nov 19, 2018

directionless Aug 23, 2019

jjerger May 28, 2020

obelisk Nov 13, 2018

jjerger commented Dec 13, 2018

jjerger commented Feb 21, 2019

muffins commented Sep 15, 2019

jjerger commented Sep 23, 2019 via email

jjerger commented May 7, 2020

directionless commented May 7, 2020

linux-foundation-easycla bot commented May 27, 2020 •

edited

Loading

jjerger commented May 27, 2020

theopolis commented May 31, 2020 •

edited

Loading

jjerger commented Jun 1, 2020

Smjert commented Jun 2, 2020

jjerger commented Jun 2, 2020

Smjert commented Jun 3, 2020 •

edited

Loading

jjerger commented Jun 3, 2020

jjerger commented Jun 3, 2020

jjerger commented Jun 3, 2020

mike-myers-tob commented Jun 4, 2020

theopolis commented Jun 4, 2020

jjerger commented Jun 5, 2020 •

edited

Loading

theopolis commented Jun 5, 2020

jjerger commented Jun 5, 2020 •

edited

Loading

jjerger commented Jun 9, 2020

jjerger commented Jun 9, 2020

mike-myers-tob commented Jun 9, 2020

jjerger commented Jun 10, 2020

directionless commented Jun 10, 2020

Add support for basic chassis information #5282

Add support for basic chassis information #5282

Conversation

jjerger commented Nov 6, 2018

obelisk Nov 13, 2018

Choose a reason for hiding this comment

jjerger Nov 19, 2018

Choose a reason for hiding this comment

directionless Aug 23, 2019

Choose a reason for hiding this comment

jjerger May 28, 2020

Choose a reason for hiding this comment

obelisk Nov 13, 2018

Choose a reason for hiding this comment

jjerger commented Dec 13, 2018

jjerger commented Feb 21, 2019

muffins commented Sep 15, 2019

jjerger commented Sep 23, 2019 via email

jjerger commented May 7, 2020

directionless commented May 7, 2020

linux-foundation-easycla bot commented May 27, 2020 • edited Loading

jjerger commented May 27, 2020

theopolis commented May 31, 2020 • edited Loading

jjerger commented Jun 1, 2020

Smjert commented Jun 2, 2020

jjerger commented Jun 2, 2020

Smjert commented Jun 3, 2020 • edited Loading

jjerger commented Jun 3, 2020

jjerger commented Jun 3, 2020

jjerger commented Jun 3, 2020

mike-myers-tob commented Jun 4, 2020

theopolis commented Jun 4, 2020

jjerger commented Jun 5, 2020 • edited Loading

theopolis commented Jun 5, 2020

jjerger commented Jun 5, 2020 • edited Loading

jjerger commented Jun 9, 2020

jjerger commented Jun 9, 2020

mike-myers-tob commented Jun 9, 2020

jjerger commented Jun 10, 2020

directionless commented Jun 10, 2020

linux-foundation-easycla bot commented May 27, 2020 •

edited

Loading

theopolis commented May 31, 2020 •

edited

Loading

Smjert commented Jun 3, 2020 •

edited

Loading

jjerger commented Jun 5, 2020 •

edited

Loading

jjerger commented Jun 5, 2020 •

edited

Loading