malware/checks: PackageTurnover skeleton #7321

woodruffw · 2020-01-30T23:21:11Z

xmunoz · 2020-02-06T01:19:56Z

warehouse/malware/checks/package_turnover/check.py

+            if has_removed_2fa_method and not user.has_two_factor:
+                self.add_verdict(
+                    project_id=project.id,
+                    classification=VerdictClassification.Threat,


I'm not sure disabling 2fa merits the Threat classification. Maybe @ewdurbin can run these queries on prod to get an idea of how noisy this might be.

I agree, although I'll point out that it's not just disabling 2FA -- it's disabling 2FA right before a new release and not adding a new method. But yes, I think it'd be a good idea to get a feel for the noise here and lower this if it's too much.

I reviewed the last few days of activity for that tag, and was actually rather delighted to find that most instances of account:two_factor:method_removed were right before or after adding another method.

On average it's been fewer than ten instances per day.

xmunoz

Minor comment but otherwise LGTM

xmunoz · 2020-02-07T22:37:16Z

warehouse/malware/checks/package_turnover/check.py

+        # NOTE: We query for releases here, but the rest of this check is
+        # project-oriented. Maybe just query for projects directly instead?
+        releases = (
+            self.db.query(Release)


self.db.query(Release.project) perhaps?

Hmm, I'm having some trouble getting this to work, probably because project is a backref and not a realized column. I could do something like this:

self.db.query(Release.project).join(Release.project).filter(...)

but this only returns a list of bools. Thoughts?

Meh, it seems fine as is. Maybe just remove/modify the comment?

* malware/checks: PackageTurnover skeleton * malware/checks: PackageTurnover: Add NOTE * malware/checks: PackageTurnoverCheck: more work * tests: blacken * malware/checks: More PackageTurnoverCheck work * malware/checks: Blacken * malware/checks: Blacken * package_turnover: Promote from indeterminate to threat * tests: Begin adding package_turnover tests * tests: Add remaining package_turnover tests * tests: Drop unused imports * warehouse: Drop (ww) from NOTE * checks/package_turnover: Drop NOTE

woodruffw · 2020-02-11T19:57:41Z

🎉

* malware/checks: PackageTurnover skeleton * malware/checks: PackageTurnover: Add NOTE * malware/checks: PackageTurnoverCheck: more work * tests: blacken * malware/checks: More PackageTurnoverCheck work * malware/checks: Blacken * malware/checks: Blacken * package_turnover: Promote from indeterminate to threat * tests: Begin adding package_turnover tests * tests: Add remaining package_turnover tests * tests: Drop unused imports * warehouse: Drop (ww) from NOTE * checks/package_turnover: Drop NOTE

* Add new models for malware detection. (#7118) * Add new models for malware detection. Fixes #7090 and #7092. * Code review changes. - FK on release_file.id field instead of md5 - Change message type from String to Text - Change Enum class in model to singular form * Add admin interface to view and enable checks (#7134) * Add admin interface to view and enable checks - Implement list, detail and change_state views (#7133) - Add unit tests for check admin view * Add comprehensive test coverage for check admin * Add initial hook-based check execution mechanism (#7160) * Add initial hook-based check execution mechanism * scratch/poc * Add initial hook-based check execution mechanism * Use sqlalchemy event hooks for malware checks * Fix unit tests * Add enum for MalwareCheckObjectType * Add unit tests for init. * Add tests for tasks, services, and utils. Also, some small bugfixes in MalwareCheckFactory and the get_enabled_checks method. * Fix spurious task test. * Add missing drop enum to downgrade function. * Added TODO to dev/environment * Be more explicit in check lookup Co-authored-by: Ernest W. Durbin III <ewdurbin@gmail.com> * Add malware check syncing mechanism (#7190) * Add malware check syncing mechanism * Code review changes. * Refactor MalwareCheckBase. Fixes #7091. (#7196) * Refactor MalwareCheckBase. Fixes #7091. Add Foreign Keys in MalwareVerdicts for other types of objects (Releases, Projects). * Change verdict dict to kwargs. * Add wipe-out functionality (#7202) * Add wipe-out functionality Related: #7133 * Call list explicitly * Add rudimentary verdicts view. Progress on #6062. (#7207) * Add rudimentary verdicts view. Progress on #6062. Also, add some better testing logic for wiped_out condition. * Code review changes. - Conditionally show fields that are populated - JSON pretty formatting * Fix unit test bug. - Use `get` instead of `filter` to look up verdict by pkey. * simplify unit tests for verdicts view * introduce malware queue (#7227) * introduce malware queue * correct syntax, apparently list of tuples documented doesn't work. * Add backfill functionality to check admin #7094 (#7232) * Add backfill functionality to check admin #7094 - Add backfill task - Change lookup of checks to check_name instead of id - Load checks that are also in "evaluation" state * Add unit tests for backfill. - Log number of runs executed by backfill - Perform basic validation on sample_rate input - Clean up other testing logic. * Remove superfluous 'all()' * Code review changes. - Set backfill size to a fix number, not configurable via web ui. - Backfill task enqueues run_check tasks - Only retry if `check.run` fails, not if loading the check fails. - Use exponential backoff for retries. * Update warehouse/admin/templates/admin/malware/checks/detail.html Co-Authored-By: Ernest W. Durbin III <ewdurbin@gmail.com> Co-authored-by: Ernest W. Durbin III <ewdurbin@gmail.com> * Refactor testing logic #7098 (#7257) - Add `schedule` field to MalwareCheck model #7096 - Move ExampleCheck into tests/common/ to remove test dependency from prod code - Rename functions and classes to differentiate between "hooked" and "scheduled" checks * Event-based Malware check (#7249) * requirements: Introduce yara * [WIP] malware/check: SetupPatternCheck In progress. Introduces SetupPatternCheck, an implementation of an event-based check that scans the `setup.py`s of release files for suspicious patterns. * malware/checks: Give MalwareCheckBase.run/scan args, kwargs * malware: Add check preparation Fiddle with the check/run signature a bit more. * malware/checks: Unpack file path correctly * docker-compose: Override FILES_BACKEND for worker The worker needs to be able to see the "files" virtual host during development so that malware checks can fetch their underlying release files. * [WIP] malware/checks: setup.py extraction * malware/checks: setup_patterns: Fix enum, seek * malware/checks: setup_patterns: Apply YARA rules Each rule match becomes a verdict. * malware/checks: setup_patterns: Prefer get over filter * warehouse/{admin,malware}: Consistent enum names Also enforce uniqueness for enum values. * warehouse/{admin,malware}: More enum changes * tests: Update admin, malware tests * tests: Fix enum, more test fixes * tests: Add prepare tests * malware/changes: base: Unpack id correctly * tests: Begin adding SetupPatternCheck tests * malware/checks: setup_patterns: Fix enum * tests: More SetupPatternCheck tests * warehouse/malware: setup_patterns: Fix enums * tests: More SetupPatternCheck tests * tests: Add license header * malware/checks: setup_patterns: Add TODO * tests: More SetupPatternCheck tests * tests: More SetupPatternCheck tests * tests: Complete extraction tests for SetupPatternCheck * tests: Fix test * malware/checks: Add docstring for prepare * malware/checks: blacken * malware/checks: Document, expand YARA rules * tests, warehouse: Restructure utilities * malware: Order some enums, reduce SetupPatternCheck verdicts * malware/models: Add missing __lt__ * malware/checks: Always embed the model object in the prepared arguments Use it instead of performing a DB request in the check itself. * malware/checks: Avoid raw bytes * malware/changes: Remove unused import * tests: Fixup malware tests * warehouse/malware: blacken * tests: Fill in malware coverage * tests, warehouse: Add a benign verdict for SetupPatternCheck * tests: blacken * Implement scheduled checks #7093 (#7271) * Implement scheduled checks #7093 - Rename `run_backfill` to `run_evaluation` in admin malware view - Modify `run` and `scan` method signatures to accept `**kwargs` - Extend `run_check` to accomodate scheduled check functionality * Reduce unit test flakiness * Code review changes. Also replace `check.hooked_object` with `check.hooked_object.value` in check detail template. * tests, warehouse: enum fixes * Fix lint error Co-authored-by: William Woodruff <william@yossarian.net> * Add verdicts view filtering capabilities #6062. (#7322) * Add verdicts view filtering capabilities #6062. * Code review changes. - Refactor tests to be parametrized. - Pass `_query` to `route_path` in template. - Remove `is None` from filter query, it adds nothing. * Add verdict administrator review. Fixes #6062. (#7339) * Add verdict administrator review. Fixes #6062. - Add new `admin.verdicts.review` endpoint - Change layout of verdict list and detail view and add forms - Change sort order of the MalwareChecks, and update the tests * Code review changes. - Rename MalwareVerdict field `administrator_verdict` to `reviewer_verdict`. - Change verdict review permission from `admin` to `moderator`. * Misc cleanup and TODOs on malware checks. (#7355) * Misc cleanup and TODOs on malware checks. - Change backfill function to invoke `IMalwareCheckService` interface - Add support for `kwargs to `IMalwareCheckService` interface - Rename variable from reserved word `file` to `release_file` - Add `FatalCheckException` for non-retryable exceptions - Replace `MALWARE_CHECK_BACKEND` in dev/environment * Make `IMalwareService` the entrypoint for `run_check` - Add `run_scheduled_check` task that invokes this interface. - Remove useless utility method - Move `FatalCheckException` into warehouse/malware/errors.py. * malware/checks: PackageTurnover skeleton (#7321) * malware/checks: PackageTurnover skeleton * malware/checks: PackageTurnover: Add NOTE * malware/checks: PackageTurnoverCheck: more work * tests: blacken * malware/checks: More PackageTurnoverCheck work * malware/checks: Blacken * malware/checks: Blacken * package_turnover: Promote from indeterminate to threat * tests: Begin adding package_turnover tests * tests: Add remaining package_turnover tests * tests: Drop unused imports * warehouse: Drop (ww) from NOTE * checks/package_turnover: Drop NOTE Co-authored-by: Cristina <hi@xmunoz.com> Co-authored-by: William Woodruff <william@yossarian.net>

woodruffw self-assigned this Jan 30, 2020

woodruffw added the malware-detection Issues related to automated malware detection. label Jan 30, 2020

woodruffw added this to the Package signing & detection/verification milestone Jan 30, 2020

woodruffw force-pushed the ww/scheduled-suspicious-check branch from 6582a71 to 689859e Compare February 3, 2020 16:29

woodruffw marked this pull request as ready for review February 5, 2020 20:18

woodruffw requested review from xmunoz and ewdurbin February 5, 2020 20:18

xmunoz reviewed Feb 6, 2020

View reviewed changes

woodruffw force-pushed the ww/scheduled-suspicious-check branch 2 times, most recently from 534ef5a to 1ab12c1 Compare February 7, 2020 21:31

woodruffw added 12 commits February 7, 2020 16:31

malware/checks: PackageTurnover skeleton

8937b1c

malware/checks: PackageTurnover: Add NOTE

fc2f252

malware/checks: PackageTurnoverCheck: more work

3e45bee

tests: blacken

ec42d63

malware/checks: More PackageTurnoverCheck work

0c8badb

malware/checks: Blacken

a81d230

malware/checks: Blacken

ab1a808

package_turnover: Promote from indeterminate to threat

0af094c

tests: Begin adding package_turnover tests

c657f0c

tests: Add remaining package_turnover tests

34d8d6c

tests: Drop unused imports

57ab860

warehouse: Drop (ww) from NOTE

3d02ba2

woodruffw force-pushed the ww/scheduled-suspicious-check branch from 1ab12c1 to 3d02ba2 Compare February 7, 2020 21:31

xmunoz approved these changes Feb 7, 2020

View reviewed changes

checks/package_turnover: Drop NOTE

e946738

ewdurbin approved these changes Feb 11, 2020

View reviewed changes

ewdurbin merged commit f9567b4 into pypi:malware-detection Feb 11, 2020

woodruffw deleted the ww/scheduled-suspicious-check branch February 11, 2020 19:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

malware/checks: PackageTurnover skeleton #7321

malware/checks: PackageTurnover skeleton #7321

woodruffw commented Jan 30, 2020 •

edited

Loading

xmunoz Feb 6, 2020 •

edited

Loading

woodruffw Feb 6, 2020

ewdurbin Feb 7, 2020

xmunoz left a comment •

edited

Loading

xmunoz Feb 7, 2020

woodruffw Feb 8, 2020

xmunoz Feb 8, 2020

woodruffw commented Feb 11, 2020

malware/checks: PackageTurnover skeleton #7321

malware/checks: PackageTurnover skeleton #7321

Conversation

woodruffw commented Jan 30, 2020 • edited Loading

xmunoz Feb 6, 2020 • edited Loading

Choose a reason for hiding this comment

woodruffw Feb 6, 2020

Choose a reason for hiding this comment

ewdurbin Feb 7, 2020

Choose a reason for hiding this comment

xmunoz left a comment • edited Loading

Choose a reason for hiding this comment

xmunoz Feb 7, 2020

Choose a reason for hiding this comment

woodruffw Feb 8, 2020

Choose a reason for hiding this comment

xmunoz Feb 8, 2020

Choose a reason for hiding this comment

woodruffw commented Feb 11, 2020

woodruffw commented Jan 30, 2020 •

edited

Loading

xmunoz Feb 6, 2020 •

edited

Loading

xmunoz left a comment •

edited

Loading