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.

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

feat(links): scan threats with Google Safe Browsing #376

Merged
merged 3 commits into from
Aug 11, 2020

Conversation

LoneRifle
Copy link
Contributor

@LoneRifle LoneRifle commented Aug 7, 2020

Problem

The Virus Scan API for websites provided by Cloudmersive was far
too aggressive, declaring Google Drive links and even a well-known
international organisation as threats. We need a more effective service
that accurately classifies websites that our users link to.

Fixes #377

Solution

Replace Cloudmersive's URL scanning with Google Safe Browsing,
which is more nuanced and more consistent with the browsing experience
for most users, who are probable Chrome users.

  • Implement SafeBrowsingService, which vets a given URL against threat
    lists maintained by Google. If we have a match, log the match and
    declare the URL threat

  • Drop SafeBrowsingService into inversify, and wire that into
    UrlCheckController

Deploy Notes

New environment variables:

  • SAFE_BROWSING_KEY - the Google Cloud API key for Safe Browsing APIs
  • REDIS_SAFE_BROWSING_URI - Redis URI for caching Safe Browsing threat matches

@LoneRifle LoneRifle requested a review from liangyuanruo August 7, 2020 02:46
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

Suggest soft launching so that we don't have to rollback in case of issues. Also, before a full rollout, we're required to implement caching to avoid spamming which could easily happen in the case of a frustrated user.

@LoneRifle LoneRifle force-pushed the feat/links/safe-browsing branch from 2f36a2b to 3ca6647 Compare August 7, 2020 10:19
@LoneRifle LoneRifle requested a review from liangyuanruo August 7, 2020 12:07
@yong-jie
Copy link
Member

Will we need to modify our warnings to satisfy Google's guidelines? They list 3 things to fulfil

@LoneRifle
Copy link
Contributor Author

Will we need to modify our warnings to satisfy Google's guidelines? They list 3 things to fulfil

I think we already fulfill the first of the three ("Link is likely to be malicious"). Can I propose that we just tweak the message so that the user gets in touch with us via e-mail if this happens? We would likely be proactive in getting in touch with the user anyway, so we can fulfill the other two parts of the guidelines manually

Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

lgtm!

Replace Cloudmersive's URL scanning with Google Safe Browsing,
which is more nuanced and more consistent with the browsing experience
for most users, who are probable Chrome users.

- Implement SafeBrowsingService, which vets a given URL against threat
  lists maintained by Google. If we have a match, log the match and
  declare the URL threat

- Drop SafeBrowsingService into inversify, and wire that into
  UrlCheckController

New env vars:

`SAFE_BROWSING_KEY` - the Google Cloud API key for Safe Browsing APIs
Cache responses from Google Safe Browsing in Redis, expiring the
entries after the duration specified in the response

- Declare and implement SafeBrowsingRepository and supporting cast
- Rework SafeBrowsingService to retrieve entries from Redis, if
  absent, query Safe Browsing, adding any matches returned
- Drop everything in via inversify
@LoneRifle LoneRifle force-pushed the feat/links/safe-browsing branch from 3ca6647 to c5c7e96 Compare August 11, 2020 05:25
@LoneRifle LoneRifle merged commit ee33d62 into develop Aug 11, 2020
@LoneRifle LoneRifle deleted the feat/links/safe-browsing branch August 11, 2020 05:45
@halfwhole halfwhole mentioned this pull request Sep 30, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caching for Google Safe Browsing APIs
3 participants