Merge pull request #305 from itk-dev/feature/digital_signature_ips

Fix IP validation in digital signature file download (CIDR support)

Author: jekuaitk

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)

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;
   }
 

modules/os2forms_digital_signature/src/Form/SettingsForm.php

@@ -40,24 +40,27 @@ 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'),
     ];
     $form['os2forms_digital_signature_submission_allowed_ips'] = [
       '#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'),
     ];