Client Does Not Honor SecurityLevel in Endpoint Selection

[MikeGranby]

The client matches the first suitable endpoint in the list returned by the server without reference to the SecurityLevel. In the attached example, the server returns three endpoints, with security policies of None, Basic256, and Basic256Sha256, all of which are available to the client. These have increasing security levels, which means the client should pick Basic256Sha256, but it instead picks None. The only way to override this is to force an encrypted connection via securityMode in the config structure, but this requires a priori knowledge of the fact that the server will indeed support this. The fix is pretty trivial: iterate through the entire list, maintaining a record of the best policy found to date and allowing it to be overridden by better ones. I've coded a fix locally, and will submit if you like.

Attachments

files.zip

Checklist

Please provide the following information:

• [X] open62541 Version (release number or git tag): v1.3.8-1478-g08d2cbeca
• [X] Other OPC UA SDKs used (client or server): Ignition 8.1.4 OPC UA Server
• [X] Operating system: Alpine 3.17
• [X] Logs (with UA_LOGLEVEL set as low as necessary) attached
• [X] Wireshark network dump attached
• [ ] Self-contained code example attached
• [ ] Critical issue

[jpfr]

Good point. You are right.

If you can provide code we make the change with attribution to you. As a code snippet in this thread or as a GitHub pull request.

[MikeGranby]

Will do. Let me tidy it up, and I'll submit.

[MikeGranby]

/* Combination of UA_Client_getEndpointsInternal and getEndpoints */
static void
responseGetEndpoints(UA_Client *client, void *userdata, UA_UInt32 requestId,
                     void *response) {
    UA_LOCK(&client->clientMutex);

    client->endpointsHandshake = false;

    UA_LOG_DEBUG(client->config.logging, UA_LOGCATEGORY_CLIENT,
                 "Received GetEndpointsResponse");

    UA_GetEndpointsResponse *resp = (UA_GetEndpointsResponse *)response;

    /* GetEndpoints not possible. Fail the connection */
    if(resp->responseHeader.serviceResult != UA_STATUSCODE_GOOD) {
        /* Fail the connection attempt if the SecureChannel is still connected.
         * If the SecureChannel is (intentionally or unintentionally) closed,
         * the connectStatus should come from there. */
        if(UA_SecureChannel_isConnected(&client->channel)) {
            client->connectStatus = resp->responseHeader.serviceResult;
            closeSecureChannel(client);
            UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT,
                         "GetEndpointRequest failed with error code %s",
                         UA_StatusCode_name(client->connectStatus));
        }

        UA_GetEndpointsResponse_clear(resp);
        UA_UNLOCK(&client->clientMutex);
        return;
    }

    /* Warn if the Endpoints look incomplete / don't match the EndpointUrl */
    Client_warnEndpointsResult(client, resp, &client->discoveryUrl);

    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) {
        UA_EndpointDescription *endpoint = &resp->endpoints[i];

        /* Do we already have a better candidate? */
        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))
            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)) {
            UA_LOG_INFO(client->config.logging, UA_LOGCATEGORY_CLIENT,
                        "Rejecting endpoint %lu: security policy doesn't match",
                        (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;
        }

        bestEndpointIndex = i;

        /* Look for a user token policy with an anonymous token */
        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;

            /* Move to the client config */
            UA_EndpointDescription_clear(&client->config.endpoint);
            client->config.endpoint = *endpoint;
            UA_EndpointDescription_init(endpoint);
            UA_UserTokenPolicy_clear(&client->config.userTokenPolicy);
            client->config.userTokenPolicy = *tokenPolicy;
            UA_UserTokenPolicy_init(tokenPolicy);

            /* Store the Server Description */
            UA_ApplicationDescription_clear(&client->serverDescription);
            UA_ApplicationDescription_copy(&endpoint->server, &client->serverDescription);

            break;
        }
    }

    if(bestEndpointIndex == notFound) {
        UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT,
                     "No suitable endpoint found");
        client->connectStatus = UA_STATUSCODE_BADINTERNALERROR;
        closeSecureChannel(client);
    } else if(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);
    } else {
#if UA_LOGLEVEL <= 300
        UA_EndpointDescription *endpoint = &client->config.endpoint;
        UA_UserTokenPolicy *tokenPolicy = &client->config.userTokenPolicy;
        UA_String *securityPolicyUri = &tokenPolicy->securityPolicyUri;
        const char *securityModeNames[3] = {"None", "Sign", "SignAndEncrypt"};
        const char *userTokenTypeNames[4] = {"Anonymous", "UserName", "Certificate",
                                             "IssuedToken"};
        /* Log the selected endpoint */
        UA_LOG_INFO(
            client->config.logging, UA_LOGCATEGORY_CLIENT,
            "Selected endpoint %lu in URL %.*s with 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);

        /* Log the selected UserTokenPolicy */
        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

        /* Close the SecureChannel if a different SecurityPolicy is defined by the
         * Endpoint */
        if(client->config.endpoint.securityMode != client->channel.securityMode ||
           !UA_String_equal(&client->config.endpoint.securityPolicyUri,
                            &client->channel.securityPolicy->policyUri))
            closeSecureChannel(client);
    }
    UA_UNLOCK(&client->clientMutex);
}

[MikeGranby]

I guess we also don't set SecurityLevel in the server's endpoint list, beyond 0 for not recommended and 1 for otherwise...

[MikeGranby]

(I'm gonna create a PR for this for you.)

[NoelGraf]

PR is merged. Thank you for your contribution!