diff mbox

Use a random initial value for next_radius_identifier so that the identifier is less likely to be reused when multiple hostapd instances are running that will appear to a RADIUS server as being from the same NAS.

Message ID CAGnO3drbURFWJEhm2gyKK90gZq8TogsVpm6RQnkb16vXYDLo+Q@mail.gmail.com
State Changes Requested
Headers show

Commit Message

Nick Lowe July 26, 2016, 2:18 p.m. UTC
[PATCH] Use a random initial value for next_radius_identifier so that
 the identifier is less likely to be reused when multiple hostapd instances
 are running that will appear to a RADIUS server as being from the same NAS.

Signed-off-by: Nick Lowe <nick.lowe@lugatech.com>
---
 src/radius/radius_client.c | 4 ++++
 1 file changed, 4 insertions(+)

  if (conf->auth_server && radius_client_init_auth(radius)) {
  radius_client_deinit(radius);

Comments

Nick Lowe July 27, 2016, 12:36 p.m. UTC | #1
Note: This is a largely cosmetic change as the UDP port will differ
and the Linux kernel will, these days, randomise the UDP port.

It potentially avoids a conceptual race in older versions of the Linux
kernel that are still in use:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=32c1da70810017a98aa6c431a5494a302b6b9a30

Nick
Jouni Malinen Sept. 18, 2016, 6:29 p.m. UTC | #2
On Wed, Jul 27, 2016 at 01:36:31PM +0100, Nick Lowe wrote:
> Note: This is a largely cosmetic change as the UDP port will differ
> and the Linux kernel will, these days, randomise the UDP port.
> 
> It potentially avoids a conceptual race in older versions of the Linux
> kernel that are still in use:
> 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=32c1da70810017a98aa6c431a5494a302b6b9a30

I'm not sure I really understand the point of this change.. What is the
case where this could help in making it less likely for RADIUS messages
to look as if being from the same NAS? If there are multiple hostapd
instances running, wouldn't each get their own UDP source port? The
RADIUS identifier is of importance when there are multiple parallel
requests from the same IP address and UDP port, but that shouldn't
really be the case for the multiple instances case mentioned in the
commit message. This is regardless of whether the kernel selects a
random source port for the UDP socket.
Alan DeKok Sept. 19, 2016, 12:20 a.m. UTC | #3
On Sep 18, 2016, at 2:29 PM, Jouni Malinen <j@w1.fi> wrote:
> I'm not sure I really understand the point of this change.. What is the
> case where this could help in making it less likely for RADIUS messages
> to look as if being from the same NAS? If there are multiple hostapd
> instances running, wouldn't each get their own UDP source port?

  Yes.

> The
> RADIUS identifier is of importance when there are multiple parallel
> requests from the same IP address and UDP port, but that shouldn't
> really be the case for the multiple instances case mentioned in the
> commit message. This is regardless of whether the kernel selects a
> random source port for the UDP socket.

  Yes.

  Alan DeKok.
diff mbox

Patch

diff --git a/src/radius/radius_client.c b/src/radius/radius_client.c
index a4edd5f..bfe42e1 100644
--- a/src/radius/radius_client.c
+++ b/src/radius/radius_client.c
@@ -1446,6 +1446,10 @@  radius_client_init(void *ctx, struct
hostapd_radius_servers *conf)
  radius->auth_serv_sock = radius->acct_serv_sock =
  radius->auth_serv_sock6 = radius->acct_serv_sock6 =
  radius->auth_sock = radius->acct_sock = -1;
+ if (os_get_random((u8 *) &radius->next_radius_identifier,
sizeof(radius->next_radius_identifier)) < 0) {
+ radius_client_deinit(radius);
+ return NULL;
+ }