From cb7cb728f72008b22157e0dc94874c502baa03d9 Mon Sep 17 00:00:00 2001 From: roman Date: Thu, 22 Aug 2024 16:12:32 +0200 Subject: [PATCH 1/4] server config BUGFIX handle delete on public-keys Fixes CESNET/netopeer2#1628 --- src/server_config.c | 50 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/src/server_config.c b/src/server_config.c index b74433a6..ab8dd9a7 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -602,12 +602,10 @@ nc_server_config_del_auth_client_pubkey(struct nc_auth_client *auth_client, stru } static void -nc_server_config_del_auth_client(struct nc_server_ssh_opts *opts, struct nc_auth_client *auth_client) +nc_server_config_del_auth_client_pubkeys(struct nc_auth_client *auth_client) { uint16_t i, pubkey_count; - free(auth_client->username); - if (auth_client->store == NC_STORE_LOCAL) { pubkey_count = auth_client->pubkey_count; for (i = 0; i < pubkey_count; i++) { @@ -615,7 +613,16 @@ nc_server_config_del_auth_client(struct nc_server_ssh_opts *opts, struct nc_auth } } else if (auth_client->store == NC_STORE_TRUSTSTORE) { free(auth_client->ts_ref); + auth_client->ts_ref = NULL; } +} + +static void +nc_server_config_del_auth_client(struct nc_server_ssh_opts *opts, struct nc_auth_client *auth_client) +{ + free(auth_client->username); + + nc_server_config_del_auth_client_pubkeys(auth_client); free(auth_client->password); @@ -2290,6 +2297,41 @@ nc_server_config_use_system_keys(const struct lyd_node *node, NC_OPERATION op) return ret; } +static int +nc_server_config_public_keys(const struct lyd_node *node, NC_OPERATION op) +{ + int ret = 0; + struct nc_auth_client *auth_client; + struct nc_ch_client *ch_client = NULL; + + assert(!strcmp(LYD_NAME(node), "public-keys")); + + /* only do something on delete */ + if (op != NC_OP_DELETE) { + return 0; + } + + /* LOCK */ + if (is_ch(node) && nc_server_config_get_ch_client_with_lock(node, &ch_client)) { + /* to avoid unlock on fail */ + return 1; + } + + if (nc_server_config_get_auth_client(node, ch_client, &auth_client)) { + ret = 1; + goto cleanup; + } + + nc_server_config_del_auth_client_pubkeys(auth_client); + +cleanup: + if (is_ch(node)) { + /* UNLOCK */ + nc_ch_client_unlock(ch_client); + } + return ret; +} + /* leaf */ static int nc_server_config_password(const struct lyd_node *node, NC_OPERATION op) @@ -3729,6 +3771,8 @@ nc_server_config_parse_netconf_server(const struct lyd_node *node, NC_OPERATION ret = nc_server_config_truststore_reference(node, op); } else if (!strcmp(name, "use-system-keys")) { ret = nc_server_config_use_system_keys(node, op); + } else if (!strcmp(name, "public-keys")) { + ret = nc_server_config_public_keys(node, op); } else if (!strcmp(name, "password")) { ret = nc_server_config_password(node, op); } else if (!strcmp(name, "use-system-auth")) { From 6e1ac9816703bdd76cda57e541831ec9a9df1372 Mon Sep 17 00:00:00 2001 From: roman Date: Thu, 22 Aug 2024 16:16:53 +0200 Subject: [PATCH 2/4] server config BUGFIX handle delete on client-auth --- src/server_config.c | 120 +++++++++++++++++++++++++++++++++++++++ src/session_server_tls.c | 39 +++++++++++++ 2 files changed, 159 insertions(+) diff --git a/src/server_config.c b/src/server_config.c index ab8dd9a7..fa5d08a8 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -763,11 +763,16 @@ nc_server_config_del_certs(struct nc_cert_grouping *certs_grp) free(certs_grp->certs[i].name); free(certs_grp->certs[i].data); } + certs_grp->cert_count = 0; free(certs_grp->certs); certs_grp->certs = NULL; } else if (certs_grp->store == NC_STORE_TRUSTSTORE) { free(certs_grp->ts_ref); + certs_grp->ts_ref = NULL; } + + /* reset to the default */ + certs_grp->store = NC_STORE_LOCAL; } static void @@ -2965,6 +2970,115 @@ nc_server_config_asymmetric_key(const struct lyd_node *node, NC_OPERATION op) return ret; } +static int +nc_server_config_client_authentication(const struct lyd_node *node, NC_OPERATION op) +{ + int ret = 0; + struct nc_server_tls_opts *opts; + struct nc_ch_client *ch_client = NULL; + + assert(!strcmp(LYD_NAME(node), "client-authentication")); + + /* only do something on delete and if we're in the TLS subtree, + * because this is a presence container unlike its SSH counterpart */ + if (!is_tls(node) || (op != NC_OP_DELETE)) { + return 0; + } + + /* LOCK */ + if (is_ch(node) && nc_server_config_get_ch_client_with_lock(node, &ch_client)) { + /* to avoid unlock on fail */ + return 1; + } + + if (nc_server_config_get_tls_opts(node, ch_client, &opts)) { + ret = 1; + goto cleanup; + } + + nc_server_config_del_certs(&opts->ca_certs); + nc_server_config_del_certs(&opts->ee_certs); + +cleanup: + if (is_ch(node)) { + /* UNLOCK */ + nc_ch_client_unlock(ch_client); + } + return ret; +} + +static int +nc_server_config_ca_certs(const struct lyd_node *node, NC_OPERATION op) +{ + int ret = 0; + struct nc_server_tls_opts *opts; + struct nc_ch_client *ch_client = NULL; + + assert(!strcmp(LYD_NAME(node), "ca-certs")); + + /* only do something on delete and if we're in the TLS subtree, + * because SSH certs are not yet supported */ + if (!is_tls(node) || (op != NC_OP_DELETE)) { + return 0; + } + + /* LOCK */ + if (is_ch(node) && nc_server_config_get_ch_client_with_lock(node, &ch_client)) { + /* to avoid unlock on fail */ + return 1; + } + + if (nc_server_config_get_tls_opts(node, ch_client, &opts)) { + ret = 1; + goto cleanup; + } + + nc_server_config_del_certs(&opts->ca_certs); + +cleanup: + if (is_ch(node)) { + /* UNLOCK */ + nc_ch_client_unlock(ch_client); + } + return ret; +} + +static int +nc_server_config_ee_certs(const struct lyd_node *node, NC_OPERATION op) +{ + int ret = 0; + struct nc_server_tls_opts *opts; + struct nc_ch_client *ch_client = NULL; + + assert(!strcmp(LYD_NAME(node), "ee-certs")); + + /* only do something on delete and if we're in the TLS subtree, + * because SSH certs are not yet supported */ + if (!is_tls(node) || (op != NC_OP_DELETE)) { + return 0; + } + + /* LOCK */ + if (is_ch(node) && nc_server_config_get_ch_client_with_lock(node, &ch_client)) { + /* to avoid unlock on fail */ + return 1; + } + + if (nc_server_config_get_tls_opts(node, ch_client, &opts)) { + ret = 1; + goto cleanup; + } + + nc_server_config_del_certs(&opts->ee_certs); + +cleanup: + if (is_ch(node)) { + /* UNLOCK */ + nc_ch_client_unlock(ch_client); + } + return ret; +} + static int nc_server_config_create_ca_certs_certificate(const struct lyd_node *node, struct nc_server_tls_opts *opts) { @@ -3795,6 +3909,12 @@ nc_server_config_parse_netconf_server(const struct lyd_node *node, NC_OPERATION ret = nc_server_config_cert_data(node, op); } else if (!strcmp(name, "asymmetric-key")) { ret = nc_server_config_asymmetric_key(node, op); + } else if (!strcmp(name, "client-authentication")) { + ret = nc_server_config_client_authentication(node, op); + } else if (!strcmp(name, "ca-certs")) { + ret = nc_server_config_ca_certs(node, op); + } else if (!strcmp(name, "ee-certs")) { + ret = nc_server_config_ee_certs(node, op); } else if (!strcmp(name, "certificate")) { ret = nc_server_config_certificate(node, op); } else if (!strcmp(name, "cert-to-name")) { diff --git a/src/session_server_tls.c b/src/session_server_tls.c index 0f8b8596..ed89fb25 100644 --- a/src/session_server_tls.c +++ b/src/session_server_tls.c @@ -766,6 +766,30 @@ nc_server_tls_accept_check(int accept_ret, void *tls_session) return accept_ret; } +/** + * @brief Get the number of certificates in a certificate grouping. + * + * @param[in] certs_grp Certificate grouping to get the number of certificates from. + * @return Number of certificates in the grouping, or -1 on error. + */ +static uint16_t +nc_server_tls_get_num_certs(struct nc_cert_grouping *certs_grp) +{ + uint16_t count = 0; + struct nc_certificate *certs; + + if (certs_grp->store == NC_STORE_LOCAL) { + count = certs_grp->cert_count; + } else if (certs_grp->store == NC_STORE_TRUSTSTORE) { + if (nc_server_tls_truststore_ref_get_certs(certs_grp->ts_ref, &certs, &count)) { + ERR(NULL, "Getting CA certificates from the truststore reference \"%s\" failed.", certs_grp->ts_ref); + return -1; + } + } + + return count; +} + int nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opts, int sock, int timeout) { @@ -774,6 +798,7 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt struct nc_tls_verify_cb_data cb_data = {0}; struct nc_endpt *referenced_endpt; void *tls_cfg, *srv_cert, *srv_pkey, *cert_store, *crl_store; + uint32_t cert_count = 0; tls_cfg = srv_cert = srv_pkey = cert_store = crl_store = NULL; @@ -818,6 +843,20 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt } } + /* Check if there are no CA/end entity certs configured, which is a valid config. + * However, that would imply not using TLS for auth, which is not (yet) supported */ + if (!opts->referenced_endpt_name) { + cert_count = nc_server_tls_get_num_certs(&opts->ca_certs) + nc_server_tls_get_num_certs(&opts->ee_certs); + } else { + cert_count = nc_server_tls_get_num_certs(&opts->ca_certs) + nc_server_tls_get_num_certs(&opts->ee_certs) + + nc_server_tls_get_num_certs(&referenced_endpt->opts.tls->ca_certs) + + nc_server_tls_get_num_certs(&referenced_endpt->opts.tls->ee_certs); + } + if (cert_count <= 0) { + ERR(session, "Neither CA nor end-entity certificates configured."); + goto fail; + } + if (nc_session_tls_crl_from_cert_ext_fetch(srv_cert, cert_store, &crl_store)) { ERR(session, "Loading server CRL failed."); goto fail; From f3d41fd710a8de9c8724b747df524b41d081b111 Mon Sep 17 00:00:00 2001 From: roman Date: Thu, 22 Aug 2024 16:18:12 +0200 Subject: [PATCH 3/4] server config BUGFIX handle delete on conn-type --- src/server_config.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/server_config.c b/src/server_config.c index fa5d08a8..669d3d57 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -3620,7 +3620,10 @@ nc_server_config_persistent(const struct lyd_node *node, NC_OPERATION op) assert(!strcmp(LYD_NAME(node), "persistent")); - (void) op; + /* switch to periodic, don't do anything */ + if (op == NC_OP_DELETE) { + return 0; + } /* LOCK */ if (nc_server_config_get_ch_client_with_lock(node, &ch_client)) { @@ -3644,7 +3647,10 @@ nc_server_config_periodic(const struct lyd_node *node, NC_OPERATION op) assert(!strcmp(LYD_NAME(node), "periodic")); - (void) op; + /* switch to persistent, don't do anything */ + if (op == NC_OP_DELETE) { + return 0; + } /* LOCK */ if (nc_server_config_get_ch_client_with_lock(node, &ch_client)) { From b8356f594ce5fd65dbbd98a9ee420d3eb4f7599c Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 23 Aug 2024 10:17:29 +0200 Subject: [PATCH 4/4] server config REFACTOR sort node names Sort node names alphabetically for better readability --- src/server_config.c | 173 +++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 89 deletions(-) diff --git a/src/server_config.c b/src/server_config.c index 669d3d57..8ce9b7be 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -3849,119 +3849,114 @@ nc_server_config_parse_netconf_server(const struct lyd_node *node, NC_OPERATION const char *name = LYD_NAME(node); int ret = 0; - if (!strcmp(name, "listen")) { - ret = nc_server_config_listen(node, op); + if (!strcmp(name, "anchor-time")) { + ret = nc_server_config_anchor_time(node, op); } else if (!strcmp(name, "call-home")) { ret = nc_server_config_ch(node, op); } else if (!strcmp(name, "endpoint")) { ret = nc_server_config_endpoint(node, op); + } else if (!strcmp(name, "idle-timeout")) { + ret = nc_server_config_idle_timeout(node, op); + } else if (!strcmp(name, "listen")) { + ret = nc_server_config_listen(node, op); + } else if (!strcmp(name, "max-attempts")) { + ret = nc_server_config_max_attempts(node, op); + } else if (!strcmp(name, "max-wait")) { + ret = nc_server_config_max_wait(node, op); + } else if (!strcmp(name, "netconf-client")) { + ret = nc_server_config_netconf_client(node, op); + } else if (!strcmp(name, "period")) { + ret = nc_server_config_period(node, op); + } else if (!strcmp(name, "periodic")) { + ret = nc_server_config_periodic(node, op); + } else if (!strcmp(name, "persistent")) { + ret = nc_server_config_persistent(node, op); + } else if (!strcmp(name, "reconnect-strategy")) { + ret = nc_server_config_reconnect_strategy(node, op); + } else if (!strcmp(name, "start-with")) { + ret = nc_server_config_start_with(node, op); } #ifdef NC_ENABLED_SSH_TLS - else if (!strcmp(name, "ssh")) { - ret = nc_server_config_ssh(node, op); + else if (!strcmp(name, "auth-timeout")) { + ret = nc_server_config_auth_timeout(node, op); + } else if (!strcmp(name, "asymmetric-key")) { + ret = nc_server_config_asymmetric_key(node, op); + } else if (!strcmp(name, "ca-certs")) { + ret = nc_server_config_ca_certs(node, op); + } else if (!strcmp(name, "central-keystore-reference")) { + ret = nc_server_config_keystore_reference(node, op); + } else if (!strcmp(name, "central-truststore-reference")) { + ret = nc_server_config_truststore_reference(node, op); + } else if (!strcmp(name, "cert-data")) { + ret = nc_server_config_cert_data(node, op); + } else if (!strcmp(name, "certificate")) { + ret = nc_server_config_certificate(node, op); + } else if (!strcmp(name, "cert-to-name")) { + ret = nc_server_config_cert_to_name(node, op); + } else if (!strcmp(name, "cipher-suite")) { + ret = nc_server_config_cipher_suite(node, op); + } else if (!strcmp(name, "cleartext-private-key")) { + ret = nc_server_config_cleartext_private_key(node, op); + } else if (!strcmp(name, "client-authentication")) { + ret = nc_server_config_client_authentication(node, op); + } else if (!strcmp(name, "ee-certs")) { + ret = nc_server_config_ee_certs(node, op); + } else if (!strcmp(name, "encryption-alg")) { + ret = nc_server_config_encryption_alg(node, op); + } else if (!strcmp(name, "endpoint-reference")) { + ret = nc_server_config_endpoint_reference(node, op); + } else if (!strcmp(name, "fingerprint")) { + ret = nc_server_config_fingerprint(node, op); + } else if (!strcmp(name, "host-key")) { + ret = nc_server_config_host_key(node, op); + } else if (!strcmp(name, "host-key-alg")) { + ret = nc_server_config_host_key_alg(node, op); + } else if (!strcmp(name, "idle-time")) { + ret = nc_server_config_idle_time(node, op); + } else if (!strcmp(name, "keepalives")) { + ret = nc_server_config_keepalives(node, op); + } else if (!strcmp(name, "key-exchange-alg")) { + ret = nc_server_config_kex_alg(node, op); } else if (!strcmp(name, "local-address")) { ret = nc_server_config_local_address(node, op); } else if (!strcmp(name, "local-port")) { ret = nc_server_config_local_port(node, op); - } else if (!strcmp(name, "keepalives")) { - ret = nc_server_config_keepalives(node, op); - } else if (!strcmp(name, "idle-time")) { - ret = nc_server_config_idle_time(node, op); + } else if (!strcmp(name, "mac-alg")) { + ret = nc_server_config_mac_alg(node, op); } else if (!strcmp(name, "max-probes")) { ret = nc_server_config_max_probes(node, op); + } else if (!strcmp(name, "none")) { + ret = nc_server_config_none(node, op); + } else if (!strcmp(name, "password")) { + ret = nc_server_config_password(node, op); + } else if (!strcmp(name, "private-key-format")) { + ret = nc_server_config_private_key_format(node, op); } else if (!strcmp(name, "probe-interval")) { ret = nc_server_config_probe_interval(node, op); - } else if (!strcmp(name, "host-key")) { - ret = nc_server_config_host_key(node, op); - } else if (!strcmp(name, "public-key-format")) { - ret = nc_server_config_public_key_format(node, op); } else if (!strcmp(name, "public-key")) { ret = nc_server_config_public_key(node, op); - } else if (!strcmp(name, "private-key-format")) { - ret = nc_server_config_private_key_format(node, op); - } else if (!strcmp(name, "cleartext-private-key")) { - ret = nc_server_config_cleartext_private_key(node, op); - } else if (!strcmp(name, "central-keystore-reference")) { - ret = nc_server_config_keystore_reference(node, op); - } else if (!strcmp(name, "user")) { - ret = nc_server_config_user(node, op); - } else if (!strcmp(name, "auth-timeout")) { - ret = nc_server_config_auth_timeout(node, op); - } else if (!strcmp(name, "central-truststore-reference")) { - ret = nc_server_config_truststore_reference(node, op); - } else if (!strcmp(name, "use-system-keys")) { - ret = nc_server_config_use_system_keys(node, op); + } else if (!strcmp(name, "public-key-format")) { + ret = nc_server_config_public_key_format(node, op); } else if (!strcmp(name, "public-keys")) { ret = nc_server_config_public_keys(node, op); - } else if (!strcmp(name, "password")) { - ret = nc_server_config_password(node, op); - } else if (!strcmp(name, "use-system-auth")) { - ret = nc_server_config_use_system_auth(node, op); - } else if (!strcmp(name, "none")) { - ret = nc_server_config_none(node, op); - } else if (!strcmp(name, "host-key-alg")) { - ret = nc_server_config_host_key_alg(node, op); - } else if (!strcmp(name, "key-exchange-alg")) { - ret = nc_server_config_kex_alg(node, op); - } else if (!strcmp(name, "encryption-alg")) { - ret = nc_server_config_encryption_alg(node, op); - } else if (!strcmp(name, "mac-alg")) { - ret = nc_server_config_mac_alg(node, op); - } else if (!strcmp(name, "endpoint-reference")) { - ret = nc_server_config_endpoint_reference(node, op); + } else if (!strcmp(name, "remote-address")) { + ret = nc_server_config_remote_address(node, op); + } else if (!strcmp(name, "remote-port")) { + ret = nc_server_config_remote_port(node, op); + } else if (!strcmp(name, "ssh")) { + ret = nc_server_config_ssh(node, op); } else if (!strcmp(name, "tls")) { ret = nc_server_config_tls(node, op); - } else if (!strcmp(name, "cert-data")) { - ret = nc_server_config_cert_data(node, op); - } else if (!strcmp(name, "asymmetric-key")) { - ret = nc_server_config_asymmetric_key(node, op); - } else if (!strcmp(name, "client-authentication")) { - ret = nc_server_config_client_authentication(node, op); - } else if (!strcmp(name, "ca-certs")) { - ret = nc_server_config_ca_certs(node, op); - } else if (!strcmp(name, "ee-certs")) { - ret = nc_server_config_ee_certs(node, op); - } else if (!strcmp(name, "certificate")) { - ret = nc_server_config_certificate(node, op); - } else if (!strcmp(name, "cert-to-name")) { - ret = nc_server_config_cert_to_name(node, op); - } else if (!strcmp(name, "fingerprint")) { - ret = nc_server_config_fingerprint(node, op); } else if (!strcmp(name, "tls-version")) { ret = nc_server_config_tls_version(node, op); - } else if (!strcmp(name, "cipher-suite")) { - ret = nc_server_config_cipher_suite(node, op); - } -#endif /* NC_ENABLED_SSH_TLS */ - else if (!strcmp(name, "netconf-client")) { - ret = nc_server_config_netconf_client(node, op); - } -#ifdef NC_ENABLED_SSH_TLS - else if (!strcmp(name, "remote-address")) { - ret = nc_server_config_remote_address(node, op); - } else if (!strcmp(name, "remote-port")) { - ret = nc_server_config_remote_port(node, op); + } else if (!strcmp(name, "user")) { + ret = nc_server_config_user(node, op); + } else if (!strcmp(name, "use-system-auth")) { + ret = nc_server_config_use_system_auth(node, op); + } else if (!strcmp(name, "use-system-keys")) { + ret = nc_server_config_use_system_keys(node, op); } #endif /* NC_ENABLED_SSH_TLS */ - else if (!strcmp(name, "persistent")) { - ret = nc_server_config_persistent(node, op); - } else if (!strcmp(name, "periodic")) { - ret = nc_server_config_periodic(node, op); - } else if (!strcmp(name, "period")) { - ret = nc_server_config_period(node, op); - } else if (!strcmp(name, "anchor-time")) { - ret = nc_server_config_anchor_time(node, op); - } else if (!strcmp(name, "idle-timeout")) { - ret = nc_server_config_idle_timeout(node, op); - } else if (!strcmp(name, "reconnect-strategy")) { - ret = nc_server_config_reconnect_strategy(node, op); - } else if (!strcmp(name, "start-with")) { - ret = nc_server_config_start_with(node, op); - } else if (!strcmp(name, "max-wait")) { - ret = nc_server_config_max_wait(node, op); - } else if (!strcmp(name, "max-attempts")) { - ret = nc_server_config_max_attempts(node, op); - } if (ret) { ERR(NULL, "Configuring node \"%s\" failed.", LYD_NAME(node));