Skip to content

Commit

Permalink
fix(client): The server can send no ServerCertificate in the asym hea…
Browse files Browse the repository at this point in the history
…der 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 open62541#6926.
  • Loading branch information
jpfr committed Dec 4, 2024
1 parent 4697450 commit f3bf755
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 6 deletions.
19 changes: 13 additions & 6 deletions src/client/ua_client_connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
35 changes: 35 additions & 0 deletions tests/client/check_client_securechannel.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit f3bf755

Please sign in to comment.