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.

Sign up for GitHub

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

Jump to bottom

Add option to collect private ssh keys #78

Merged
merged 3 commits into from
Jul 19, 2023
Merged

Add option to collect private ssh keys #78

merged 3 commits into from
Jul 19, 2023

Conversation

Miauwkeru
Copy link
Contributor

  • Iterate over /Users/* in from_user_home for osx
  • Create file_filter inside the collector
  • Use a context manager to bind the module
  • Add python test modules and stuff
    (DIS-1869)

@Miauwkeru Miauwkeru requested a review from pyrco July 18, 2023 11:52
@Miauwkeru
Copy link
Contributor Author

@Zawadidone I did change the ssh code a lil bit more here. ':) To allow for a filter option

@codecov
Copy link

codecov bot commented Jul 18, 2023 •
edited
Loading

Codecov Report

Merging #78 (d23023e) into main (1c771ea) will increase coverage by 0.51%.
The diff coverage is 55.55%.

❗ Current head d23023e differs from pull request most recent head 5e92f22. Consider uploading reports for the commit 5e92f22 to get more accurate results

@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   41.95%   42.47%   +0.51%     
==========================================
  Files          21       21              
  Lines        2722     2743      +21     
==========================================
+ Hits         1142     1165      +23     
+ Misses       1580     1578       -2
Flag Coverage Δ
unittests 42.47% <55.55%> (+0.51%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
acquire/collector.py 57.26% <42.10%> (-1.02%) ⬇️
acquire/acquire.py 40.73% <62.85%> (+1.56%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Zawadidone
Copy link
Contributor

@Zawadidone I did change the ssh code a lil bit more here. ':) To allow for a filter option

Nice work! FYI I am currently working on this fox-it/dissect.target#300.

pyrco
pyrco reviewed Jul 19, 2023
View reviewed changes
Copy link
Contributor

@pyrco pyrco left a comment

Choose a reason for hiding this comment

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

Could you split this change set in 3 commits (and not squash them on merge):

  • the new/stub from_user_home() changes
  • the collector.bind_module() changes
  • the ssh private keys & file filter changes

As they are changes in different sets of fucntionality.

acquire/collector.py Outdated Show resolved Hide resolved
acquire/collector.py Outdated Show resolved Hide resolved
acquire/collector.py Outdated Show resolved Hide resolved
acquire/collector.py Outdated Show resolved Hide resolved
@Miauwkeru Miauwkeru force-pushed the DIS-1869_add-option-to-collect-private-keys branch 2 times, most recently from 34de75d to 3594f7f Compare July 19, 2023 09:42
@Miauwkeru Miauwkeru requested a review from pyrco July 19, 2023 09:42
@Miauwkeru Miauwkeru force-pushed the DIS-1869_add-option-to-collect-private-keys branch from 3594f7f to a7940cf Compare July 19, 2023 10:55
pyrco
pyrco reviewed Jul 19, 2023
View reviewed changes
acquire/acquire.py Outdated Show resolved Hide resolved
@Miauwkeru Miauwkeru force-pushed the DIS-1869_add-option-to-collect-private-keys branch 2 times, most recently from dbb8e50 to d23023e Compare July 19, 2023 11:42
@Miauwkeru Miauwkeru requested a review from pyrco July 19, 2023 11:42
pyrco
pyrco requested changes Jul 19, 2023
View reviewed changes
acquire/acquire.py Show resolved Hide resolved
acquire/acquire.py Outdated Show resolved Hide resolved
acquire/acquire.py Show resolved Hide resolved
acquire/acquire.py Outdated
yield homedir


MISC_MAPPING = {"windows": misc_windows_user_homes}
Copy link
Contributor

@pyrco pyrco Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We can add osx here preemptively, so when target.user_details works, we can just remove the code at the start of from_user_home() and be done right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I fixed that part

@Miauwkeru
Add misc utilities for unix and osx systems
3e62236 
(DIS-1869)
@Miauwkeru
Add a bind_module context manager to bind the collector to a Module…
8af7918 
… class

(DIS-1869)
@Miauwkeru
Add the option to acquire private openssh keys
5e92f22 
Additionally, it adds the options to filter out
certain paths if the filter function evaluates to `True`

(DIS-1869)
@Miauwkeru Miauwkeru force-pushed the DIS-1869_add-option-to-collect-private-keys branch from d23023e to 5e92f22 Compare July 19, 2023 13:52
@Miauwkeru Miauwkeru requested a review from pyrco July 19, 2023 13:58
@Miauwkeru Miauwkeru merged commit 1c1cee0 into main Jul 19, 2023
16 checks passed
@Miauwkeru Miauwkeru deleted the DIS-1869_add-option-to-collect-private-keys branch July 19, 2023 14:27
@Miauwkeru Miauwkeru mentioned this pull request Aug 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pyrco pyrco pyrco approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Miauwkeru @Zawadidone @pyrco