From 40e34379f7abad47aef581354d3efdb5f1322411 Mon Sep 17 00:00:00 2001
From: Sander van Harmelen <sander@vanharmelen.nl>
Date: Tue, 12 Apr 2022 13:48:08 +0200
Subject: [PATCH] Make the logic to ignore clock skew more robust
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We encounted a Heuft OPC server that didn’t send a timestamp in the `OpenSecureChannelResponse` message.

Yet it did send the correct timestamp in all other responses, so by adding a few more checks and also updating the offset on session creation fixes the problem.

And at the same time it can help to prevent new clock skew in cases where the channel is kept alive very long and new sessions are created within the existing channel.
---
 client/src/session/session.rs       |  7 +++++++
 client/src/session/session_state.rs | 10 +++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/client/src/session/session.rs b/client/src/session/session.rs
index b0d893ad5..14b5e0145 100644
--- a/client/src/session/session.rs
+++ b/client/src/session/session.rs
@@ -1443,6 +1443,13 @@ impl SessionService for Session {
                     let _ = secure_channel
                         .set_remote_cert_from_byte_string(&response.server_certificate);
                 }
+                // When ignoring clock skew, we calculate the time offset between the client
+                // and the server and use that to compensate for the difference in time.
+                if self.ignore_clock_skew && !response.response_header.timestamp.is_null() {
+                    let offset = response.response_header.timestamp - DateTime::now();
+                    // Update the client offset by adding the new offset.
+                    session_state.set_client_offset(offset);
+                }
                 session_state.session_id()
             };
 
diff --git a/client/src/session/session_state.rs b/client/src/session/session_state.rs
index d6e7839e9..46488b5ab 100644
--- a/client/src/session/session_state.rs
+++ b/client/src/session/session_state.rs
@@ -193,6 +193,11 @@ impl SessionState {
         self.id
     }
 
+    pub fn set_client_offset(&mut self, offset: Duration) {
+        self.client_offset = self.client_offset + offset;
+        debug!("Client offset set to {}", self.client_offset);
+    }
+
     pub fn set_session_id(&mut self, session_id: NodeId) {
         self.session_id = session_id
     }
@@ -452,15 +457,14 @@ impl SessionState {
             // server and use that offset to compensate for the difference in time when setting
             // the timestamps in the request headers and when decoding timestamps in messages
             // received from the server.
-            if self.ignore_clock_skew {
+            if self.ignore_clock_skew && !response.response_header.timestamp.is_null() {
                 let offset = response.response_header.timestamp - DateTime::now();
                 // Make sure to apply the offset to the security token in the current response.
                 security_token.created_at = security_token.created_at - offset;
                 // Update the client offset by adding the new offset. When the secure channel is
                 // renewed its already using the client offset calculated when issuing the secure
                 // channel and only needs to be updated to accommodate any additional clock skew.
-                self.client_offset = self.client_offset + offset;
-                debug!("Client offset set to {}", self.client_offset);
+                self.set_client_offset(offset);
             }
 
             debug!("Setting transport's security token");