From baf897757800413a7b8c8276ed6a23f9891db5cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Sun, 1 Dec 2024 19:19:04 +0100 Subject: [PATCH] Correctly handle IP addresses containing RFC 4007 scoping --- .../LogSystem/SecurityEventLogEntry.php | 4 +- src/Entity/LogSystem/UserLoginLogEntry.php | 5 +- src/Entity/LogSystem/UserLogoutLogEntry.php | 4 +- src/Helpers/IPAnonymizer.php | 49 +++++++++++++++++++ tests/Helpers/IPAnonymizerTest.php | 44 +++++++++++++++++ 5 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 src/Helpers/IPAnonymizer.php create mode 100644 tests/Helpers/IPAnonymizerTest.php diff --git a/src/Entity/LogSystem/SecurityEventLogEntry.php b/src/Entity/LogSystem/SecurityEventLogEntry.php index 1b65ca0b..12e8e65e 100644 --- a/src/Entity/LogSystem/SecurityEventLogEntry.php +++ b/src/Entity/LogSystem/SecurityEventLogEntry.php @@ -44,9 +44,9 @@ namespace App\Entity\LogSystem; use App\Entity\Base\AbstractDBElement; use App\Entity\UserSystem\User; use App\Events\SecurityEvents; +use App\Helpers\IPAnonymizer; use Doctrine\ORM\Mapping as ORM; use InvalidArgumentException; -use Symfony\Component\HttpFoundation\IpUtils; /** * This log entry is created when something security related to a user happens. @@ -134,7 +134,7 @@ class SecurityEventLogEntry extends AbstractLogEntry public function setIPAddress(string $ip, bool $anonymize = true): self { if ($anonymize) { - $ip = IpUtils::anonymize($ip); + $ip = IPAnonymizer::anonymize($ip); } $this->extra['i'] = $ip; diff --git a/src/Entity/LogSystem/UserLoginLogEntry.php b/src/Entity/LogSystem/UserLoginLogEntry.php index db62eb01..0719a740 100644 --- a/src/Entity/LogSystem/UserLoginLogEntry.php +++ b/src/Entity/LogSystem/UserLoginLogEntry.php @@ -22,8 +22,9 @@ declare(strict_types=1); namespace App\Entity\LogSystem; +use App\Helpers\IPAnonymizer; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\HttpFoundation\IpUtils; + /** * This log entry is created when a user logs in. @@ -59,7 +60,7 @@ class UserLoginLogEntry extends AbstractLogEntry public function setIPAddress(string $ip, bool $anonymize = true): self { if ($anonymize) { - $ip = IpUtils::anonymize($ip); + $ip = IPAnonymizer::anonymize($ip); } $this->extra['i'] = $ip; diff --git a/src/Entity/LogSystem/UserLogoutLogEntry.php b/src/Entity/LogSystem/UserLogoutLogEntry.php index 275b1d4e..f9f9a3dc 100644 --- a/src/Entity/LogSystem/UserLogoutLogEntry.php +++ b/src/Entity/LogSystem/UserLogoutLogEntry.php @@ -22,8 +22,8 @@ declare(strict_types=1); namespace App\Entity\LogSystem; +use App\Helpers\IPAnonymizer; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\HttpFoundation\IpUtils; #[ORM\Entity] class UserLogoutLogEntry extends AbstractLogEntry @@ -56,7 +56,7 @@ class UserLogoutLogEntry extends AbstractLogEntry public function setIPAddress(string $ip, bool $anonymize = true): self { if ($anonymize) { - $ip = IpUtils::anonymize($ip); + $ip = IPAnonymizer::anonymize($ip); } $this->extra['i'] = $ip; diff --git a/src/Helpers/IPAnonymizer.php b/src/Helpers/IPAnonymizer.php new file mode 100644 index 00000000..9662852f --- /dev/null +++ b/src/Helpers/IPAnonymizer.php @@ -0,0 +1,49 @@ +. + */ + +declare(strict_types=1); + + +namespace App\Helpers; + +use Symfony\Component\HttpFoundation\IpUtils; + +/** + * Utils to assist with IP anonymization. + * The IPUtils::anonymize has a certain edgecase with local-link addresses, which is handled here. + * See: https://github.com/Part-DB/Part-DB-server/issues/782 + */ +final class IPAnonymizer +{ + public static function anonymize(string $ip): string + { + /** + * If the IP contains a % symbol, then it is a local-link address with scoping according to RFC 4007 + * In that case, we only care about the part before the % symbol, as the following functions, can only work with + * the IP address itself. As the scope can leak information (containing interface name), we do not want to + * include it in our anonymized IP data. + */ + if (str_contains($ip, '%')) { + $ip = substr($ip, 0, strpos($ip, '%')); + } + + return IpUtils::anonymize($ip); + } +} \ No newline at end of file diff --git a/tests/Helpers/IPAnonymizerTest.php b/tests/Helpers/IPAnonymizerTest.php new file mode 100644 index 00000000..464d6f18 --- /dev/null +++ b/tests/Helpers/IPAnonymizerTest.php @@ -0,0 +1,44 @@ +. + */ + +namespace App\Tests\Helpers; + +use App\Helpers\IPAnonymizer; +use PHPUnit\Framework\TestCase; + +class IPAnonymizerTest extends TestCase +{ + + public function anonymizeDataProvider(): \Generator + { + yield ['127.0.0.0', '127.0.0.23']; + yield ['2001:0db8:85a3::', '2001:0db8:85a3:0000:0000:8a2e:0370:7334']; + //RFC 4007 format + yield ['fe80::', 'fe80::1fc4:15d8:78db:2319%enp4s0']; + } + + /** + * @dataProvider anonymizeDataProvider + */ + public function testAnonymize(string $expected, string $input): void + { + $this->assertSame($expected, IPAnonymizer::anonymize($input)); + } +}