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

@@ -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));
 }

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) {

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);