From de2282a3ad6edc60fc6c18ed8d6585d568c3fbde Mon Sep 17 00:00:00 2001 From: Jesper Kristensen Date: Tue, 17 Feb 2026 16:28:55 +0100 Subject: [PATCH 1/2] Fix IP validation in digital signature file download (CIDR support) --- CHANGELOG.md | 2 + .../os2forms_digital_signature.module | 59 +++++++++++++++++-- .../os2forms_digital_signature.services.yml | 13 ++++ .../src/Form/SettingsForm.php | 5 +- 4 files changed, 73 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6f3e756..89a90989 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ before starting to add changes. Use example [placed in the end of the page](#exa ## [Unreleased] +- Fix IP validation in digital signature file download (CIDR support) + ## [5.0.0] 2025-11-18 - [PR-192](https://github.com/OS2Forms/os2forms/pull/192) diff --git a/modules/os2forms_digital_signature/os2forms_digital_signature.module b/modules/os2forms_digital_signature/os2forms_digital_signature.module index 6e12a210..49f9a496 100644 --- a/modules/os2forms_digital_signature/os2forms_digital_signature.module +++ b/modules/os2forms_digital_signature/os2forms_digital_signature.module @@ -57,21 +57,70 @@ function os2forms_digital_signature_file_download($uri) { $config = \Drupal::config(SettingsForm::$configName); $allowedIps = $config->get('os2forms_digital_signature_submission_allowed_ips'); - $allowedIpsArr = explode(',', $allowedIps); - $remoteIp = Drupal::request()->getClientIp(); + $allowedIpsArr = array_map('trim', explode(',', $allowedIps)); + // Remove empty entries (e.g. from trailing comma or empty config). + $allowedIpsArr = array_filter($allowedIpsArr); + $remoteIp = \Drupal::request()->getClientIp(); - // IP list is empty, or request IP is allowed. - if (empty($allowedIpsArr) || in_array($remoteIp, $allowedIpsArr)) { + // IP list is empty, allow access. + if (empty($allowedIpsArr)) { $basename = basename($uri); return [ 'Content-disposition' => 'attachment; filename="' . $basename . '"', ]; } - // Otherwise - Deny access. + // Check if remote IP matches any allowed IP or CIDR range. + foreach ($allowedIpsArr as $allowedIp) { + if ($remoteIp === $allowedIp || os2forms_digital_signature_ip_in_cidr($remoteIp, $allowedIp)) { + $basename = basename($uri); + return [ + 'Content-disposition' => 'attachment; filename="' . $basename . '"', + ]; + } + } + + // Deny access and log warning. + \Drupal::logger('os2forms_digital_signature')->warning('File download denied for IP @ip on URI @uri. Allowed IPs: @allowed', [ + '@ip' => $remoteIp, + '@uri' => $uri, + '@allowed' => $allowedIps, + ]); + return -1; } // Not submission file, allow normal access. return NULL; } + +/** + * Check if an IP address is within a CIDR range. + * + * @param string $ip + * The IP address to check. + * @param string $cidr + * The CIDR range (e.g. "172.16.0.0/16"). + * + * @return bool + * TRUE if the IP is within the CIDR range, FALSE otherwise. + */ +function os2forms_digital_signature_ip_in_cidr(string $ip, string $cidr): bool { + if (!str_contains($cidr, '/')) { + return FALSE; + } + + [$subnet, $bits] = explode('/', $cidr, 2); + $bits = (int) $bits; + + $ip = ip2long($ip); + $subnet = ip2long($subnet); + + if ($ip === FALSE || $subnet === FALSE || $bits < 0 || $bits > 32) { + return FALSE; + } + + $mask = -1 << (32 - $bits); + + return ($ip & $mask) === ($subnet & $mask); +} diff --git a/modules/os2forms_digital_signature/os2forms_digital_signature.services.yml b/modules/os2forms_digital_signature/os2forms_digital_signature.services.yml index 33848830..47d0fd68 100644 --- a/modules/os2forms_digital_signature/os2forms_digital_signature.services.yml +++ b/modules/os2forms_digital_signature/os2forms_digital_signature.services.yml @@ -11,3 +11,16 @@ services: - '@config.factory' - '@entity_type.manager' - '@logger.channel.os2forms_digital_signature' +services: + logger.channel.os2forms_digital_signature: + parent: logger.channel_base + arguments: [ 'os2forms_digital_signature' ] + + os2forms_digital_signature.signing_service: + class: Drupal\os2forms_digital_signature\Service\SigningService + arguments: + - '@http_client' + - '@datetime.time' + - '@config.factory' + - '@entity_type.manager' + - '@logger.channel.os2forms_digital_signature' diff --git a/modules/os2forms_digital_signature/src/Form/SettingsForm.php b/modules/os2forms_digital_signature/src/Form/SettingsForm.php index a05ec886..ab821c26 100644 --- a/modules/os2forms_digital_signature/src/Form/SettingsForm.php +++ b/modules/os2forms_digital_signature/src/Form/SettingsForm.php @@ -40,12 +40,14 @@ public function buildForm(array $form, FormStateInterface $form_state) { $form['os2forms_digital_signature_remote_service_url'] = [ '#type' => 'textfield', '#title' => $this->t('Signature server URL'), + '#required' => TRUE, '#default_value' => $this->config(self::$configName)->get('os2forms_digital_signature_remote_service_url'), '#description' => $this->t('E.g. https://signering.bellcom.dk/sign.php?'), ]; $form['os2forms_digital_signature_sign_hash_salt'] = [ '#type' => 'textfield', '#title' => $this->t('Hash Salt used for signature'), + '#required' => TRUE, '#default_value' => $this->config(self::$configName)->get('os2forms_digital_signature_sign_hash_salt'), '#description' => $this->t('Must match hash salt on the signature server'), ]; @@ -53,11 +55,12 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#type' => 'textfield', '#title' => $this->t('List IPs which can download unsigned PDF submissions'), '#default_value' => $this->config(self::$configName)->get('os2forms_digital_signature_submission_allowed_ips'), - '#description' => $this->t('Comma separated. e.g. 192.168.1.1,192.168.2.1'), + '#description' => $this->t('Comma separated. e.g. 192.168.1.1,192.168.2.1 or 172.16.0.0/16. If left empty no restrictions will be applied.'), ]; $form['os2forms_digital_signature_submission_retention_period'] = [ '#type' => 'textfield', '#title' => $this->t('Unsigned submission timespan (s)'), + '#required' => TRUE, '#default_value' => ($this->config(self::$configName)->get('os2forms_digital_signature_submission_retention_period')) ?? 300, '#description' => $this->t('How many seconds can unsigned submission exist before being automatically deleted'), ]; From a95445d6e90eda165acdd58c4ebe128ed3490b3b Mon Sep 17 00:00:00 2001 From: Jesper Kristensen Date: Wed, 18 Feb 2026 09:13:57 +0100 Subject: [PATCH 2/2] Switch digital signatur ip range check to use symfony function --- .../os2forms_digital_signature.module | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/modules/os2forms_digital_signature/os2forms_digital_signature.module b/modules/os2forms_digital_signature/os2forms_digital_signature.module index 49f9a496..35a72ef6 100644 --- a/modules/os2forms_digital_signature/os2forms_digital_signature.module +++ b/modules/os2forms_digital_signature/os2forms_digital_signature.module @@ -8,6 +8,7 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\StreamWrapper\StreamWrapperManager; use Drupal\os2forms_digital_signature\Form\SettingsForm; +use Symfony\Component\HttpFoundation\IpUtils; /** * Implements hook_cron(). @@ -106,21 +107,5 @@ function os2forms_digital_signature_file_download($uri) { * TRUE if the IP is within the CIDR range, FALSE otherwise. */ function os2forms_digital_signature_ip_in_cidr(string $ip, string $cidr): bool { - if (!str_contains($cidr, '/')) { - return FALSE; - } - - [$subnet, $bits] = explode('/', $cidr, 2); - $bits = (int) $bits; - - $ip = ip2long($ip); - $subnet = ip2long($subnet); - - if ($ip === FALSE || $subnet === FALSE || $bits < 0 || $bits > 32) { - return FALSE; - } - - $mask = -1 << (32 - $bits); - - return ($ip & $mask) === ($subnet & $mask); + return IpUtils::checkIp($ip, $cidr); }