Implicit withdrawals are now generated also on correct subprefix routing

[vkotronis]

Description of PR

What component(s) does this PR affect?

• Back-End (Database, Detection/Configuration/etc. Microservices)
• Front-End (Flask, API, UI, etc)
• Monitor (RIPE RIS, BGPStream RV/RIS/CAIDA, etc.)
• Docs (incl. wiki)
• Build System

Does the PR require changes on other components? If yes, please mark the components:

• Back-End (Database, Detection/Configuration/etc. Microservices)
• Front-End (Flask, API, UI, etc)
• Monitor (RIPE RIS, BGPStream RV/RIS/CAIDA, etc.)
• Docs (incl. wiki)
• Build System

Related Issue

Resolves #438

Blocks #436 #437

Solution

The main logic of this PR is the following:

1. Assume we have an active hijack for prefix N/M (where N = network address and M = prefix length) .
2. Currently, we treat correct routing updates for N/M as implicit withdrawals for the monitor that saw both the hijack and the "healed" path.
3. With this adjustment, we also treat correct routing updates for either N_1/M+1 or N_2/M+1 (where N_1, N_2 the network addresses of the direct more specifics of the original prefix) as implicit withdrawals for the hijacked N/M.

Note that in a future issue we need to carefully account for what prefix space is currently healed; this PR is an approximation for direct more specifics advertised during the deaggregation process (it is OK to assume that subprefix routing done for the entire prefix space will result in the hijack being mitigated, even if we do not wait to see all subprefix announcements).

Type

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Docs update
• None of the above

Checklist:

• I have read the contributing guide and my code conforms to the guidelines.
• This change requires a change in the documentation.
• I have updated the documentation accordingly.

[artemis-pr-bot]

🏃 Benchmark Results 🏃

• RMQ Update inserts: 463/s
• RMQ Hijack inserts: 154/s
• PG Update inserts: 194/s
• PG Update updates: 153/s
• PG Hijack inserts: 153/s

[codecov]

Codecov Report

Merging #439 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #439      +/-   ##
==========================================
+ Coverage   81.87%   81.91%   +0.03%     
==========================================
  Files           6        6              
  Lines        2047     2051       +4     
==========================================
+ Hits         1676     1680       +4     
  Misses        371      371              

Impacted Files	Coverage Δ
backend/core/database.py	94.73% <ø> (+<0.01%)	⬆️
backend/core/detection.py	97.36% <ø> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 774a932...89921ba. Read the comment docs.

[slowr]

Can you provide unittests for this part of the code? (with mocking redis)

[vkotronis]

Can you provide unittests for this part of the code? (with mocking redis)

@slowr I added the unit tests, just let me know if we have a good way of also simulating redis state on the fly (I tried mockredis but it does not create state as expected).

[vkotronis]

unit tests with mock redis exists also added.

[vkotronis]

Note that full partial withdrawal support will be implemented in 2.0.0 (issue #440)

[slowr]

LGTM! Only comment I have is for maybe moving the rules within the unit tests instead (have like a static function to set them inside the unit test itself or something)

[vkotronis]

LGTM! Only comment I have is for maybe moving the rules within the unit tests instead (have like a static function to set them inside the unit test itself or something)

Will be addressed in separate issue #441