git: 2736dc8c28a3 - main - ctld: Add a label string to auth_groups
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 11 Apr 2025 14:03:56 UTC
The branch main has been updated by jhb:
URL: https://cgit.FreeBSD.org/src/commit/?id=2736dc8c28a33ba911fd59f87b587a3d9722e975
commit 2736dc8c28a33ba911fd59f87b587a3d9722e975
Author: John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2025-04-11 14:00:14 +0000
Commit: John Baldwin <jhb@FreeBSD.org>
CommitDate: 2025-04-11 14:00:14 +0000
ctld: Add a label string to auth_groups
This holds the abstract name of an auth-group for use in warning
messages. For anonymous groups associated with a target, the label
includes the target name.
Abstracting this out removes a lot of code duplication of
nearly-identical warning messages.
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D49643
---
usr.sbin/ctld/conf.cc | 33 +++++---------------
usr.sbin/ctld/ctld.cc | 85 +++++++++++++++++++++++----------------------------
usr.sbin/ctld/ctld.h | 4 ++-
3 files changed, 50 insertions(+), 72 deletions(-)
diff --git a/usr.sbin/ctld/conf.cc b/usr.sbin/ctld/conf.cc
index ac82d06ad8fa..e86b44ee5004 100644
--- a/usr.sbin/ctld/conf.cc
+++ b/usr.sbin/ctld/conf.cc
@@ -126,25 +126,13 @@ _auth_group_set_type(struct auth_group *ag, const char *str)
} else if (strcmp(str, "chap-mutual") == 0) {
type = AG_TYPE_CHAP_MUTUAL;
} else {
- if (ag->ag_name != NULL)
- log_warnx("invalid auth-type \"%s\" for auth-group "
- "\"%s\"", str, ag->ag_name);
- else
- log_warnx("invalid auth-type \"%s\" for target "
- "\"%s\"", str, ag->ag_target->t_name);
+ log_warnx("invalid auth-type \"%s\" for %s", str, ag->ag_label);
return (false);
}
if (ag->ag_type != AG_TYPE_UNKNOWN && ag->ag_type != type) {
- if (ag->ag_name != NULL) {
- log_warnx("cannot set auth-type to \"%s\" for "
- "auth-group \"%s\"; already has a different "
- "type", str, ag->ag_name);
- } else {
- log_warnx("cannot set auth-type to \"%s\" for target "
- "\"%s\"; already has a different type",
- str, ag->ag_target->t_name);
- }
+ log_warnx("cannot set auth-type to \"%s\" for %s; "
+ "already has a different type", str, ag->ag_label);
return (false);
}
@@ -531,10 +519,9 @@ target_add_chap(const char *user, const char *secret)
return (false);
}
} else {
- target->t_auth_group = auth_group_new(conf, NULL);
+ target->t_auth_group = auth_group_new(conf, target);
if (target->t_auth_group == NULL)
return (false);
- target->t_auth_group->ag_target = target;
}
return (auth_new_chap(target->t_auth_group, user, secret));
}
@@ -550,10 +537,9 @@ target_add_chap_mutual(const char *user, const char *secret,
return (false);
}
} else {
- target->t_auth_group = auth_group_new(conf, NULL);
+ target->t_auth_group = auth_group_new(conf, target);
if (target->t_auth_group == NULL)
return (false);
- target->t_auth_group->ag_target = target;
}
return (auth_new_chap_mutual(target->t_auth_group, user, secret, user2,
secret2));
@@ -569,10 +555,9 @@ target_add_initiator_name(const char *name)
return (false);
}
} else {
- target->t_auth_group = auth_group_new(conf, NULL);
+ target->t_auth_group = auth_group_new(conf, target);
if (target->t_auth_group == NULL)
return (false);
- target->t_auth_group->ag_target = target;
}
return (auth_name_new(target->t_auth_group, name));
}
@@ -588,10 +573,9 @@ target_add_initiator_portal(const char *addr)
return (false);
}
} else {
- target->t_auth_group = auth_group_new(conf, NULL);
+ target->t_auth_group = auth_group_new(conf, target);
if (target->t_auth_group == NULL)
return (false);
- target->t_auth_group->ag_target = target;
}
return (auth_portal_new(target->t_auth_group, addr));
}
@@ -701,10 +685,9 @@ target_set_auth_type(const char *type)
return (false);
}
} else {
- target->t_auth_group = auth_group_new(conf, NULL);
+ target->t_auth_group = auth_group_new(conf, target);
if (target->t_auth_group == NULL)
return (false);
- target->t_auth_group->ag_target = target;
}
return (_auth_group_set_type(target->t_auth_group, type));
}
diff --git a/usr.sbin/ctld/ctld.cc b/usr.sbin/ctld/ctld.cc
index 6cb15283503a..6360a88e5c97 100644
--- a/usr.sbin/ctld/ctld.cc
+++ b/usr.sbin/ctld/ctld.cc
@@ -189,24 +189,14 @@ auth_check_secret_length(const struct auth_group *ag, const char *user,
len = strlen(secret);
if (len > 16) {
- if (ag->ag_name != NULL)
- log_warnx("%s for user \"%s\", auth-group \"%s\", "
- "is too long; it should be at most 16 characters "
- "long", secret_type, user, ag->ag_name);
- else
- log_warnx("%s for user \"%s\", target \"%s\", "
- "is too long; it should be at most 16 characters "
- "long", secret_type, user, ag->ag_target->t_name);
+ log_warnx("%s for user \"%s\", %s, is too long; it should be "
+ "at most 16 characters long", secret_type, user,
+ ag->ag_label);
}
if (len < 12) {
- if (ag->ag_name != NULL)
- log_warnx("%s for user \"%s\", auth-group \"%s\", "
- "is too short; it should be at least 12 characters "
- "long", secret_type, user, ag->ag_name);
- else
- log_warnx("%s for user \"%s\", target \"%s\", "
- "is too short; it should be at least 12 characters "
- "long", secret_type, user, ag->ag_target->t_name);
+ log_warnx("%s for user \"%s\", %s, is too short; it should be "
+ "at least 12 characters long", secret_type, user,
+ ag->ag_label);
}
}
@@ -219,13 +209,8 @@ auth_new_chap(struct auth_group *ag, const char *user,
if (ag->ag_type == AG_TYPE_UNKNOWN)
ag->ag_type = AG_TYPE_CHAP;
if (ag->ag_type != AG_TYPE_CHAP) {
- if (ag->ag_name != NULL)
- log_warnx("cannot mix \"chap\" authentication with "
- "other types for auth-group \"%s\"", ag->ag_name);
- else
- log_warnx("cannot mix \"chap\" authentication with "
- "other types for target \"%s\"",
- ag->ag_target->t_name);
+ log_warnx("cannot mix \"chap\" authentication with "
+ "other types for %s", ag->ag_label);
return (false);
}
@@ -247,14 +232,8 @@ auth_new_chap_mutual(struct auth_group *ag, const char *user,
if (ag->ag_type == AG_TYPE_UNKNOWN)
ag->ag_type = AG_TYPE_CHAP_MUTUAL;
if (ag->ag_type != AG_TYPE_CHAP_MUTUAL) {
- if (ag->ag_name != NULL)
- log_warnx("cannot mix \"chap-mutual\" authentication "
- "with other types for auth-group \"%s\"",
- ag->ag_name);
- else
- log_warnx("cannot mix \"chap-mutual\" authentication "
- "with other types for target \"%s\"",
- ag->ag_target->t_name);
+ log_warnx("cannot mix \"chap-mutual\" authentication "
+ "with other types for %s", ag->ag_label);
return (false);
}
@@ -453,24 +432,17 @@ auth_portal_check(const struct auth_group *ag, const struct sockaddr_storage *sa
return (true);
}
-struct auth_group *
-auth_group_new(struct conf *conf, const char *name)
+static struct auth_group *
+auth_group_create(struct conf *conf, const char *name, char *label)
{
struct auth_group *ag;
- if (name != NULL) {
- ag = auth_group_find(conf, name);
- if (ag != NULL) {
- log_warnx("duplicated auth-group \"%s\"", name);
- return (NULL);
- }
- }
-
ag = reinterpret_cast<struct auth_group *>(calloc(1, sizeof(*ag)));
if (ag == NULL)
log_err(1, "calloc");
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);
@@ -480,6 +452,31 @@ auth_group_new(struct conf *conf, const char *name)
return (ag);
}
+struct auth_group *
+auth_group_new(struct conf *conf, const char *name)
+{
+ struct auth_group *ag;
+ char *label;
+
+ ag = auth_group_find(conf, name);
+ if (ag != NULL) {
+ log_warnx("duplicated auth-group \"%s\"", name);
+ return (NULL);
+ }
+
+ asprintf(&label, "auth-group \"%s\"", name);
+ return (auth_group_create(conf, name, label));
+}
+
+struct auth_group *
+auth_group_new(struct conf *conf, struct target *target)
+{
+ char *label;
+
+ asprintf(&label, "target \"%s\"", target->t_name);
+ return (auth_group_create(conf, NULL, label));
+}
+
void
auth_group_delete(struct auth_group *ag)
{
@@ -496,6 +493,7 @@ auth_group_delete(struct auth_group *ag)
TAILQ_FOREACH_SAFE(auth_portal, &ag->ag_portals, ap_next,
auth_portal_tmp)
auth_portal_delete(auth_portal);
+ free(ag->ag_label);
free(ag->ag_name);
free(ag);
}
@@ -1540,11 +1538,6 @@ conf_verify(struct conf *conf)
}
}
TAILQ_FOREACH(ag, &conf->conf_auth_groups, ag_next) {
- if (ag->ag_name == NULL)
- assert(ag->ag_target != NULL);
- else
- assert(ag->ag_target == NULL);
-
found = false;
TAILQ_FOREACH(targ, &conf->conf_targets, t_next) {
if (targ->t_auth_group == ag) {
diff --git a/usr.sbin/ctld/ctld.h b/usr.sbin/ctld/ctld.h
index c76708daafe5..2cc9139fed1d 100644
--- a/usr.sbin/ctld/ctld.h
+++ b/usr.sbin/ctld/ctld.h
@@ -82,7 +82,7 @@ struct auth_group {
TAILQ_ENTRY(auth_group) ag_next;
struct conf *ag_conf;
char *ag_name;
- struct target *ag_target;
+ char *ag_label;
int ag_type;
TAILQ_HEAD(, auth) ag_auths;
TAILQ_HEAD(, auth_name) ag_names;
@@ -257,6 +257,8 @@ void conf_start(struct conf *new_conf);
bool conf_verify(struct conf *conf);
struct auth_group *auth_group_new(struct conf *conf, const char *name);
+struct auth_group *auth_group_new(struct conf *conf,
+ struct target *target);
void auth_group_delete(struct auth_group *ag);
struct auth_group *auth_group_find(const struct conf *conf,
const char *name);