Make "secret" become a required configuration for both client and

radius module.  "secret" for client was required already, but it
hadn't cause an error if it's missing.  Original diff from IIJ.

3 files changed, 38 insertions, 31 deletions

diff --git a/usr.sbin/radiusd/parse.y b/usr.sbin/radiusd/parse.y
index 45611f78167..50ce7b7521c 100644
--- a/usr.sbin/radiusd/parse.y
+++ b/usr.sbin/radiusd/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.10 2019/03/29 07:07:48 yasuoka Exp $	*/
+/*	$OpenBSD: parse.y,v 1.11 2019/04/01 09:25:14 yasuoka Exp $	*/
 
 /*
  * Copyright (c) 2002, 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -170,6 +170,11 @@ optport		: { $$ = 0; }
 client		: CLIENT prefix optnl clientopts_b {
 			struct radiusd_client *client0;
 
+			if (client.secret[0] == '\0') {
+				yyerror("secret is required for client");
+				YYERROR;
+			}
+
 			client0 = calloc(1, sizeof(struct radiusd_client));
 			if (client0 == NULL)
 				goto outofmemory;
diff --git a/usr.sbin/radiusd/radiusd.conf.5 b/usr.sbin/radiusd/radiusd.conf.5
index d8993af5026..90eff773a48 100644
--- a/usr.sbin/radiusd/radiusd.conf.5
+++ b/usr.sbin/radiusd/radiusd.conf.5
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: radiusd.conf.5,v 1.8 2017/03/13 21:48:07 jca Exp $
+.\"	$OpenBSD: radiusd.conf.5,v 1.9 2019/04/01 09:25:14 yasuoka Exp $
 .\"
 .\" Copyright (c) 2014 Esdenera Networks GmbH
 .\" Copyright (c) 2014 Internet Initiative Japan Inc.
@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: March 13 2017 $
+.Dd $Mdocdate: April 1 2019 $
 .Dt RADIUSD.CONF 5
 .Os
 .Sh NAME
@@ -110,6 +110,7 @@ is omitted, 1812 is used.
 This configuration can be specified multiple times.
 .It Ic secret Ar secret
 Specify the shared secret with the servers.
+This configuration cannot be omitted.
 .It Ic max-tries Ar number
 Specify the maximum number of retransmissions for a server.
 .Xr radiusd 8
diff --git a/usr.sbin/radiusd/radiusd_radius.c b/usr.sbin/radiusd/radiusd_radius.c
index 474198bc63e..a35bcb6e727 100644
--- a/usr.sbin/radiusd/radiusd_radius.c
+++ b/usr.sbin/radiusd/radiusd_radius.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: radiusd_radius.c,v 1.13 2017/05/30 16:30:22 yasuoka Exp $	*/
+/*	$OpenBSD: radiusd_radius.c,v 1.14 2019/04/01 09:25:14 yasuoka Exp $	*/
 
 /*
  * Copyright (c) 2013 Internet Initiative Japan Inc.
@@ -240,6 +240,12 @@ module_radius_start(void *ctx)
 		return;
 	}
 
+	if (module->secret[0] == '\0') {
+		module_send_message(module->base, IMSG_NG,
+		    "`secret' configuration is required");
+		return;
+	}
+
 	for (i = 0; i < module->nserver; i++) {
 		module->server[i].module = module;
 		if (radius_server_start(&module->server[i]) != 0) {
@@ -251,8 +257,7 @@ module_radius_start(void *ctx)
 	}
 	module_send_message(module->base, IMSG_OK, NULL);
 
-	if (module->secret[0] != '\0')
-		module_notify_secret(module->base, module->secret);
+	module_notify_secret(module->base, module->secret);
 }
 
 static void
@@ -301,8 +306,7 @@ module_radius_access_request(void *ctx, u_int q_id, const u_char *pkt,
 	 * secret.
 	 */
 	attrlen = sizeof(attrbuf);
-	if (module->secret[0] != '\0' &&
-	    radius_get_raw_attr(req->q_pkt, RADIUS_TYPE_USER_PASSWORD,
+	if (radius_get_raw_attr(req->q_pkt, RADIUS_TYPE_USER_PASSWORD,
 		    attrbuf, &attrlen) == 0) {
 		attrbuf[attrlen] = '\0';
 		radius_del_attr_all(req->q_pkt, RADIUS_TYPE_USER_PASSWORD);
@@ -426,27 +430,25 @@ radius_server_on_event(int fd, short evmask, void *ctx)
 	}
 	radius_set_request_packet(radpkt, req->q_pkt);
 
-	if (server->module->secret[0] != '\0') {
-		if (radius_check_response_authenticator(radpkt,
+	if (radius_check_response_authenticator(radpkt,
+	    server->module->secret) != 0) {
+		module_radius_log(server->module, LOG_WARNING,
+		    "server=%s Received radius message(id=%d) has bad "
+		    "authenticator",
+		    addrport_tostring(peer, peer->sa_len, buf,
+		    sizeof(buf)), res_id);
+		goto out;
+	}
+	if (radius_has_attr(radpkt,
+	    RADIUS_TYPE_MESSAGE_AUTHENTICATOR) &&
+	    radius_check_message_authenticator(radpkt,
 		    server->module->secret) != 0) {
-			module_radius_log(server->module, LOG_WARNING,
-			    "server=%s Received radius message(id=%d) has bad "
-			    "authenticator",
-			    addrport_tostring(peer, peer->sa_len, buf,
-			    sizeof(buf)), res_id);
-			goto out;
-		}
-		if (radius_has_attr(radpkt,
-		    RADIUS_TYPE_MESSAGE_AUTHENTICATOR) &&
-		    radius_check_message_authenticator(radpkt,
-			    server->module->secret) != 0) {
-			module_radius_log(server->module, LOG_WARNING,
-			    "server=%s Received radius message(id=%d) has bad "
-			    "message authenticator",
-			    addrport_tostring(peer, peer->sa_len, buf,
-			    sizeof(buf)), res_id);
-			goto out;
-		}
+		module_radius_log(server->module, LOG_WARNING,
+		    "server=%s Received radius message(id=%d) has bad "
+		    "message authenticator",
+		    addrport_tostring(peer, peer->sa_len, buf,
+		    sizeof(buf)), res_id);
+		goto out;
 	}
 
 	module_radius_log(server->module, LOG_INFO,
@@ -605,9 +607,8 @@ module_radius_req_reset_msgauth(struct module_radius_req *req)
 	if (radius_has_attr(req->q_pkt, RADIUS_TYPE_MESSAGE_AUTHENTICATOR))
 		radius_del_attr_all(req->q_pkt,
 		    RADIUS_TYPE_MESSAGE_AUTHENTICATOR);
-	if (req->module->secret[0] != '\0')
-		radius_put_message_authenticator(req->q_pkt,
-		    req->module->secret);
+	radius_put_message_authenticator(req->q_pkt,
+	    req->module->secret);
 }
 
 static void