diff --git a/CHANGELOG.md b/CHANGELOG.md index f5613cf4..09e00b97 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] +- [PR-305](https://github.com/OS2Forms/os2forms/pull/305) + Fix IP validation in digital signature file download (CIDR support) - [PR-317](https://github.com/OS2Forms/os2forms/pull/317) Updated code analysis script. - [PR-306](https://github.com/OS2Forms/os2forms/pull/306) diff --git a/modules/os2forms_digital_signature/os2forms_digital_signature.module b/modules/os2forms_digital_signature/os2forms_digital_signature.module index 6e12a210..a3618fda 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(). @@ -57,18 +58,26 @@ 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)) { + // Check if remote IP matches any allowed IP or CIDR range. + if (empty($allowedIpsArr) || IpUtils::checkIp($remoteIp, $allowedIpsArr)) { $basename = basename($uri); return [ 'Content-disposition' => 'attachment; filename="' . $basename . '"', ]; } - // Otherwise - Deny access. + // 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; } 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'), ];