Browse Source

Solved issues related to SecurityToken renewal

* previousSecurityToken structure of UA_SecureChannel was not cleared
after generating the new remote symmetric key. This caused a server
to renew the remote key every time a message chunk was received. If
the client requested a renewal of the secure channel and new nonces
were exchanged, but the client continued to use the "old"
SecurityToken for some more message chunks the server generated a
remote key based on the new nonces when it should have used the key
of the current SecurityToken.
* UA_SecureChannelManager_cleanupTimedOut() changed currently used
SecurityToken to the next one if present. According to Part 4,
chapter 5.5.2 OpenSecureChannel the server shall use the existing
SecurityToken until it expires or the server receives a message
secured with the new SecurityToken. Hence, it shouldn't trigg itself
change the SecurityToken.
* Fixed issues in renew channel test cases of the client
Jonas Green 4 years ago
parent
commit
e923698177
3 changed files with 12 additions and 6 deletions
  1. 0 5
      src/server/ua_securechannel_manager.c
  2. 2 0
      src/ua_securechannel.c
  3. 10 1
      tests/client/check_client.c

+ 0 - 5
src/server/ua_securechannel_manager.c

@@ -87,11 +87,6 @@ UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm,
             removeSecureChannel(cm, entry);
             continue;
         }
-
-        /* Revolve the channel tokens */
-        if(entry->channel.nextSecurityToken.tokenId > 0) {
-            UA_SecureChannel_revolveTokens(&entry->channel);
-        }
     }
 }
 

+ 2 - 0
src/ua_securechannel.c

@@ -1326,6 +1326,7 @@ checkSymHeader(UA_SecureChannel *const channel,
         if(channel->securityToken.tokenId == tokenId) {
             retval = UA_SecureChannel_generateRemoteKeys(channel, channel->securityPolicy);
             UA_ChannelSecurityToken_deleteMembers(&channel->previousSecurityToken);
+            UA_ChannelSecurityToken_init(&channel->previousSecurityToken);
             return retval;
         }
     }
@@ -1338,6 +1339,7 @@ checkSymHeader(UA_SecureChannel *const channel,
         UA_StatusCode retval =
             UA_SecureChannel_generateRemoteKeys(channel, channel->securityPolicy);
         UA_ChannelSecurityToken_deleteMembers(&channel->previousSecurityToken);
+        UA_ChannelSecurityToken_init(&channel->previousSecurityToken);
         return retval;
     }
 

+ 10 - 1
tests/client/check_client.c

@@ -171,6 +171,10 @@ START_TEST(Client_renewSecureChannel) {
     UA_ClientConfig *cc = UA_Client_getConfig(client);
     UA_fakeSleep((UA_UInt32)((UA_Double)cc->secureChannelLifeTime * 0.8));
 
+    /* Make the client renew the channel */
+    retval = UA_Client_run_iterate(client, 0);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+
     /* Now read */
     UA_Variant val;
     UA_NodeId nodeId = UA_NODEID_STRING(1, "my.variable");
@@ -193,7 +197,12 @@ START_TEST(Client_renewSecureChannelWithActiveSubscription) {
     ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
 
     UA_CreateSubscriptionRequest request = UA_CreateSubscriptionRequest_default();
-    request.requestedLifetimeCount = 1000;
+    /* Force the server to send keep alive responses every second to trigg
+     * the client to send new publish requests. Requests from the client
+     * will make the server to change to the new SecurityToken after renewal.
+     */
+    request.requestedPublishingInterval = 1000;
+    request.requestedMaxKeepAliveCount = 1;
     UA_CreateSubscriptionResponse response = UA_Client_Subscriptions_create(client, request,
                                                                             NULL, NULL, NULL);