improve timeout handling in sessions

src/server/ua_server.c

@@ -147,7 +147,8 @@ static void UA_Server_cleanup(UA_Server *server, void *nothing) {
         if(sentry->session.validTill < now) {
             session_list_entry *next = LIST_NEXT(sentry, pointers);
             LIST_REMOVE(sentry, pointers);
-            UA_SecureChannel_detachSession(sentry->session.channel);
+            if(sentry->session.channel)
+                UA_SecureChannel_detachSession(sentry->session.channel);
             UA_Session_deleteMembers(&sentry->session);
             UA_free(sentry);
             sentry = next;
@@ -384,9 +385,9 @@ UA_Server * UA_Server_new(UA_ServerConfig config) {
 
     server->nodestore = UA_NodeStore_new();
 
-   // UA_WorkItem cleanup = {.type = UA_WORKITEMTYPE_METHODCALL,
-   //                        .work.methodCall = {.method = UA_Server_cleanup, .data = NULL} };
-   // UA_Server_addRepeatedWorkItem(server, &cleanup, 10000, NULL);
+    UA_WorkItem cleanup = {.type = UA_WORKITEMTYPE_METHODCALL,
+                           .work.methodCall = {.method = UA_Server_cleanup, .data = NULL} };
+    UA_Server_addRepeatedWorkItem(server, &cleanup, 10000, NULL);
 
     /**********************/
     /* Server Information */

src/server/ua_server_binary.c

@@ -123,35 +123,44 @@ static void init_response_header(const UA_RequestHeader *p, UA_ResponseHeader *r
     r->timestamp = UA_DateTime_now();
 }
 
+/* The request/response are casted to the header (first element of their struct) */
+static void invoke_service(UA_Server *server, UA_SecureChannel *channel,
+                           UA_RequestHeader *request, UA_ResponseHeader *response,
+                           void (*service)(UA_Server*, UA_Session*, void*, void*)) {
+    init_response_header(request, response);
+    if(!channel->session ||
+        !UA_NodeId_equal(&channel->session->authenticationToken,
+                         &request->authenticationToken))
+        response->serviceResult = UA_STATUSCODE_BADSESSIONIDINVALID;
+    else if(channel->session->channel != channel)
+            response->serviceResult = UA_STATUSCODE_BADSECURECHANNELIDINVALID;
+    else if(channel->session->activated == UA_FALSE)
+            response->serviceResult = UA_STATUSCODE_BADSESSIONNOTACTIVATED;
+    else {
+            UA_Session_updateLifetime(channel->session);
+            service(server, channel->session, request, response);
+        }
+}
+
 #define INVOKE_SERVICE(TYPE) do {                                       \
-        UA_##TYPE##Request p;                                           \
-        UA_##TYPE##Response r;                                          \
-        if(UA_##TYPE##Request_decodeBinary(msg, pos, &p))               \
-            return;                                                     \
+        UA_##TYPE##Request p;                                           \
+        UA_##TYPE##Response r;                                          \
+        if(UA_##TYPE##Request_decodeBinary(msg, pos, &p))               \
+            return;                                                     \
         UA_##TYPE##Response_init(&r);                                   \
-        init_response_header(&p.requestHeader, &r.responseHeader);      \
-        if(!clientChannel->session || !UA_NodeId_equal(&clientChannel->session->authenticationToken,\
-                &p.requestHeader.authenticationToken))                  \
-            r.responseHeader.serviceResult = UA_STATUSCODE_BADSESSIONIDINVALID;     \
-        else if(clientChannel->session->channel != clientChannel){      \
-            r.responseHeader.serviceResult = UA_STATUSCODE_BADSECURECHANNELIDINVALID; \
-        }                                                               \
-        else if(clientChannel->session->activated == UA_FALSE){         \
-            UA_SessionManager_removeSession(&server->sessionManager, &clientChannel->session->sessionId); \
-            r.responseHeader.serviceResult = UA_STATUSCODE_BADSESSIONNOTACTIVATED; \
-        }else{                                                          \
-            clientSession = clientChannel->session;                     \
-            Service_##TYPE(server, clientSession, &p, &r);              \
-        }                                                               \
+        invoke_service(server, clientChannel, &p.requestHeader,         \
+                       &r.responseHeader,                               \
+                       (void (*)(UA_Server*, UA_Session*, void*,void*))Service_##TYPE); \
         UA_##TYPE##Request_deleteMembers(&p);                           \
-        retval = connection->getBuffer(connection, &message, headerSize + UA_##TYPE##Response_calcSizeBinary(&r)); \
-        if(retval != UA_STATUSCODE_GOOD) {                              \
-            UA_##TYPE##Response_deleteMembers(&r);                      \
-            return;                                                     \
-        }                                                               \
-        UA_##TYPE##Response_encodeBinary(&r, &message, &messagePos);    \
+        retval = connection->getBuffer(connection, &message,            \
+                     headerSize + UA_##TYPE##Response_calcSizeBinary(&r)); \
+        if(retval != UA_STATUSCODE_GOOD) {                              \
+            UA_##TYPE##Response_deleteMembers(&r);                      \
+            return;                                                     \
+        }                                                               \
+        UA_##TYPE##Response_encodeBinary(&r, &message, &messagePos);    \
         UA_##TYPE##Response_deleteMembers(&r);                          \
-    } while(0)
+} while(0)
 
 static void processMSG(UA_Connection *connection, UA_Server *server, const UA_ByteString *msg, size_t *pos) {
     // 1) Read in the securechannel
@@ -165,6 +174,7 @@ static void processMSG(UA_Connection *connection, UA_Server *server, const UA_By
     UA_SecureChannel anonymousChannel;
     if(!clientChannel) {
         UA_SecureChannel_init(&anonymousChannel);
+        anonymousChannel->session = &anonymousSession;
         clientChannel = &anonymousChannel;
     }
 #endif
@@ -182,12 +192,6 @@ static void processMSG(UA_Connection *connection, UA_Server *server, const UA_By
     clientChannel->sequenceNumber = sequenceHeader.sequenceNumber;
     clientChannel->requestId = sequenceHeader.requestId;
 
-    UA_Session *clientSession = UA_NULL;
-#ifdef EXTENSION_STATELESS
-    if(clientChannel == &anonymousChannel)
-        clientSession = &anonymousSession;
-#endif
-
     // 3) Build the header and compute the header size
     UA_SecureConversationMessageHeader respHeader;
     respHeader.messageHeader.messageTypeAndFinal = UA_MESSAGETYPEANDFINAL_MSGF;

src/server/ua_services_session.c

@@ -41,20 +41,13 @@ void Service_CreateSession(UA_Server *server, UA_SecureChannel *channel,
     }
 }
 
-#ifdef RETURN
-#undef RETURN
-#endif
-#define RETURN  UA_UserIdentityToken_deleteMembers(&token); \
-                UA_UserNameIdentityToken_deleteMembers(&username_token); \
-                return
 void Service_ActivateSession(UA_Server *server,UA_SecureChannel *channel,
                              const UA_ActivateSessionRequest *request,
                              UA_ActivateSessionResponse *response) {
     // make the channel know about the session
-	UA_Session *foundSession;
-	UA_SessionManager_getSessionByToken(&server->sessionManager,
-                                        (const UA_NodeId*)&request->requestHeader.authenticationToken,
-                                        &foundSession);
+	UA_Session *foundSession =
+        UA_SessionManager_getSession(&server->sessionManager,
+                                     (const UA_NodeId*)&request->requestHeader.authenticationToken);
 
 	if(foundSession == UA_NULL){
         response->responseHeader.serviceResult = UA_STATUSCODE_BADSESSIONIDINVALID;
@@ -63,7 +56,6 @@ void Service_ActivateSession(UA_Server *server,UA_SecureChannel *channel,
 
 	if(foundSession->validTill < UA_DateTime_now()){
         response->responseHeader.serviceResult = UA_STATUSCODE_BADSESSIONIDINVALID;
-        //TODO: maybe delete session? or wait for a recurring cleanup?
         return;
 	}
 
@@ -75,72 +67,58 @@ void Service_ActivateSession(UA_Server *server,UA_SecureChannel *channel,
     UA_UserNameIdentityToken username_token;
     UA_UserNameIdentityToken_init(&username_token);
 
-    //check policies
-
-    if(token.policyId.data == UA_NULL){ //user identity token is NULL
+    if(token.policyId.data == UA_NULL) {
+        /* 1) no policy defined */
         response->responseHeader.serviceResult = UA_STATUSCODE_BADIDENTITYTOKENINVALID;
-        //todo cleanup session
-        RETURN;
-    }
-
-    //anonymous logins
-    if(server->config.Login_enableAnonymous && UA_String_equalchars(&token.policyId, ANONYMOUS_POLICY)){
-        //success - activate
-        channel->session = foundSession;
-        channel->session->activated = UA_TRUE;
-        //TODO: not sure if we have to do this, tests seem to work
-        //if(foundSession->channel) //in case session is being rebound
-        //    foundSession->channel->session = UA_NULL;
-        foundSession->channel=channel;
-        RETURN;
-    //username logins
-    }else if(server->config.Login_enableUsernamePassword && UA_String_equalchars(&token.policyId, USERNAME_POLICY)){
+    } else if(server->config.Login_enableAnonymous && UA_String_equalchars(&token.policyId, ANONYMOUS_POLICY)) {
+        /* 2) anonymous logins */
+        if(foundSession->channel)
+            UA_Session_detachSecureChannel(foundSession);
+        UA_SecureChannel_attachSession(channel, foundSession);
+        foundSession->activated = UA_TRUE;
+        UA_Session_updateLifetime(foundSession);
+    } else if(server->config.Login_enableUsernamePassword && UA_String_equalchars(&token.policyId, USERNAME_POLICY)) {
+        /* 3) username logins */
         offset = 0;
         UA_UserNameIdentityToken_decodeBinary(&request->userIdentityToken.body, &offset, &username_token);
-        if(username_token.encryptionAlgorithm.data != UA_NULL){
-            //we only support encryption
+        if(username_token.encryptionAlgorithm.data != UA_NULL) {
+            /* 3.1) we only support encryption */
             response->responseHeader.serviceResult = UA_STATUSCODE_BADIDENTITYTOKENINVALID;
-            //todo cleanup session
-            RETURN;
-        }
-        if(username_token.userName.length == -1 && username_token.password.length == -1){
-            //empty username and password
+        } else  if(username_token.userName.length == -1 || username_token.password.length == -1){
+            /* 3.2) empty username and password */
             response->responseHeader.serviceResult = UA_STATUSCODE_BADIDENTITYTOKENINVALID;
-            //todo cleanup session
-            RETURN;
-        }
-        for(UA_UInt32 i=0;i<server->config.Login_loginsCount;++i){
-            if(UA_String_equalchars(&username_token.userName, server->config.Login_usernames[i])
-            && UA_String_equalchars(&username_token.password, server->config.Login_passwords[i])){
-                //success - activate
-                channel->session = foundSession;
-                channel->session->activated = UA_TRUE;
-                //TODO: not sure if we have to do this, tests seem to work
-                //if(foundSession->channel) //in case session is being rebound
-                //    foundSession->channel->session = UA_NULL;
-                foundSession->channel=channel;
-                RETURN;
+        } else {
+            /* 3.3) ok, trying to match the username */
+            UA_UInt32 i = 0;
+            for(; i < server->config.Login_loginsCount; ++i) {
+                if(UA_String_equalchars(&username_token.userName, server->config.Login_usernames[i])
+                    && UA_String_equalchars(&username_token.password, server->config.Login_passwords[i])) {
+                    /* success - activate */
+                    if(foundSession->channel)
+                        UA_Session_detachSecureChannel(foundSession);
+                    UA_SecureChannel_attachSession(channel, foundSession);
+                    foundSession->activated = UA_TRUE;
+                    UA_Session_updateLifetime(foundSession);
+                    break;
+                }
             }
+            /* no username/pass matched */
+            if(i >= server->config.Login_loginsCount)
+                response->responseHeader.serviceResult = UA_STATUSCODE_BADUSERACCESSDENIED;
         }
-        //no username/pass matched
-       response->responseHeader.serviceResult = UA_STATUSCODE_BADUSERACCESSDENIED;
-       //todo cleanup session
-       RETURN;
+    } else {
+        response->responseHeader.serviceResult = UA_STATUSCODE_BADIDENTITYTOKENINVALID;
     }
-
-    //default case - no login
-    response->responseHeader.serviceResult = UA_STATUSCODE_BADIDENTITYTOKENINVALID;
-    //todo cleanup session
-    RETURN;
-
+    UA_UserIdentityToken_deleteMembers(&token);
+    UA_UserNameIdentityToken_deleteMembers(&username_token);
+    return;
 }
-#undef RETURN
 
 void Service_CloseSession(UA_Server *server, UA_Session *session, const UA_CloseSessionRequest *request,
                           UA_CloseSessionResponse *response) {
-	UA_Session *foundSession;
-	UA_SessionManager_getSessionByToken(&server->sessionManager,
-			(const UA_NodeId*)&request->requestHeader.authenticationToken, &foundSession);
+	UA_Session *foundSession =
+        UA_SessionManager_getSession(&server->sessionManager,
+		                             (const UA_NodeId*)&request->requestHeader.authenticationToken);
 
 	if(foundSession == UA_NULL){
 		response->responseHeader.serviceResult = UA_STATUSCODE_BADSESSIONIDINVALID;

src/server/ua_session_manager.c

@@ -31,56 +31,16 @@ void UA_SessionManager_deleteMembers(UA_SessionManager *sessionManager) {
     }
 }
 
-UA_StatusCode
-UA_SessionManager_getSessionById(UA_SessionManager *sessionManager, const UA_NodeId *sessionId,
-                                 UA_Session **session)
-{
-    if(sessionManager == UA_NULL) {
-        *session = UA_NULL;
-        return UA_STATUSCODE_BADINTERNALERROR;
-    }
-
-    session_list_entry *current = UA_NULL;
-    LIST_FOREACH(current, &sessionManager->sessions, pointers) {
-        if(UA_NodeId_equal(&current->session.sessionId, sessionId))
-            break;
-    }
-
-    if(!current) {
-        *session = UA_NULL;
-        return UA_STATUSCODE_BADINTERNALERROR;
-    }
-
-    // Lifetime handling is not done here, but in a regular cleanup by the
-    // server. If the session still exists, then it is valid.
-    *session = &current->session;
-    return UA_STATUSCODE_GOOD;
-}
-
-UA_StatusCode
-UA_SessionManager_getSessionByToken(UA_SessionManager *sessionManager, const UA_NodeId *token,
-                                    UA_Session **session)
-{
-    if(sessionManager == UA_NULL) {
-        *session = UA_NULL;
-        return UA_STATUSCODE_BADINTERNALERROR;
-    }
-
+UA_Session * UA_SessionManager_getSession(UA_SessionManager *sessionManager,
+                                          const UA_NodeId *token) {
     session_list_entry *current = UA_NULL;
     LIST_FOREACH(current, &sessionManager->sessions, pointers) {
         if(UA_NodeId_equal(&current->session.authenticationToken, token))
             break;
     }
-
-    if(!current) {
-        *session = UA_NULL;
-        return UA_STATUSCODE_BADINTERNALERROR;
-    }
-
-    // Lifetime handling is not done here, but in a regular cleanup by the
-    // server. If the session still exists, then it is valid.
-    *session = &current->session;
-    return UA_STATUSCODE_GOOD;
+    if(!current || UA_DateTime_now() > current->session.validTill)
+        return UA_NULL;
+    return &current->session;
 }
 
 /** Creates and adds a session. */
@@ -103,7 +63,7 @@ UA_SessionManager_createSession(UA_SessionManager *sessionManager, UA_SecureChan
         (request->requestedSessionTimeout <= sessionManager->maxSessionLifeTime &&
          request->requestedSessionTimeout>0) ?
         request->requestedSessionTimeout : sessionManager->maxSessionLifeTime;
-    UA_Session_setExpirationDate(&newentry->session);
+    UA_Session_updateLifetime(&newentry->session);
 
     sessionManager->currentSessionCount++;
     LIST_INSERT_HEAD(&sessionManager->sessions, newentry, pointers);

src/server/ua_session_manager.h

@@ -30,14 +30,7 @@ UA_StatusCode UA_SessionManager_createSession(UA_SessionManager *sessionManager,
 UA_StatusCode UA_SessionManager_removeSession(UA_SessionManager *sessionManager,
                                               const UA_NodeId *sessionId);
 
-/** Finds the session which is identified by the sessionId */
-UA_StatusCode UA_SessionManager_getSessionById(UA_SessionManager *sessionManager,
-                                               const UA_NodeId *sessionId, UA_Session **session);
-
-UA_StatusCode UA_SessionManager_getSessionByToken(UA_SessionManager *sessionManager,
-                                                  const UA_NodeId *token, UA_Session **session);
-
-//UA_Int32 UA_SessionManager_updateSessions();
-//UA_Int32 UA_SessionManager_generateToken(UA_Session session, UA_Int32 requestedLifeTime, SecurityTokenRequestType requestType, UA_ChannelSecurityToken* newToken);
+/** Finds the session which is identified by the authentication token */
+UA_Session * UA_SessionManager_getSession(UA_SessionManager *sessionManager, const UA_NodeId *token);
 
 #endif /* UA_SESSION_MANAGER_H_ */

src/ua_session.c

@@ -30,14 +30,6 @@ UA_Session adminSession = {
     .timeout = UA_INT64_MAX, .validTill = UA_INT64_MAX, .channel = UA_NULL,
     .continuationPoints = {UA_NULL}};
 
-/* TODO: Nobody seems to call this function right now */
-static UA_StatusCode UA_Session_generateToken(UA_NodeId *newToken, UA_UInt32 *seed) {
-    newToken->namespaceIndex = 0; // where else?
-    newToken->identifierType = UA_NODEIDTYPE_GUID;
-    newToken->identifier.guid = UA_Guid_random(seed);
-    return UA_STATUSCODE_GOOD;
-}
-
 void UA_Session_init(UA_Session *session) {
     UA_ApplicationDescription_init(&session->clientDescription);
     session->activated = UA_FALSE;
@@ -67,20 +59,8 @@ void UA_Session_deleteMembers(UA_Session *session) {
     }
 }
 
-UA_StatusCode UA_Session_setExpirationDate(UA_Session *session) {
-    if(!session)
-        return UA_STATUSCODE_BADINTERNALERROR;
-
+void UA_Session_updateLifetime(UA_Session *session) {
     session->validTill = UA_DateTime_now() + session->timeout * 10000; //timeout in ms
-    return UA_STATUSCODE_GOOD;
-}
-
-UA_StatusCode UA_Session_getPendingLifetime(UA_Session *session, UA_Double *pendingLifetime_ms) {
-    if(!session)
-        return UA_STATUSCODE_BADINTERNALERROR;
-
-    *pendingLifetime_ms = (session->validTill - UA_DateTime_now())/10000; //difference in ms
-    return UA_STATUSCODE_GOOD;
 }
 
 void UA_Session_detachSecureChannel(UA_Session *session) {

src/ua_session.h

@@ -39,12 +39,8 @@ extern UA_Session adminSession; ///< Local access to the services (for startup a
 void UA_Session_init(UA_Session *session);
 void UA_Session_deleteMembers(UA_Session *session);
 
-/** If any activity on a session happens, the timeout must be extended */
-UA_StatusCode UA_Session_updateLifetime(UA_Session *session);
-/** Set up the point in time till the session is valid */
-UA_StatusCode UA_Session_setExpirationDate(UA_Session *session);
-/** Gets the sessions pending lifetime (calculated from the timeout which was set) */
-UA_StatusCode UA_Session_getPendingLifetime(UA_Session *session, UA_Double *pendingLifetime);
+/** If any activity on a session happens, the timeout is extended */
+void UA_Session_updateLifetime(UA_Session *session);
 
 void UA_Session_detachSecureChannel(UA_Session *session);