diff mbox

Move client socket to server structure.

Message ID 52668388.4020402@deployingradius.com
State Deferred
Headers show

Commit Message

Alan DeKok Oct. 22, 2013, 1:54 p.m. UTC 
This patch has no functional effect, but it makes the code a little
simpler.  It also prepares the way for doing RADIUS over TCP, or even
TLS if so desired.

  There is a new function radius_client_sock().  It takes care of
opening a socket from the client to the server.

  radius_change_server() has been modified to call this function.  It
also now adds the new socket to the event loop, and removes any old
socket from the event loop.

  The references to auth_serv_sock, acct_serv_sock, auth_serv_sock6, and
acct_serv_sock6 have been removed.

  It used to create these sockets pro-actively, and then not bind them
to anything until radius_change_server() was called.  It now creates
sockets only when necessary.

  The code has been tested with eapol_test, and it works.

  I should have patches shortly to do RADIUS over TCP.  Now that the
socket code is abstracted in one function, that should be easy:

1) update the server structure to add "protocol", with a default of UDP.

2) update radius_client_socket() to create SOCK_STREAM if
protocol==IPPROTO_TCP

3) update radius_client_data to have a dynamically allocated buffer and
size, to allow partial reads from a TCP socket.

3) update radius_client_receive() to do partial reads on TCP sockets.


  The main reason for the current patch is to replace multiple instances
of socket(.. SOCK_DGRAM ... ) with only one in radius_client_socket().
That makes it much easier to add TCP functionality, as there's now only
one place to change the code.

  Alan DeKok.
From 13cb3eda7842ff04c5d4dc5a3b73459688ca73cb Mon Sep 17 00:00:00 2001
From: "Alan T. DeKok" <aland@freeradius.org>
Date: Tue, 22 Oct 2013 09:38:58 -0400
Subject: [PATCH] Move client socket to server structure.

Each server now has a "sock" entry.  This is the socket used to
communicate with that server.

When a client calls the "change server" function, that function
takes care of creating the socket, and registering the read events.
---
 src/radius/radius_client.c | 240 ++++++++++++++++-----------------------------
 src/radius/radius_client.h |   5 +
 2 files changed, 91 insertions(+), 154 deletions(-)
diff mbox

Patch

diff --git a/src/radius/radius_client.c b/src/radius/radius_client.c
index 425ad93..6df81b8 100644
--- a/src/radius/radius_client.c
+++ b/src/radius/radius_client.c
@@ -163,26 +163,6 @@  struct radius_client_data {
 	struct hostapd_radius_servers *conf;
 
 	/**
-	 * auth_serv_sock - IPv4 socket for RADIUS authentication messages
-	 */
-	int auth_serv_sock;
-
-	/**
-	 * acct_serv_sock - IPv4 socket for RADIUS accounting messages
-	 */
-	int acct_serv_sock;
-
-	/**
-	 * auth_serv_sock6 - IPv6 socket for RADIUS authentication messages
-	 */
-	int auth_serv_sock6;
-
-	/**
-	 * acct_serv_sock6 - IPv6 socket for RADIUS accounting messages
-	 */
-	int acct_serv_sock6;
-
-	/**
 	 * auth_sock - Currently used socket for RADIUS authentication server
 	 */
 	int auth_sock;
@@ -233,10 +213,10 @@  static int
 radius_change_server(struct radius_client_data *radius,
 		     struct hostapd_radius_server *nserv,
 		     struct hostapd_radius_server *oserv,
-		     int sock, int sock6, int auth);
+		     int auth);
 static int radius_client_init_acct(struct radius_client_data *radius);
 static int radius_client_init_auth(struct radius_client_data *radius);
-
+static int radius_client_disable_pmtu_discovery(int s);
 
 static void radius_client_msg_free(struct radius_msg_list *req)
 {
@@ -448,9 +428,7 @@  static void radius_client_timer(void *eloop_ctx, void *timeout_ctx)
 		if (next > &(conf->auth_servers[conf->num_auth_servers - 1]))
 			next = conf->auth_servers;
 		conf->auth_server = next;
-		radius_change_server(radius, next, old,
-				     radius->auth_serv_sock,
-				     radius->auth_serv_sock6, 1);
+		radius_change_server(radius, next, old, 1);
 	}
 
 	if (acct_failover && conf->num_acct_servers > 1) {
@@ -473,9 +451,7 @@  static void radius_client_timer(void *eloop_ctx, void *timeout_ctx)
 		if (next > &conf->acct_servers[conf->num_acct_servers - 1])
 			next = conf->acct_servers;
 		conf->acct_server = next;
-		radius_change_server(radius, next, old,
-				     radius->acct_serv_sock,
-				     radius->acct_serv_sock6, 0);
+		radius_change_server(radius, next, old, 0);
 	}
 }
 
@@ -929,12 +905,45 @@  static void radius_client_update_acct_msgs(struct radius_client_data *radius,
 	}
 }
 
+static int
+radius_client_sock(struct hostapd_radius_server *serv,
+		   struct sockaddr *serv_addr,
+		   socklen_t addrlen,
+		   struct sockaddr *cl_addr,
+		   socklen_t claddrlen)
+{
+	int sock;
+
+	sock = socket(PF_INET, SOCK_DGRAM, 0);
+	if (sock < 0) {
+		perror("socket[PF_INET,SOCK_DGRAM]");
+		return -1;
+	}
+
+	radius_client_disable_pmtu_discovery(sock);
+
+	if (cl_addr) {
+		if (bind(sock, cl_addr, claddrlen) < 0) {
+			close(sock);
+			perror("bind[radius]");
+			return -1;
+		}
+	}
+
+	if (connect(sock, serv_addr, addrlen) < 0) {
+		close(sock);
+		perror("connect[radius]");
+		return -1;
+	}
+
+	return sock;
+}
 
 static int
 radius_change_server(struct radius_client_data *radius,
 		     struct hostapd_radius_server *nserv,
 		     struct hostapd_radius_server *oserv,
-		     int sock, int sock6, int auth)
+		     int auth)
 {
 	struct sockaddr_in serv, claddr;
 #ifdef CONFIG_IPV6
@@ -943,7 +952,7 @@  radius_change_server(struct radius_client_data *radius,
 	struct sockaddr *addr, *cl_addr;
 	socklen_t addrlen, claddrlen;
 	char abuf[50];
-	int sel_sock;
+	int sock;
 	struct radius_msg_list *entry;
 	struct hostapd_radius_servers *conf = radius->conf;
 
@@ -954,6 +963,12 @@  radius_change_server(struct radius_client_data *radius,
 		       hostapd_ip_txt(&nserv->addr, abuf, sizeof(abuf)),
 		       nserv->port);
 
+	if (oserv) {
+		eloop_unregister_read_sock(oserv->sock);
+		close(oserv->sock);
+		oserv->sock = -1;
+	}
+
 	if (!oserv || nserv->shared_secret_len != oserv->shared_secret_len ||
 	    os_memcmp(nserv->shared_secret, oserv->shared_secret,
 		      nserv->shared_secret_len) != 0) {
@@ -997,7 +1012,6 @@  radius_change_server(struct radius_client_data *radius,
 		serv.sin_port = htons(nserv->port);
 		addr = (struct sockaddr *) &serv;
 		addrlen = sizeof(serv);
-		sel_sock = sock;
 		break;
 #ifdef CONFIG_IPV6
 	case AF_INET6:
@@ -1008,7 +1022,6 @@  radius_change_server(struct radius_client_data *radius,
 		serv6.sin6_port = htons(nserv->port);
 		addr = (struct sockaddr *) &serv6;
 		addrlen = sizeof(serv6);
-		sel_sock = sock6;
 		break;
 #endif /* CONFIG_IPV6 */
 	default:
@@ -1040,29 +1053,27 @@  radius_change_server(struct radius_client_data *radius,
 			return -1;
 		}
 
-		if (bind(sel_sock, cl_addr, claddrlen) < 0) {
-			perror("bind[radius]");
-			return -1;
-		}
+	} else {
+		cl_addr = NULL;
+		claddrlen = 0;
 	}
 
-	if (connect(sel_sock, addr, addrlen) < 0) {
-		perror("connect[radius]");
+	sock = radius_client_sock(nserv, addr, addrlen, cl_addr, claddrlen);
+	if (sock < 0)
 		return -1;
-	}
 
 #ifndef CONFIG_NATIVE_WINDOWS
 	switch (nserv->addr.af) {
 	case AF_INET:
 		claddrlen = sizeof(claddr);
-		getsockname(sel_sock, (struct sockaddr *) &claddr, &claddrlen);
+		getsockname(sock, (struct sockaddr *) &claddr, &claddrlen);
 		wpa_printf(MSG_DEBUG, "RADIUS local address: %s:%u",
 			   inet_ntoa(claddr.sin_addr), ntohs(claddr.sin_port));
 		break;
 #ifdef CONFIG_IPV6
 	case AF_INET6: {
 		claddrlen = sizeof(claddr6);
-		getsockname(sel_sock, (struct sockaddr *) &claddr6,
+		getsockname(sock, (struct sockaddr *) &claddr6,
 			    &claddrlen);
 		wpa_printf(MSG_DEBUG, "RADIUS local address: %s:%u",
 			   inet_ntop(AF_INET6, &claddr6.sin6_addr,
@@ -1074,10 +1085,31 @@  radius_change_server(struct radius_client_data *radius,
 	}
 #endif /* CONFIG_NATIVE_WINDOWS */
 
-	if (auth)
-		radius->auth_sock = sel_sock;
-	else
-		radius->acct_sock = sel_sock;
+	if (auth) {
+		if (eloop_register_read_sock(sock,
+					     radius_client_receive, radius,
+					     (void *) RADIUS_AUTH)) {
+			printf("Could not register read socket for authentication "
+				       "server\n");
+				close(sock);
+				nserv->sock= -1;
+				return -1;
+		}
+		
+		radius->auth_sock = sock;
+	} else {
+		if (eloop_register_read_sock(sock,
+					     radius_client_receive, radius,
+					     (void *) RADIUS_ACCT)) {
+			printf("Could not register read socket for accounting "
+			       "server\n");
+			close(sock);
+			nserv->sock = -1;
+			return -1;
+		}
+		
+		radius->acct_sock = sock;
+	}
 
 	return 0;
 }
@@ -1093,18 +1125,14 @@  static void radius_retry_primary_timer(void *eloop_ctx, void *timeout_ctx)
 	    conf->auth_server != conf->auth_servers) {
 		oserv = conf->auth_server;
 		conf->auth_server = conf->auth_servers;
-		radius_change_server(radius, conf->auth_server, oserv,
-				     radius->auth_serv_sock,
-				     radius->auth_serv_sock6, 1);
+		radius_change_server(radius, conf->auth_server, oserv, 1);
 	}
 
 	if (radius->acct_sock >= 0 && conf->acct_servers &&
 	    conf->acct_server != conf->acct_servers) {
 		oserv = conf->acct_server;
 		conf->acct_server = conf->acct_servers;
-		radius_change_server(radius, conf->acct_server, oserv,
-				     radius->acct_serv_sock,
-				     radius->acct_serv_sock6, 0);
+		radius_change_server(radius, conf->acct_server, oserv, 0);
 	}
 
 	if (conf->retry_primary_interval)
@@ -1133,104 +1161,16 @@  static int radius_client_disable_pmtu_discovery(int s)
 static int radius_client_init_auth(struct radius_client_data *radius)
 {
 	struct hostapd_radius_servers *conf = radius->conf;
-	int ok = 0;
-
-	radius->auth_serv_sock = socket(PF_INET, SOCK_DGRAM, 0);
-	if (radius->auth_serv_sock < 0)
-		perror("socket[PF_INET,SOCK_DGRAM]");
-	else {
-		radius_client_disable_pmtu_discovery(radius->auth_serv_sock);
-		ok++;
-	}
-
-#ifdef CONFIG_IPV6
-	radius->auth_serv_sock6 = socket(PF_INET6, SOCK_DGRAM, 0);
-	if (radius->auth_serv_sock6 < 0)
-		perror("socket[PF_INET6,SOCK_DGRAM]");
-	else
-		ok++;
-#endif /* CONFIG_IPV6 */
-
-	if (ok == 0)
-		return -1;
 
-	radius_change_server(radius, conf->auth_server, NULL,
-			     radius->auth_serv_sock, radius->auth_serv_sock6,
-			     1);
-
-	if (radius->auth_serv_sock >= 0 &&
-	    eloop_register_read_sock(radius->auth_serv_sock,
-				     radius_client_receive, radius,
-				     (void *) RADIUS_AUTH)) {
-		printf("Could not register read socket for authentication "
-		       "server\n");
-		return -1;
-	}
-
-#ifdef CONFIG_IPV6
-	if (radius->auth_serv_sock6 >= 0 &&
-	    eloop_register_read_sock(radius->auth_serv_sock6,
-				     radius_client_receive, radius,
-				     (void *) RADIUS_AUTH)) {
-		printf("Could not register read socket for authentication "
-		       "server\n");
-		return -1;
-	}
-#endif /* CONFIG_IPV6 */
-
-	return 0;
+	return radius_change_server(radius, conf->auth_server, NULL, 1);
 }
 
 
 static int radius_client_init_acct(struct radius_client_data *radius)
 {
 	struct hostapd_radius_servers *conf = radius->conf;
-	int ok = 0;
 
-	radius->acct_serv_sock = socket(PF_INET, SOCK_DGRAM, 0);
-	if (radius->acct_serv_sock < 0)
-		perror("socket[PF_INET,SOCK_DGRAM]");
-	else {
-		radius_client_disable_pmtu_discovery(radius->acct_serv_sock);
-		ok++;
-	}
-
-#ifdef CONFIG_IPV6
-	radius->acct_serv_sock6 = socket(PF_INET6, SOCK_DGRAM, 0);
-	if (radius->acct_serv_sock6 < 0)
-		perror("socket[PF_INET6,SOCK_DGRAM]");
-	else
-		ok++;
-#endif /* CONFIG_IPV6 */
-
-	if (ok == 0)
-		return -1;
-
-	radius_change_server(radius, conf->acct_server, NULL,
-			     radius->acct_serv_sock, radius->acct_serv_sock6,
-			     0);
-
-	if (radius->acct_serv_sock >= 0 &&
-	    eloop_register_read_sock(radius->acct_serv_sock,
-				     radius_client_receive, radius,
-				     (void *) RADIUS_ACCT)) {
-		printf("Could not register read socket for accounting "
-		       "server\n");
-		return -1;
-	}
-
-#ifdef CONFIG_IPV6
-	if (radius->acct_serv_sock6 >= 0 &&
-	    eloop_register_read_sock(radius->acct_serv_sock6,
-				     radius_client_receive, radius,
-				     (void *) RADIUS_ACCT)) {
-		printf("Could not register read socket for accounting "
-		       "server\n");
-		return -1;
-	}
-#endif /* CONFIG_IPV6 */
-
-	return 0;
+	return radius_change_server(radius, conf->acct_server, NULL, 0);
 }
 
 
@@ -1255,9 +1195,7 @@  radius_client_init(void *ctx, struct hostapd_radius_servers *conf)
 
 	radius->ctx = ctx;
 	radius->conf = conf;
-	radius->auth_serv_sock = radius->acct_serv_sock =
-		radius->auth_serv_sock6 = radius->acct_serv_sock6 =
-		radius->auth_sock = radius->acct_sock = -1;
+	radius->auth_sock = radius->acct_sock = -1;
 
 	if (conf->auth_server && radius_client_init_auth(radius)) {
 		radius_client_deinit(radius);
@@ -1287,16 +1225,10 @@  void radius_client_deinit(struct radius_client_data *radius)
 	if (!radius)
 		return;
 
-	if (radius->auth_serv_sock >= 0)
-		eloop_unregister_read_sock(radius->auth_serv_sock);
-	if (radius->acct_serv_sock >= 0)
-		eloop_unregister_read_sock(radius->acct_serv_sock);
-#ifdef CONFIG_IPV6
-	if (radius->auth_serv_sock6 >= 0)
-		eloop_unregister_read_sock(radius->auth_serv_sock6);
-	if (radius->acct_serv_sock6 >= 0)
-		eloop_unregister_read_sock(radius->acct_serv_sock6);
-#endif /* CONFIG_IPV6 */
+	if (radius->auth_sock >= 0)
+		eloop_unregister_read_sock(radius->auth_sock);
+	if (radius->acct_sock >= 0)
+		eloop_unregister_read_sock(radius->acct_sock);
 
 	eloop_cancel_timeout(radius_retry_primary_timer, radius, NULL);
 
diff --git a/src/radius/radius_client.h b/src/radius/radius_client.h
index 3db16aa..81ae463 100644
--- a/src/radius/radius_client.h
+++ b/src/radius/radius_client.h
@@ -36,6 +36,11 @@  struct hostapd_radius_server {
 	int port;
 
 	/**
+	 * sock - socket used by clients for communication with the server
+	 */
+	int sock;
+
+	/**
 	 * shared_secret - Shared secret for authenticating RADIUS messages
 	 */
 	u8 *shared_secret;