From 90871d4ae4c3ff494540a7afd0bc400f97bab275 Mon Sep 17 00:00:00 2001 From: Julius Pfrommer Date: Wed, 6 Nov 2024 08:22:44 +0100 Subject: [PATCH] fix(server): Consider allowNonePolicyPassword in GetEndpoints Remove all UserTokenPolicies that would allow an unencrypted password transmitted. --- src/server/ua_services_discovery.c | 60 +++++++++++++++++----------- tests/client/check_activateSession.c | 3 ++ tests/client/check_client.c | 1 + tests/server/check_accesscontrol.c | 3 ++ tests/server/check_server_password.c | 1 + 5 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/server/ua_services_discovery.c b/src/server/ua_services_discovery.c index 50afaa32dee..b9fe2ac96fd 100644 --- a/src/server/ua_services_discovery.c +++ b/src/server/ua_services_discovery.c @@ -294,8 +294,7 @@ getDefaultEncryptedSecurityPolicy(UA_Server *server) { if(!UA_String_equal(&UA_SECURITY_POLICY_NONE_URI, &sp->policyUri)) return sp; } - return server->config.securityPoliciesSize > 0 ? - &server->config.securityPolicies[0] : NULL; + return NULL; /* No encrypted policy found */ } const char *securityModeStrs[4] = {"-invalid", "-none", "-sign", "-sign+encrypt"}; @@ -321,30 +320,45 @@ updateEndpointUserIdentityToken(UA_Server *server, UA_EndpointDescription *ed) { /* Copy the UserTokenPolicies from the AccessControl plugin, but only the matching ones to the securityPolicyUri. * TODO: Different instances of the AccessControl plugin per Endpoint */ UA_StatusCode res = UA_STATUSCODE_GOOD; - for(size_t i = 0; i < server->config.accessControl.userTokenPoliciesSize; i++) { - UA_UserTokenPolicy *utp = &server->config.accessControl.userTokenPolicies[i]; - if(UA_String_equal(&ed->securityPolicyUri, &utp->securityPolicyUri)) { - res = UA_Array_appendCopy((void**)&ed->userIdentityTokens, - &ed->userIdentityTokensSize, - utp, - &UA_TYPES[UA_TYPES_USERTOKENPOLICY]); - if(res != UA_STATUSCODE_GOOD) - return res; - } - } - - for(size_t i = 0; i < ed->userIdentityTokensSize; i++) { - /* Use the securityPolicy of the SecureChannel. But not if the - * SecureChannel is unencrypted and there is a non-anonymous token. */ - UA_UserTokenPolicy *utp = &ed->userIdentityTokens[i]; + UA_ServerConfig *sc = &server->config; + for(size_t i = 0; i < sc->accessControl.userTokenPoliciesSize; i++) { + UA_UserTokenPolicy *utp = &sc->accessControl.userTokenPolicies[i]; + res = UA_Array_appendCopy((void**)&ed->userIdentityTokens, + &ed->userIdentityTokensSize, utp, + &UA_TYPES[UA_TYPES_USERTOKENPOLICY]); + if(res != UA_STATUSCODE_GOOD) + return res; + + /* Select the SecurityPolicy for the UserTokenType. + * If not set, the SecurityPolicy of the SecureChannel is used. */ + utp = &ed->userIdentityTokens[ed->userIdentityTokensSize - 1]; UA_String_clear(&utp->securityPolicyUri); - if((!server->config.allowNonePolicyPassword || ed->userIdentityTokens[i].tokenType != UA_USERTOKENTYPE_USERNAME) && - UA_String_equal(&ed->securityPolicyUri, &UA_SECURITY_POLICY_NONE_URI) && - utp->tokenType != UA_USERTOKENTYPE_ANONYMOUS) { +#ifdef UA_ENABLE_ENCRYPTION + /* Anonymous tokens don't need encryption. All other tokens require + * encryption with the exception of Username/Password if also the + * allowNonePolicyPassword option has been set. The same logic is used + * in selectEndpointAndTokenPolicy (ua_services_session.c). */ + if(utp->tokenType != UA_USERTOKENTYPE_ANONYMOUS && + !(sc->allowNonePolicyPassword && utp->tokenType == UA_USERTOKENTYPE_USERNAME) && + UA_String_equal(&ed->securityPolicyUri, &UA_SECURITY_POLICY_NONE_URI)) { UA_SecurityPolicy *encSP = getDefaultEncryptedSecurityPolicy(server); - if(encSP) - res |= UA_String_copy(&encSP->policyUri, &utp->securityPolicyUri); + if(!encSP) { + /* No encrypted SecurityPolicy available */ + UA_LOG_WARNING(sc->logging, UA_LOGCATEGORY_CLIENT, + "Removing a UserTokenPolicy that would allow the " + "password to be transmitted without encryption " + "(Can be enabled via config->allowNonePolicyPassword)"); + UA_StatusCode res2 = + UA_Array_resize((void **)&ed->userIdentityTokens, + &ed->userIdentityTokensSize, + ed->userIdentityTokensSize - 1, + &UA_TYPES[UA_TYPES_USERTOKENPOLICY]); + (void)res2; + continue; + } + res |= UA_String_copy(&encSP->policyUri, &utp->securityPolicyUri); } +#endif /* Append the SecurityMode and SecurityPolicy postfix to the PolicyId to * make it unique */ diff --git a/tests/client/check_activateSession.c b/tests/client/check_activateSession.c index edd23716d6e..348ee9cacf8 100644 --- a/tests/client/check_activateSession.c +++ b/tests/client/check_activateSession.c @@ -168,6 +168,9 @@ START_TEST(Client_activateSession) { END_TEST START_TEST(Client_activateSession_username) { + UA_ServerConfig *sc = UA_Server_getConfig(server); + sc->allowNonePolicyPassword = true; + UA_Client *client = UA_Client_new(); UA_ClientConfig *config = UA_Client_getConfig(client); UA_ClientConfig_setDefault(config); diff --git a/tests/client/check_client.c b/tests/client/check_client.c index 0e95e57e3ac..28f64f513da 100644 --- a/tests/client/check_client.c +++ b/tests/client/check_client.c @@ -69,6 +69,7 @@ static void setup(void) { UA_SecurityPolicy *sp = &config->securityPolicies[config->securityPoliciesSize-1]; UA_AccessControl_default(config, true, &sp->policyUri, usernamePasswordsSize, usernamePasswords); + config->allowNonePolicyPassword = true; UA_Server_run_startup(server); addVariable(VARLENGTH); diff --git a/tests/server/check_accesscontrol.c b/tests/server/check_accesscontrol.c index a0a66db566f..9ecd23ded98 100644 --- a/tests/server/check_accesscontrol.c +++ b/tests/server/check_accesscontrol.c @@ -34,6 +34,9 @@ static void setup(void) { server = UA_Server_new(); ck_assert(server != NULL); + UA_ServerConfig *sc = UA_Server_getConfig(server); + sc->allowNonePolicyPassword = true; + /* Instatiate a new AccessControl plugin that knows username/pw */ UA_ServerConfig *config = UA_Server_getConfig(server); UA_SecurityPolicy *sp = &config->securityPolicies[config->securityPoliciesSize-1]; diff --git a/tests/server/check_server_password.c b/tests/server/check_server_password.c index a282190077d..614682b0c9c 100644 --- a/tests/server/check_server_password.c +++ b/tests/server/check_server_password.c @@ -144,6 +144,7 @@ static void setup(void) { ck_assert_msg(server, "UA_Server_new"); UA_ServerConfig *config = UA_Server_getConfig(server); UA_ServerConfig_setDefault(config); + config->allowNonePolicyPassword = true; UA_String policy = UA_STRING_STATIC("http://opcfoundation.org/UA/SecurityPolicy#None"); UA_UsernamePasswordLogin login[] = { { UA_STRING_STATIC("user"),