The branch main has been updated by jhb:
URL:
https://cgit.FreeBSD.org/src/commit/?id=7f912714c53644ea18fc58ffa364918ccfa22999
commit 7f912714c53644ea18fc58ffa364918ccfa22999
Author: John Baldwin <j...@freebsd.org>
AuthorDate: 2025-08-04 19:38:06 +0000
Commit: John Baldwin <j...@freebsd.org>
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<struct auth *>(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<struct auth_group *>(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 <libiscsiutil.h>
#include <libutil.h>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
#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<std::string, auth> 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;
}
