[InvalidSignature] Consider expired SecureToken for up to 25% of its lifetime

asyncua/common/connection.py

@@ -19,8 +19,7 @@ class MessageChunk(ua.FrozenClass):
     """
     Message Chunk, as described in OPC UA specs Part 6, 6.7.2.
     """
-    def __init__(self, security_policy, body=b'', msg_type=ua.MessageType.SecureMessage,
-                 chunk_type=ua.ChunkType.Single):
+    def __init__(self, security_policy, body=b'', msg_type=ua.MessageType.SecureMessage, chunk_type=ua.ChunkType.Single):
         self.MessageHeader = ua.Header(msg_type, chunk_type)
         if msg_type in (ua.MessageType.SecureMessage, ua.MessageType.SecureClose):
             self.SecurityHeader = ua.SymmetricAlgorithmHeader()
@@ -63,10 +62,7 @@ class MessageChunk(ua.FrozenClass):
         if signature_size > 0:
             signature = decrypted[-signature_size:]
             decrypted = decrypted[:-signature_size]
-            crypto.verify(
-                header_to_binary(obj.MessageHeader) + struct_to_binary(obj.SecurityHeader) + decrypted,
-                signature
-            )
+            crypto.verify(header_to_binary(obj.MessageHeader) + struct_to_binary(obj.SecurityHeader) + decrypted, signature)
         data = ua.utils.Buffer(crypto.remove_padding(decrypted))
         obj.SequenceHeader = struct_from_binary(ua.SequenceHeader, data)
         obj.Body = data.read(len(data))
@@ -95,8 +91,7 @@ class MessageChunk(ua.FrozenClass):
         return max_plain_size - ua.SequenceHeader.max_size() - crypto.signature_size() - crypto.min_padding_size()
 
     @staticmethod
-    def message_to_chunks(security_policy, body, max_chunk_size, message_type=ua.MessageType.SecureMessage,
-                          channel_id=1, request_id=1, token_id=1):
+    def message_to_chunks(security_policy, body, max_chunk_size, message_type=ua.MessageType.SecureMessage, channel_id=1, request_id=1, token_id=1):
         """
         Pack message body (as binary string) into one or more chunks.
         Size of each chunk will not exceed max_chunk_size.
@@ -259,9 +254,15 @@ class SecureConnection:
         The only supported types are SecureOpen, SecureMessage, SecureClose.
         If message_type is SecureMessage, the AlgorithmHeader should be passed as arg.
         """
-        chunks = MessageChunk.message_to_chunks(self.security_policy, message, self._max_chunk_size,
-                                                message_type=message_type, channel_id=self.security_token.ChannelId,
-                                                request_id=request_id, token_id=self.security_token.TokenId)
+        chunks = MessageChunk.message_to_chunks(
+            self.security_policy,
+            message,
+            self._max_chunk_size,
+            message_type=message_type,
+            channel_id=self.security_token.ChannelId,
+            request_id=request_id,
+            token_id=self.security_token.TokenId,
+        )
         for chunk in chunks:
             self._sequence_number += 1
             if self._sequence_number >= (1 << 32):
@@ -292,8 +293,7 @@ class SecureConnection:
             timeout = self.prev_security_token.CreatedAt + \
                 timedelta(milliseconds=self.prev_security_token.RevisedLifetime * 1.25)
             if timeout < datetime.utcnow():
-                raise ua.UaError(f"Security token id {security_hdr.TokenId} has timed out "
-                                 f"({timeout} < {datetime.utcnow()})")
+                raise ua.UaError(f"Security token id {security_hdr.TokenId} has timed out " f"({timeout} < {datetime.utcnow()})")
             return
 
         expected_tokens = [self.security_token.TokenId, self.next_security_token.TokenId]
@@ -306,12 +306,10 @@ class SecureConnection:
             raise ValueError(f'Expected chunk, got: {chunk}')
         if chunk.MessageHeader.MessageType != ua.MessageType.SecureOpen:
             if chunk.MessageHeader.ChannelId != self.security_token.ChannelId:
-                raise ua.UaError(f'Wrong channel id {chunk.MessageHeader.ChannelId},'
-                                 f' expected {self.security_token.ChannelId}')
+                raise ua.UaError(f'Wrong channel id {chunk.MessageHeader.ChannelId},' f' expected {self.security_token.ChannelId}')
         if self._incoming_parts:
             if self._incoming_parts[0].SequenceHeader.RequestId != chunk.SequenceHeader.RequestId:
-                raise ua.UaError(f'Wrong request id {chunk.SequenceHeader.RequestId},'
-                                 f' expected {self._incoming_parts[0].SequenceHeader.RequestId}')
+                raise ua.UaError(f'Wrong request id {chunk.SequenceHeader.RequestId},' f' expected {self._incoming_parts[0].SequenceHeader.RequestId}')
         # The sequence number must monotonically increase (but it can wrap around)
         seq_num = chunk.SequenceHeader.SequenceNumber
         if self._peer_sequence_number is not None:
@@ -322,9 +320,7 @@ class SecureConnection:
                     logger.debug('Sequence number wrapped: %d -> %d', self._peer_sequence_number, seq_num)
                 else:
                     # Condition for monotonically increase is not met
-                    raise ua.UaError(f"Received chunk: {chunk} with wrong sequence expecting:"
-                                     f" {self._peer_sequence_number}, received: {seq_num},"
-                                     f" spec says to close connection")
+                    raise ua.UaError(f"Received chunk: {chunk} with wrong sequence expecting:" f" {self._peer_sequence_number}, received: {seq_num}," f" spec says to close connection")
         self._peer_sequence_number = seq_num
 
     def receive_from_header_and_body(self, header, body):

asyncua/crypto/security_policies.py

@@ -586,8 +586,9 @@ class SecurityPolicyBasic256(SecurityPolicy):
             self.symmetric_cryptography.Prev_Decryptor = self.symmetric_cryptography.Decryptor
             self.symmetric_cryptography.prev_key_expiration = self.symmetric_cryptography.key_expiration
 
-        # lifetime is in ms
-        self.symmetric_cryptography.key_expiration = time.time() + (lifetime * 0.001)
+        # convert lifetime to seconds and add the 25% extra-margin (Part4/5.5.2)
+        lifetime = lifetime * 1.25 * 0.001
+        self.symmetric_cryptography.key_expiration = time.time() + lifetime
         self.symmetric_cryptography.Verifier = VerifierAesCbc(sigkey)
         self.symmetric_cryptography.Decryptor = DecryptorAesCbc(key, init_vec)
 

tests/test_crypto_connect.py

@@ -308,8 +308,9 @@ async def test_secure_channel_key_expiration(srv_crypto_one_cert, mocker):
         assert clt.uaclient.security_policy.symmetric_cryptography.Prev_Decryptor is None
 
         await asyncio.sleep(timeout)
-        prev_verifier = clt.uaclient.security_policy.symmetric_cryptography.Prev_Verifier
-        prev_decryptor = clt.uaclient.security_policy.symmetric_cryptography.Prev_Decryptor
+        sym_crypto = clt.uaclient.security_policy.symmetric_cryptography
+        prev_verifier = sym_crypto.Prev_Verifier
+        prev_decryptor = sym_crypto.Prev_Decryptor
         assert isinstance(prev_verifier, Verifier)
         assert isinstance(prev_decryptor, Decryptor)
 
@@ -320,6 +321,11 @@ async def test_secure_channel_key_expiration(srv_crypto_one_cert, mocker):
 
         await asyncio.sleep(timeout*0.3)
         assert await clt.get_objects_node().get_children()
+
+        assert sym_crypto.key_expiration > 0
+        assert sym_crypto.prev_key_expiration > 0
+        assert sym_crypto.key_expiration > sym_crypto.prev_key_expiration
+
         assert mock_decry_reset.call_count == 1
         assert mock_verif_reset.call_count == 1
         assert clt.uaclient.security_policy.symmetric_cryptography.Prev_Verifier is None