code review 001 (coming soon)

Created on 28 December 2022, over 1 year ago
Updated 16 January 2023, over 1 year ago

// Your module's code is very good I'll have to be picky to suggest changes

==> crowdsec/src/Commands/CrowdSec.php

Are you going to call the refreshRemediation via a cron too?

==> crowdsec/src/Middleware.php

The message could contain this URL to unblock IP if people think it's a mistake: https://www.crowdsec.net/remove-ip-crowdsec-blocklist.
Their case will be studied, but it's very unlikely that someone's IP reached the blocklist by mistake.
It's just an option we offer; often we inform them that their device is infected by a malware taking control for malicious activities

Question: won't this 403 trigger a signal? It's not critical but if it can be avoided, it'd be better

==> crowdsec/src/Form/Settings.php

::signalScenarios()

We could rename the signals and eventually split the whispers

  • flood : if i'm not mistaken this is authentication flood, and this would trigger when attempts threshold is reached
    • If so we could call it drupal/auth-bruteforce
  • ban : Could be named drupal/manual-ban
  • whisper : after discussing it with colleagues We think it could serve as a "http-scan" or "route-scan" signal
    • More details in the Buffer.php::addWhisperSignal() comments i'm making further down

::scenarios()
Looks good to me for a first version

::buildForm(...)

  • '#title' => $this->t('Whipser'), //typo Whisper
  • We could soften the description to:
    • 'Signal IPs generating 4xx error'
    • // We don't automatically block them, only if we observe that behaviours on many different signal sources
  • range could be renamed 'delay'

==> crowdsec/src/EventSubscriber/CrowdSec.php

::onResponse()
We need to check if 404 triggered by missing asset would generate this.
Ideally we'd send signal only for a non existing url (or route)
We could split 4xx and 5xx in 2 types of signals

==> crowdsec/src/Buffer.php
::addWhisperSignal()
We're rethinking our whisper concept and your system may be the better version of it.
Your system is very good and we could use this to detect scans

  • The idea would be to detect someone scanning your website routes
  • so we'd need to only look at 403 and 404 on php files or route and not on static resources to avoid false positives
    ** the 500 error could lead to too many false positive if a drupal is buggy

So if we set a threshold of minimum 5 in a delay of 5 secondes this could work

==> MISC
Allowing to enroll in console

  • It would ne nice to have a text field in the setting called enrollId (console enroll it)
  • If this field is not empty and enroll call has not been done yet we could make an enroll call via the capi client (watcher)
  • I'll give you more details about how to test the console and the enrollement
    this couldbe good as you'd have a "near real time" view of the signals you sent in our console
    NB enrolling and refusing (via console) or deleting from the console makes it impossible to enroll this machineId again for now unfortunatly. for testing multiple enrolls we'd need to change machineId every time (things we don't want to do in prod)

For the threshold system something that works fine in security to detect burst is the "leaky bucket"
Right now, you're removing whispers older than range and check if the count > threshold

the leaky bucket system is a little different, a bucket has:

  • a max capacity (similar to threshold)
  • a leak delay : empties by 1 unit every N secondes (delay of leak)

This allows to better catch irregular burst by making the delay/(range of time) about activity tolerence rather than a strict cut
In our agent we setup the capacity to 10 and the leak speed at 10s

We also have a notion of "blackhole" time (5m in our scan scenario case) meaning that we ignore this kind of alert for this attacking IP for 5 minutes. But this might be a bit heavier on to implement, you don't have to do this.

📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇫🇷France rr404

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

No activities found.

Production build 0.71.5 2024