From nobody Mon Aug 4 19:46:42 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4bwnBC1s2Yz63RJF; Mon, 04 Aug 2025 19:46:43 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bwnBB4zrBz4KXc; Mon, 04 Aug 2025 19:46:42 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1754336802; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=gLcYFrE+qkpgrL61BRAs8ChzQx0xZ8UthFJa42kePYQ=; b=lhizlA3DuAwwyuUAq5d4Wnu+ZZ7SEZg0mqP4JjQgQp+bFs+BP7ScOTjhYBwsO/X2wa+MOn 6SQTwHMJ5P/95TFlpd6iWMTeIqNkBzanix3fJMQaIkZdf90CDVfS44GzUXo8nc+JpG72zS ixLehPtBWzBaNGDqmTrFPp0YqlL/Q5IyWOxT73JpTK5INSfVqRacACeYt5APb+pBxL1lox V62fPtdS6o7sp7Rq4Cb1DSeS3RPYCg9RbOQQcBdmfgPk64J9UdyolpyaLJxB+wohD/JuV8 YP+C1mUcKwXAHuaKds75TQt4hTPtRUEJ5Q2dDL0Q3mbWYco2gE4nR/Vhs/afYA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1754336802; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=gLcYFrE+qkpgrL61BRAs8ChzQx0xZ8UthFJa42kePYQ=; b=jWxJ0OQ28dBhSPVmvUa4PVzbQXsEsxvGdhVNgRrBVNfGy6beBy7U3dSa91kmFZo5Ftnim+ mIKexpK7fwSd7eVnnmDkrM/NRW+eggnaRhD4R6RtsiE63WmNbfTA4R1Rc16uW8WzOU/8yw nCRW6BwKoVNARdAQ13MnThkZjK+uEmBqzZbW0FWYo7q6lNqfFMGbbDvqUBWcdK5cFWhNhb F7T4Czmdff7gBfQ1gTyKyxZ3K4fcEETTK0EK/avD/Qft2RbOr5wG4mesSo2n7IsHHpXtku SAYfscDFffCVF2rGUyVpQUBhKj380PtpUSwueDfCHxFyxhZVVSGMV8XaoOBygA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1754336802; a=rsa-sha256; cv=none; b=crXNpyo1F825PYYOEgEdJiTC63U+dabnI653MVGp2NMFW6/K2LusNoeeeiSOkTDOx0xUD6 7JGg0Rx06CfrmhNgX5AYmLdDIq3NVtnqNwlAXJypx8xhNhZWsjuSoPqlVRw1PrOf6Au9Wv MDpqe5JKRKH3oINXEvEfHiQFoE2I6aHjfLuduD7ALvTn/rHhPBvhGynC7RBZTtUjJNoW43 NpTBtGjleh0DPOj94CDYTFz2/ZbSwKlXrnalgwCm5Y7om4JGJQhbFoXbS127DxQ6bCjXwn fh4XofmLG0fw175FckKzrFBD5Qfk4vCABFrNiQ4dqsY4RpOxsSXK85OAaU0yxw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4bwnBB4Z8pz13B8; Mon, 04 Aug 2025 19:46:42 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 574Jkg5I098893; Mon, 4 Aug 2025 19:46:42 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 574JkgJC098890; Mon, 4 Aug 2025 19:46:42 GMT (envelope-from git) Date: Mon, 4 Aug 2025 19:46:42 GMT Message-Id: <202508041946.574JkgJC098890@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: John Baldwin Subject: git: 7f912714c536 - main - ctld: Convert struct auth to a C++ class List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jhb X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 7f912714c53644ea18fc58ffa364918ccfa22999 Auto-Submitted: auto-generated The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=7f912714c53644ea18fc58ffa364918ccfa22999 commit 7f912714c53644ea18fc58ffa364918ccfa22999 Author: John Baldwin AuthorDate: 2025-08-04 19:38:06 +0000 Commit: John Baldwin CommitDate: 2025-08-04 19:38:06 +0000 ctld: Convert struct auth to a C++ class Use private std::string to hold secret and mutual authentication strings along with accessors to retrieve constant C versions of those strings. Add a helper function to determine if an auth object contains mutual credentials. Instead of storing the user name in the structure, use an unordered_map<> with the username as the key for the ag_auths member of auth_group. Add a parse error if multiple credentials specify the same user. Previously the code always used the first credential when verifying and ignored additional credentials silently. Sponsored by: Chelsio Communications Pull Request: https://github.com/freebsd/freebsd-src/pull/1794 --- usr.sbin/ctld/ctld.cc | 72 +++++++++++++--------------------------------- usr.sbin/ctld/ctld.hh | 28 +++++++++++++----- usr.sbin/ctld/discovery.cc | 2 +- usr.sbin/ctld/login.cc | 31 ++++++++++---------- 4 files changed, 57 insertions(+), 76 deletions(-) diff --git a/usr.sbin/ctld/ctld.cc b/usr.sbin/ctld/ctld.cc index ba65befa2d0a..558ddb8ac6aa 100644 --- a/usr.sbin/ctld/ctld.cc +++ b/usr.sbin/ctld/ctld.cc @@ -143,42 +143,14 @@ conf_delete(struct conf *conf) free(conf); } -static struct auth * -auth_new(struct auth_group *ag) -{ - struct auth *auth; - - auth = reinterpret_cast(calloc(1, sizeof(*auth))); - if (auth == NULL) - log_err(1, "calloc"); - auth->a_auth_group = ag; - TAILQ_INSERT_TAIL(&ag->ag_auths, auth, a_next); - return (auth); -} - -static void -auth_delete(struct auth *auth) -{ - TAILQ_REMOVE(&auth->a_auth_group->ag_auths, auth, a_next); - - free(auth->a_user); - free(auth->a_secret); - free(auth->a_mutual_user); - free(auth->a_mutual_secret); - free(auth); -} - const struct auth * auth_find(const struct auth_group *ag, const char *user) { - const struct auth *auth; + auto it = ag->ag_auths.find(user); + if (it == ag->ag_auths.end()) + return (nullptr); - TAILQ_FOREACH(auth, &ag->ag_auths, a_next) { - if (strcmp(auth->a_user, user) == 0) - return (auth); - } - - return (NULL); + return (&it->second); } static void @@ -188,6 +160,7 @@ auth_check_secret_length(const struct auth_group *ag, const char *user, size_t len; len = strlen(secret); + assert(len != 0); if (len > 16) { log_warnx("%s for user \"%s\", %s, is too long; it should be " "at most 16 characters long", secret_type, user, @@ -204,8 +177,6 @@ bool auth_new_chap(struct auth_group *ag, const char *user, const char *secret) { - struct auth *auth; - if (ag->ag_type == AG_TYPE_UNKNOWN) ag->ag_type = AG_TYPE_CHAP; if (ag->ag_type != AG_TYPE_CHAP) { @@ -216,9 +187,12 @@ auth_new_chap(struct auth_group *ag, const char *user, auth_check_secret_length(ag, user, secret, "secret"); - auth = auth_new(ag); - auth->a_user = checked_strdup(user); - auth->a_secret = checked_strdup(secret); + const auto &pair = ag->ag_auths.try_emplace(user, secret); + if (!pair.second) { + log_warnx("duplicate credentials for user \"%s\" for %s", + user, ag->ag_label); + return (false); + } return (true); } @@ -227,8 +201,6 @@ bool auth_new_chap_mutual(struct auth_group *ag, const char *user, const char *secret, const char *user2, const char *secret2) { - struct auth *auth; - if (ag->ag_type == AG_TYPE_UNKNOWN) ag->ag_type = AG_TYPE_CHAP_MUTUAL; if (ag->ag_type != AG_TYPE_CHAP_MUTUAL) { @@ -240,11 +212,13 @@ auth_new_chap_mutual(struct auth_group *ag, const char *user, auth_check_secret_length(ag, user, secret, "secret"); auth_check_secret_length(ag, user, secret2, "mutual secret"); - auth = auth_new(ag); - auth->a_user = checked_strdup(user); - auth->a_secret = checked_strdup(secret); - auth->a_mutual_user = checked_strdup(user2); - auth->a_mutual_secret = checked_strdup(secret2); + const auto &pair = ag->ag_auths.try_emplace(user, secret, user2, + secret2); + if (!pair.second) { + log_warnx("duplicate credentials for user \"%s\" for %s", + user, ag->ag_label); + return (false); + } return (true); } @@ -442,13 +416,10 @@ auth_group_create(struct conf *conf, const char *name, char *label) { struct auth_group *ag; - ag = reinterpret_cast(calloc(1, sizeof(*ag))); - if (ag == NULL) - log_err(1, "calloc"); + ag = new auth_group(); if (name != NULL) ag->ag_name = checked_strdup(name); ag->ag_label = label; - TAILQ_INIT(&ag->ag_auths); TAILQ_INIT(&ag->ag_names); TAILQ_INIT(&ag->ag_portals); ag->ag_conf = conf; @@ -485,14 +456,11 @@ auth_group_new(struct conf *conf, struct target *target) void auth_group_delete(struct auth_group *ag) { - struct auth *auth, *auth_tmp; struct auth_name *auth_name, *auth_name_tmp; struct auth_portal *auth_portal, *auth_portal_tmp; TAILQ_REMOVE(&ag->ag_conf->conf_auth_groups, ag, ag_next); - TAILQ_FOREACH_SAFE(auth, &ag->ag_auths, a_next, auth_tmp) - auth_delete(auth); TAILQ_FOREACH_SAFE(auth_name, &ag->ag_names, an_next, auth_name_tmp) auth_name_delete(auth_name); TAILQ_FOREACH_SAFE(auth_portal, &ag->ag_portals, ap_next, @@ -500,7 +468,7 @@ auth_group_delete(struct auth_group *ag) auth_portal_delete(auth_portal); free(ag->ag_label); free(ag->ag_name); - free(ag); + delete ag; } struct auth_group * diff --git a/usr.sbin/ctld/ctld.hh b/usr.sbin/ctld/ctld.hh index 61e453d8dc23..a2cedeaf77da 100644 --- a/usr.sbin/ctld/ctld.hh +++ b/usr.sbin/ctld/ctld.hh @@ -41,6 +41,10 @@ #include #include +#include +#include +#include + #define DEFAULT_CONFIG_PATH "/etc/ctl.conf" #define DEFAULT_PIDFILE "/var/run/ctld.pid" #define DEFAULT_BLOCKSIZE 512 @@ -50,12 +54,22 @@ #define SOCKBUF_SIZE 1048576 struct auth { - TAILQ_ENTRY(auth) a_next; - struct auth_group *a_auth_group; - char *a_user; - char *a_secret; - char *a_mutual_user; - char *a_mutual_secret; + auth(std::string_view secret) : a_secret(secret) {} + auth(std::string_view secret, std::string_view mutual_user, + std::string_view mutual_secret) : + a_secret(secret), a_mutual_user(mutual_user), + a_mutual_secret(mutual_secret) {} + + bool mutual() const { return !a_mutual_user.empty(); } + + const char *secret() const { return a_secret.c_str(); } + const char *mutual_user() const { return a_mutual_user.c_str(); } + const char *mutual_secret() const { return a_mutual_secret.c_str(); } + +private: + std::string a_secret; + std::string a_mutual_user; + std::string a_mutual_secret; }; struct auth_name { @@ -84,7 +98,7 @@ struct auth_group { char *ag_name; char *ag_label; int ag_type; - TAILQ_HEAD(, auth) ag_auths; + std::unordered_map ag_auths; TAILQ_HEAD(, auth_name) ag_names; TAILQ_HEAD(, auth_portal) ag_portals; }; diff --git a/usr.sbin/ctld/discovery.cc b/usr.sbin/ctld/discovery.cc index 3cf1b94a9619..c4bfb94669f9 100644 --- a/usr.sbin/ctld/discovery.cc +++ b/usr.sbin/ctld/discovery.cc @@ -196,7 +196,7 @@ discovery_target_filtered_out(const struct ctld_connection *conn, return (true); } - error = chap_authenticate(conn->conn_chap, auth->a_secret); + error = chap_authenticate(conn->conn_chap, auth->secret()); if (error != 0) { log_debugx("password for CHAP user \"%s\" doesn't " "match target \"%s\"; skipping", diff --git a/usr.sbin/ctld/login.cc b/usr.sbin/ctld/login.cc index e891a3ed4b67..549fa0c397ad 100644 --- a/usr.sbin/ctld/login.cc +++ b/usr.sbin/ctld/login.cc @@ -336,7 +336,7 @@ login_send_chap_c(struct pdu *request, struct chap *chap) static struct pdu * login_receive_chap_r(struct connection *conn, struct auth_group *ag, - struct chap *chap, const struct auth **authp) + struct chap *chap, const struct auth **authp, std::string &user) { struct pdu *request; struct keys *request_keys; @@ -376,15 +376,13 @@ login_receive_chap_r(struct connection *conn, struct auth_group *ag, chap_n); } - assert(auth->a_secret != NULL); - assert(strlen(auth->a_secret) > 0); - - error = chap_authenticate(chap, auth->a_secret); + error = chap_authenticate(chap, auth->secret()); if (error != 0) { login_send_error(request, 0x02, 0x01); log_errx(1, "CHAP authentication failed for user \"%s\"", - auth->a_user); + chap_n); } + user = chap_n; keys_delete(request_keys); @@ -394,7 +392,7 @@ login_receive_chap_r(struct connection *conn, struct auth_group *ag, static void login_send_chap_success(struct pdu *request, - const struct auth *auth) + const struct auth *auth, const std::string &user) { struct pdu *response; struct keys *request_keys, *response_keys; @@ -424,17 +422,17 @@ login_send_chap_success(struct pdu *request, log_errx(1, "initiator requested target " "authentication, but didn't send CHAP_C"); } - if (auth->a_auth_group->ag_type != AG_TYPE_CHAP_MUTUAL) { + if (!auth->mutual()) { login_send_error(request, 0x02, 0x01); log_errx(1, "initiator requests target authentication " "for user \"%s\", but mutual user/secret " - "is not set", auth->a_user); + "is not set", user.c_str()); } log_debugx("performing mutual authentication as user \"%s\"", - auth->a_mutual_user); + auth->mutual_user()); - rchap = rchap_new(auth->a_mutual_secret); + rchap = rchap_new(auth->mutual_secret()); error = rchap_receive(rchap, chap_i, chap_c); if (error != 0) { login_send_error(request, 0x02, 0x07); @@ -444,7 +442,7 @@ login_send_chap_success(struct pdu *request, chap_r = rchap_get_response(rchap); rchap_delete(rchap); response_keys = keys_new(); - keys_add(response_keys, "CHAP_N", auth->a_mutual_user); + keys_add(response_keys, "CHAP_N", auth->mutual_user()); keys_add(response_keys, "CHAP_R", chap_r); free(chap_r); keys_save_pdu(response_keys, response); @@ -461,6 +459,7 @@ login_send_chap_success(struct pdu *request, static void login_chap(struct ctld_connection *conn, struct auth_group *ag) { + std::string user; const struct auth *auth; struct chap *chap; struct pdu *request; @@ -488,20 +487,20 @@ login_chap(struct ctld_connection *conn, struct auth_group *ag) * Receive CHAP_N/CHAP_R PDU and authenticate. */ log_debugx("waiting for CHAP_N/CHAP_R"); - request = login_receive_chap_r(&conn->conn, ag, chap, &auth); + request = login_receive_chap_r(&conn->conn, ag, chap, &auth, user); /* * Yay, authentication succeeded! */ log_debugx("authentication succeeded for user \"%s\"; " - "transitioning to operational parameter negotiation", auth->a_user); - login_send_chap_success(request, auth); + "transitioning to operational parameter negotiation", user.c_str()); + login_send_chap_success(request, auth, user); pdu_delete(request); /* * Leave username and CHAP information for discovery(). */ - conn->conn_user = auth->a_user; + conn->conn_user = checked_strdup(user.c_str()); conn->conn_chap = chap; }