Merge pull request #5787 from nicksinger/hmac_tolerance

Implement option to configure the hmac time difference tolerance
os-autoinst · Jul 23, 2024 · 51f9636 · 51f9636
2 parents 468dc67 + 3883e61
commit 51f9636
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 2 deletions.
diff --git a/etc/openqa/openqa.ini b/etc/openqa/openqa.ini
@@ -98,6 +98,12 @@
 ## (e.g. 8080) then just set `service_port_delta = 0`.
 # service_port_delta = 2
 
+## Allowed time difference in hmac validation in seconds.
+## Higher values introduce higher risks for replay attacks but make API requests more
+## resilient in case of a high load on the web UI. Lower values reduce this risk but
+## can cause jobs to incomplete with "timestamp mismatch" error messages.
+# api_hmac_time_tolerance = 300
+
 #[scm git]
 # name of remote to get updates from before committing changes (e.g. origin, leave out-commented to disable remote update)
 #update_remote = origin

diff --git a/lib/OpenQA/Setup.pm b/lib/OpenQA/Setup.pm
@@ -54,6 +54,7 @@ sub read_config ($app) {
             parallel_children_collapsable_results => join(' ', OK_RESULTS),
             service_port_delta => $ENV{OPENQA_SERVICE_PORT_DELTA} // 2,
             access_control_allow_origin_header => undef,
+            api_hmac_time_tolerance => 300,
         },
         rate_limits => {
             search => 5,

diff --git a/lib/OpenQA/Shared/Controller/Auth.pm b/lib/OpenQA/Shared/Controller/Auth.pm
@@ -98,10 +98,12 @@ sub auth_admin ($self) {
 
 sub _is_timestamp_valid ($self, $our_timestamp, $remote_timestamp) {
     my $log = $self->app->log;
+    my $tolerance = $self->config->{api_hmac_time_tolerance}
+      // 300;    # make extra sure this value is never empty to avoid security issues
 
-    return 1 if ($our_timestamp - $remote_timestamp <= 300);
+    return 1 if (abs($our_timestamp - $remote_timestamp) <= $tolerance);
     $log->debug(
-qq{Timestamp mismatch over 300s; our_timestamp: $our_timestamp, X-API-Microtime (from worker): $remote_timestamp}
+qq{Timestamp mismatch over ${tolerance}s; our_timestamp: $our_timestamp, X-API-Microtime (from worker): $remote_timestamp}
     );
     return 0;
 }

diff --git a/t/config.t b/t/config.t
@@ -52,6 +52,7 @@ subtest 'Test configuration default modes' => sub {
             force_result_regex => '',
             parallel_children_collapsable_results_sel => ' .status:not(.result_passed):not(.result_softfailed)',
             auto_clone_limit => 20,
+            api_hmac_time_tolerance => 300,
         },
         rate_limits => {
             search => 5,