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

Initial implementation. #1

Merged

ChrisSchinnerl merged 25 commits into main from ivo/impl

Nov 30, 2021

Collaborator

ro-tex commented Oct 28, 2021 •

edited

Loading

PULL REQUEST

Overview

This PR introduces the initial implementation of the service.

What is supported so far is:

submit a new skylink for scanning: POST /scan/:skylink
- the skylink will be stored in MongoDB
- it will be automatically downloaded and streamed to ClamAV (without being stored on disk)
- after being scanned, the skylink itself will be removed from MongoDB and only its hash will be kept

Planned follow-up features:

list all known malicious skylinks' hashes
scan skylinks for CSAM as well

Known limitations:

currently only base64 skylinks are suported. This depends on Add support for base32 skylinks skynet-accounts#98

Example for Visual Changes

Checklist

Review and complete the checklist to ensure that the PR is complete before assigned to an approver.

All new methods or updated methods have clear docstrings
Testing added or updated for new methods
Verify if any changes impact the WebPortal Health Checks
Approriate documentation updated
Changelog file created

Issues Closed


          Initial implementation.

b7bc075

ro-tex self-assigned this

ro-tex requested review from ChrisSchinnerl and MSevey

October 28, 2021 14:04

MSevey reviewed

View reviewed changes

api/handlers.go Outdated Show resolved Hide resolved

MSevey reviewed

View reviewed changes

api/handlers.go Outdated Show resolved Hide resolved

MSevey reviewed

View reviewed changes

api/handlers.go Outdated Show resolved Hide resolved

MSevey reviewed

View reviewed changes

api/api.go Outdated Show resolved Hide resolved

MSevey reviewed

View reviewed changes

api/api.go Show resolved Hide resolved

MSevey reviewed

View reviewed changes

database/db.go Show resolved Hide resolved

MSevey reviewed

View reviewed changes

database/db.go Show resolved Hide resolved

MSevey reviewed

View reviewed changes

api/handlers.go Outdated Show resolved Hide resolved

MSevey reviewed

View reviewed changes

database/skylink.go Show resolved Hide resolved

MSevey reviewed

View reviewed changes

api/handlers.go Outdated Show resolved Hide resolved

MSevey suggested changes

View reviewed changes

Contributor

MSevey left a comment

mostly docstring updates but the one comment about updating the entries is the main thing to address.

ro-tex added 4 commits

November 3, 2021 11:00


          Add docstrings.

f1f9212

Add a Makefile with lints and lint configuration.


          String status.

0f5c36d


          Categorise files after scanning.

5ac9b4b


          Replace skylink update with fetch-edit-save and add flow validation.

7d8fd9f

ro-tex requested a review from MSevey

November 3, 2021 14:41

ro-tex added 2 commits

November 3, 2021 17:01


          Store the skylink's hash in the DB and remove the string representati…

a95c6c9

…on, once the skylink has been scanned.


          Make the hash unique.

f4a98c6

Make the skylink string not unique (most of them will be "").

ro-tex changed the title ~~Initial implementation.~~ WIP: Initial implementation.

ro-tex removed request for ChrisSchinnerl and MSevey

November 9, 2021 14:37

ro-tex added 5 commits

November 9, 2021 16:53


          Implement an integration with ClamAV and a scanner loop.

903584e


          Rename project to malware-scanner.

a8775dd

Fix lint and other minor issues.


          Implement an unlocker background task.

e1c1555

Sleep between scans if there was nothing to scan.


          Current an import loop.

3b19cba


          Anticipate ClamAV nullpointers.

2787ee3

Connect to nearest MongoDB replica.

ro-tex changed the title ~~WIP: Initial implementation.~~ Initial implementation.

ChrisSchinnerl reviewed

View reviewed changes

database/db.go Outdated Show resolved Hide resolved

database/db.go Outdated Show resolved Hide resolved

database/db.go Outdated Show resolved Hide resolved

database/db.go Outdated Show resolved Hide resolved

database/db.go Show resolved Hide resolved

database/db.go Show resolved Hide resolved

database/skylink.go Outdated Show resolved Hide resolved

api/api.go Outdated Show resolved Hide resolved

clamav/clamav.go Show resolved Hide resolved

ro-tex requested a review from ChrisSchinnerl

November 15, 2021 17:41

ro-tex added 2 commits

November 15, 2021 18:46


          Address PR comments.

84b5619


          Use HEAD /skynet/skylink/:skylink instead of GET /skynet/metadata/:sk…

49f2da5

…ylink.

ro-tex requested a review from peterjan

November 16, 2021 12:30

ChrisSchinnerl reviewed

View reviewed changes

ChrisSchinnerl left a comment

Looks good overall but requires some testing.

ro-tex added 2 commits

November 17, 2021 13:20


          Mark which skylinks have been fully scanned, what's their size, and w…

3870e5a

…hether we've tried all possible offsets on the underlying merkle root.


          Add unit tests for the skylink package.

0177ac0

MSevey previously approved these changes

View reviewed changes

Contributor

MSevey left a comment

I'm OK with my comments being follow ups to be able to review the iterations in more detail. Overall structure is good.

.golangci.yml Outdated Show resolved Hide resolved

.golangci.yml Outdated Show resolved Hide resolved

.golangci.yml Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

api/handlers.go Outdated Show resolved Hide resolved

scanner/scanner.go Show resolved Hide resolved

database/db.go Outdated Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Show resolved Hide resolved

ro-tex added 2 commits

November 25, 2021 17:47


          User client options to connect to mongodb.

b6f8907


          Minor fixes.

e0de669

ro-tex mentioned this pull request

Restrict the incoming skylinks to V1 only? #5

Open


          Address PR comments.

256b4fc

ro-tex requested a review from ChrisSchinnerl

November 26, 2021 14:50


          On scanning error, sleep for a while. The sleep period increases with…

f5bca32

… subsequent errors from 100ms to 100s in 4 steps.

ChrisSchinnerl requested changes

View reviewed changes

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

scanner/scanner.go Show resolved Hide resolved

scanner/scanner.go Outdated Show resolved Hide resolved

main.go Outdated Show resolved Hide resolved


          Address PR comments.

7caae7a

ro-tex dismissed MSevey’s stale review via

7caae7a

November 29, 2021 11:52

ro-tex requested review from ChrisSchinnerl and MSevey

November 29, 2021 11:52

ro-tex added 2 commits

November 29, 2021 13:53


          Add a ticker and if-else chain.

f04d6aa


          Add a separate goroutine that cancels the scanning.

283d174

ro-tex mentioned this pull request

F/U: Move the timeout backoff to a function #6

Open

ChrisSchinnerl approved these changes

View reviewed changes

scanner/scanner.go Show resolved Hide resolved

scanner/scanner.go Show resolved Hide resolved

MSevey approved these changes

View reviewed changes

Contributor

MSevey left a comment

Let's handle all the comments as follow ups

ro-tex added a commit that referenced this pull request


          Reduce code indentation. (f/u from #1)

a44c50c

ChrisSchinnerl mentioned this pull request

A check for portal - sure, makes sense. #7

Closed

ChrisSchinnerl merged commit cee102d into main

ro-tex deleted the ivo/impl branch

November 30, 2021 09:11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet