darwin/firewall: Fixes for alf_exceptions, make alf_services an alias for sharing_preferences #5378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akindyakov
reviewed
Jan 31, 2019
There was a problem hiding this comment.
Thank you for your contribution, @woodruffw !
Traditional question about table integration tests, may I ask you to update it?
|
Alright, I've added integration tests for |
|
Anything else I can do to help get this merged, @akindyakov? |
|
Polite ping on this. Is there anything blocking another review/merge? |
theopolis
requested changes
Sep 30, 2019
|
|
||
| /// Parse a nested (base-64 encoded) plist's alias data for its path | ||
| /// Observed experimentally with /Library/Preferences/com.apple.alf.plist | ||
| Status pathFromNestedPlistAliasData(const std::string& data, |
There was a problem hiding this comment.
Do you mind uploading test data into ./tests and making a unit test for this new parsing.
I can +1 to the observation, it's the same format on a 10.14.6 test device.
There was a problem hiding this comment.
Yeah, let me see if I can whip up a sanitized version of a local sample.
|
Looking good! Last thing needed is the unit test. |
|
Looks like the CI is failing on Linux due to disk exhaustion? |
|
@mike-myers-tob or @Smjert, see above about disk exhaustion. Is this something we have control over within Azure? |
Some plists (like com.apple.alf) contain a weird nested (base64-encoded) plist, which in turn contains a single NSData value. This commit updates `filterPlist` to handle this NSData case, and adds a new convenience method for extracting the POSIX path from the alias data of these weird nested plists.
Should make it a bit easier to debug these in the future, if Apple decides to change the format up.
The alf_services row was generated from an apparently unused/meaningless array in the ALF plist. sharing_preferences represents the same services and has accurate status information, so just substitute it instead.
The `alf_services` table is now an alias for `sharing_preferences`, so the functionality test should just go there.
theopolis
approved these changes
Oct 4, 2019
muffins
pushed a commit
to muffins/osquery
that referenced
this pull request
Oct 15, 2019
… for sharing_preferences (osquery#5378)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds the
pathFromNestedPlistAliasDatafunction to handle an edge case in parsing some of thealiasfields in the ALF plist, and turnsalf_servicesinto an alias forsharing_preferences(the original backing plist was vestigial and didn't accurately represent the state of the application firewall).