Skip to content

Commit 9699f4f

Browse files
Roytakmichalvasko
Roytak
authored and
michalvasko
committed
server config BUGFIX replace asserts with err checks
Fixes #598
1 parent 5b462c5 commit 9699f4f

2 files changed

Lines changed: 143 additions & 27 deletions

File tree

‎src/server_config.c‎

Lines changed: 86 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,10 @@ config_local_bind(const struct lyd_node *node, enum nc_operation parent_op, stru
633633
break;
634634
}
635635
}
636-
assert(i < LY_ARRAY_COUNT(endpt->binds));
636+
if (i == LY_ARRAY_COUNT(endpt->binds)) {
637+
ERR(NULL, "Local bind with address \"%s\" not found.", local_addr);
638+
return 1;
639+
}
637640
bind = &endpt->binds[i];
638641
} else if (op == NC_OP_CREATE) {
639642
/* create a new bind */
@@ -1003,7 +1006,10 @@ config_ssh_hostkey(const struct lyd_node *node, enum nc_operation parent_op, str
10031006
break;
10041007
}
10051008
}
1006-
assert(i < LY_ARRAY_COUNT(ssh->hostkeys));
1009+
if (i == LY_ARRAY_COUNT(ssh->hostkeys)) {
1010+
ERR(NULL, "Hostkey with name \"%s\" not found.", name);
1011+
return 1;
1012+
}
10071013
hostkey = &ssh->hostkeys[i];
10081014
} else if (op == NC_OP_CREATE) {
10091015
/* create a new hostkey */
@@ -1143,7 +1149,10 @@ config_ssh_user_public_key(const struct lyd_node *node, enum nc_operation parent
11431149
break;
11441150
}
11451151
}
1146-
assert(i < LY_ARRAY_COUNT(user->pubkeys));
1152+
if (i == LY_ARRAY_COUNT(user->pubkeys)) {
1153+
ERR(NULL, "Public key with name \"%s\" not found.", name);
1154+
return 1;
1155+
}
11471156
key = &user->pubkeys[i];
11481157
} else if (op == NC_OP_CREATE) {
11491158
/* create a new public key */
@@ -1379,7 +1388,10 @@ config_ssh_user(const struct lyd_node *node, enum nc_operation parent_op, struct
13791388
break;
13801389
}
13811390
}
1382-
assert(i < LY_ARRAY_COUNT(ssh->auth_clients));
1391+
if (i == LY_ARRAY_COUNT(ssh->auth_clients)) {
1392+
ERR(NULL, "SSH user with name \"%s\" not found.", name);
1393+
return 1;
1394+
}
13831395
user = &ssh->auth_clients[i];
13841396
} else if (op == NC_OP_CREATE) {
13851397
/* create a new user */
@@ -2214,7 +2226,10 @@ config_tls_client_auth_ca_cert(const struct lyd_node *node,
22142226
break;
22152227
}
22162228
}
2217-
assert(i < LY_ARRAY_COUNT(client_auth->ca_certs));
2229+
if (i == LY_ARRAY_COUNT(client_auth->ca_certs)) {
2230+
ERR(NULL, "CA certificate \"%s\" not found.", name);
2231+
return 1;
2232+
}
22182233
cert = &client_auth->ca_certs[i];
22192234
} else if (op == NC_OP_CREATE) {
22202235
/* create a new ca-cert */
@@ -2337,7 +2352,10 @@ config_tls_client_auth_ee_cert(const struct lyd_node *node,
23372352
break;
23382353
}
23392354
}
2340-
assert(i < LY_ARRAY_COUNT(client_auth->ee_certs));
2355+
if (i == LY_ARRAY_COUNT(client_auth->ee_certs)) {
2356+
ERR(NULL, "End-entity certificate \"%s\" not found.", name);
2357+
return 1;
2358+
}
23412359
cert = &client_auth->ee_certs[i];
23422360
} else if (op == NC_OP_CREATE) {
23432361
/* create a new ee-cert */
@@ -2801,18 +2819,16 @@ config_cert_to_name(const struct lyd_node *node, enum nc_operation parent_op, st
28012819

28022820
if ((op == NC_OP_DELETE) || (op == NC_OP_NONE)) {
28032821
/* find the ctn we are deleting/modifying */
2804-
if (!tls->ctn) {
2805-
ERR(NULL, "Trying to modify/delete a non-existing cert-to-name mapping with ID %" PRIu32 ".", id);
2806-
return 1;
2807-
}
2808-
28092822
iter = tls->ctn;
28102823
prev = NULL;
28112824
while (iter && (iter->id != id)) {
28122825
prev = iter;
28132826
iter = iter->next;
28142827
}
2815-
assert(iter);
2828+
if (!iter) {
2829+
ERR(NULL, "Cert-to-name mapping with id %" PRIu32 " not found.", id);
2830+
return 1;
2831+
}
28162832
ctn = iter;
28172833
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
28182834
/* create a new ctn */
@@ -2959,15 +2975,21 @@ config_unix_socket_path(const struct lyd_node *node, enum nc_operation parent_op
29592975
if (op == NC_OP_DELETE) {
29602976
/* the endpoint must have a single binding, so we can just free it,
29612977
* the socket will be closed in ::nc_server_config_free() */
2962-
assert(endpt->binds);
2978+
if (!endpt->binds) {
2979+
ERR(NULL, "No UNIX socket path binding to delete.");
2980+
return 1;
2981+
}
29632982
free(endpt->binds[0].address);
29642983
LY_ARRAY_FREE(endpt->binds);
29652984

29662985
/* also clear the cleartext path flag */
29672986
opts->path_type = NC_UNIX_SOCKET_PATH_UNKNOWN;
29682987
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
29692988
/* the endpoint must not have any bindings yet, so we can just create one */
2970-
assert(!endpt->binds);
2989+
if (endpt->binds) {
2990+
ERR(NULL, "UNIX socket path binding already exists.");
2991+
return 1;
2992+
}
29712993
LY_ARRAY_NEW_RET(LYD_CTX(node), endpt->binds, bind, 1);
29722994
bind->address = strdup(lyd_get_value(node));
29732995
NC_CHECK_ERRMEM_RET(!bind->address, 1);
@@ -2994,15 +3016,21 @@ config_unix_hidden_path(const struct lyd_node *node, enum nc_operation parent_op
29943016
if (op == NC_OP_DELETE) {
29953017
/* the endpoint must have a single binding, so we can just free it,
29963018
* the socket will be closed in ::nc_server_config_free() */
2997-
assert(endpt->binds);
3019+
if (!endpt->binds) {
3020+
ERR(NULL, "No UNIX socket hidden path binding to delete.");
3021+
return 1;
3022+
}
29983023
LY_ARRAY_FREE(endpt->binds);
29993024

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

@@ -3208,7 +3236,10 @@ config_unix_user_mapping(const struct lyd_node *node, enum nc_operation parent_o
32083236
break;
32093237
}
32103238
}
3211-
assert(i < LY_ARRAY_COUNT(unix->user_mappings));
3239+
if (i == LY_ARRAY_COUNT(unix->user_mappings)) {
3240+
ERR(NULL, "UNIX user mapping with system user \"%s\" not found.", system_user);
3241+
return 1;
3242+
}
32123243
mapping = &unix->user_mappings[i];
32133244
} else if (op == NC_OP_CREATE) {
32143245
/* create a new user mapping */
@@ -3344,7 +3375,10 @@ config_endpoint(const struct lyd_node *node, enum nc_operation parent_op,
33443375
break;
33453376
}
33463377
}
3347-
assert(i < LY_ARRAY_COUNT(config->endpts));
3378+
if (i == LY_ARRAY_COUNT(config->endpts)) {
3379+
ERR(NULL, "Endpoint \"%s\" not found.", name);
3380+
return 1;
3381+
}
33483382
endpt = &config->endpts[i];
33493383
} else if (op == NC_OP_CREATE) {
33503384
/* create a new endpoint */
@@ -3695,7 +3729,10 @@ config_ch_client_endpoint(const struct lyd_node *node, enum nc_operation parent_
36953729
break;
36963730
}
36973731
}
3698-
assert(i < LY_ARRAY_COUNT(ch_client->ch_endpts));
3732+
if (i == LY_ARRAY_COUNT(ch_client->ch_endpts)) {
3733+
ERR(NULL, "Call Home client \"%s\" endpoint \"%s\" not found.", ch_client->name, name);
3734+
return 1;
3735+
}
36993736
endpt = &ch_client->ch_endpts[i];
37003737
} else if (op == NC_OP_CREATE) {
37013738
/* create a new endpoint and init it */
@@ -3958,7 +3995,10 @@ config_netconf_client(const struct lyd_node *node, enum nc_operation parent_op,
39583995
break;
39593996
}
39603997
}
3961-
assert(i < LY_ARRAY_COUNT(config->ch_clients));
3998+
if (i == LY_ARRAY_COUNT(config->ch_clients)) {
3999+
ERR(NULL, "Call Home client \"%s\" not found.", name);
4000+
return 1;
4001+
}
39624002
ch_client = &config->ch_clients[i];
39634003
} else if (op == NC_OP_CREATE) {
39644004
/* create a new client */
@@ -4184,7 +4224,10 @@ config_asymmetric_key_cert(const struct lyd_node *node, enum nc_operation parent
41844224
break;
41854225
}
41864226
}
4187-
assert(i < LY_ARRAY_COUNT(entry->certs));
4227+
if (i == LY_ARRAY_COUNT(entry->certs)) {
4228+
ERR(NULL, "Certificate \"%s\" not found for asymmetric key \"%s\".", name, entry->asym_key.name);
4229+
return 1;
4230+
}
41884231
cert = &entry->certs[i];
41894232
} else if (op == NC_OP_CREATE) {
41904233
/* create a new certificate */
@@ -4256,7 +4299,10 @@ config_asymmetric_key(const struct lyd_node *node, enum nc_operation parent_op,
42564299
break;
42574300
}
42584301
}
4259-
assert(i < LY_ARRAY_COUNT(keystore->entries));
4302+
if (i == LY_ARRAY_COUNT(keystore->entries)) {
4303+
ERR(NULL, "Asymmetric key \"%s\" not found.", name);
4304+
return 1;
4305+
}
42604306
entry = &keystore->entries[i];
42614307
} else if (op == NC_OP_CREATE) {
42624308
/* create a new asymmetric key entry */
@@ -4475,7 +4521,10 @@ config_certificate_bag_cert(const struct lyd_node *node, enum nc_operation paren
44754521
break;
44764522
}
44774523
}
4478-
assert(i < LY_ARRAY_COUNT(bag->certs));
4524+
if (i == LY_ARRAY_COUNT(bag->certs)) {
4525+
ERR(NULL, "Certificate \"%s\" not found in certificate bag \"%s\".", name, bag->name);
4526+
return 1;
4527+
}
44794528
cert = &bag->certs[i];
44804529
} else if (op == NC_OP_CREATE) {
44814530
/* create a new certificate */
@@ -4533,7 +4582,10 @@ config_certificate_bag(const struct lyd_node *node, enum nc_operation parent_op,
45334582
break;
45344583
}
45354584
}
4536-
assert(i < LY_ARRAY_COUNT(truststore->cert_bags));
4585+
if (i == LY_ARRAY_COUNT(truststore->cert_bags)) {
4586+
ERR(NULL, "Certificate bag \"%s\" not found.", name);
4587+
return 1;
4588+
}
45374589
bag = &truststore->cert_bags[i];
45384590
} else if (op == NC_OP_CREATE) {
45394591
/* create a new certificate bag */
@@ -4664,7 +4716,10 @@ config_public_key_bag_pubkey(const struct lyd_node *node, enum nc_operation pare
46644716
break;
46654717
}
46664718
}
4667-
assert(i < LY_ARRAY_COUNT(bag->pubkeys));
4719+
if (i == LY_ARRAY_COUNT(bag->pubkeys)) {
4720+
ERR(NULL, "Public key \"%s\" not found in public key bag \"%s\".", name, bag->name);
4721+
return 1;
4722+
}
46684723
pubkey = &bag->pubkeys[i];
46694724
} else if (op == NC_OP_CREATE) {
46704725
/* create a new public key */
@@ -4722,7 +4777,10 @@ config_public_key_bag(const struct lyd_node *node, enum nc_operation parent_op,
47224777
break;
47234778
}
47244779
}
4725-
assert(i < LY_ARRAY_COUNT(truststore->pubkey_bags));
4780+
if (i == LY_ARRAY_COUNT(truststore->pubkey_bags)) {
4781+
ERR(NULL, "Public key bag \"%s\" not found.", name);
4782+
return 1;
4783+
}
47264784
bag = &truststore->pubkey_bags[i];
47274785
} else if (op == NC_OP_CREATE) {
47284786
/* create a new public key bag */
@@ -4897,6 +4955,7 @@ config_cert_exp_notif_anchor(const struct lyd_node *node, enum nc_operation pare
48974955
if (op == NC_OP_DELETE) {
48984956
memset(anchor, 0, sizeof *anchor);
48994957
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
4958+
/* list key */
49004959
value = lyd_get_value(node);
49014960
assert(value);
49024961
NC_CHECK_RET(nc_server_config_cert_exp_time_from_str(value, anchor));
@@ -4916,6 +4975,7 @@ config_cert_exp_notif_period(const struct lyd_node *node, enum nc_operation pare
49164975
if (op == NC_OP_DELETE) {
49174976
memset(period, 0, sizeof *period);
49184977
} else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) {
4978+
/* list key */
49194979
value = lyd_get_value(node);
49204980
assert(value);
49214981
NC_CHECK_RET(nc_server_config_cert_exp_time_from_str(value, period));

‎tests/test_config.c‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,60 @@ test_unusupported_asymkey_format(void **state)
937937
lyd_free_all(tree);
938938
}
939939

940+
static void
941+
test_invalid_diff(void **state)
942+
{
943+
int ret;
944+
struct lyd_node *diff = NULL, *node = NULL;
945+
struct ln2_test_ctx *test_ctx = *state;
946+
const struct lys_module *yang_mod;
947+
948+
yang_mod = ly_ctx_get_module_implemented(test_ctx->ctx, "yang");
949+
assert_non_null(yang_mod);
950+
951+
/* dup the keystore config */
952+
ret = lyd_find_path(test_ctx->test_data, "/ietf-keystore:keystore", 0, &node);
953+
assert_int_equal(ret, 0);
954+
ret = lyd_dup_single(node, NULL, LYD_DUP_RECURSIVE, &diff);
955+
assert_int_equal(ret, 0);
956+
957+
/* add none operation to the root */
958+
ret = lyd_new_meta(test_ctx->ctx, diff, yang_mod, "operation", "none", 0, NULL);
959+
assert_int_equal(ret, 0);
960+
961+
/* Delete the "hostkey" asymmetric key from the keystore */
962+
ret = lyd_find_path(diff, "/ietf-keystore:keystore/asymmetric-keys/asymmetric-key[name='hostkey']", 0, &node);
963+
assert_int_equal(ret, 0);
964+
ret = lyd_new_meta(test_ctx->ctx, node, yang_mod, "operation", "delete", 0, NULL);
965+
assert_int_equal(ret, 0);
966+
967+
ret = nc_server_config_setup_diff(diff);
968+
assert_int_equal(ret, 0);
969+
970+
/* Now apply a diff with op=none on the deleted key and op=delete on its previously existing child public-key */
971+
ret = lyd_new_meta(test_ctx->ctx, node, yang_mod, "operation", "none", 0, NULL);
972+
assert_int_equal(ret, 0);
973+
974+
/* add delete op to the previously existing child public-key */
975+
ret = lyd_find_path(diff,
976+
"/ietf-keystore:keystore/asymmetric-keys/asymmetric-key[name='hostkey']/public-key", 0, &node);
977+
assert_int_equal(ret, 0);
978+
ret = lyd_new_meta(test_ctx->ctx, node, yang_mod, "operation", "delete", 0, NULL);
979+
assert_int_equal(ret, 0);
980+
981+
/* should fail because the key was already deleted and the diff is invalid, but it should not crash */
982+
ret = nc_server_config_setup_diff(diff);
983+
assert_int_equal(ret, 1);
984+
985+
lyd_free_all(diff);
986+
}
987+
988+
static void
989+
test_config_data_free(void *data)
990+
{
991+
lyd_free_all(data);
992+
}
993+
940994
static int
941995
setup_f(void **state)
942996
{
@@ -969,7 +1023,8 @@ setup_f(void **state)
9691023
ret = nc_server_set_unix_socket_path("unix", "netconf-test-server.sock");
9701024
assert_int_equal(ret, 0);
9711025

972-
lyd_free_all(tree);
1026+
test_ctx->test_data = tree;
1027+
test_ctx->free_test_data = test_config_data_free;
9731028
return 0;
9741029
}
9751030

@@ -982,6 +1037,7 @@ main(void)
9821037
cmocka_unit_test_setup_teardown(test_transport_params_oper_get, setup_f, ln2_glob_test_teardown),
9831038
cmocka_unit_test_setup_teardown(test_config_all_nodes, setup_f, ln2_glob_test_teardown),
9841039
cmocka_unit_test_setup_teardown(test_unusupported_asymkey_format, setup_f, ln2_glob_test_teardown),
1040+
cmocka_unit_test_setup_teardown(test_invalid_diff, setup_f, ln2_glob_test_teardown),
9851041
};
9861042

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

0 commit comments

Comments
 (0)