The branch stable/14 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=e87f835bf13bcc514a54f224b8b4d5f85befb213
commit e87f835bf13bcc514a54f224b8b4d5f85befb213 Author: John Baldwin <[email protected]> AuthorDate: 2025-04-11 14:00:14 +0000 Commit: John Baldwin <[email protected]> CommitDate: 2026-01-27 18:15:57 +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 (cherry picked from commit 2736dc8c28a33ba911fd59f87b587a3d9722e975) --- 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 1c2d9779e697..8172e36e97f9 100644 --- a/usr.sbin/ctld/ctld.cc +++ b/usr.sbin/ctld/ctld.cc @@ -190,24 +190,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); } } @@ -220,13 +210,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); } @@ -248,14 +233,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); } @@ -454,24 +433,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); @@ -481,6 +453,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) { @@ -497,6 +494,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); } @@ -1541,11 +1539,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);
