Skip to content

Commit 61e852d

Browse files
mobula9mobula9
committed
fix large ip ranges bug
1 parent f0c8606 commit 61e852d

File tree

6 files changed

+132
-71
lines changed

6 files changed

+132
-71
lines changed

‎src/ApiCache.php

+82-45
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use DateTime;
88
use IPLib\Address\AddressInterface;
9+
use IPLib\Address\Type;
910
use IPLib\Factory;
1011
use IPLib\Range\Subnet;
1112
use Psr\Log\LoggerInterface;
@@ -245,14 +246,49 @@ private function defferUpdateCacheConfig(array $config): void
245246
$this->adapter->saveDeferred($cacheConfigItem);
246247
}
247248

249+
/**
250+
* Update the cached remediation of the specified IP from these new decisions.
251+
*/
252+
private function saveRemediationsForIp(array $decisions, string $ip): string
253+
{
254+
$remediationResult = Constants::REMEDIATION_BYPASS;
255+
if (\count($decisions)) {
256+
foreach ($decisions as $decision) {
257+
if (!\in_array($decision['type'], Constants::ORDERED_REMEDIATIONS)) {
258+
$this->logger->warning('', ['type' => 'UNKNOWN_REMEDIATION', 'unknown' => $decision['type'], 'fallback' => $this->fallbackRemediation]);
259+
$decision['type'] = $this->fallbackRemediation;
260+
}
261+
$remediation = $this->formatRemediationFromDecision($decision);
262+
$type = $remediation[0];
263+
$exp = $remediation[1];
264+
$id = $remediation[2];
265+
$remediationResult = $this->addRemediationToCacheItem($ip, $type, $exp, $id);
266+
}
267+
} else {
268+
$remediation = $this->formatRemediationFromDecision(null);
269+
$type = $remediation[0];
270+
$exp = $remediation[1];
271+
$id = $remediation[2];
272+
$remediationResult = $this->addRemediationToCacheItem($ip, $type, $exp, $id);
273+
}
274+
$this->commit();
275+
276+
return $remediationResult;
277+
}
278+
248279
/**
249280
* Update the cached remediations from these new decisions.
250281
*/
251-
private function saveRemediations(array $decisions): bool
282+
private function saveRemediations(array $decisions): array
252283
{
284+
$errors = [];
253285
foreach ($decisions as $decision) {
254286
$remediation = $this->formatRemediationFromDecision($decision);
255287
$type = $remediation[0];
288+
if (!\in_array($remediation[0], Constants::ORDERED_REMEDIATIONS)) {
289+
$this->logger->warning('', ['type' => 'UNKNOWN_REMEDIATION', 'unknown' => $remediation[0], 'fallback' => $this->fallbackRemediation]);
290+
$remediation[0] = $this->fallbackRemediation;
291+
}
256292
$exp = $remediation[1];
257293
$id = $remediation[2];
258294

@@ -262,7 +298,17 @@ private function saveRemediations(array $decisions): bool
262298
} elseif ('Range' === $decision['scope']) {
263299
$range = Subnet::fromString($decision['value']);
264300

301+
$addressType = $range->getAddressType();
302+
$isIpv6 = (Type::T_IPv6 === $addressType);
303+
if ($isIpv6 || ($range->getNetworkPrefix() < 24)) {
304+
$error = ['type' => 'DECISION_RANGE_TO_ADD_IS_TOO_LARGE', 'decision' => $decision['id'], 'range' => $decision['value'], 'remediation' => $type, 'expiration' => $exp];
305+
$errors[] = $error;
306+
$this->logger->warning('', $error);
307+
continue;
308+
}
265309
$comparableEndAddress = $range->getEndAddress()->getComparableString();
310+
311+
$comparableEndAddress = $range->getComparableEndString();
266312
$address = $range->getStartAddress();
267313
$this->addRemediationToCacheItem(Bouncer::formatIpAsCacheKey($address), $type, $exp, $id);
268314
$ipCount = 1;
@@ -277,11 +323,12 @@ private function saveRemediations(array $decisions): bool
277323
}
278324
}
279325

280-
return $this->commit();
326+
return ['success' => $this->commit(), 'errors' => $errors];
281327
}
282328

283-
private function removeRemediations(array $decisions): int
329+
private function removeRemediations(array $decisions): array
284330
{
331+
$errors = [];
285332
$count = 0;
286333
foreach ($decisions as $decision) {
287334
if ('Ip' === $decision['scope']) {
@@ -298,7 +345,16 @@ private function removeRemediations(array $decisions): int
298345
} elseif ('Range' === $decision['scope']) {
299346
$range = Subnet::fromString($decision['value']);
300347

301-
$comparableEndAddress = $range->getEndAddress()->getComparableString();
348+
$addressType = $range->getAddressType();
349+
$isIpv6 = (Type::T_IPv6 === $addressType);
350+
if ($isIpv6 || ($range->getNetworkPrefix() < 24)) {
351+
$error = ['type' => 'DECISION_RANGE_TO_REMOVE_IS_TOO_LARGE', 'decision' => $decision['id'], 'range' => $decision['value']];
352+
$errors[] = $error;
353+
$this->logger->warning('', $error);
354+
continue;
355+
}
356+
357+
$comparableEndAddress = $range->getComparableEndString();
302358
$address = $range->getStartAddress();
303359
if (!$this->removeDecisionFromRemediationItem(Bouncer::formatIpAsCacheKey($address), $decision['id'])) {
304360
$this->logger->debug('', ['type' => 'DECISION_TO_REMOVE_NOT_FOUND_IN_CACHE', 'decision' => $decision['id']]);
@@ -333,37 +389,7 @@ private function removeRemediations(array $decisions): int
333389

334390
$this->commit();
335391

336-
return $count;
337-
}
338-
339-
/**
340-
* Update the cached remediation of the specified IP from these new decisions.
341-
*/
342-
private function saveRemediationsForIp(array $decisions, string $ip): string
343-
{
344-
$remediationResult = Constants::REMEDIATION_BYPASS;
345-
if (\count($decisions)) {
346-
foreach ($decisions as $decision) {
347-
if (!\in_array($decision['type'], Constants::ORDERED_REMEDIATIONS)) {
348-
$this->logger->warning('', ['type' => 'UNKNOWN_REMEDIATION', 'unknown' => $decision['type'], 'fallback' => $this->fallbackRemediation]);
349-
$decision['type'] = $this->fallbackRemediation;
350-
}
351-
$remediation = $this->formatRemediationFromDecision($decision);
352-
$type = $remediation[0];
353-
$exp = $remediation[1];
354-
$id = $remediation[2];
355-
$remediationResult = $this->addRemediationToCacheItem($ip, $type, $exp, $id);
356-
}
357-
} else {
358-
$remediation = $this->formatRemediationFromDecision(null);
359-
$type = $remediation[0];
360-
$exp = $remediation[1];
361-
$id = $remediation[2];
362-
$remediationResult = $this->addRemediationToCacheItem($ip, $type, $exp, $id);
363-
}
364-
$this->commit();
365-
366-
return $remediationResult;
392+
return ['count' => $count, 'errors' => $errors];
367393
}
368394

369395
public function clear(): bool
@@ -387,10 +413,12 @@ public function clear(): bool
387413
* Warm the cache up.
388414
* Used when the stream mode has just been activated.
389415
*
390-
* @return int number of decisions added
416+
* @return array "count": number of decisions added, "errors": decisions not added
391417
*/
392-
public function warmUp(): int
418+
public function warmUp(): array
393419
{
420+
$addErrors = [];
421+
394422
if ($this->warmedUp) {
395423
$this->clear();
396424
}
@@ -401,7 +429,9 @@ public function warmUp(): int
401429

402430
$nbNew = 0;
403431
if ($newDecisions) {
404-
$this->warmedUp = $this->saveRemediations($newDecisions);
432+
$saveResult = $this->saveRemediations($newDecisions);
433+
$addErrors = $saveResult['errors'];
434+
$this->warmedUp = $saveResult['success'];
405435
$this->defferUpdateCacheConfig(['warmed_up' => $this->warmedUp]);
406436
$this->commit();
407437
if (!$this->warmedUp) {
@@ -416,20 +446,25 @@ public function warmUp(): int
416446
$this->commit();
417447
$this->logger->info('', ['type' => 'CACHE_WARMED_UP', 'added_decisions' => $nbNew]);
418448

419-
return $nbNew;
449+
return ['count' => $nbNew, 'errors' => $addErrors];
420450
}
421451

422452
/**
423453
* Used in stream mode only.
424454
* Pull decisions updates from the API and update the cached remediations.
425455
* Used for the stream mode when we have to update the remediations list.
426456
*
427-
* @return array number of deleted and new decisions
457+
* @return array number of deleted and new decisions, and errors when processing decisions
428458
*/
429459
public function pullUpdates(): array
430460
{
461+
$deletionErrors = [];
462+
$addErrors = [];
431463
if (!$this->warmedUp) {
432-
return ['deleted' => 0, 'new' => $this->warmUp()];
464+
$warmUpResult = $this->warmUp();
465+
$addErrors = $warmUpResult['errors'];
466+
467+
return ['deleted' => 0, 'new' => $warmUpResult['count'], 'deletionErrors' => $deletionErrors, 'addErrors' => $addErrors];
433468
}
434469

435470
$this->logger->debug('', ['type' => 'START_CACHE_UPDATE']);
@@ -439,18 +474,21 @@ public function pullUpdates(): array
439474

440475
$nbDeleted = 0;
441476
if ($deletedDecisions) {
442-
$nbDeleted = $this->removeRemediations($deletedDecisions);
477+
$removingResult = $this->removeRemediations($deletedDecisions);
478+
$deletionErrors = $removingResult['errors'];
479+
$nbDeleted = $removingResult['count'];
443480
}
444481

445482
$nbNew = 0;
446483
if ($newDecisions) {
447-
$this->saveRemediations($newDecisions);
484+
$saveResult = $this->saveRemediations($newDecisions);
485+
$addErrors = $saveResult['errors'];
448486
$nbNew = \count($newDecisions);
449487
}
450488

451489
$this->logger->debug('', ['type' => 'CACHE_UPDATED', 'deleted' => $nbDeleted, 'new' => $nbNew]);
452490

453-
return ['deleted' => $nbDeleted, 'new' => $nbNew];
491+
return ['deleted' => $nbDeleted, 'new' => $nbNew, 'deletionErrors' => $deletionErrors, 'addErrors' => $addErrors];
454492
}
455493

456494
/**
@@ -468,7 +506,6 @@ private function miss(string $ipToQuery, string $cacheKey): string
468506
$decisions = $this->apiClient->getFilteredDecisions(['ip' => $ipToQuery]);
469507
}
470508

471-
472509
return $this->saveRemediationsForIp($decisions, $cacheKey);
473510
}
474511

‎src/Bouncer.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,9 @@ public static function getCaptchaHtmlTemplate(bool $error, string $captchaImageS
161161
* Used in stream mode only.
162162
* This method should be called only to force a cache warm up.
163163
*
164-
* @return int number of decisions added
164+
* @return array "count": number of decisions added, "errors": decisions not added
165165
*/
166-
public function warmBlocklistCacheUp(): int
166+
public function warmBlocklistCacheUp(): array
167167
{
168168
return $this->apiCache->warmUp();
169169
}
@@ -172,7 +172,7 @@ public function warmBlocklistCacheUp(): int
172172
* Used in stream mode only.
173173
* This method should be called periodically (ex: crontab) in a asynchronous way to update the bouncer cache.
174174
*
175-
* @return array number of deleted and new decisions
175+
* @return array number of deleted and new decisions, and errors when processing decisions
176176
*/
177177
public function refreshBlocklistCache(): array
178178
{

‎src/RestClient.php

+1
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ public function request(
9999
'method' => $method,
100100
'uri' => $this->baseUri.$endpoint,
101101
'content' => 'POST' === $method ? $config['http']['content'] : null,
102+
//'header' => $header, # Do not display header to avoid logging sensible data
102103
]);
103104

104105
$response = file_get_contents($this->baseUri.$endpoint, false, $context);

0 commit comments

Comments
 (0)