From f3bf755f8148bc42ff8f322cc004a9b02b155ed0 Mon Sep 17 00:00:00 2001
From: Julius Pfrommer <julius.pfrommer@web.de>
Date: Wed, 4 Dec 2024 19:37:15 +0100
Subject: [PATCH] fix(client): The server can send no ServerCertificate in the
 asym header for #None

Even if a ServerCertificate is defined for the Endpoint. Some servers in
the wild show this behavior. This was reported by @mario-reif in #6926.
---
 src/client/ua_client_connect.c            | 19 ++++++++----
 tests/client/check_client_securechannel.c | 35 +++++++++++++++++++++++
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/src/client/ua_client_connect.c b/src/client/ua_client_connect.c
index 45da870bf31..a9b4d060514 100644
--- a/src/client/ua_client_connect.c
+++ b/src/client/ua_client_connect.c
@@ -523,8 +523,7 @@ processOPNResponse(UA_Client *client, const UA_ByteString *message) {
 
     /* Check whether the nonce was reused */
     if(client->channel.securityMode != UA_MESSAGESECURITYMODE_NONE &&
-       UA_ByteString_equal(&client->channel.remoteNonce,
-                           &response.serverNonce)) {
+       UA_ByteString_equal(&client->channel.remoteNonce, &response.serverNonce)) {
         UA_LOG_ERROR_CHANNEL(client->config.logging, &client->channel,
                              "The server reused the last nonce");
         client->connectStatus = UA_STATUSCODE_BADSECURITYCHECKSFAILED;
@@ -1458,11 +1457,19 @@ verifyClientSecureChannelHeader(void *application, UA_SecureChannel *channel,
         return UA_STATUSCODE_BADSECURITYCHECKSFAILED;
     }
 
-    /* If encryption is used, check that the server certificate for the
-     * endpoint is used for the SecureChannel */
+    /* Get the remote certificate.
+     * Omit the remainder if an entire certificate chain was sent. */
     UA_ByteString serverCert = getLeafCertificate(asymHeader->senderCertificate);
-    if(client->endpoint.serverCertificate.length > 0 &&
-       !UA_String_equal(&client->endpoint.serverCertificate, &serverCert)) {
+
+    /* If encryption is enabled, then a server certificate is defined.
+     * Otherwise the creation of the SecureChannel would have failed. */
+    UA_assert(channel->securityMode == UA_MESSAGESECURITYMODE_NONE ||
+              serverCert.length > 0);
+
+    /* If a server certificate is sent in the asymHeader, check that the same
+     * certificate was defined for the endpoint */
+    if(serverCert.length > 0 &&
+       !UA_String_equal(&serverCert, &client->endpoint.serverCertificate)) {
         UA_LOG_ERROR(client->config.logging, UA_LOGCATEGORY_CLIENT,
                      "The server certificate is different from the EndpointDescription");
         return UA_STATUSCODE_BADSECURITYCHECKSFAILED;
diff --git a/tests/client/check_client_securechannel.c b/tests/client/check_client_securechannel.c
index d4c37caf4cc..2c315724a7b 100644
--- a/tests/client/check_client_securechannel.c
+++ b/tests/client/check_client_securechannel.c
@@ -220,6 +220,40 @@ START_TEST(SecureChannel_cableunplugged) {
 }
 END_TEST
 
+/* Some servers have a certificate in their endpoint which they do not use for
+ * #None SecureChannels. To be compatible with them, only check if the
+ * certificate matches IF it gets sent in th asymHeader of the OPN message. */
+START_TEST(SecureChannel_serverCert) {
+    UA_Client *client = UA_Client_new();
+    UA_ClientConfig_setDefault(UA_Client_getConfig(client));
+
+    UA_StatusCode retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+
+    /* Copy the endpont and disconnect */
+    UA_EndpointDescription endpoint;
+    UA_EndpointDescription_copy(&client->endpoint, &endpoint);
+    UA_Client_disconnect(client);
+
+    /* Add a fake certificate information to the endpoint - which is not getting
+     * used during the connect with #None. */
+    endpoint.serverCertificate = UA_STRING_ALLOC("random 123");
+    client->config.endpoint = endpoint;
+
+    /* Reconnect */
+    retval = UA_Client_connect(client, "opc.tcp://localhost:4840");
+
+    UA_Variant val;
+    UA_Variant_init(&val);
+    UA_NodeId nodeId = UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER_SERVERSTATUS_STATE);
+    retval = UA_Client_readValueAttribute(client, nodeId, &val);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+
+    UA_Variant_clear(&val);
+    UA_Client_delete(client);
+}
+END_TEST
+
 int main(void) {
     TCase *tc_sc = tcase_create("Client SecureChannel");
     tcase_add_checked_fixture(tc_sc, setup, teardown);
@@ -229,6 +263,7 @@ int main(void) {
     tcase_add_test(tc_sc, SecureChannel_networkfail);
     tcase_add_test(tc_sc, SecureChannel_reconnect);
     tcase_add_test(tc_sc, SecureChannel_cableunplugged);
+    tcase_add_test(tc_sc, SecureChannel_serverCert);
 
     Suite *s = suite_create("Client");
     suite_add_tcase(s, tc_sc);