From fd0c3306bb8aabb2e7e37020eefb208bafebc45f Mon Sep 17 00:00:00 2001
From: nicksinger <nsinger@suse.de>
Date: Tue, 23 Jul 2024 12:18:56 +0200
Subject: [PATCH 1/2] Add hmac time tolerance setting

---
 etc/openqa/openqa.ini                | 6 ++++++
 lib/OpenQA/Setup.pm                  | 1 +
 lib/OpenQA/Shared/Controller/Auth.pm | 6 ++++--
 t/config.t                           | 1 +
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/etc/openqa/openqa.ini b/etc/openqa/openqa.ini
index ca36445c14c..1089f8d3bf4 100644
--- 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
index ab8dcf5d4e6..31be28f4782 100644
--- 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
index 2bdf7d252ca..b6b2bdaf67c 100644
--- 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 ($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
index a526aa67db2..6548582c60d 100644
--- 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,

From 3883e6143ea12f62d35567ca670f9c017925982f Mon Sep 17 00:00:00 2001
From: nicksinger <nsinger@suse.de>
Date: Tue, 23 Jul 2024 13:13:25 +0200
Subject: [PATCH 2/2] Use absolute timestamp delta to check hmac tolerance

This avoids unexpected behaviour if the timestamp of a worker is ahead
of the webuis time.
---
 lib/OpenQA/Shared/Controller/Auth.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/OpenQA/Shared/Controller/Auth.pm b/lib/OpenQA/Shared/Controller/Auth.pm
index b6b2bdaf67c..910f172ffce 100644
--- a/lib/OpenQA/Shared/Controller/Auth.pm
+++ b/lib/OpenQA/Shared/Controller/Auth.pm
@@ -101,7 +101,7 @@ sub _is_timestamp_valid ($self, $our_timestamp, $remote_timestamp) {
     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 <= $tolerance);
+    return 1 if (abs($our_timestamp - $remote_timestamp) <= $tolerance);
     $log->debug(
 qq{Timestamp mismatch over ${tolerance}s; our_timestamp: $our_timestamp, X-API-Microtime (from worker): $remote_timestamp}
     );