Skip to content

Replace server config asserts with error + return #599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
michalvasko merged 3 commits into from
Apr 21, 2026
+145 −29
Merged

Replace server config asserts with error + return #599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2efc988
server config BUGFIX replace asserts with err checks
Roytak Apr 21, 2026
51745f5
SOVERSION bump to version 5.3.10
Roytak Apr 21, 2026
feedd1b
VERSION bump to version 4.3.4
Roytak Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ set(CMAKE_MACOSX_RPATH TRUE)
# micro version is changed with a set of small changes or bugfixes anywhere in the project.
set(LIBNETCONF2_MAJOR_VERSION 4)
set(LIBNETCONF2_MINOR_VERSION 3)
set(LIBNETCONF2_MICRO_VERSION 3)
set(LIBNETCONF2_MICRO_VERSION 4)
set(LIBNETCONF2_VERSION ${LIBNETCONF2_MAJOR_VERSION}.${LIBNETCONF2_MINOR_VERSION}.${LIBNETCONF2_MICRO_VERSION})

# Version of the library
# Major version is changed with every backward non-compatible API/ABI change in the library, minor version changes
# with backward compatible change and micro version is connected with any internal change of the library.
set(LIBNETCONF2_MAJOR_SOVERSION 5)
set(LIBNETCONF2_MINOR_SOVERSION 3)
set(LIBNETCONF2_MICRO_SOVERSION 9)
set(LIBNETCONF2_MICRO_SOVERSION 10)
set(LIBNETCONF2_SOVERSION_FULL ${LIBNETCONF2_MAJOR_SOVERSION}.${LIBNETCONF2_MINOR_SOVERSION}.${LIBNETCONF2_MICRO_SOVERSION})
set(LIBNETCONF2_SOVERSION ${LIBNETCONF2_MAJOR_SOVERSION})

Expand Down
112 changes: 86 additions & 26 deletions src/server_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,10 @@ config_local_bind(const struct lyd_node *node, enum nc_operation parent_op, stru
break;
}
}
assert(i < LY_ARRAY_COUNT(endpt->binds));
if (i == LY_ARRAY_COUNT(endpt->binds)) {
ERR(NULL, "Local bind with address \"%s\" not found.", local_addr);
return 1;
}
bind = &endpt->binds[i];
} else if (op == NC_OP_CREATE) {
/* create a new bind */
Expand Down Expand Up @@ -1003,7 +1006,10 @@ config_ssh_hostkey(const struct lyd_node *node, enum nc_operation parent_op, str
break;
}
}
assert(i < LY_ARRAY_COUNT(ssh->hostkeys));
if (i == LY_ARRAY_COUNT(ssh->hostkeys)) {
ERR(NULL, "Hostkey with name \"%s\" not found.", name);
return 1;
}
hostkey = &ssh->hostkeys[i];
} else if (op == NC_OP_CREATE) {
/* create a new hostkey */
Expand Down Expand Up @@ -1143,7 +1149,10 @@ config_ssh_user_public_key(const struct lyd_node *node, enum nc_operation parent
break;
}
}
assert(i < LY_ARRAY_COUNT(user->pubkeys));
if (i == LY_ARRAY_COUNT(user->pubkeys)) {
ERR(NULL, "Public key with name \"%s\" not found.", name);
return 1;
}
key = &user->pubkeys[i];
} else if (op == NC_OP_CREATE) {
/* create a new public key */
Expand Down Expand Up @@ -1379,7 +1388,10 @@ config_ssh_user(const struct lyd_node *node, enum nc_operation parent_op, struct
break;
}
}
assert(i < LY_ARRAY_COUNT(ssh->auth_clients));
if (i == LY_ARRAY_COUNT(ssh->auth_clients)) {
ERR(NULL, "SSH user with name \"%s\" not found.", name);
return 1;
}
user = &ssh->auth_clients[i];
} else if (op == NC_OP_CREATE) {
/* create a new user */
Expand Down Expand Up @@ -2214,7 +2226,10 @@ config_tls_client_auth_ca_cert(const struct lyd_node *node,
break;
}
}
assert(i < LY_ARRAY_COUNT(client_auth->ca_certs));
if (i == LY_ARRAY_COUNT(client_auth->ca_certs)) {
ERR(NULL, "CA certificate \"%s\" not found.", name);
return 1;
}
cert = &client_auth->ca_certs[i];
} else if (op == NC_OP_CREATE) {
/* create a new ca-cert */
Expand Down Expand Up @@ -2337,7 +2352,10 @@ config_tls_client_auth_ee_cert(const struct lyd_node *node,
break;
}
}
assert(i < LY_ARRAY_COUNT(client_auth->ee_certs));
if (i == LY_ARRAY_COUNT(client_auth->ee_certs)) {
ERR(NULL, "End-entity certificate \"%s\" not found.", name);
return 1;
}
cert = &client_auth->ee_certs[i];
} else if (op == NC_OP_CREATE) {
/* create a new ee-cert */
Expand Down Expand Up @@ -2801,18 +2819,16 @@ config_cert_to_name(const struct lyd_node *node, enum nc_operation parent_op, st

if ((op == NC_OP_DELETE) || (op == NC_OP_NONE)) {
/* find the ctn we are deleting/modifying */
if (!tls->ctn) {
ERR(NULL, "Trying to modify/delete a non-existing cert-to-name mapping with ID %" PRIu32 ".", id);
return 1;
}

iter = tls->ctn;
prev = NULL;
while (iter && (iter->id != id)) {
prev = iter;
iter = iter->next;
}
assert(iter);
if (!iter) {
ERR(NULL, "Cert-to-name mapping with id %" PRIu32 " not found.", id);
return 1;
}
ctn = iter;
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
/* create a new ctn */
Expand Down Expand Up @@ -2959,15 +2975,21 @@ config_unix_socket_path(const struct lyd_node *node, enum nc_operation parent_op
if (op == NC_OP_DELETE) {
/* the endpoint must have a single binding, so we can just free it,
* the socket will be closed in ::nc_server_config_free() */
assert(endpt->binds);
if (!endpt->binds) {
ERR(NULL, "No UNIX socket path binding to delete.");
return 1;
}
free(endpt->binds[0].address);
LY_ARRAY_FREE(endpt->binds);

/* also clear the cleartext path flag */
opts->path_type = NC_UNIX_SOCKET_PATH_UNKNOWN;
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
/* the endpoint must not have any bindings yet, so we can just create one */
assert(!endpt->binds);
if (endpt->binds) {
ERR(NULL, "UNIX socket path binding already exists.");
return 1;
}
LY_ARRAY_NEW_RET(LYD_CTX(node), endpt->binds, bind, 1);
bind->address = strdup(lyd_get_value(node));
NC_CHECK_ERRMEM_RET(!bind->address, 1);
Expand All @@ -2994,15 +3016,21 @@ config_unix_hidden_path(const struct lyd_node *node, enum nc_operation parent_op
if (op == NC_OP_DELETE) {
/* the endpoint must have a single binding, so we can just free it,
* the socket will be closed in ::nc_server_config_free() */
assert(endpt->binds);
if (!endpt->binds) {
ERR(NULL, "No UNIX socket hidden path binding to delete.");
return 1;
}
LY_ARRAY_FREE(endpt->binds);

/* also clear the hidden path flag */
opts->path_type = NC_UNIX_SOCKET_PATH_UNKNOWN;
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
/* the endpoint must not have any bindings yet, so we can just create one
* and since the path is hidden, there is no address to set */
assert(!endpt->binds);
if (endpt->binds) {
ERR(NULL, "UNIX socket hidden path binding already exists.");
return 1;
}
LY_ARRAY_NEW_RET(LYD_CTX(node), endpt->binds, bind, 1);
bind->sock = -1;

Expand Down Expand Up @@ -3208,7 +3236,10 @@ config_unix_user_mapping(const struct lyd_node *node, enum nc_operation parent_o
break;
}
}
assert(i < LY_ARRAY_COUNT(unix->user_mappings));
if (i == LY_ARRAY_COUNT(unix->user_mappings)) {
ERR(NULL, "UNIX user mapping with system user \"%s\" not found.", system_user);
return 1;
}
mapping = &unix->user_mappings[i];
} else if (op == NC_OP_CREATE) {
/* create a new user mapping */
Expand Down Expand Up @@ -3344,7 +3375,10 @@ config_endpoint(const struct lyd_node *node, enum nc_operation parent_op,
break;
}
}
assert(i < LY_ARRAY_COUNT(config->endpts));
if (i == LY_ARRAY_COUNT(config->endpts)) {
ERR(NULL, "Endpoint \"%s\" not found.", name);
return 1;
}
endpt = &config->endpts[i];
} else if (op == NC_OP_CREATE) {
/* create a new endpoint */
Expand Down Expand Up @@ -3695,7 +3729,10 @@ config_ch_client_endpoint(const struct lyd_node *node, enum nc_operation parent_
break;
}
}
assert(i < LY_ARRAY_COUNT(ch_client->ch_endpts));
if (i == LY_ARRAY_COUNT(ch_client->ch_endpts)) {
ERR(NULL, "Call Home client \"%s\" endpoint \"%s\" not found.", ch_client->name, name);
return 1;
}
endpt = &ch_client->ch_endpts[i];
} else if (op == NC_OP_CREATE) {
/* create a new endpoint and init it */
Expand Down Expand Up @@ -3958,7 +3995,10 @@ config_netconf_client(const struct lyd_node *node, enum nc_operation parent_op,
break;
}
}
assert(i < LY_ARRAY_COUNT(config->ch_clients));
if (i == LY_ARRAY_COUNT(config->ch_clients)) {
ERR(NULL, "Call Home client \"%s\" not found.", name);
return 1;
}
ch_client = &config->ch_clients[i];
} else if (op == NC_OP_CREATE) {
/* create a new client */
Expand Down Expand Up @@ -4184,7 +4224,10 @@ config_asymmetric_key_cert(const struct lyd_node *node, enum nc_operation parent
break;
}
}
assert(i < LY_ARRAY_COUNT(entry->certs));
if (i == LY_ARRAY_COUNT(entry->certs)) {
ERR(NULL, "Certificate \"%s\" not found for asymmetric key \"%s\".", name, entry->asym_key.name);
return 1;
}
cert = &entry->certs[i];
} else if (op == NC_OP_CREATE) {
/* create a new certificate */
Expand Down Expand Up @@ -4256,7 +4299,10 @@ config_asymmetric_key(const struct lyd_node *node, enum nc_operation parent_op,
break;
}
}
assert(i < LY_ARRAY_COUNT(keystore->entries));
if (i == LY_ARRAY_COUNT(keystore->entries)) {
ERR(NULL, "Asymmetric key \"%s\" not found.", name);
return 1;
}
entry = &keystore->entries[i];
} else if (op == NC_OP_CREATE) {
/* create a new asymmetric key entry */
Expand Down Expand Up @@ -4475,7 +4521,10 @@ config_certificate_bag_cert(const struct lyd_node *node, enum nc_operation paren
break;
}
}
assert(i < LY_ARRAY_COUNT(bag->certs));
if (i == LY_ARRAY_COUNT(bag->certs)) {
ERR(NULL, "Certificate \"%s\" not found in certificate bag \"%s\".", name, bag->name);
return 1;
}
cert = &bag->certs[i];
} else if (op == NC_OP_CREATE) {
/* create a new certificate */
Expand Down Expand Up @@ -4533,7 +4582,10 @@ config_certificate_bag(const struct lyd_node *node, enum nc_operation parent_op,
break;
}
}
assert(i < LY_ARRAY_COUNT(truststore->cert_bags));
if (i == LY_ARRAY_COUNT(truststore->cert_bags)) {
ERR(NULL, "Certificate bag \"%s\" not found.", name);
return 1;
}
bag = &truststore->cert_bags[i];
} else if (op == NC_OP_CREATE) {
/* create a new certificate bag */
Expand Down Expand Up @@ -4664,7 +4716,10 @@ config_public_key_bag_pubkey(const struct lyd_node *node, enum nc_operation pare
break;
}
}
assert(i < LY_ARRAY_COUNT(bag->pubkeys));
if (i == LY_ARRAY_COUNT(bag->pubkeys)) {
ERR(NULL, "Public key \"%s\" not found in public key bag \"%s\".", name, bag->name);
return 1;
}
pubkey = &bag->pubkeys[i];
} else if (op == NC_OP_CREATE) {
/* create a new public key */
Expand Down Expand Up @@ -4722,7 +4777,10 @@ config_public_key_bag(const struct lyd_node *node, enum nc_operation parent_op,
break;
}
}
assert(i < LY_ARRAY_COUNT(truststore->pubkey_bags));
if (i == LY_ARRAY_COUNT(truststore->pubkey_bags)) {
ERR(NULL, "Public key bag \"%s\" not found.", name);
return 1;
}
bag = &truststore->pubkey_bags[i];
} else if (op == NC_OP_CREATE) {
/* create a new public key bag */
Expand Down Expand Up @@ -4897,6 +4955,7 @@ config_cert_exp_notif_anchor(const struct lyd_node *node, enum nc_operation pare
if (op == NC_OP_DELETE) {
memset(anchor, 0, sizeof *anchor);
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
/* list key */
value = lyd_get_value(node);
assert(value);
NC_CHECK_RET(nc_server_config_cert_exp_time_from_str(value, anchor));
Expand All @@ -4916,6 +4975,7 @@ config_cert_exp_notif_period(const struct lyd_node *node, enum nc_operation pare
if (op == NC_OP_DELETE) {
memset(period, 0, sizeof *period);
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
/* list key */
value = lyd_get_value(node);
assert(value);
NC_CHECK_RET(nc_server_config_cert_exp_time_from_str(value, period));
Expand Down
58 changes: 57 additions & 1 deletion tests/test_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,60 @@ test_unusupported_asymkey_format(void **state)
lyd_free_all(tree);
}

static void
test_invalid_diff(void **state)
{
int ret;
struct lyd_node *diff = NULL, *node = NULL;
struct ln2_test_ctx *test_ctx = *state;
const struct lys_module *yang_mod;

yang_mod = ly_ctx_get_module_implemented(test_ctx->ctx, "yang");
assert_non_null(yang_mod);

/* dup the keystore config */
ret = lyd_find_path(test_ctx->test_data, "/ietf-keystore:keystore", 0, &node);
assert_int_equal(ret, 0);
ret = lyd_dup_single(node, NULL, LYD_DUP_RECURSIVE, &diff);
assert_int_equal(ret, 0);

/* add none operation to the root */
ret = lyd_new_meta(test_ctx->ctx, diff, yang_mod, "operation", "none", 0, NULL);
assert_int_equal(ret, 0);

/* Delete the "hostkey" asymmetric key from the keystore */
ret = lyd_find_path(diff, "/ietf-keystore:keystore/asymmetric-keys/asymmetric-key[name='hostkey']", 0, &node);
assert_int_equal(ret, 0);
ret = lyd_new_meta(test_ctx->ctx, node, yang_mod, "operation", "delete", 0, NULL);
assert_int_equal(ret, 0);

ret = nc_server_config_setup_diff(diff);
assert_int_equal(ret, 0);

/* Now apply a diff with op=none on the deleted key and op=delete on its previously existing child public-key */
ret = lyd_new_meta(test_ctx->ctx, node, yang_mod, "operation", "none", 0, NULL);
assert_int_equal(ret, 0);

/* add delete op to the previously existing child public-key */
ret = lyd_find_path(diff,
"/ietf-keystore:keystore/asymmetric-keys/asymmetric-key[name='hostkey']/public-key", 0, &node);
assert_int_equal(ret, 0);
ret = lyd_new_meta(test_ctx->ctx, node, yang_mod, "operation", "delete", 0, NULL);
assert_int_equal(ret, 0);

/* should fail because the key was already deleted and the diff is invalid, but it should not crash */
ret = nc_server_config_setup_diff(diff);
assert_int_equal(ret, 1);

lyd_free_all(diff);
}

static void
test_config_data_free(void *data)
{
lyd_free_all(data);
}

static int
setup_f(void **state)
{
Expand Down Expand Up @@ -969,7 +1023,8 @@ setup_f(void **state)
ret = nc_server_set_unix_socket_path("unix", "netconf-test-server.sock");
assert_int_equal(ret, 0);

lyd_free_all(tree);
test_ctx->test_data = tree;
test_ctx->free_test_data = test_config_data_free;
return 0;
}

Expand All @@ -982,6 +1037,7 @@ main(void)
cmocka_unit_test_setup_teardown(test_transport_params_oper_get, setup_f, ln2_glob_test_teardown),
cmocka_unit_test_setup_teardown(test_config_all_nodes, setup_f, ln2_glob_test_teardown),
cmocka_unit_test_setup_teardown(test_unusupported_asymkey_format, setup_f, ln2_glob_test_teardown),
cmocka_unit_test_setup_teardown(test_invalid_diff, setup_f, ln2_glob_test_teardown),
};

/* try to get ports from the environment, otherwise use the default */
Expand Down