feat(origins count): Store list with scenario for origins count (#31)

crowdsecurity · Dec 13, 2024 · 60cec12 · 60cec12
1 parent 8a7430a
commit 60cec12
Show file tree

Hide file tree

Showing 7 changed files with 137 additions and 18 deletions.
diff --git a/src/AbstractRemediation.php b/src/AbstractRemediation.php
@@ -113,6 +113,17 @@ public function pruneCache(): bool
      */
     abstract public function refreshDecisions(): array;
 
+    private function handleDecisionOrigin(array $rawDecision): string
+    {
+        $origin = $this->normalize($rawDecision['origin']);
+        if (Constants::ORIGIN_LISTS === $origin) {
+            // The existence of the $rawDecision['scenario'] must be guaranteed by the validateRawDecision method
+            $origin .= Constants::ORIGIN_LISTS_SEPARATOR . $this->normalize($rawDecision['scenario']);
+        }
+
+        return $origin;
+    }
+
     protected function convertRawDecision(array $rawDecision): ?Decision
     {
         if (!$this->validateRawDecision($rawDecision)) {
@@ -121,7 +132,7 @@ protected function convertRawDecision(array $rawDecision): ?Decision
         // The existence of the following indexes must be guaranteed by the validateRawDecision method
         $value = $rawDecision['value'];
         $type = $this->normalize($rawDecision['type']);
-        $origin = $this->normalize($rawDecision['origin']);
+        $origin = $this->handleDecisionOrigin($rawDecision);
         $duration = $rawDecision['duration'];
         $scope = $this->normalize($rawDecision['scope']);
 
@@ -158,8 +169,7 @@ protected function getAllCachedDecisions(string $ip, string $country): array
         // Ask cache for Range scoped decision
         $rangeDecisions = Type::T_IPv4 === $this->getIpType($ip)
             ? $this->cacheStorage->retrieveDecisionsForIp(Constants::SCOPE_RANGE, $ip)
-            : []
-        ;
+            : [];
         // Ask cache for Country scoped decision
         $countryDecisions = $country ? $this->cacheStorage->retrieveDecisionsForCountry($country) : [];
 
@@ -499,21 +509,32 @@ private function sortDecisionsByPriority(array $decisions): array
 
     private function validateRawDecision(array $rawDecision): bool
     {
+        $result = false;
+
         if (
             !empty($rawDecision['scope'])
             && !empty($rawDecision['value'])
             && !empty($rawDecision['type'])
             && !empty($rawDecision['origin'])
             && !empty($rawDecision['duration'])
         ) {
-            return true;
+            $result = true;
+            // We don't want blocklists decisions without a scenario
+            if (
+                Constants::ORIGIN_LISTS === $rawDecision['origin']
+                && empty($rawDecision['scenario'])
+            ) {
+                $result = false;
+            }
         }
 
-        $this->logger->error('Retrieved raw decision is not as expected', [
-            'type' => 'REM_RAW_DECISION_NOT_AS_EXPECTED',
-            'raw_decision' => json_encode($rawDecision),
-        ]);
+        if (false === $result) {
+            $this->logger->error('Retrieved raw decision is not as expected', [
+                'type' => 'REM_RAW_DECISION_NOT_AS_EXPECTED',
+                'raw_decision' => json_encode($rawDecision),
+            ]);
+        }
 
-        return false;
+        return $result;
     }
 }
diff --git a/src/CapiRemediation.php b/src/CapiRemediation.php
@@ -154,6 +154,7 @@ private function handleListDecisions(array $blocklists): array
                         'type' => $type,
                         'origin' => $origin,
                         'duration' => $duration,
+                        'scenario' => $listName,
                     ];
 
                     $lastPullCacheKey = $this->getCacheStorage()->getCacheKey(

diff --git a/src/Constants.php b/src/Constants.php
@@ -66,4 +66,8 @@ class Constants extends CommonConstants
      * @var string The current version of this library
      */
     public const VERSION = 'v3.5.0';
+    /**
+     * @var string The separator between the origin and scenario for the stored origin
+     */
+    public const ORIGIN_LISTS_SEPARATOR = ':';
 }
diff --git a/tests/Constants.php b/tests/Constants.php
@@ -23,6 +23,7 @@ class Constants
     public const IP_V4 = '1.2.3.4';
     public const IP_V4_2 = '5.6.7.8';
     public const IP_V4_3 = '9.10.11.12';
+    public const IP_V4_4 = '12.13.14.15';
     public const IP_V4_2_CACHE_KEY = RemConstants::SCOPE_IP . AbstractCache::SEP . self::IP_V4_2;
     /*
      * 66051 = intdiv(ip2long(IP_V4),256)

diff --git a/tests/Unit/AppSecLapiRemediationTest.php b/tests/Unit/AppSecLapiRemediationTest.php
@@ -284,7 +284,7 @@ public function testGetAppSecRemediation($cacheType)
         // Test 0.2: exceeded body and allow action
         $remediationConfigs = ['appsec_body_size_exceeded_action' => 'allow', 'appsec_max_body_size_kb' => 1024];
         $remediation = new LapiRemediation($remediationConfigs, $this->bouncer, $this->cacheStorage, null);
-        $result = $remediation->getAppSecRemediation($appSecHeaders, str_repeat('a', 1024*1024+1));
+        $result = $remediation->getAppSecRemediation($appSecHeaders, str_repeat('a', 1024 * 1024 + 1));
         $this->assertEquals(
             Constants::REMEDIATION_BYPASS,
             $result,
@@ -293,7 +293,7 @@ public function testGetAppSecRemediation($cacheType)
         // Test 0.3: exceeded body and block action
         $remediationConfigs = ['appsec_body_size_exceeded_action' => 'block', 'appsec_max_body_size_kb' => 12];
         $remediation = new LapiRemediation($remediationConfigs, $this->bouncer, $this->cacheStorage, null);
-        $result = $remediation->getAppSecRemediation($appSecHeaders, str_repeat('a', 12*1024+1));
+        $result = $remediation->getAppSecRemediation($appSecHeaders, str_repeat('a', 12 * 1024 + 1));
         $this->assertEquals(
             Constants::REMEDIATION_BAN,
             $result,
@@ -308,7 +308,7 @@ public function testGetAppSecRemediation($cacheType)
             $originsCount,
             'Origins count should be empty'
         );
-        $result = $remediation->getAppSecRemediation($appSecHeaders, str_repeat('a', 1024*1024+1));
+        $result = $remediation->getAppSecRemediation($appSecHeaders, str_repeat('a', 1024 * 1024 + 1));
         $this->assertEquals(
             Constants::REMEDIATION_BYPASS,
             $result,

diff --git a/tests/Unit/CapiRemediationTest.php b/tests/Unit/CapiRemediationTest.php
@@ -69,6 +69,7 @@
  * @covers   \CrowdSec\RemediationEngine\CapiRemediation::handleListDecisions
  *
  * @uses \CrowdSec\RemediationEngine\Configuration\Capi::addCapiNodes
+ * @uses \CrowdSec\RemediationEngine\AbstractRemediation::handleDecisionOrigin
  *
  * @covers \CrowdSec\RemediationEngine\CapiRemediation::formatIfModifiedSinceHeader
  * @covers \CrowdSec\RemediationEngine\CapiRemediation::handleListPullHeaders

diff --git a/tests/Unit/LapiRemediationTest.php b/tests/Unit/LapiRemediationTest.php
@@ -66,6 +66,7 @@
  * @covers \CrowdSec\RemediationEngine\AbstractRemediation::getOriginsCount
  *
  * @uses \CrowdSec\RemediationEngine\AbstractRemediation::sortDecisionsByPriority
+ * @covers \CrowdSec\RemediationEngine\AbstractRemediation::handleDecisionOrigin
  *
  * @covers \CrowdSec\RemediationEngine\AbstractRemediation::updateRemediationOriginCount
  * @covers \CrowdSec\RemediationEngine\AbstractRemediation::getCacheStorage
@@ -447,8 +448,10 @@ public function testGetIpRemediationInLiveMode($cacheType)
                     $expectedCleanTime,
                     'clean-bypass-ip-1.2.3.4',
                     'clean',
-                ]]],                            // Test 6 : retrieve cached bypass
-                [AbstractCache::STORED => []]  // Test 6 : retrieve empty range
+                ]]],                            // Test 5 bis : retrieve cached bypass
+                [AbstractCache::STORED => []],  // Test 5 bis : retrieve empty range
+                [AbstractCache::STORED => []],  // Test 6 : retrieve empty IP decisions
+                [AbstractCache::STORED => []]  // Test 6 : retrieve empty range decisions
             )
         );
         $this->bouncer->method('getFilteredDecisions')->will(
@@ -478,7 +481,17 @@ public function testGetIpRemediationInLiveMode($cacheType)
                         'origin' => 'lapi',
                         'duration' => '1h',
                     ],
-                ]   // Test 4 : IPv6 range scoped
+                ],   // Test 4 : IPv6 range scoped
+                [
+                    [
+                        'scope' => 'ip',
+                        'value' => TestConstants::IP_V4_4,
+                        'type' => 'ban',
+                        'origin' => 'lists',
+                        'scenario' => 'crowdsec_proxy',
+                        'duration' => '1h',
+                    ],
+                ] // Test 6 : origin lists
             )
         );
 
@@ -502,8 +515,8 @@ public function testGetIpRemediationInLiveMode($cacheType)
             'Default ordered remediation should be as expected'
         );
 
-        // Direct LAPI call will be done only if there is no cached decisions (Test1, Test 3)
-        $this->bouncer->expects($this->exactly(3))->method('getFilteredDecisions');
+        // Direct LAPI call will be done only if there is no cached decisions (Test1, Test 3, Test 6)
+        $this->bouncer->expects($this->exactly(4))->method('getFilteredDecisions');
 
         // Test 1 (No cached items and no active decision)
         $originsCount = $remediation->getOriginsCount();
@@ -619,7 +632,7 @@ public function testGetIpRemediationInLiveMode($cacheType)
             $originsCount,
             'Origin count should be updated'
         );
-        // Test 5 : merge origins count
+        // Test 5 bis : merge origins count
         $remediation->getIpRemediation(TestConstants::IP_V4);
         $originsCount = $remediation->getOriginsCount();
         $this->assertEquals(
@@ -629,6 +642,23 @@ public function testGetIpRemediationInLiveMode($cacheType)
             $originsCount,
             'Origin count should be updated'
         );
+        // Test 6 : origin lists
+        $result = $remediation->getIpRemediation(TestConstants::IP_V4_4);
+        $this->assertEquals(
+            Constants::REMEDIATION_BAN,
+            $result,
+            'Should return a ban remediation'
+        );
+        $originsCount = $remediation->getOriginsCount();
+        $this->assertEquals(
+            [
+                'lists:crowdsec_proxy' => 1,
+                'clean' => 2,
+                'lapi' => 1,
+            ],
+            $originsCount,
+            'Origin count should be updated'
+        );
     }
 
     /**
@@ -1294,6 +1324,67 @@ public function testPrivateOrProtectedMethods()
             $decision->getIdentifier(),
             'Should have created a correct decision even with id'
         );
+        // Test 4: bad raw decision for lists
+        $rawDecisions = [
+            [
+                'value' => '1.2.3.4',
+                'origin' => 'lists',
+                'duration' => '147h',
+                'type' => 'ban',
+                'scope' => 'ip',
+            ],
+        ];
+        $result = PHPUnitUtil::callMethod(
+            $remediation,
+            'convertRawDecisionsToDecisions',
+            [$rawDecisions]
+        );
+        $this->assertCount(
+            0,
+            $result,
+            'Should return empty array'
+        );
+
+        PHPUnitUtil::assertRegExp(
+            $this,
+            '/.*400.*"type":"REM_RAW_DECISION_NOT_AS_EXPECTED"/',
+            file_get_contents($this->root->url() . '/' . $this->prodFile),
+            'Prod log content should be correct'
+        );
+        // Test 5 : with lists and scenario
+        $rawDecisions = [
+            [
+                'scope' => 'IP',
+                'value' => '1.2.3.4',
+                'type' => 'ban',
+                'origin' => 'lists',
+                'scenario' => 'crowdsec_proxy',
+                'duration' => '147h',
+            ],
+        ];
+        $result = PHPUnitUtil::callMethod(
+            $remediation,
+            'convertRawDecisionsToDecisions',
+            [$rawDecisions]
+        );
+
+        $this->assertCount(
+            1,
+            $result,
+            'Should return array'
+        );
+
+        $decision = $result[0];
+        $this->assertEquals(
+            'lists:crowdsec_proxy-ban-ip-1.2.3.4',
+            $decision->getIdentifier(),
+            'Should have created a correct decision even with lists'
+        );
+        $this->assertEquals(
+            'lists:crowdsec_proxy',
+            $decision->getOrigin(),
+            'Should have created a correct decision origin'
+        );
     }
 
     protected function tearDown(): void