From 0a485919909f9db2be916b5ee7c57c3e98c85aa9 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 11:12:07 +0200 Subject: [PATCH 01/18] fix(plugins): Disable the Basic128Rsa15 and Basic256 SecurityPolicy --- .../mbedtls/ua_securitypolicy_basic128rsa15.c | 4 ++ .../mbedtls/ua_securitypolicy_basic256.c | 5 ++ .../crypto/openssl/ua_openssl_basic128rsa15.c | 4 ++ plugins/crypto/openssl/ua_openssl_basic256.c | 5 ++ plugins/ua_config_default.c | 68 ++++++++++--------- .../check_encryption_basic128rsa15.c | 31 ++++++++- tests/encryption/check_encryption_basic256.c | 26 +++++++ 7 files changed, 109 insertions(+), 34 deletions(-) diff --git a/plugins/crypto/mbedtls/ua_securitypolicy_basic128rsa15.c b/plugins/crypto/mbedtls/ua_securitypolicy_basic128rsa15.c index e708c78fdc1..5ede408d31b 100644 --- a/plugins/crypto/mbedtls/ua_securitypolicy_basic128rsa15.c +++ b/plugins/crypto/mbedtls/ua_securitypolicy_basic128rsa15.c @@ -746,6 +746,10 @@ UA_SecurityPolicy_Basic128Rsa15(UA_SecurityPolicy *policy, const UA_ByteString l memset(policy, 0, sizeof(UA_SecurityPolicy)); policy->logger = logger; + UA_LOG_WARNING(logger, UA_LOGCATEGORY_SECURITYPOLICY, + "!! WARNING !! The Basic128Rsa15 SecurityPolicy is unsecure. " + "There are known attacks that break the encryption."); + policy->policyUri = UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#Basic128Rsa15\0"); UA_SecurityPolicyAsymmetricModule *const asymmetricModule = &policy->asymmetricModule; diff --git a/plugins/crypto/mbedtls/ua_securitypolicy_basic256.c b/plugins/crypto/mbedtls/ua_securitypolicy_basic256.c index bf0196bb7f8..fd4ddc008a4 100644 --- a/plugins/crypto/mbedtls/ua_securitypolicy_basic256.c +++ b/plugins/crypto/mbedtls/ua_securitypolicy_basic256.c @@ -667,6 +667,11 @@ policyContext_newContext_sp_basic256(UA_SecurityPolicy *securityPolicy, UA_StatusCode UA_SecurityPolicy_Basic256(UA_SecurityPolicy *policy, const UA_ByteString localCertificate, const UA_ByteString localPrivateKey, const UA_Logger *logger) { + + UA_LOG_WARNING(logger, UA_LOGCATEGORY_SECURITYPOLICY, + "!! WARNING !! The Basic256 SecurityPolicy is unsecure. " + "There are known attacks that break the encryption."); + memset(policy, 0, sizeof(UA_SecurityPolicy)); policy->logger = logger; diff --git a/plugins/crypto/openssl/ua_openssl_basic128rsa15.c b/plugins/crypto/openssl/ua_openssl_basic128rsa15.c index 71affd31655..7e47e4d0729 100644 --- a/plugins/crypto/openssl/ua_openssl_basic128rsa15.c +++ b/plugins/crypto/openssl/ua_openssl_basic128rsa15.c @@ -500,6 +500,10 @@ UA_SecurityPolicy_Basic128Rsa15 (UA_SecurityPolicy * policy, const UA_ByteString localPrivateKey, const UA_Logger * logger) { + UA_LOG_WARNING(logger, UA_LOGCATEGORY_SECURITYPOLICY, + "!! WARNING !! The Basic128Rsa15 SecurityPolicy is unsecure. " + "There are known attacks that break the encryption."); + UA_SecurityPolicyAsymmetricModule * const asymmetricModule = &policy->asymmetricModule; UA_SecurityPolicySymmetricModule * const symmetricModule = &policy->symmetricModule; UA_SecurityPolicyChannelModule * const channelModule = &policy->channelModule; diff --git a/plugins/crypto/openssl/ua_openssl_basic256.c b/plugins/crypto/openssl/ua_openssl_basic256.c index e54042a7a02..a3c8903364c 100644 --- a/plugins/crypto/openssl/ua_openssl_basic256.c +++ b/plugins/crypto/openssl/ua_openssl_basic256.c @@ -501,6 +501,11 @@ UA_SecurityPolicy_Basic256 (UA_SecurityPolicy * policy, const UA_ByteString localCertificate, const UA_ByteString localPrivateKey, const UA_Logger * logger) { + + UA_LOG_WARNING(logger, UA_LOGCATEGORY_SECURITYPOLICY, + "!! WARNING !! The Basic256 SecurityPolicy is unsecure. " + "There are known attacks that break the encryption."); + UA_SecurityPolicyAsymmetricModule * const asymmetricModule = &policy->asymmetricModule; UA_SecurityPolicySymmetricModule * const symmetricModule = &policy->symmetricModule; UA_SecurityPolicyChannelModule * const channelModule = &policy->channelModule; diff --git a/plugins/ua_config_default.c b/plugins/ua_config_default.c index 8ab5f547f28..b1445975c63 100644 --- a/plugins/ua_config_default.c +++ b/plugins/ua_config_default.c @@ -639,19 +639,21 @@ UA_ServerConfig_addAllSecurityPolicies(UA_ServerConfig *config, UA_StatusCode_name(retval)); } - retval = UA_ServerConfig_addSecurityPolicyBasic128Rsa15(config, &localCertificate, &localPrivateKey); - if(retval != UA_STATUSCODE_GOOD) { - UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", - UA_StatusCode_name(retval)); - } - - retval = UA_ServerConfig_addSecurityPolicyBasic256(config, &localCertificate, &localPrivateKey); - if(retval != UA_STATUSCODE_GOOD) { - UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Basic256 with error code %s", - UA_StatusCode_name(retval)); - } + /* Basic128Rsa15 should no longer be used */ + /* retval = UA_ServerConfig_addSecurityPolicyBasic128Rsa15(config, &localCertificate, &localPrivateKey); */ + /* if(retval != UA_STATUSCODE_GOOD) { */ + /* UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ + + /* Basic256 should no longer be used */ + /* retval = UA_ServerConfig_addSecurityPolicyBasic256(config, &localCertificate, &localPrivateKey); */ + /* if(retval != UA_STATUSCODE_GOOD) { */ + /* UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic256 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ retval = UA_ServerConfig_addSecurityPolicyBasic256Sha256(config, &localCertificate, &localPrivateKey); if(retval != UA_STATUSCODE_GOOD) { @@ -838,25 +840,27 @@ UA_ClientConfig_setDefaultEncryption(UA_ClientConfig *config, return UA_STATUSCODE_BADOUTOFMEMORY; config->securityPolicies = sp; - retval = UA_SecurityPolicy_Basic128Rsa15(&config->securityPolicies[config->securityPoliciesSize], - localCertificate, privateKey, &config->logger); - if(retval == UA_STATUSCODE_GOOD) { - ++config->securityPoliciesSize; - } else { - UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", - UA_StatusCode_name(retval)); - } - - retval = UA_SecurityPolicy_Basic256(&config->securityPolicies[config->securityPoliciesSize], - localCertificate, privateKey, &config->logger); - if(retval == UA_STATUSCODE_GOOD) { - ++config->securityPoliciesSize; - } else { - UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Basic256 with error code %s", - UA_StatusCode_name(retval)); - } + /* Basic128Rsa15 should no longer be used */ + /* retval = UA_SecurityPolicy_Basic128Rsa15(&config->securityPolicies[config->securityPoliciesSize], */ + /* localCertificate, privateKey, &config->logger); */ + /* if(retval == UA_STATUSCODE_GOOD) { */ + /* ++config->securityPoliciesSize; */ + /* } else { */ + /* UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ + + /* Basic256 should no longer be used */ + /* retval = UA_SecurityPolicy_Basic256(&config->securityPolicies[config->securityPoliciesSize], */ + /* localCertificate, privateKey, &config->logger); */ + /* if(retval == UA_STATUSCODE_GOOD) { */ + /* ++config->securityPoliciesSize; */ + /* } else { */ + /* UA_LOG_WARNING(&config->logger, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic256 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ retval = UA_SecurityPolicy_Basic256Sha256(&config->securityPolicies[config->securityPoliciesSize], localCertificate, privateKey, &config->logger); diff --git a/tests/encryption/check_encryption_basic128rsa15.c b/tests/encryption/check_encryption_basic128rsa15.c index 9c547fa5411..80bb043163f 100644 --- a/tests/encryption/check_encryption_basic128rsa15.c +++ b/tests/encryption/check_encryption_basic128rsa15.c @@ -75,6 +75,11 @@ static void setup(void) { config->certificateVerification.clear(&config->certificateVerification); UA_CertificateVerification_AcceptAll(&config->certificateVerification); + /* Manually add the Basic128Rsa15 SecurityPolicy. + * It does not get added by default as it is considered unsecure. */ + UA_ServerConfig_addSecurityPolicyBasic128Rsa15(config, &certificate, &privateKey); + UA_ServerConfig_addAllEndpoints(config); + /* Set the ApplicationUri used in the certificate */ UA_String_clear(&config->applicationDescription.applicationUri); config->applicationDescription.applicationUri = @@ -118,7 +123,9 @@ START_TEST(encryption_connect) { * security mode as none to see the server's capability * and certificate */ client = UA_Client_new(); - UA_ClientConfig_setDefault(UA_Client_getConfig(client)); + UA_ClientConfig *cc = UA_Client_getConfig(client); + UA_ClientConfig_setDefault(cc); + ck_assert(client != NULL); UA_StatusCode retval = UA_Client_getEndpoints(client, "opc.tcp://localhost:4840", &endpointArraySize, &endpointArray); @@ -147,12 +154,22 @@ START_TEST(encryption_connect) { /* Secure client initialization */ client = UA_Client_new(); - UA_ClientConfig *cc = UA_Client_getConfig(client); + cc = UA_Client_getConfig(client); UA_ClientConfig_setDefaultEncryption(cc, certificate, privateKey, trustList, trustListSize, revocationList, revocationListSize); cc->certificateVerification.clear(&cc->certificateVerification); UA_CertificateVerification_AcceptAll(&cc->certificateVerification); + + /* Manually add the Basic128Rsa15 SecurityPolicy. + * It does not get added by default as it is considered unsecure. */ + cc->securityPolicies = (UA_SecurityPolicy *) + UA_realloc(cc->securityPolicies, sizeof(UA_SecurityPolicy) * + (cc->securityPoliciesSize + 1)); + UA_SecurityPolicy_Basic128Rsa15(&cc->securityPolicies[cc->securityPoliciesSize], + certificate, privateKey, &cc->logger); + cc->securityPoliciesSize++; + cc->securityPolicyUri = UA_STRING_ALLOC("http://opcfoundation.org/UA/SecurityPolicy#Basic128Rsa15"); ck_assert(client != NULL); @@ -236,6 +253,16 @@ START_TEST(encryption_connect_pem) { revocationList, revocationListSize); cc->certificateVerification.clear(&cc->certificateVerification); UA_CertificateVerification_AcceptAll(&cc->certificateVerification); + + /* Manually add the Basic128Rsa15 SecurityPolicy. + * It does not get added by default as it is considered unsecure. */ + cc->securityPolicies = (UA_SecurityPolicy *) + UA_realloc(cc->securityPolicies, sizeof(UA_SecurityPolicy) * + (cc->securityPoliciesSize + 1)); + UA_SecurityPolicy_Basic128Rsa15(&cc->securityPolicies[cc->securityPoliciesSize], + certificate, privateKey, &cc->logger); + cc->securityPoliciesSize++; + cc->securityPolicyUri = UA_STRING_ALLOC("http://opcfoundation.org/UA/SecurityPolicy#Basic128Rsa15"); ck_assert(client != NULL); diff --git a/tests/encryption/check_encryption_basic256.c b/tests/encryption/check_encryption_basic256.c index 48d14a4978b..57335b2d29e 100644 --- a/tests/encryption/check_encryption_basic256.c +++ b/tests/encryption/check_encryption_basic256.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -78,6 +79,11 @@ static void setup(void) { config->certificateVerification.clear(&config->certificateVerification); UA_CertificateVerification_AcceptAll(&config->certificateVerification); + /* Manually add the Basic256 SecurityPolicy. + * It does not get added by default as it is considered unsecure. */ + UA_ServerConfig_addSecurityPolicyBasic256(config, &certificate, &privateKey); + UA_ServerConfig_addAllEndpoints(config); + /* Set the ApplicationUri used in the certificate */ UA_String_clear(&config->applicationDescription.applicationUri); config->applicationDescription.applicationUri = @@ -156,6 +162,16 @@ START_TEST(encryption_connect) { revocationList, revocationListSize); cc->certificateVerification.clear(&cc->certificateVerification); UA_CertificateVerification_AcceptAll(&cc->certificateVerification); + + /* Manually add the Basic256 SecurityPolicy. + * It does not get added by default as it is considered unsecure. */ + cc->securityPolicies = (UA_SecurityPolicy *) + UA_realloc(cc->securityPolicies, sizeof(UA_SecurityPolicy) * + (cc->securityPoliciesSize + 1)); + UA_SecurityPolicy_Basic256(&cc->securityPolicies[cc->securityPoliciesSize], + certificate, privateKey, &cc->logger); + cc->securityPoliciesSize++; + cc->securityPolicyUri = UA_STRING_ALLOC("http://opcfoundation.org/UA/SecurityPolicy#Basic256"); ck_assert(client != NULL); @@ -239,6 +255,16 @@ START_TEST(encryption_connect_pem) { revocationList, revocationListSize); cc->certificateVerification.clear(&cc->certificateVerification); UA_CertificateVerification_AcceptAll(&cc->certificateVerification); + + /* Manually add the Basic256 SecurityPolicy. + * It does not get added by default as it is considered unsecure. */ + cc->securityPolicies = (UA_SecurityPolicy *) + UA_realloc(cc->securityPolicies, sizeof(UA_SecurityPolicy) * + (cc->securityPoliciesSize + 1)); + UA_SecurityPolicy_Basic256(&cc->securityPolicies[cc->securityPoliciesSize], + certificate, privateKey, &cc->logger); + cc->securityPoliciesSize++; + cc->securityPolicyUri = UA_STRING_ALLOC("http://opcfoundation.org/UA/SecurityPolicy#Basic256"); ck_assert(client != NULL); From 43afb0471a81c71dfb1d1e33589308762d5a6d18 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 11:17:08 +0200 Subject: [PATCH 02/18] refactor(build): Bump version to 1.3.14 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3ff203aeee4..bcf1fea7e06 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,7 +43,7 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) # overwritten with more detailed information if git is available. set(OPEN62541_VER_MAJOR 1) set(OPEN62541_VER_MINOR 3) -set(OPEN62541_VER_PATCH 13) +set(OPEN62541_VER_PATCH 14) set(OPEN62541_VER_LABEL "-undefined") # like "-rc1" or "-g4538abcd" or "-g4538abcd-dirty" set(OPEN62541_VER_COMMIT "unknown-commit") From f9e660dc75ae6cd9839148b42e2612f67ac66d57 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 23:34:16 +0200 Subject: [PATCH 03/18] refactor(core): Remove const annotation in SecureChannel We need it non-const for some of the crypto operations. --- src/ua_securechannel.c | 2 +- src/ua_securechannel.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ua_securechannel.c b/src/ua_securechannel.c index 16203166173..d8beed471af 100644 --- a/src/ua_securechannel.c +++ b/src/ua_securechannel.c @@ -36,7 +36,7 @@ UA_SecureChannel_init(UA_SecureChannel *channel) { UA_StatusCode UA_SecureChannel_setSecurityPolicy(UA_SecureChannel *channel, - const UA_SecurityPolicy *securityPolicy, + UA_SecurityPolicy *securityPolicy, const UA_ByteString *remoteCertificate) { /* Is a policy already configured? */ UA_CHECK_ERROR(!channel->securityPolicy, return UA_STATUSCODE_BADINTERNALERROR, diff --git a/src/ua_securechannel.h b/src/ua_securechannel.h index 4c5a0a180f5..424d65b34e0 100644 --- a/src/ua_securechannel.h +++ b/src/ua_securechannel.h @@ -123,7 +123,7 @@ struct UA_SecureChannel { * See the renewState. */ /* The endpoint and context of the channel */ - const UA_SecurityPolicy *securityPolicy; + UA_SecurityPolicy *securityPolicy; void *channelContext; /* For interaction with the security policy */ /* Asymmetric encryption info */ @@ -181,7 +181,7 @@ UA_SecureChannel_processHELACK(UA_SecureChannel *channel, UA_StatusCode UA_SecureChannel_setSecurityPolicy(UA_SecureChannel *channel, - const UA_SecurityPolicy *securityPolicy, + UA_SecurityPolicy *securityPolicy, const UA_ByteString *remoteCertificate); UA_Boolean From 2fd84ab53da63e531fc3a8b998559bfc5e50c79b Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 20:19:17 +0200 Subject: [PATCH 04/18] fix(server): Clear the SecureChannel only after logging and statistics counting --- src/server/ua_server_binary.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/server/ua_server_binary.c b/src/server/ua_server_binary.c index 7569d8b6d52..889e52847cd 100644 --- a/src/server/ua_server_binary.c +++ b/src/server/ua_server_binary.c @@ -142,10 +142,6 @@ setBinaryProtocolManagerState(UA_Server *server, static void deleteServerSecureChannel(UA_BinaryProtocolManager *bpm, UA_SecureChannel *channel) { - /* Clean up the SecureChannel. This is the only place where - * UA_SecureChannel_clear must be called within the server code-base. */ - UA_SecureChannel_clear(channel); - /* Detach the channel from the server list */ TAILQ_REMOVE(&bpm->channels, (channel_entry*)channel, pointers); @@ -178,6 +174,9 @@ deleteServerSecureChannel(UA_BinaryProtocolManager *bpm, break; } + /* Clean up the SecureChannel. This is the only place where + * UA_SecureChannel_clear must be called within the server code-base. */ + UA_SecureChannel_clear(channel); UA_free(channel); } From 6c7a51e892f18e2acfddc45b941aefc2d4df9fd1 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 20:36:38 +0200 Subject: [PATCH 05/18] fix(client): Don't starting sending the old AuthenticationToken right away during reconnect Instead wait until the ActivateSession request. --- src/client/ua_client.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/client/ua_client.c b/src/client/ua_client.c index 9916fb512e1..d64e1786319 100644 --- a/src/client/ua_client.c +++ b/src/client/ua_client.c @@ -330,10 +330,14 @@ sendRequest(UA_Client *client, const void *request, return client->connectStatus; /* Adjusting the request header. The const attribute is violated, but we - * only touch the following members: */ + * reset to the original state before returning. Use the AuthenticationToken + * only once the session is active (or to activate / close it). */ UA_RequestHeader *rr = (UA_RequestHeader*)(uintptr_t)request; UA_NodeId oldToken = rr->authenticationToken; /* Put back in place later */ - rr->authenticationToken = client->authenticationToken; + if(client->sessionState == UA_SESSIONSTATE_ACTIVATED || + requestType == &UA_TYPES[UA_TYPES_ACTIVATESESSIONREQUEST] || + requestType == &UA_TYPES[UA_TYPES_CLOSESESSIONREQUEST]) + rr->authenticationToken = client->authenticationToken; rr->timestamp = UA_DateTime_now(); /* Create a unique handle >100,000 if not manually defined. The handle is From e4f728648fd069542b20fd9a857e76c7cb387489 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 14:27:36 +0200 Subject: [PATCH 06/18] refactor(client): Refactor the endpoint and UserIdentityToken selection This no writes into the client config. Instead the selected endpoint is stored internally in the client. The UserIdentityToken is selected from this endpoint at runtime. --- src/client/ua_client.c | 4 +- src/client/ua_client_connect.c | 435 +++++++++--------- src/client/ua_client_internal.h | 3 +- .../encryption/check_username_connect_none.c | 2 +- tests/server/check_server_password.c | 2 +- 5 files changed, 224 insertions(+), 222 deletions(-) diff --git a/src/client/ua_client.c b/src/client/ua_client.c index d64e1786319..388c94949c5 100644 --- a/src/client/ua_client.c +++ b/src/client/ua_client.c @@ -212,7 +212,7 @@ UA_Client_clear(UA_Client *client) { UA_Client_disconnect(client); UA_String_clear(&client->discoveryUrl); - UA_ApplicationDescription_clear(&client->serverDescription); + UA_EndpointDescription_clear(&client->endpoint); UA_String_clear(&client->serverSessionNonce); UA_String_clear(&client->clientSessionNonce); @@ -1091,7 +1091,7 @@ getConnectionttribute(UA_Client *client, const UA_QualifiedName key, if(UA_QualifiedName_equal(&key, &connectionAttributes[0])) { /* ServerDescription */ - UA_Variant_setScalar(&localAttr, &client->serverDescription, + UA_Variant_setScalar(&localAttr, &client->endpoint.server, &UA_TYPES[UA_TYPES_APPLICATIONDESCRIPTION]); } else if(UA_QualifiedName_equal(&key, &connectionAttributes[1])) { /* SecurityPolicyUri */ diff --git a/src/client/ua_client_connect.c b/src/client/ua_client_connect.c index dd76b57d0ae..1452c1ee8b7 100644 --- a/src/client/ua_client_connect.c +++ b/src/client/ua_client_connect.c @@ -34,14 +34,16 @@ static void initConnect(UA_Client *client); static UA_StatusCode createSessionAsync(UA_Client *client); +static UA_UserTokenPolicy * +findUserTokenPolicy(UA_Client *client, UA_EndpointDescription *endpoint); /* Get the EndpointUrl to be used right now. * This is adjusted during the discovery process. * We fall back if connecting to an EndpointUrl fails. */ static UA_String getEndpointUrl(UA_Client *client) { - if(client->config.endpoint.endpointUrl.length > 0) { - return client->config.endpoint.endpointUrl; + if(client->endpoint.endpointUrl.length > 0) { + return client->endpoint.endpointUrl; } else if(client->discoveryUrl.length > 0) { return client->discoveryUrl; } else { @@ -63,19 +65,19 @@ fallbackEndpointUrl(UA_Client* client) { return UA_STATUSCODE_BADCONNECTIONREJECTED; } - if(client->config.endpoint.endpointUrl.length > 0) { + if(client->endpoint.endpointUrl.length > 0) { /* Overwrite the EndpointUrl of the Endpoint */ UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_CLIENT, "Could not open a TCP connection to the Endpoint at %.*s. " "Overriding the endpoint description with the initial " "EndpointUrl at %.*s.", - (int)client->config.endpoint.endpointUrl.length, - client->config.endpoint.endpointUrl.data, + (int)client->endpoint.endpointUrl.length, + client->endpoint.endpointUrl.data, (int)client->config.endpointUrl.length, client->config.endpointUrl.data); - UA_String_clear(&client->config.endpoint.endpointUrl); + UA_String_clear(&client->endpoint.endpointUrl); return UA_String_copy(&client->config.endpointUrl, - &client->config.endpoint.endpointUrl); + &client->endpoint.endpointUrl); } else { /* Overwrite the DiscoveryUrl returned by FindServers */ UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_CLIENT, @@ -93,6 +95,8 @@ fallbackEndpointUrl(UA_Client* client) { static UA_SecurityPolicy * getSecurityPolicy(UA_Client *client, UA_String policyUri) { + if(policyUri.length == 0) + policyUri = UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#None"); for(size_t i = 0; i < client->config.securityPoliciesSize; i++) { if(UA_String_equal(&policyUri, &client->config.securityPolicies[i].policyUri)) return &client->config.securityPolicies[i]; @@ -113,11 +117,10 @@ getAuthSecurityPolicy(UA_Client *client, UA_String policyUri) { /* The endpoint is unconfigured if the description is all zeroed-out */ static UA_Boolean -endpointUnconfigured(UA_Client *client) { +endpointUnconfigured(const UA_EndpointDescription *endpoint) { UA_EndpointDescription tmp; UA_EndpointDescription_init(&tmp); - return UA_equal(&tmp, &client->config.endpoint, - &UA_TYPES[UA_TYPES_ENDPOINTDESCRIPTION]); + return UA_equal(&tmp, endpoint, &UA_TYPES[UA_TYPES_ENDPOINTDESCRIPTION]); } UA_Boolean @@ -130,9 +133,12 @@ isFullyConnected(UA_Client *client) { if(client->channel.state != UA_SECURECHANNELSTATE_OPEN) return false; - /* Handshake ongoing or not yet done */ - if(client->endpointsHandshake || endpointUnconfigured(client) || - client->findServersHandshake || client->discoveryUrl.length == 0) + /* GetEndpoints handshake ongoing or not yet done */ + if(client->endpointsHandshake || endpointUnconfigured(&client->endpoint)) + return false; + + /* FindServers handshake ongoing or not yet done */ + if(client->findServersHandshake || client->discoveryUrl.length == 0) return false; return true; @@ -148,6 +154,13 @@ signActivateSessionRequest(UA_Client *client, UA_SecureChannel *channel, channel->securityMode != UA_MESSAGESECURITYMODE_SIGNANDENCRYPT) return UA_STATUSCODE_GOOD; + UA_UserTokenPolicy *utp = findUserTokenPolicy(client, &client->endpoint); + if(!utp) { + UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_NETWORK, + "Could not find a matching UserTokenPolicy in the endpoint"); + return UA_STATUSCODE_BADINTERNALERROR; + } + const UA_SecurityPolicy *sp = channel->securityPolicy; UA_SignatureData *sd = &request->clientSignature; const UA_SecurityPolicySignatureAlgorithm *signAlg = @@ -183,11 +196,15 @@ signActivateSessionRequest(UA_Client *client, UA_SecureChannel *channel, if(retval != UA_STATUSCODE_GOOD) goto cleanup; - /* Prepare the userTokenSignature */ - if(client->config.userTokenPolicy.tokenType == UA_USERTOKENTYPE_CERTIFICATE) { - /* Get the correct securityPolicy for authentication */ - UA_SecurityPolicy *utsp = - getAuthSecurityPolicy(client, client->config.authSecurityPolicyUri); + /* Prepare the UserTokenSignature */ + if(utp->tokenType == UA_USERTOKENTYPE_CERTIFICATE) { + /* Get the SecurityPolicy for authentication. If the UserTokenPolicy + * does not define it, the SecurityPolicyUri of the overall endpoint is + * used. The SecurityPolicy must be retrieved with getAuthSecurityPolicy + * as it can use a certificate different from the SecureChannel. */ + UA_String securityPolicyUri = (utp->securityPolicyUri.length > 0) ? + utp->securityPolicyUri : client->endpoint.securityPolicyUri; + UA_SecurityPolicy *utsp = getAuthSecurityPolicy(client, securityPolicyUri); if(!utsp) { UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT, "The configured SecurityPolicy for certificate " @@ -254,7 +271,7 @@ signActivateSessionRequest(UA_Client *client, UA_SecureChannel *channel, } static UA_StatusCode -encryptUserIdentityToken(UA_Client *client, const UA_String *userTokenSecurityPolicy, +encryptUserIdentityToken(UA_Client *client, const UA_String userTokenSecurityPolicy, UA_ExtensionObject *userIdentityToken) { UA_IssuedIdentityToken *iit = NULL; UA_UserNameIdentityToken *unit = NULL; @@ -272,12 +289,12 @@ encryptUserIdentityToken(UA_Client *client, const UA_String *userTokenSecurityPo /* No encryption */ const UA_String none = UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#None"); - if(userTokenSecurityPolicy->length == 0 || - UA_String_equal(userTokenSecurityPolicy, &none)) { + if(userTokenSecurityPolicy.length == 0 || + UA_String_equal(&userTokenSecurityPolicy, &none)) { return UA_STATUSCODE_GOOD; } - UA_SecurityPolicy *sp = getSecurityPolicy(client, *userTokenSecurityPolicy); + UA_SecurityPolicy *sp = getSecurityPolicy(client, userTokenSecurityPolicy); if(!sp) { UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_NETWORK, "Could not find the required SecurityPolicy for the UserToken"); @@ -288,7 +305,7 @@ encryptUserIdentityToken(UA_Client *client, const UA_String *userTokenSecurityPo void *channelContext; UA_StatusCode retval = sp->channelModule. - newContext(sp, &client->config.endpoint.serverCertificate, &channelContext); + newContext(sp, &client->endpoint.serverCertificate, &channelContext); if(retval != UA_STATUSCODE_GOOD) { UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_NETWORK, "Could not instantiate the SecurityPolicy for the UserToken"); @@ -770,6 +787,13 @@ activateSessionAsync(UA_Client *client) { return UA_STATUSCODE_BADSESSIONCLOSED; } + const UA_UserTokenPolicy *utp = findUserTokenPolicy(client, &client->endpoint); + if(!utp) { + UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_NETWORK, + "Could not find a matching UserTokenPolicy in the endpoint"); + return UA_STATUSCODE_BADINTERNALERROR; + } + UA_ActivateSessionRequest request; UA_ActivateSessionRequest_init(&request); UA_StatusCode retval = @@ -802,15 +826,12 @@ activateSessionAsync(UA_Client *client) { /* Set the correct PolicyId from the endpoint */ UA_String_clear((UA_String*)request.userIdentityToken.content.decoded.data); - retval = UA_String_copy(&client->config.userTokenPolicy.policyId, + retval = UA_String_copy(&utp->policyId, (UA_String*)request.userIdentityToken.content.decoded.data); #ifdef UA_ENABLE_ENCRYPTION /* Encrypt the UserIdentityToken */ - const UA_String *userTokenPolicy = &client->channel.securityPolicy->policyUri; - if(client->config.userTokenPolicy.securityPolicyUri.length > 0) - userTokenPolicy = &client->config.userTokenPolicy.securityPolicyUri; - retval |= encryptUserIdentityToken(client, userTokenPolicy, &request.userIdentityToken); + retval |= encryptUserIdentityToken(client, utp->securityPolicyUri, &request.userIdentityToken); retval |= signActivateSessionRequest(client, &client->channel, &request); #endif @@ -832,6 +853,129 @@ activateSessionAsync(UA_Client *client) { return retval; } +static const UA_String binaryTransport = + UA_STRING_STATIC("http://opcfoundation.org/UA-Profile/Transport/uatcp-uasc-uabinary"); + +/* Find a matching endpoint -- the UserTokenPolicy is matched later */ +static UA_Boolean +matchEndpoint(UA_Client *client, const UA_EndpointDescription *endpoint, unsigned i) { + /* Matching ApplicationUri if defined */ + if(client->config.applicationUri.length > 0 && + !UA_String_equal(&client->config.applicationUri, + &endpoint->server.applicationUri)) { + UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, + "Rejecting endpoint %u: application uri not match", i); + return false; + } + + /* Look out for binary transport endpoints. + * Note: Siemens returns empty ProfileUrl, we will accept it as binary. */ + if(endpoint->transportProfileUri.length != 0 && + !UA_String_equal(&endpoint->transportProfileUri, &binaryTransport)) { + UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, + "Rejecting endpoint %u: transport profile does not match", i); + return false; + } + + /* Valid SecurityMode? */ + if(endpoint->securityMode < 1 || endpoint->securityMode > 3) { + UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, + "Rejecting endpoint %u: invalid security mode", i); + return false; + } + + /* Selected SecurityMode? */ + if(client->config.securityMode > 0 && + client->config.securityMode != endpoint->securityMode) { + UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, + "Rejecting endpoint %u: security mode does not match", i); + return false; + } + + /* Matching SecurityPolicy? */ + if(client->config.securityPolicyUri.length > 0 && + !UA_String_equal(&client->config.securityPolicyUri, &endpoint->securityPolicyUri)) { + UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, + "Rejecting endpoint %u: security policy does not match the configuration", i); + return false; + } + + /* SecurityPolicy available? */ + if(!getSecurityPolicy(client, endpoint->securityPolicyUri)) { + UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, + "Rejecting endpoint %u: security policy not available", i); + return false; + } + + return true; +} + +/* Match the policy with the configured user token */ +static UA_Boolean +matchUserToken(UA_Client *client, + const UA_UserTokenPolicy *tokenPolicy) { + const UA_DataType *tokenType = + client->config.userIdentityToken.content.decoded.type; + + if(tokenPolicy->tokenType == UA_USERTOKENTYPE_ANONYMOUS && + (tokenType == &UA_TYPES[UA_TYPES_ANONYMOUSIDENTITYTOKEN] || !tokenType)) + return true; + + if(tokenPolicy->tokenType == UA_USERTOKENTYPE_USERNAME && + tokenType == &UA_TYPES[UA_TYPES_USERNAMEIDENTITYTOKEN]) + return true; + + if(tokenPolicy->tokenType == UA_USERTOKENTYPE_CERTIFICATE && + tokenType == &UA_TYPES[UA_TYPES_X509IDENTITYTOKEN]) + return true; + + if(tokenPolicy->tokenType == UA_USERTOKENTYPE_ISSUEDTOKEN && + tokenType == &UA_TYPES[UA_TYPES_ISSUEDIDENTITYTOKEN]) + return true; + + return false; +} + +/* Returns the locally configured UserTokenPolicy (if it exists) or a match from + * the EndpointDescription. If localTokenPolicy != NULL, then we require an + * exact match with the configured UserTokenPolicy. */ +static UA_UserTokenPolicy * +findUserTokenPolicy(UA_Client *client, UA_EndpointDescription *endpoint) { + /* Was a UserTokenPolicy configured? Then we need an exact match. */ + UA_UserTokenPolicy *requiredTokenPolicy = NULL; + UA_UserTokenPolicy tmp; + UA_UserTokenPolicy_init(&tmp); + if(!UA_equal(&tmp, &client->config.userTokenPolicy, &UA_TYPES[UA_TYPES_USERTOKENPOLICY])) + requiredTokenPolicy = &client->config.userTokenPolicy; + + for(size_t j = 0; j < endpoint->userIdentityTokensSize; ++j) { + /* Is the SecurityPolicy available? */ + UA_UserTokenPolicy *tokenPolicy = &endpoint->userIdentityTokens[j]; + if(!getSecurityPolicy(client, tokenPolicy->securityPolicyUri)) + continue; + + /* Required SecurityPolicyUri in the configuration? */ + if(!UA_String_isEmpty(&client->config.authSecurityPolicyUri) && + !UA_String_equal(&client->config.authSecurityPolicyUri, + &tokenPolicy->securityPolicyUri)) + continue; + + /* Match (entire) UserTokenPolicy if defined in the configuration? */ + if(requiredTokenPolicy && + !UA_equal(requiredTokenPolicy, tokenPolicy, &UA_TYPES[UA_TYPES_USERTOKENPOLICY])) + continue; + + /* Match with the configured UserToken */ + if(!matchUserToken(client, tokenPolicy)) + continue; + + /* Found a match? */ + return tokenPolicy; + } + + return NULL; +} + /* Combination of UA_Client_getEndpointsInternal and getEndpoints */ static void responseGetEndpoints(UA_Client *client, void *userdata, @@ -868,10 +1012,7 @@ responseGetEndpoints(UA_Client *client, void *userdata, const size_t notFound = (size_t)-1; size_t bestEndpointIndex = notFound; - size_t bestTokenIndex = notFound; UA_Byte bestEndpointLevel = 0; - const UA_String binaryTransport = - UA_STRING("http://opcfoundation.org/UA-Profile/Transport/uatcp-uasc-uabinary"); /* Find a matching combination of Endpoint and UserTokenPolicy */ for(size_t i = 0; i < resp->endpointsSize; ++i) { @@ -881,201 +1022,57 @@ responseGetEndpoints(UA_Client *client, void *userdata, if(endpoint->securityLevel < bestEndpointLevel) continue; - /* Filter by the ApplicationURI if defined */ - if(client->config.applicationUri.length > 0 && - !UA_String_equal(&client->config.applicationUri, - &endpoint->server.applicationUri)) + /* Does the endpoint match the client configuration? */ + if(!matchEndpoint(client, endpoint, (unsigned)i)) continue; - /* Look out for binary transport endpoints. - * Note: Siemens returns empty ProfileUrl, we will accept it as binary. */ - if(endpoint->transportProfileUri.length != 0 && - !UA_String_equal(&endpoint->transportProfileUri, &binaryTransport)) - continue; - - /* Valid SecurityMode? */ - if(endpoint->securityMode < 1 || endpoint->securityMode > 3) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting endpoint %lu: invalid security mode", - (long unsigned)i); - continue; - } - - /* Selected SecurityMode? */ - if(client->config.securityMode > 0 && - client->config.securityMode != endpoint->securityMode) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting endpoint %lu: security mode doesn't match", - (long unsigned)i); - continue; - } - - /* Matching SecurityPolicy? */ - if(client->config.securityPolicyUri.length > 0 && - !UA_String_equal(&client->config.securityPolicyUri, - &endpoint->securityPolicyUri)) { + /* Do we want a session? If yes, then the endpoint needs to have a + * UserTokenPolicy that matches the configuration. */ + if(!client->config.noSession && !findUserTokenPolicy(client, endpoint)) { UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting endpoint %lu: security policy doesn't match", + "Rejecting endpoint %lu: No matching UserTokenPolicy", (long unsigned)i); continue; } - /* SecurityPolicy available? */ - if(!getSecurityPolicy(client, endpoint->securityPolicyUri)) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting endpoint %lu: security policy not available", - (long unsigned)i); - continue; - } - - /* We have found a matching endpoint. - * But maybe not a amtching user token policy. */ + /* Best endpoint so far */ + bestEndpointLevel = endpoint->securityLevel; bestEndpointIndex = i; - - /* Compare the available UserTokenPolicies */ - for(size_t j = 0; j < endpoint->userIdentityTokensSize; ++j) { - UA_UserTokenPolicy *tokenPolicy = &endpoint->userIdentityTokens[j]; - const UA_DataType *tokenType = - client->config.userIdentityToken.content.decoded.type; - - /* Usertokens also have a security policy... */ - if(tokenPolicy->tokenType != UA_USERTOKENTYPE_ANONYMOUS && - tokenPolicy->securityPolicyUri.length > 0 && - !getSecurityPolicy(client, tokenPolicy->securityPolicyUri)) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting UserTokenPolicy %lu in endpoint %lu: " - "security policy '%.*s' not available", - (long unsigned)j, (long unsigned)i, - (int)tokenPolicy->securityPolicyUri.length, - tokenPolicy->securityPolicyUri.data); - continue; - } - - if(tokenPolicy->tokenType > 3) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting UserTokenPolicy %lu in endpoint %lu: " - "invalid token type", - (long unsigned)j, (long unsigned)i); - continue; - } - - if(tokenPolicy->tokenType == UA_USERTOKENTYPE_ANONYMOUS && - tokenType != &UA_TYPES[UA_TYPES_ANONYMOUSIDENTITYTOKEN] && - tokenType != NULL) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting UserTokenPolicy %lu (anonymous) in endpoint %lu: " - "configuration doesn't match", - (long unsigned)j, (long unsigned)i); - continue; - } - if(tokenPolicy->tokenType == UA_USERTOKENTYPE_USERNAME && - tokenType != &UA_TYPES[UA_TYPES_USERNAMEIDENTITYTOKEN]) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting UserTokenPolicy %lu (username) in endpoint %lu: " - "configuration doesn't match", - (long unsigned)j, (long unsigned)i); - continue; - } - if(tokenPolicy->tokenType == UA_USERTOKENTYPE_CERTIFICATE && - tokenType != &UA_TYPES[UA_TYPES_X509IDENTITYTOKEN]) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting UserTokenPolicy %lu (certificate) in endpoint %lu: " - "configuration doesn't match", - (long unsigned)j, (long unsigned)i); - continue; - } - if(tokenPolicy->tokenType == UA_USERTOKENTYPE_ISSUEDTOKEN && - tokenType != &UA_TYPES[UA_TYPES_ISSUEDIDENTITYTOKEN]) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Rejecting UserTokenPolicy %lu (token) in endpoint %lu: " - "configuration doesn't match", - (long unsigned)j, (long unsigned)i); - continue; - } - - /* Endpoint with matching UserTokenPolicy found */ - - /* If not explicit PolicyUri for authentication is configured, use - * that from the TokenPolicy or (if that is not defined) from the - * Endpoint SecurityPolicy. */ - UA_String *securityPolicyUri = &tokenPolicy->securityPolicyUri; - if(securityPolicyUri->length == 0) - securityPolicyUri = &endpoint->securityPolicyUri; - if(UA_String_isEmpty(&client->config.authSecurityPolicyUri)) - UA_String_copy(securityPolicyUri, &client->config.authSecurityPolicyUri); - - /* Update tracking */ - bestEndpointLevel = endpoint->securityLevel; - bestTokenIndex = j; - - /* Stop search for the UserTokenPolicy. But we go on searching for - * an endpoints with a better security level. */ - break; - } } + /* No matching endpoint found */ if(bestEndpointIndex == notFound) { UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT, "No suitable endpoint found"); - client->connectStatus = UA_STATUSCODE_BADINTERNALERROR; + client->connectStatus = UA_STATUSCODE_BADIDENTITYTOKENREJECTED; closeSecureChannel(client); UA_UNLOCK(&client->clientMutex); return; } - if(!client->config.noSession && bestTokenIndex == notFound) { - UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT, - "No suitable UserTokenPolicy found for the possible endpoints"); - client->connectStatus = UA_STATUSCODE_BADIDENTITYTOKENINVALID; - closeSecureChannel(client); - UA_UNLOCK(&client->clientMutex); - return; - } - - /* Move the application description and endpoint information to the client config */ - UA_EndpointDescription *endpoint = &resp->endpoints[bestEndpointIndex]; - UA_ApplicationDescription_clear(&client->serverDescription); - UA_ApplicationDescription_copy(&endpoint->server, &client->serverDescription); - UA_EndpointDescription_clear(&client->config.endpoint); - client->config.endpoint = *endpoint; + /* Store the endpoint description in the client. It contains the + * ApplicationDescription and the UserTokenPolicies. We continue to look up + * the matching UserTokenPolicy from there. */ + UA_EndpointDescription_clear(&client->endpoint); + client->endpoint = resp->endpoints[bestEndpointIndex]; + UA_EndpointDescription_init(&resp->endpoints[bestEndpointIndex]); #if UA_LOGLEVEL <= 300 const char *securityModeNames[3] = {"None", "Sign", "SignAndEncrypt"}; UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Selected endpoint %lu in URL %.*s with SecurityMode " + "Selected endpoint with EndpointUrl %.*s, SecurityMode " "%s and SecurityPolicy %.*s", - (long unsigned)bestEndpointIndex, (int)endpoint->endpointUrl.length, - endpoint->endpointUrl.data, securityModeNames[endpoint->securityMode - 1], - (int)endpoint->securityPolicyUri.length, endpoint->securityPolicyUri.data); + (int)client->endpoint.endpointUrl.length, + client->endpoint.endpointUrl.data, + securityModeNames[client->endpoint.securityMode - 1], + (int)client->endpoint.securityPolicyUri.length, + client->endpoint.securityPolicyUri.data); #endif - /* Move the UserTokenPolicy information to the client config */ - if(bestTokenIndex != notFound) { - UA_UserTokenPolicy *tokenPolicy = &endpoint->userIdentityTokens[bestTokenIndex]; - UA_assert(tokenPolicy); - UA_UserTokenPolicy_clear(&client->config.userTokenPolicy); - client->config.userTokenPolicy = *tokenPolicy; - UA_UserTokenPolicy_init(tokenPolicy); -#if UA_LOGLEVEL <= 300 - UA_String *securityPolicyUri = &tokenPolicy->securityPolicyUri; - const char *userTokenTypeNames[4] = - {"Anonymous", "UserName", "Certificate", "IssuedToken"}; - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Selected UserTokenPolicy %.*s with UserTokenType %s " - "and SecurityPolicy %.*s", - (int)tokenPolicy->policyId.length, tokenPolicy->policyId.data, - userTokenTypeNames[tokenPolicy->tokenType], - (int)securityPolicyUri->length, securityPolicyUri->data); -#endif - } - - /* Don't clean up later -- was moved to the client config */ - UA_EndpointDescription_init(endpoint); - - /* Close the SecureChannel -- a different SecurityMode or SecurityPolicy is - * defined by the Endpoint. */ - if(client->config.endpoint.securityMode != client->channel.securityMode || - !UA_String_equal(&client->config.endpoint.securityPolicyUri, + /* A different SecurityMode or SecurityPolicy is defined by the Endpoint. + * Close the SecureChannel and reconnect. */ + if(client->endpoint.securityMode != client->channel.securityMode || + !UA_String_equal(&client->endpoint.securityPolicyUri, &client->channel.securityPolicy->policyUri)) { closeSecureChannel(client); UA_UNLOCK(&client->clientMutex); @@ -1086,8 +1083,7 @@ responseGetEndpoints(UA_Client *client, void *userdata, * Close the SecureChannel to reopen with the EndpointUrl. If an endpoint * was selected, then we use the endpointUrl for the HEL message. */ if(client->discoveryUrl.length > 0 && - !UA_String_equal(&client->discoveryUrl, - &client->config.endpoint.endpointUrl)) { + !UA_String_equal(&client->discoveryUrl, &client->endpoint.endpointUrl)) { closeSecureChannel(client); UA_UNLOCK(&client->clientMutex); return; @@ -1318,7 +1314,7 @@ createSessionAsync(UA_Client *client) { request.clientNonce = client->clientSessionNonce; request.requestedSessionTimeout = client->config.requestedSessionTimeout; request.maxResponseMessageSize = UA_INT32_MAX; - request.endpointUrl = client->config.endpoint.endpointUrl; + request.endpointUrl = client->endpoint.endpointUrl; request.clientDescription = client->config.clientDescription; request.sessionName = client->config.sessionName; if(client->channel.securityMode == UA_MESSAGESECURITYMODE_SIGN || @@ -1344,13 +1340,8 @@ createSessionAsync(UA_Client *client) { static UA_StatusCode initSecurityPolicy(UA_Client *client) { /* Find the SecurityPolicy */ - UA_SecurityPolicy *sp = NULL; - if(client->config.endpoint.securityPolicyUri.length == 0) { - sp = getSecurityPolicy(client, - UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#None")); - } else { - sp = getSecurityPolicy(client, client->config.endpoint.securityPolicyUri); - } + UA_SecurityPolicy *sp = + getSecurityPolicy(client, client->endpoint.securityPolicyUri); /* Unknown SecurityPolicy -- we would never select such an endpoint */ if(!sp) @@ -1362,13 +1353,13 @@ initSecurityPolicy(UA_Client *client) { UA_STATUSCODE_GOOD : UA_STATUSCODE_BADINTERNALERROR; /* Set the SecurityMode -- none if no endpoint is selected so far */ - client->channel.securityMode = client->config.endpoint.securityMode; + client->channel.securityMode = client->endpoint.securityMode; if(client->channel.securityMode == UA_MESSAGESECURITYMODE_INVALID) client->channel.securityMode = UA_MESSAGESECURITYMODE_NONE; /* Instantiate the SecurityPolicy context with the remote certificate */ return UA_SecureChannel_setSecurityPolicy(&client->channel, sp, - &client->config.endpoint.serverCertificate); + &client->endpoint.serverCertificate); } static void @@ -1440,7 +1431,7 @@ connectActivity(UA_Client *client) { /* GetEndpoints to identify the remote side and/or reset the SecureChannel * with encryption */ - if(endpointUnconfigured(client)) { + if(endpointUnconfigured(&client->endpoint)) { client->connectStatus = requestGetEndpoints(client); return; } @@ -1492,8 +1483,8 @@ verifyClientSecureChannelHeader(void *application, UA_SecureChannel *channel, /* If encryption is used, check that the server certificate for the * endpoint is used for the SecureChannel */ UA_ByteString serverCert = getLeafCertificate(asymHeader->senderCertificate); - if(client->config.endpoint.serverCertificate.length > 0 && - !UA_String_equal(&client->config.endpoint.serverCertificate, &serverCert)) { + if(client->endpoint.serverCertificate.length > 0 && + !UA_String_equal(&client->endpoint.serverCertificate, &serverCert)) { UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT, "The server certificate is different from the EndpointDescription"); return UA_STATUSCODE_BADSECURITYCHECKSFAILED; @@ -1666,6 +1657,14 @@ initConnect(UA_Client *client) { return; } + /* An exact endpoint was configured. Use it. */ + if(!endpointUnconfigured(&client->config.endpoint)) { + UA_EndpointDescription_clear(&client->endpoint); + client->connectStatus = + UA_EndpointDescription_copy(&client->config.endpoint, &client->endpoint); + UA_CHECK_STATUS(client->connectStatus, return); + } + /* Start the EventLoop if not already started */ client->connectStatus = __UA_Client_startup(client); UA_CHECK_STATUS(client->connectStatus, return); @@ -2198,8 +2197,10 @@ cleanupSession(UA_Client *client) { static void disconnectSecureChannel(UA_Client *client, UA_Boolean sync) { - /* Clean the DiscoveryUrl when the connection is explicitly closed */ + /* Clean the DiscoveryUrl and endpoint description when the connection is + * explicitly closed */ UA_String_clear(&client->discoveryUrl); + UA_EndpointDescription_clear(&client->endpoint); /* Close the SecureChannel */ closeSecureChannel(client); diff --git a/src/client/ua_client_internal.h b/src/client/ua_client_internal.h index f03df801f26..6ed8f28e879 100644 --- a/src/client/ua_client_internal.h +++ b/src/client/ua_client_internal.h @@ -132,7 +132,8 @@ struct UA_Client { * EndpointUrl != DiscoveryUrl. */ UA_String discoveryUrl; - UA_ApplicationDescription serverDescription; + /* Contains the Server description, etc. */ + UA_EndpointDescription endpoint; /* SecureChannel */ UA_SecureChannel channel; diff --git a/tests/encryption/check_username_connect_none.c b/tests/encryption/check_username_connect_none.c index 3db48f7dd60..2a74a84d2c2 100644 --- a/tests/encryption/check_username_connect_none.c +++ b/tests/encryption/check_username_connect_none.c @@ -88,7 +88,7 @@ START_TEST(none_policy_connect_fail) { UA_StatusCode retval = UA_Client_connectUsername(client, "opc.tcp://localhost:4840", "user1", "password"); - ck_assert_uint_eq(retval, UA_STATUSCODE_BADIDENTITYTOKENINVALID); + ck_assert_uint_eq(retval, UA_STATUSCODE_BADIDENTITYTOKENREJECTED); UA_Client_disconnect(client); UA_Client_delete(client); diff --git a/tests/server/check_server_password.c b/tests/server/check_server_password.c index 61e0ce6d6d0..a282190077d 100644 --- a/tests/server/check_server_password.c +++ b/tests/server/check_server_password.c @@ -231,7 +231,7 @@ START_TEST(Password_none) { UA_ClientConfig_setDefault(config); UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4840"); - ck_assert_uint_eq(retval, UA_STATUSCODE_BADIDENTITYTOKENINVALID); + ck_assert_uint_eq(retval, UA_STATUSCODE_BADIDENTITYTOKENREJECTED); UA_Client_disconnect(client); UA_Client_delete(client); } END_TEST From 7ebb8cc329c93d044fb7d30c68ad2bf9334ef582 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 19:10:31 +0200 Subject: [PATCH 07/18] refactor(client): Improve the documentation of endpoint and userTokenPolicy in the client config --- include/open62541/client.h | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/include/open62541/client.h b/include/open62541/client.h index 31dcbe39547..6913f3f70e3 100644 --- a/include/open62541/client.h +++ b/include/open62541/client.h @@ -116,27 +116,16 @@ struct UA_ClientConfig { * connection when the Session is lost. */ /** - * If either endpoint or userTokenPolicy has been set (at least one non-zero - * byte in either structure), then the selected Endpoint and UserTokenPolicy - * overwrite the settings in the basic connection configuration. The - * userTokenPolicy array in the EndpointDescription is ignored. The selected - * userTokenPolicy is set in the dedicated configuration field. - * - * If the advanced configuration is not set, the client will write to it the - * selected Endpoint and UserTokenPolicy during GetEndpoints. - * - * The information in the advanced configuration is used during reconnect - * when the SecureChannel was broken. */ + * If either endpoint or userTokenPolicy has been set, then they are used + * directly. Otherwise this information comes from the GetEndpoints response + * from the server (filtered and selected for the SecurityMode, etc.). */ UA_EndpointDescription endpoint; UA_UserTokenPolicy userTokenPolicy; /** * If the EndpointDescription has not been defined, the ApplicationURI - * constrains the servers considered in the FindServers service and the - * Endpoints considered in the GetEndpoints service. - * - * If empty the applicationURI is not used to filter. - */ + * filters the servers considered in the FindServers service and the + * Endpoints considered in the GetEndpoints service. */ UA_String applicationUri; /** @@ -179,6 +168,7 @@ struct UA_ClientConfig { * secure channel is selected.*/ size_t authSecurityPoliciesSize; UA_SecurityPolicy *authSecurityPolicies; + /* SecurityPolicyUri for the Authentication. */ UA_String authSecurityPolicyUri; From 93a4b986e9f257b262bff588495accc651fdd7d8 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 19:11:21 +0200 Subject: [PATCH 08/18] refactor(client): Improve the documentation of securityMode in the client config --- include/open62541/client.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/open62541/client.h b/include/open62541/client.h index 6913f3f70e3..7ab9ace17c7 100644 --- a/include/open62541/client.h +++ b/include/open62541/client.h @@ -101,9 +101,9 @@ struct UA_ClientConfig { * message. */ UA_ExtensionObject userIdentityToken; /* Configured User-Identity Token */ UA_MessageSecurityMode securityMode; /* None, Sign, SignAndEncrypt. The - * default is invalid. This indicates - * the client to select any matching - * endpoint. */ + * default is "invalid". This + * indicates the client to select any + * matching endpoint. */ UA_String securityPolicyUri; /* SecurityPolicy for the SecureChannel. An * empty string indicates the client to select * any matching SecurityPolicy. */ From a2950d7209c9b7c2f7c7e7672f65e19c03bca13f Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 4 Oct 2024 09:20:23 +0200 Subject: [PATCH 09/18] refactor(client): Small cleanup in responseFindServers --- src/client/ua_client_connect.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/client/ua_client_connect.c b/src/client/ua_client_connect.c index 1452c1ee8b7..6026971c3ea 100644 --- a/src/client/ua_client_connect.c +++ b/src/client/ua_client_connect.c @@ -1121,7 +1121,6 @@ static void responseFindServers(UA_Client *client, void *userdata, UA_UInt32 requestId, void *response) { client->findServersHandshake = false; - UA_LOG_DEBUG(client->config.logging, UA_LOGCATEGORY_CLIENT, "Received FindServersResponse"); @@ -1135,6 +1134,7 @@ responseFindServers(UA_Client *client, void *userdata, UA_StatusCode_name(fsr->responseHeader.serviceResult), (int)client->config.endpointUrl.length, client->config.endpointUrl.data); + UA_String_clear(&client->discoveryUrl); UA_String_copy(&client->config.endpointUrl, &client->discoveryUrl); return; } @@ -1145,18 +1145,14 @@ responseFindServers(UA_Client *client, void *userdata, /* Filter by the ApplicationURI if defined */ if(client->config.applicationUri.length > 0 && - !UA_String_equal(&client->config.applicationUri, - &server->applicationUri)) + !UA_String_equal(&client->config.applicationUri, &server->applicationUri)) continue; for(size_t j = 0; j < server->discoveryUrlsSize; j++) { if(UA_String_equal(&client->config.endpointUrl, &server->discoveryUrls[j])) { - UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "The initially defined EndpointURL %.*s " - "is valid for the server", - (int)client->config.endpointUrl.length, - client->config.endpointUrl.data); - UA_String_copy(&client->config.endpointUrl, &client->discoveryUrl); + UA_String_clear(&client->discoveryUrl); + client->discoveryUrl = server->discoveryUrls[j]; + UA_String_init(&server->discoveryUrls[j]); return; } } @@ -1173,8 +1169,7 @@ responseFindServers(UA_Client *client, void *userdata, /* Filter by the ApplicationURI if defined */ if(client->config.applicationUri.length > 0 && - !UA_String_equal(&client->config.applicationUri, - &server->applicationUri)) + !UA_String_equal(&client->config.applicationUri, &server->applicationUri)) continue; for(size_t j = 0; j < server->discoveryUrlsSize; j++) { @@ -1183,8 +1178,7 @@ responseFindServers(UA_Client *client, void *userdata, UA_String hostname, path; UA_UInt16 port; UA_StatusCode res = - UA_parseEndpointUrl(&server->discoveryUrls[j], &hostname, - &port, &path); + UA_parseEndpointUrl(&server->discoveryUrls[j], &hostname, &port, &path); if(res != UA_STATUSCODE_GOOD) continue; @@ -1194,11 +1188,12 @@ responseFindServers(UA_Client *client, void *userdata, UA_String_init(&server->discoveryUrls[j]); UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT, - "Use the EndpointURL %.*s returned from FindServers", + "Use the EndpointURL %.*s returned from FindServers and reconnect", (int)client->discoveryUrl.length, client->discoveryUrl.data); /* Close the SecureChannel to build it up new with the correct - * EndpointURL in the HEL/ACK handshake */ + * EndpointURL in the HEL/ACK handshake. In closeSecureChannel the + * received client->endpoint is reset also. */ closeSecureChannel(client); return; } From 2f1d82ed851356b5e097699706878867d39f8104 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 4 Oct 2024 13:23:56 +0200 Subject: [PATCH 10/18] fix(pubsub): Fix potential double free in the SKS client config --- src/pubsub/ua_pubsub_keystorage.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pubsub/ua_pubsub_keystorage.c b/src/pubsub/ua_pubsub_keystorage.c index 6ec48e8567c..799cca43275 100644 --- a/src/pubsub/ua_pubsub_keystorage.c +++ b/src/pubsub/ua_pubsub_keystorage.c @@ -503,6 +503,8 @@ sksClientCleanupCb(void *client, void *context) { * be freed in UA_PubSubKeyStorage_delete. */ sksClient->config.securityPolicies = NULL; sksClient->config.securityPoliciesSize = 0; + sksClient->config.authSecurityPolicies = NULL; + sksClient->config.authSecurityPoliciesSize = 0; sksClient->config.certificateVerification.context = NULL; sksClient->config.logging = NULL; sksClient->config.clientContext = NULL; From b540d891476a49f2f31eeac9f26df83325d87f26 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 4 Oct 2024 16:12:16 +0200 Subject: [PATCH 11/18] fix(plugins): Don't enable Basic128Rsa15 and Basic256 SecurityPolicies by default for authentication --- plugins/ua_config_default.c | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/plugins/ua_config_default.c b/plugins/ua_config_default.c index aaf2a9c51b6..aa73b6a948f 100644 --- a/plugins/ua_config_default.c +++ b/plugins/ua_config_default.c @@ -1330,25 +1330,27 @@ UA_ClientConfig_setAuthenticationCert(UA_ClientConfig *config, return UA_STATUSCODE_BADOUTOFMEMORY; config->authSecurityPolicies = sp; - retval = UA_SecurityPolicy_Basic128Rsa15(&config->authSecurityPolicies[config->authSecurityPoliciesSize], - certificateAuth, privateKeyAuth, config->logging); - if(retval == UA_STATUSCODE_GOOD) { - ++config->authSecurityPoliciesSize; - } else { - UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", - UA_StatusCode_name(retval)); - } + /* Basic128Rsa15 is unsecure and should not be used */ + /* retval = UA_SecurityPolicy_Basic128Rsa15(&config->authSecurityPolicies[config->authSecurityPoliciesSize], */ + /* certificateAuth, privateKeyAuth, config->logging); */ + /* if(retval == UA_STATUSCODE_GOOD) { */ + /* ++config->authSecurityPoliciesSize; */ + /* } else { */ + /* UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ - retval = UA_SecurityPolicy_Basic256(&config->authSecurityPolicies[config->authSecurityPoliciesSize], - certificateAuth, privateKeyAuth, config->logging); - if(retval == UA_STATUSCODE_GOOD) { - ++config->authSecurityPoliciesSize; - } else { - UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Basic256 with error code %s", - UA_StatusCode_name(retval)); - } + /* Basic256 is unsecure and should not be used */ + /* retval = UA_SecurityPolicy_Basic256(&config->authSecurityPolicies[config->authSecurityPoliciesSize], */ + /* certificateAuth, privateKeyAuth, config->logging); */ + /* if(retval == UA_STATUSCODE_GOOD) { */ + /* ++config->authSecurityPoliciesSize; */ + /* } else { */ + /* UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic256 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ retval = UA_SecurityPolicy_Aes256Sha256RsaPss(&config->authSecurityPolicies[config->authSecurityPoliciesSize], certificateAuth, privateKeyAuth, config->logging); From 0feab11c3f4da51676cd37dd80460938d2637b11 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 4 Oct 2024 20:23:03 +0200 Subject: [PATCH 12/18] refactor(plugin): Set the AuthenticationSecurityPolicies by default --- plugins/ua_config_default.c | 148 +++++++++++++++++++++--------------- 1 file changed, 85 insertions(+), 63 deletions(-) diff --git a/plugins/ua_config_default.c b/plugins/ua_config_default.c index aa73b6a948f..0c22c991372 100644 --- a/plugins/ua_config_default.c +++ b/plugins/ua_config_default.c @@ -1183,6 +1183,83 @@ UA_ClientConfig_setDefault(UA_ClientConfig *config) { #ifdef UA_ENABLE_ENCRYPTION +static UA_StatusCode +clientConfig_setAuthenticationSecurityPolicies(UA_ClientConfig *config, + UA_ByteString certificateAuth, + UA_ByteString privateKeyAuth) { + UA_SecurityPolicy *sp = (UA_SecurityPolicy*) + UA_realloc(config->authSecurityPolicies, sizeof(UA_SecurityPolicy) * 3); + if(!sp) + return UA_STATUSCODE_BADOUTOFMEMORY; + config->authSecurityPolicies = sp; + + /* Clean up old SecurityPolicies */ + for(size_t i = 0; i < config->authSecurityPoliciesSize; i++) { + config->authSecurityPolicies[i].clear(&config->authSecurityPolicies[i]); + } + config->authSecurityPoliciesSize = 0; + + /* Basic128Rsa15 is unsecure and should not be used */ + /* sp = &config->authSecurityPolicies[config->authSecurityPoliciesSize]; */ + /* retval = UA_SecurityPolicy_Basic128Rsa15(sp, certificateAuth, privateKeyAuth, config->logging); */ + /* if(retval == UA_STATUSCODE_GOOD) { */ + /* ++config->authSecurityPoliciesSize; */ + /* } else { */ + /* UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ + + /* Basic256 is unsecure and should not be used */ + /* sp = &config->authSecurityPolicies[config->authSecurityPoliciesSize]; */ + /* retval = UA_SecurityPolicy_Basic256(sp, certificateAuth, privateKeyAuth, config->logging); */ + /* if(retval == UA_STATUSCODE_GOOD) { */ + /* ++config->authSecurityPoliciesSize; */ + /* } else { */ + /* UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, */ + /* "Could not add SecurityPolicy#Basic256 with error code %s", */ + /* UA_StatusCode_name(retval)); */ + /* } */ + + UA_StatusCode retval; + sp = &config->authSecurityPolicies[config->authSecurityPoliciesSize]; + retval = UA_SecurityPolicy_Aes256Sha256RsaPss(sp, certificateAuth, privateKeyAuth, config->logging); + if(retval == UA_STATUSCODE_GOOD) { + ++config->authSecurityPoliciesSize; + } else { + UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, + "Could not add SecurityPolicy#Aes256Sha256RsaPss with error code %s", + UA_StatusCode_name(retval)); + } + + sp = &config->authSecurityPolicies[config->authSecurityPoliciesSize]; + retval = UA_SecurityPolicy_Basic256Sha256(sp, certificateAuth, privateKeyAuth, config->logging); + if(retval == UA_STATUSCODE_GOOD) { + ++config->authSecurityPoliciesSize; + } else { + UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, + "Could not add SecurityPolicy#Basic256Sha256 with error code %s", + UA_StatusCode_name(retval)); + } + + sp = &config->authSecurityPolicies[config->authSecurityPoliciesSize]; + retval = UA_SecurityPolicy_Aes128Sha256RsaOaep(sp, certificateAuth, privateKeyAuth, config->logging); + if(retval == UA_STATUSCODE_GOOD) { + ++config->authSecurityPoliciesSize; + } else { + UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, + "Could not add SecurityPolicy#Aes128Sha256RsaOaep with error code %s", + UA_StatusCode_name(retval)); + } + + if(config->authSecurityPoliciesSize == 0) { + UA_free(config->authSecurityPolicies); + config->authSecurityPolicies = NULL; + } + + return retval; +} + UA_StatusCode UA_ClientConfig_setDefaultEncryption(UA_ClientConfig *config, UA_ByteString localCertificate, UA_ByteString privateKey, @@ -1287,6 +1364,12 @@ UA_ClientConfig_setDefaultEncryption(UA_ClientConfig *config, UA_StatusCode_name(retval)); } + /* Set the same certificate also for authentication. + * Can be overridden with a different certificate. */ + if(config->authSecurityPoliciesSize == 0) + clientConfig_setAuthenticationSecurityPolicies(config, localCertificate, + decryptedPrivateKey); + UA_ByteString_memZero(&decryptedPrivateKey); UA_ByteString_clear(&decryptedPrivateKey); @@ -1309,6 +1392,7 @@ UA_ClientConfig_setAuthenticationCert(UA_ClientConfig *config, "Certificate authentication with LibreSSL as crypto backend is not supported."); return UA_STATUSCODE_BADNOTIMPLEMENTED; #endif + /* Create UserIdentityToken */ UA_X509IdentityToken* identityToken = UA_X509IdentityToken_new(); if(!identityToken) @@ -1324,68 +1408,6 @@ UA_ClientConfig_setAuthenticationCert(UA_ClientConfig *config, config->userIdentityToken.content.decoded.data = identityToken; /* Populate SecurityPolicies */ - UA_SecurityPolicy *sp = (UA_SecurityPolicy*) - UA_realloc(config->authSecurityPolicies, sizeof(UA_SecurityPolicy) * 5); - if(!sp) - return UA_STATUSCODE_BADOUTOFMEMORY; - config->authSecurityPolicies = sp; - - /* Basic128Rsa15 is unsecure and should not be used */ - /* retval = UA_SecurityPolicy_Basic128Rsa15(&config->authSecurityPolicies[config->authSecurityPoliciesSize], */ - /* certificateAuth, privateKeyAuth, config->logging); */ - /* if(retval == UA_STATUSCODE_GOOD) { */ - /* ++config->authSecurityPoliciesSize; */ - /* } else { */ - /* UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, */ - /* "Could not add SecurityPolicy#Basic128Rsa15 with error code %s", */ - /* UA_StatusCode_name(retval)); */ - /* } */ - - /* Basic256 is unsecure and should not be used */ - /* retval = UA_SecurityPolicy_Basic256(&config->authSecurityPolicies[config->authSecurityPoliciesSize], */ - /* certificateAuth, privateKeyAuth, config->logging); */ - /* if(retval == UA_STATUSCODE_GOOD) { */ - /* ++config->authSecurityPoliciesSize; */ - /* } else { */ - /* UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, */ - /* "Could not add SecurityPolicy#Basic256 with error code %s", */ - /* UA_StatusCode_name(retval)); */ - /* } */ - - retval = UA_SecurityPolicy_Aes256Sha256RsaPss(&config->authSecurityPolicies[config->authSecurityPoliciesSize], - certificateAuth, privateKeyAuth, config->logging); - if(retval == UA_STATUSCODE_GOOD) { - ++config->authSecurityPoliciesSize; - } else { - UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Aes256Sha256RsaPss with error code %s", - UA_StatusCode_name(retval)); - } - - retval = UA_SecurityPolicy_Basic256Sha256(&config->authSecurityPolicies[config->authSecurityPoliciesSize], - certificateAuth, privateKeyAuth, config->logging); - if(retval == UA_STATUSCODE_GOOD) { - ++config->authSecurityPoliciesSize; - } else { - UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Basic256Sha256 with error code %s", - UA_StatusCode_name(retval)); - } - - retval = UA_SecurityPolicy_Aes128Sha256RsaOaep(&config->authSecurityPolicies[config->authSecurityPoliciesSize], - certificateAuth, privateKeyAuth, config->logging); - if(retval == UA_STATUSCODE_GOOD) { - ++config->authSecurityPoliciesSize; - } else { - UA_LOG_WARNING(config->logging, UA_LOGCATEGORY_USERLAND, - "Could not add SecurityPolicy#Aes128Sha256RsaOaep with error code %s", - UA_StatusCode_name(retval)); - } - - if(config->authSecurityPoliciesSize == 0) { - UA_free(config->authSecurityPolicies); - config->authSecurityPolicies = NULL; - } - return UA_STATUSCODE_GOOD; + return clientConfig_setAuthenticationSecurityPolicies(config, certificateAuth, privateKeyAuth); } #endif From 257252e28a2664bb391b2824a873b0517abd4ef5 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 4 Oct 2024 20:23:44 +0200 Subject: [PATCH 13/18] refactor(server): Silence a noisy warning in discovery --- src/server/ua_services_discovery.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/server/ua_services_discovery.c b/src/server/ua_services_discovery.c index 1270d63c8f1..3e28e013a0a 100644 --- a/src/server/ua_services_discovery.c +++ b/src/server/ua_services_discovery.c @@ -294,11 +294,8 @@ getDefaultEncryptedSecurityPolicy(UA_Server *server) { if(!UA_String_equal(&UA_SECURITY_POLICY_NONE_URI, &sp->policyUri)) return sp; } - UA_LOG_WARNING(server->config.logging, UA_LOGCATEGORY_CLIENT, - "Could not find a SecurityPolicy with encryption for the " - "UserTokenPolicy. Using an unencrypted policy."); return server->config.securityPoliciesSize > 0 ? - &server->config.securityPolicies[0]: NULL; + &server->config.securityPolicies[0] : NULL; } const char *securityModeStrs[4] = {"-invalid", "-none", "-sign", "-sign+encrypt"}; From 8def851476113888950237991a652e2c5c28e1e6 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 4 Oct 2024 20:24:27 +0200 Subject: [PATCH 14/18] feat(server): Decrypt received IssuedIdentityToken --- src/server/ua_services_session.c | 60 ++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/server/ua_services_session.c b/src/server/ua_services_session.c index 85235e13766..23b29f3e034 100644 --- a/src/server/ua_services_session.c +++ b/src/server/ua_services_session.c @@ -533,22 +533,23 @@ selectEndpointAndTokenPolicy(UA_Server *server, UA_SecureChannel *channel, #ifdef UA_ENABLE_ENCRYPTION static UA_StatusCode -decryptUserNamePW(UA_Server *server, UA_Session *session, - const UA_SecurityPolicy *sp, - UA_UserNameIdentityToken *userToken) { +decryptUserToken(UA_Server *server, UA_Session *session, + UA_SecureChannel *channel, const UA_SecurityPolicy *sp, + const UA_String encryptionAlgorithm, UA_String *encrypted) { /* If SecurityPolicy is None there shall be no EncryptionAlgorithm */ if(UA_String_equal(&sp->policyUri, &UA_SECURITY_POLICY_NONE_URI)) { - if(userToken->encryptionAlgorithm.length > 0) + if(encryptionAlgorithm.length > 0) return UA_STATUSCODE_BADIDENTITYTOKENINVALID; - - UA_LOG_WARNING_SESSION(server->config.logging, session, "ActivateSession: " - "Received an unencrypted username/passwort. " - "Is the server misconfigured to allow that?"); + if(channel->securityMode == UA_MESSAGESECURITYMODE_NONE) { + UA_LOG_WARNING_SESSION(server->config.logging, session, "ActivateSession: " + "Received an unencrypted UserToken. " + "Is the server misconfigured to allow that?"); + } return UA_STATUSCODE_GOOD; } /* Test if the correct encryption algorithm is used */ - if(!UA_String_equal(&userToken->encryptionAlgorithm, + if(!UA_String_equal(&encryptionAlgorithm, &sp->asymmetricModule.cryptoModule.encryptionAlgorithm.uri)) return UA_STATUSCODE_BADIDENTITYTOKENINVALID; @@ -580,13 +581,12 @@ decryptUserNamePW(UA_Server *server, UA_Session *session, res = UA_STATUSCODE_BADIDENTITYTOKENINVALID; /* Decrypt the secret */ - if(UA_ByteString_copy(&userToken->password, &secret) != UA_STATUSCODE_GOOD || + if(UA_ByteString_copy(encrypted, &secret) != UA_STATUSCODE_GOOD || asymEnc->decrypt(tempChannelContext, &secret) != UA_STATUSCODE_GOOD) goto cleanup; /* The secret starts with a UInt32 length for the content */ - if(UA_UInt32_decodeBinary(&secret, &offset, - &secretLen) != UA_STATUSCODE_GOOD) + if(UA_UInt32_decodeBinary(&secret, &offset, &secretLen) != UA_STATUSCODE_GOOD) goto cleanup; /* The decrypted data must be large enough to include the Encrypted Token @@ -616,9 +616,9 @@ decryptUserNamePW(UA_Server *server, UA_Session *session, * decrypted password. The encryptionAlgorithm and policyId fields are left * in the UserToken as an indication for the AccessControl plugin that * evaluates the decrypted content. */ - memcpy(userToken->password.data, + memcpy(encrypted->data, &secret.data[sizeof(UA_UInt32)], secretLen - sn->length); - userToken->password.length = secretLen - sn->length; + encrypted->length = secretLen - sn->length; res = UA_STATUSCODE_GOOD; cleanup: @@ -642,7 +642,7 @@ static UA_StatusCode checkActivateSessionX509(UA_Server *server, UA_Session *session, const UA_SecurityPolicy *sp, UA_X509IdentityToken* token, const UA_SignatureData *tokenSignature) { - /* The SecurityPolicy must be None */ + /* The SecurityPolicy must not be None for the signature */ if(UA_String_equal(&sp->policyUri, &UA_SECURITY_POLICY_NONE_URI)) return UA_STATUSCODE_BADIDENTITYTOKENINVALID; @@ -753,29 +753,37 @@ Service_ActivateSession(UA_Server *server, UA_SecureChannel *channel, goto rejected; } + /* Decrypt (or validate the signature) of the UserToken. The DataType of the + * UserToken was already checked in selectEndpointAndTokenPolicy */ #ifdef UA_ENABLE_ENCRYPTION if(utp->tokenType == UA_USERTOKENTYPE_USERNAME) { /* If it is a UserNameIdentityToken, the password may be encrypted */ UA_UserNameIdentityToken *userToken = (UA_UserNameIdentityToken *) req->userIdentityToken.content.decoded.data; resp->responseHeader.serviceResult = - decryptUserNamePW(server, session, tokenSp, userToken); - if(resp->responseHeader.serviceResult != UA_STATUSCODE_GOOD) - goto securityRejected; + decryptUserToken(server, session, channel, tokenSp, + userToken->encryptionAlgorithm, &userToken->password); } else if(utp->tokenType == UA_USERTOKENTYPE_CERTIFICATE) { /* If it is a X509IdentityToken, check the userTokenSignature. Note this * only validates that the user has the corresponding private key for - * the given user cetificate. Checking whether the user certificate is + * the given user certificate. Checking whether the user certificate is * trusted has to be implemented in the access control plugin. The * entire token is forwarded in the call to ActivateSession. */ - UA_X509IdentityToken* token = (UA_X509IdentityToken*) + UA_X509IdentityToken* x509token = (UA_X509IdentityToken*) req->userIdentityToken.content.decoded.data; - resp->responseHeader.serviceResult = - checkActivateSessionX509(server, session, tokenSp, - token, &req->userTokenSignature); - if(resp->responseHeader.serviceResult != UA_STATUSCODE_GOOD) - goto securityRejected; - } + resp->responseHeader.serviceResult = + checkActivateSessionX509(server, session, tokenSp, + x509token, &req->userTokenSignature); + } else if(utp->tokenType == UA_USERTOKENTYPE_ISSUEDTOKEN) { + /* IssuedTokens are encrypted */ + UA_IssuedIdentityToken *issuedToken = (UA_IssuedIdentityToken*) + req->userIdentityToken.content.decoded.data; + resp->responseHeader.serviceResult = decryptUserToken( + server, session, channel, tokenSp, issuedToken->encryptionAlgorithm, + &issuedToken->tokenData); + } /* else Anonymous */ + if(resp->responseHeader.serviceResult != UA_STATUSCODE_GOOD) + goto securityRejected; #endif /* Callback into userland access control */ From 8312860037b48f4b1b0dabcb3ff5573b23ad52a9 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Fri, 4 Oct 2024 21:19:51 +0200 Subject: [PATCH 15/18] refactor(client): Pull out activate session signing and encryption into dedicated methods --- src/client/ua_client_connect.c | 293 ++++++++++++++++----------------- 1 file changed, 139 insertions(+), 154 deletions(-) diff --git a/src/client/ua_client_connect.c b/src/client/ua_client_connect.c index 6026971c3ea..458837f8c91 100644 --- a/src/client/ua_client_connect.c +++ b/src/client/ua_client_connect.c @@ -146,21 +146,11 @@ isFullyConnected(UA_Client *client) { #ifdef UA_ENABLE_ENCRYPTION -/* Function to create a signature using remote certificate and nonce */ +/* Function to create a signature using remote certificate and nonce. + * This uses the SecurityPolicy of the SecureChannel. */ static UA_StatusCode -signActivateSessionRequest(UA_Client *client, UA_SecureChannel *channel, - UA_ActivateSessionRequest *request) { - if(channel->securityMode != UA_MESSAGESECURITYMODE_SIGN && - channel->securityMode != UA_MESSAGESECURITYMODE_SIGNANDENCRYPT) - return UA_STATUSCODE_GOOD; - - UA_UserTokenPolicy *utp = findUserTokenPolicy(client, &client->endpoint); - if(!utp) { - UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_NETWORK, - "Could not find a matching UserTokenPolicy in the endpoint"); - return UA_STATUSCODE_BADINTERNALERROR; - } - +signClientSignature(UA_Client *client, UA_ActivateSessionRequest *request) { + UA_SecureChannel *channel = &client->channel; const UA_SecurityPolicy *sp = channel->securityPolicy; UA_SignatureData *sd = &request->clientSignature; const UA_SecurityPolicySignatureAlgorithm *signAlg = @@ -177,101 +167,71 @@ signActivateSessionRequest(UA_Client *client, UA_SecureChannel *channel, if(retval != UA_STATUSCODE_GOOD) return retval; - /* Allocate a temporary buffer */ - UA_ByteString dataToSign; - size_t dataToSignSize = + /* Create a temporary buffer */ + size_t signDataSize = channel->remoteCertificate.length + client->serverSessionNonce.length; - if(dataToSignSize > MAX_DATA_SIZE) + if(signDataSize > MAX_DATA_SIZE) return UA_STATUSCODE_BADINTERNALERROR; - retval = UA_ByteString_allocBuffer(&dataToSign, dataToSignSize); - if(retval != UA_STATUSCODE_GOOD) - return retval; /* sd->signature is cleaned up with the response */ - - /* Sign the clientSignature */ - memcpy(dataToSign.data, channel->remoteCertificate.data, - channel->remoteCertificate.length); - memcpy(dataToSign.data + channel->remoteCertificate.length, - client->serverSessionNonce.data, client->serverSessionNonce.length); - retval = signAlg->sign(channel->channelContext, &dataToSign, &sd->signature); - if(retval != UA_STATUSCODE_GOOD) - goto cleanup; - - /* Prepare the UserTokenSignature */ - if(utp->tokenType == UA_USERTOKENTYPE_CERTIFICATE) { - /* Get the SecurityPolicy for authentication. If the UserTokenPolicy - * does not define it, the SecurityPolicyUri of the overall endpoint is - * used. The SecurityPolicy must be retrieved with getAuthSecurityPolicy - * as it can use a certificate different from the SecureChannel. */ - UA_String securityPolicyUri = (utp->securityPolicyUri.length > 0) ? - utp->securityPolicyUri : client->endpoint.securityPolicyUri; - UA_SecurityPolicy *utsp = getAuthSecurityPolicy(client, securityPolicyUri); - if(!utsp) { - UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT, - "The configured SecurityPolicy for certificate " - "authentication could not be found"); - retval = UA_STATUSCODE_BADSECURITYPOLICYREJECTED; - goto cleanup; - } + UA_Byte buf[MAX_DATA_SIZE]; + UA_ByteString signData = {signDataSize, buf}; - UA_SignatureData *utsd = &request->userTokenSignature; - UA_X509IdentityToken *token = (UA_X509IdentityToken *) - request->userIdentityToken.content.decoded.data; - UA_SecurityPolicySignatureAlgorithm *utpSignAlg = - &utsp->asymmetricModule.cryptoModule.signatureAlgorithm; - - /* Check the size of the content for signing */ - UA_ByteString signData = UA_BYTESTRING_NULL; - size_t signDataSize = - channel->remoteCertificate.length + client->serverSessionNonce.length; - if(signDataSize > MAX_DATA_SIZE) { - retval = UA_STATUSCODE_BADINTERNALERROR; - goto cleanup; - } - - /* Copy the algorithm identifier */ - retval = UA_String_copy(&utpSignAlg->uri, &utsd->algorithm); - if(retval != UA_STATUSCODE_GOOD) - goto cleanup; - - /* We need a channel context with the user certificate in order to reuse - * the code for signing. */ - void *tmpCtx; - retval = utsp->channelModule.newContext(utsp, &token->certificateData, &tmpCtx); - if(retval != UA_STATUSCODE_GOOD) - goto cleanup; + /* Sign the ClientSignature */ + memcpy(buf, channel->remoteCertificate.data, channel->remoteCertificate.length); + memcpy(buf + channel->remoteCertificate.length, client->serverSessionNonce.data, + client->serverSessionNonce.length); + return signAlg->sign(channel->channelContext, &signData, &sd->signature); +} - /* Allocate memory for the signature */ - retval = UA_ByteString_allocBuffer(&utsd->signature, - utpSignAlg->getLocalSignatureSize(tmpCtx)); - if(retval != UA_STATUSCODE_GOOD) - goto cleanup_utp; +static UA_StatusCode +signUserTokenSignature(UA_Client *client, UA_SecurityPolicy *utsp, + UA_ActivateSessionRequest *request) { + /* Check the size of the content for signing and create a temporary buffer */ + UA_StatusCode retval = UA_STATUSCODE_GOOD; + size_t signDataSize = + client->channel.remoteCertificate.length + client->serverSessionNonce.length; + if(signDataSize > MAX_DATA_SIZE) + return UA_STATUSCODE_BADINTERNALERROR; + UA_Byte buf[MAX_DATA_SIZE]; + UA_ByteString signData = {signDataSize, buf}; + + /* Copy the algorithm identifier */ + UA_SecurityPolicySignatureAlgorithm *utpSignAlg = + &utsp->asymmetricModule.cryptoModule.signatureAlgorithm; + UA_SignatureData *utsd = &request->userTokenSignature; + retval = UA_String_copy(&utpSignAlg->uri, &utsd->algorithm); + if(retval != UA_STATUSCODE_GOOD) + return retval; - /* Allocate a temporary buffer for the data to be signed */ - retval = UA_ByteString_allocBuffer(&signData, signDataSize); - if(retval != UA_STATUSCODE_GOOD) - goto cleanup_utp; + /* We need a channel context with the user certificate in order to reuse the + * code for signing. */ + void *tmpCtx; + retval = utsp->channelModule.newContext(utsp, &client->channel.remoteCertificate, &tmpCtx); + if(retval != UA_STATUSCODE_GOOD) + return retval; - /* Create the userTokenSignature */ - memcpy(signData.data, channel->remoteCertificate.data, - channel->remoteCertificate.length); - memcpy(signData.data + channel->remoteCertificate.length, - client->serverSessionNonce.data, client->serverSessionNonce.length); - retval = utpSignAlg->sign(tmpCtx, &signData, &utsd->signature); + /* Allocate memory for the signature */ + retval = UA_ByteString_allocBuffer(&utsd->signature, + utpSignAlg->getLocalSignatureSize(tmpCtx)); + if(retval != UA_STATUSCODE_GOOD) + goto cleanup_utp; - /* Clean up */ - cleanup_utp: - UA_ByteString_clear(&signData); - utsp->channelModule.deleteContext(tmpCtx); - } + /* Create the userTokenSignature */ + memcpy(buf, client->channel.remoteCertificate.data, + client->channel.remoteCertificate.length); + memcpy(buf + client->channel.remoteCertificate.length, + client->serverSessionNonce.data, client->serverSessionNonce.length); + retval = utpSignAlg->sign(tmpCtx, &signData, &utsd->signature); /* Clean up */ - cleanup: - UA_ByteString_clear(&dataToSign); + cleanup_utp: + utsp->channelModule.deleteContext(tmpCtx); return retval; } +/* UserName and IssuedIdentity are transferred encrypted. + * X509 and Anonymous are not. */ static UA_StatusCode -encryptUserIdentityToken(UA_Client *client, const UA_String userTokenSecurityPolicy, +encryptUserIdentityToken(UA_Client *client, UA_SecurityPolicy *utsp, UA_ExtensionObject *userIdentityToken) { UA_IssuedIdentityToken *iit = NULL; UA_UserNameIdentityToken *unit = NULL; @@ -287,25 +247,11 @@ encryptUserIdentityToken(UA_Client *client, const UA_String userTokenSecurityPol return UA_STATUSCODE_GOOD; } - /* No encryption */ - const UA_String none = UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#None"); - if(userTokenSecurityPolicy.length == 0 || - UA_String_equal(&userTokenSecurityPolicy, &none)) { - return UA_STATUSCODE_GOOD; - } - - UA_SecurityPolicy *sp = getSecurityPolicy(client, userTokenSecurityPolicy); - if(!sp) { - UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_NETWORK, - "Could not find the required SecurityPolicy for the UserToken"); - return UA_STATUSCODE_BADSECURITYPOLICYREJECTED; - } - /* Create a temp channel context */ void *channelContext; - UA_StatusCode retval = sp->channelModule. - newContext(sp, &client->endpoint.serverCertificate, &channelContext); + UA_StatusCode retval = utsp->channelModule. + newContext(utsp, &client->endpoint.serverCertificate, &channelContext); if(retval != UA_STATUSCODE_GOOD) { UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_NETWORK, "Could not instantiate the SecurityPolicy for the UserToken"); @@ -313,12 +259,11 @@ encryptUserIdentityToken(UA_Client *client, const UA_String userTokenSecurityPol } /* Compute the encrypted length (at least one byte padding) */ - size_t plainTextBlockSize = sp->asymmetricModule.cryptoModule. + size_t plainTextBlockSize = utsp->asymmetricModule.cryptoModule. encryptionAlgorithm.getRemotePlainTextBlockSize(channelContext); - size_t encryptedBlockSize = sp->asymmetricModule.cryptoModule. + size_t encryptedBlockSize = utsp->asymmetricModule.cryptoModule. encryptionAlgorithm.getRemoteBlockSize(channelContext); - UA_UInt32 length = (UA_UInt32)(tokenData->length + - client->serverSessionNonce.length); + UA_UInt32 length = (UA_UInt32)(tokenData->length + client->serverSessionNonce.length); UA_UInt32 totalLength = length + 4; /* Including the length field */ size_t blocks = totalLength / plainTextBlockSize; if(totalLength % plainTextBlockSize != 0) @@ -329,7 +274,7 @@ encryptUserIdentityToken(UA_Client *client, const UA_String userTokenSecurityPol UA_ByteString encrypted; retval = UA_ByteString_allocBuffer(&encrypted, encryptedLength); if(retval != UA_STATUSCODE_GOOD) { - sp->channelModule.deleteContext(channelContext); + utsp->channelModule.deleteContext(channelContext); return UA_STATUSCODE_BADOUTOFMEMORY; } @@ -352,24 +297,22 @@ encryptUserIdentityToken(UA_Client *client, const UA_String userTokenSecurityPol encrypted.data[i] = 0; encrypted.length = paddedLength; - retval = sp->asymmetricModule.cryptoModule.encryptionAlgorithm. + retval = utsp->asymmetricModule.cryptoModule.encryptionAlgorithm. encrypt(channelContext, &encrypted); encrypted.length = encryptedLength; + UA_ByteString_clear(tokenData); + *tokenData = encrypted; + + /* Delete the temporary channel context */ + utsp->channelModule.deleteContext(channelContext); if(iit) { - retval |= UA_String_copy(&sp->asymmetricModule.cryptoModule.encryptionAlgorithm.uri, + retval |= UA_String_copy(&utsp->asymmetricModule.cryptoModule.encryptionAlgorithm.uri, &iit->encryptionAlgorithm); } else { - retval |= UA_String_copy(&sp->asymmetricModule.cryptoModule.encryptionAlgorithm.uri, + retval |= UA_String_copy(&utsp->asymmetricModule.cryptoModule.encryptionAlgorithm.uri, &unit->encryptionAlgorithm); } - - UA_ByteString_clear(tokenData); - *tokenData = encrypted; - - /* Delete the temp channel context */ - sp->channelModule.deleteContext(channelContext); - return retval; } @@ -794,14 +737,12 @@ activateSessionAsync(UA_Client *client) { return UA_STATUSCODE_BADINTERNALERROR; } + /* Initialize the request */ UA_ActivateSessionRequest request; UA_ActivateSessionRequest_init(&request); - UA_StatusCode retval = - UA_ExtensionObject_copy(&client->config.userIdentityToken, - &request.userIdentityToken); - if(retval != UA_STATUSCODE_GOOD) - return retval; + /* Set the requested LocaleIds */ + UA_StatusCode retval = UA_STATUSCODE_GOOD; if(client->config.sessionLocaleIdsSize && client->config.sessionLocaleIds) { retval = UA_Array_copy(client->config.sessionLocaleIds, client->config.sessionLocaleIdsSize, @@ -811,38 +752,80 @@ activateSessionAsync(UA_Client *client) { request.localeIdsSize = client->config.sessionLocaleIdsSize; } - /* If not token is set, use anonymous */ - if(request.userIdentityToken.encoding == UA_EXTENSIONOBJECT_ENCODED_NOBODY) { - UA_AnonymousIdentityToken *t = UA_AnonymousIdentityToken_new(); - if(!t) { - UA_ActivateSessionRequest_clear(&request); - return UA_STATUSCODE_BADOUTOFMEMORY; + /* Set the User Identity Token. If not defined use an anonymous token. Use + * the PolicyId from the UserTokenPolicy. All token types have the PolicyId + * string as the first element. */ + UA_AnonymousIdentityToken anonToken; + retval = UA_ExtensionObject_copy(&client->config.userIdentityToken, + &request.userIdentityToken); + if(request.userIdentityToken.encoding != UA_EXTENSIONOBJECT_ENCODED_NOBODY) { + UA_String *policyId = (UA_String*)request.userIdentityToken.content.decoded.data; + UA_String_clear(policyId); + retval = UA_String_copy(&utp->policyId, policyId); + } else { + UA_AnonymousIdentityToken_init(&anonToken); + UA_ExtensionObject_setValueNoDelete(&request.userIdentityToken, &anonToken, + &UA_TYPES[UA_TYPES_ANONYMOUSIDENTITYTOKEN]); + anonToken.policyId = utp->policyId; + } + if(retval != UA_STATUSCODE_GOOD) + return retval; + +#ifdef UA_ENABLE_ENCRYPTION + UA_SecurityPolicy *utsp = NULL; + UA_SecureChannel *channel = &client->channel; + + /* Does the UserTokenPolicy have encryption? If not specifically defined in + * the UserTokenPolicy, then the SecurityPolicy of the underlying endpoint + * (SecureChannel) is used. */ + UA_String tokenSecurityPolicyUri = (utp->securityPolicyUri.length > 0) ? + utp->securityPolicyUri : client->endpoint.securityPolicyUri; + const UA_String none = UA_STRING("http://opcfoundation.org/UA/SecurityPolicy#None"); + if(UA_String_equal(&none, &tokenSecurityPolicyUri)) { + if(UA_String_equal(&none, &client->channel.securityPolicy->policyUri) && + request.userIdentityToken.content.decoded.type != + &UA_TYPES[UA_TYPES_ANONYMOUSIDENTITYTOKEN]) { + UA_LOG_WARNING(client->config.logging, UA_LOGCATEGORY_CLIENT, + "!!! Warning !!! AuthenticationToken is transmitted " + "without encryption"); } - request.userIdentityToken.content.decoded.data = t; - request.userIdentityToken.content.decoded.type = - &UA_TYPES[UA_TYPES_ANONYMOUSIDENTITYTOKEN]; - request.userIdentityToken.encoding = UA_EXTENSIONOBJECT_DECODED; + goto utp_done; } - /* Set the correct PolicyId from the endpoint */ - UA_String_clear((UA_String*)request.userIdentityToken.content.decoded.data); - retval = UA_String_copy(&utp->policyId, - (UA_String*)request.userIdentityToken.content.decoded.data); + /* Get the SecurityPolicy for authentication */ + utsp = getAuthSecurityPolicy(client, tokenSecurityPolicyUri); + if(!utsp) { + UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT, + "UserTokenPolicy %.*s not available for authentication", + (int)tokenSecurityPolicyUri.length, tokenSecurityPolicyUri.data); + retval = UA_STATUSCODE_BADSECURITYPOLICYREJECTED; + goto utp_done; + } -#ifdef UA_ENABLE_ENCRYPTION /* Encrypt the UserIdentityToken */ - retval |= encryptUserIdentityToken(client, utp->securityPolicyUri, &request.userIdentityToken); - retval |= signActivateSessionRequest(client, &client->channel, &request); + retval = encryptUserIdentityToken(client, utsp, &request.userIdentityToken); + + /* Create the UserTokenSignature if this is possible for the token. + * The certificate is already loaded into the utsp. */ + if(utp->tokenType == UA_USERTOKENTYPE_CERTIFICATE) + retval |= signUserTokenSignature(client, utsp, &request); + + utp_done: + /* Create the client signature with the SecurityPolicy of the SecurteChannel */ + if(channel->securityMode == UA_MESSAGESECURITYMODE_SIGN || + channel->securityMode == UA_MESSAGESECURITYMODE_SIGNANDENCRYPT) + retval |= signClientSignature(client, &request); #endif - if(retval == UA_STATUSCODE_GOOD) + /* Send the request */ + if(UA_LIKELY(retval == UA_STATUSCODE_GOOD)) retval = __Client_AsyncService(client, &request, &UA_TYPES[UA_TYPES_ACTIVATESESSIONREQUEST], (UA_ClientAsyncServiceCallback)responseActivateSession, &UA_TYPES[UA_TYPES_ACTIVATESESSIONRESPONSE], NULL, NULL); - UA_ActivateSessionRequest_clear(&request); + /* On success, advance the session state */ if(retval == UA_STATUSCODE_GOOD) client->sessionState = UA_SESSIONSTATE_ACTIVATE_REQUESTED; else @@ -850,6 +833,8 @@ activateSessionAsync(UA_Client *client) { "ActivateSession failed when sending the request with error code %s", UA_StatusCode_name(retval)); + /* Clean up */ + UA_ActivateSessionRequest_clear(&request); return retval; } @@ -936,9 +921,9 @@ matchUserToken(UA_Client *client, return false; } -/* Returns the locally configured UserTokenPolicy (if it exists) or a match from - * the EndpointDescription. If localTokenPolicy != NULL, then we require an - * exact match with the configured UserTokenPolicy. */ +/* Returns a matching UserTokenPolicy from the EndpointDescription. If a + * UserTokenPolicy is configured in the client config, then we need an exact + * match. */ static UA_UserTokenPolicy * findUserTokenPolicy(UA_Client *client, UA_EndpointDescription *endpoint) { /* Was a UserTokenPolicy configured? Then we need an exact match. */ From 92d69c2785e37020cac4de4d2492b6c8b33453ec Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Sat, 5 Oct 2024 21:49:46 +0200 Subject: [PATCH 16/18] refactor(client): Use SO_REUSEADDR sockets for the client listening for reverse connections The client is often restarted in short order. Then the socket was blocked for 2 minutes. --- src/client/ua_client_connect.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/ua_client_connect.c b/src/client/ua_client_connect.c index 458837f8c91..4f4d4eda2c6 100644 --- a/src/client/ua_client_connect.c +++ b/src/client/ua_client_connect.c @@ -2067,7 +2067,7 @@ UA_Client_startListeningForReverseConnect(UA_Client *client, client->channel.connectionManager = cm; - UA_KeyValuePair params[3]; + UA_KeyValuePair params[4]; bool booleanTrue = true; params[0].key = UA_QUALIFIEDNAME(0, "port"); UA_Variant_setScalar(¶ms[0].value, &port, &UA_TYPES[UA_TYPES_UINT16]); @@ -2076,10 +2076,12 @@ UA_Client_startListeningForReverseConnect(UA_Client *client, listenHostnamesLength, &UA_TYPES[UA_TYPES_STRING]); params[2].key = UA_QUALIFIEDNAME(0, "listen"); UA_Variant_setScalar(¶ms[2].value, &booleanTrue, &UA_TYPES[UA_TYPES_BOOLEAN]); + params[3].key = UA_QUALIFIEDNAME(0, "reuse"); + UA_Variant_setScalar(¶ms[3].value, &booleanTrue, &UA_TYPES[UA_TYPES_BOOLEAN]); UA_KeyValueMap paramMap; paramMap.map = params; - paramMap.mapSize = 3; + paramMap.mapSize = 4; UA_UNLOCK(&client->clientMutex); res = cm->openConnection(cm, ¶mMap, client, NULL, __Client_reverseConnectCallback); From 8c1536866515ffb03bc3d74ff7ed694cf5d569b8 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Sat, 5 Oct 2024 21:52:27 +0200 Subject: [PATCH 17/18] fix (client): The client can listen on multiple sockets for reverse connections Listening on "localhost" is now possible for IPv4 and IPv6 at the same time. --- src/client/ua_client_connect.c | 122 +++++++++++++++++++++----------- src/client/ua_client_internal.h | 4 ++ 2 files changed, 83 insertions(+), 43 deletions(-) diff --git a/src/client/ua_client_connect.c b/src/client/ua_client_connect.c index 4f4d4eda2c6..fbaaf6de6c1 100644 --- a/src/client/ua_client_connect.c +++ b/src/client/ua_client_connect.c @@ -30,7 +30,6 @@ #define UA_MINMESSAGESIZE 8192 #define UA_SESSION_LOCALNONCELENGTH 32 #define MAX_DATA_SIZE 4096 -#define REVERSE_CONNECT_INDICATOR (void *)(uintptr_t)0xFFFFFFFF static void initConnect(UA_Client *client); static UA_StatusCode createSessionAsync(UA_Client *client); @@ -1360,6 +1359,7 @@ connectActivity(UA_Client *client) { switch(client->channel.state) { /* Nothing to do if the connection has not opened fully */ case UA_SECURECHANNELSTATE_CONNECTING: + case UA_SECURECHANNELSTATE_REVERSE_CONNECTED: case UA_SECURECHANNELSTATE_CLOSING: case UA_SECURECHANNELSTATE_HEL_SENT: case UA_SECURECHANNELSTATE_OPN_SENT: @@ -1582,8 +1582,6 @@ __Client_networkCallback(UA_ConnectionManager *cm, uintptr_t connectionId, /* The connection is now open on the TCP level. Set the SecureChannel * state to reflect this. Otherwise later consistency checks for the * received messages fail. */ - if(client->channel.state == UA_SECURECHANNELSTATE_REVERSE_LISTENING) - client->channel.state = UA_SECURECHANNELSTATE_REVERSE_CONNECTED; if(client->channel.state < UA_SECURECHANNELSTATE_CONNECTED) client->channel.state = UA_SECURECHANNELSTATE_CONNECTED; } else /* state == UA_CONNECTIONSTATE_OPENING */ { @@ -1946,60 +1944,96 @@ UA_Client_activateSessionAsync(UA_Client *client, } static void -__Client_reverseConnectCallback(UA_ConnectionManager *cm, uintptr_t connectionId, - void *application, void **connectionContext, - UA_ConnectionState state, const UA_KeyValueMap *params, - UA_ByteString msg) { +disconnectListenSockets(UA_Client *client) { + UA_ConnectionManager *cm = client->reverseConnectionCM; + for(size_t i = 0; i < 16; i++) { + if(client->reverseConnectionIds[i] != 0) { + cm->closeConnection(cm, client->reverseConnectionIds[i]); + } + } +} +/* ConnectionContext meaning: + * - NULL: New listen connection + * - &client->channel: Established active socket to a server + * - &client->reverseConnectionIds[*] == connectionId: Established listen socket + * - &client->reverseConnectionIds[*] != connectionId: New active socket */ +static void +__Client_reverseConnectCallback(UA_ConnectionManager *cm, uintptr_t connectionId, + void *application, void **connectionContext, + UA_ConnectionState state, const UA_KeyValueMap *params, + UA_ByteString msg) { UA_Client *client = (UA_Client*)application; - UA_LOCK(&client->clientMutex); - /* This is the first call for the listening socket, attach the - * REVERSE_CONNECT_INDICATOR marker and set the ID to the channel */ - if(!client->channel.connectionId) { - client->channel.connectionId = connectionId; - *connectionContext = REVERSE_CONNECT_INDICATOR; - } - - /* Last call for the listening connection while it is being closed. Only - * notify a state change if no reverse connection is being or has been - * established by now */ - if(*connectionContext == REVERSE_CONNECT_INDICATOR && - state == UA_CONNECTIONSTATE_CLOSING) { - if(client->channel.connectionId == connectionId) { - client->channel.state = UA_SECURECHANNELSTATE_CLOSED; - notifyClientState(client); + if(!*connectionContext) { + /* Store the new listen connection */ + size_t i = 0; + for(; i < 16; i++) { + if(client->reverseConnectionIds[i] == 0) { + client->reverseConnectionIds[i] = connectionId; + client->reverseConnectionCM = cm; + *connectionContext = &client->reverseConnectionIds[i]; + if(client->channel.state == UA_SECURECHANNELSTATE_CLOSED) + client->channel.state = UA_SECURECHANNELSTATE_REVERSE_LISTENING; + break; + } } - UA_UNLOCK(&client->clientMutex); - return; - } + /* All slots are full, close */ + if(i == 16) { + cm->closeConnection(cm, connectionId); + UA_UNLOCK(&client->clientMutex); + return; + } + } else if(*connectionContext == &client->channel || + *(uintptr_t*)*connectionContext != connectionId) { + /* Active socket */ + + /* New active socket */ + if(*connectionContext != &client->channel) { + /* The client already has an active connection */ + if(client->channel.connectionId) { + cm->closeConnection(cm, connectionId); + UA_UNLOCK(&client->clientMutex); + return; + } - /* Second callback for the listening socket, it is now listening for - * incoming connections */ - if(client->channel.connectionId == connectionId && - *connectionContext == REVERSE_CONNECT_INDICATOR) { - client->channel.state = UA_SECURECHANNELSTATE_REVERSE_LISTENING; - notifyClientState(client); - } + /* Set the connection the SecureChannel */ + client->channel.connectionId = connectionId; + client->channel.connectionManager = cm; + *connectionContext = &client->channel; - /* This is a connection initiated by a server, disconnect the listener and - * reset secure channel information */ - if(client->channel.connectionId != connectionId) { - cm->closeConnection(cm, client->channel.connectionId); - client->channel.connectionId = 0; - *connectionContext = NULL; - } + /* Don't keep the listen sockets when an active connection is open */ + disconnectListenSockets(client); - /* Forward all calls belonging to the reverse connection estblished by the - * server to the regular network callback */ - if(*connectionContext != REVERSE_CONNECT_INDICATOR) { + /* Set the channel state. The notification callback is called within + * __Client_reverseConnectCallback. */ + if(client->channel.state == UA_SECURECHANNELSTATE_REVERSE_LISTENING) + client->channel.state = UA_SECURECHANNELSTATE_REVERSE_CONNECTED; + } + + /* Handle the active connection in the normal network callback */ UA_UNLOCK(&client->clientMutex); __Client_networkCallback(cm, connectionId, application, connectionContext, state, params, msg); return; } + /* Close the listen socket. Was this the last one? */ + if(state == UA_CONNECTIONSTATE_CLOSING) { + UA_Byte count = 0; + for(size_t i = 0; i < 16; i++) { + if(client->reverseConnectionIds[i] == connectionId) + client->reverseConnectionIds[i] = 0; + if(client->reverseConnectionIds[i] != 0) + count++; + } + /* The last connection was closed */ + if(count == 0 && client->channel.connectionId == 0) + client->channel.state = UA_SECURECHANNELSTATE_CLOSED; + } + + notifyClientState(client); UA_UNLOCK(&client->clientMutex); } @@ -2118,6 +2152,8 @@ closeSecureChannel(UA_Client *client) { UA_LOG_DEBUG_CHANNEL(client->config.logging, &client->channel, "Closing the channel"); + disconnectListenSockets(client); + /* Send CLO if the SecureChannel is open */ if(client->channel.state == UA_SECURECHANNELSTATE_OPEN) { UA_LOG_DEBUG_CHANNEL(client->config.logging, &client->channel, diff --git a/src/client/ua_client_internal.h b/src/client/ua_client_internal.h index 6ed8f28e879..53a05f3c6dc 100644 --- a/src/client/ua_client_internal.h +++ b/src/client/ua_client_internal.h @@ -140,6 +140,10 @@ struct UA_Client { UA_UInt32 requestId; /* Unique, internally defined for each request */ UA_DateTime nextChannelRenewal; + /* Reverse connect (listen) connections */ + UA_ConnectionManager *reverseConnectionCM; + uintptr_t reverseConnectionIds[16]; + /* Session */ UA_SessionState sessionState; UA_NodeId authenticationToken; From 1f19dca8ce1f73860083586c6bfdda1d1bf70a6e Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Thu, 3 Oct 2024 13:49:22 +0200 Subject: [PATCH 18/18] refactor(build): Bump version to v1.4.6 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fbf1e58a971..c18a457aaaf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,7 +40,7 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) # overwritten with more detailed information if git is available. set(OPEN62541_VER_MAJOR 1) set(OPEN62541_VER_MINOR 4) -set(OPEN62541_VER_PATCH 5) +set(OPEN62541_VER_PATCH 6) set(OPEN62541_VER_LABEL "-undefined") # like "-rc1" or "-g4538abcd" or "-g4538abcd-dirty" set(OPEN62541_VER_COMMIT "unknown-commit")