Possible bug in configurable failover

[Brian Candler]

I think I've found a bug in configurable failover - or at least, it doesn't
correspond to any behaviour I can see documented, but I thought I'd raise it
here before making a ticket.

To replicate: start with stock freeradius 2.1.x, uncomment the "steve" entry
from the users file, and apply the following config changes:

--- etc/raddb/sites-available/default.orig	2011-03-13 19:48:20.584961000 +0000
+++ etc/raddb/sites-available/default	2011-03-13 19:47:09.244961001 +0000
@@ -237,7 +237,11 @@
 	#  in the 'authorize' section supplies a password.  The
 	#  password can be clear-text, or encrypted.
 	Auth-Type PAP {
-		pap
+		pap {
+                        ok = return
+                        reject = 1
+		}
+		testing_module
 	}
 
 	#
--- etc/raddb/policy.conf.orig	2011-03-13 19:48:26.154961000 +0000
+++ etc/raddb/policy.conf	2011-03-13 19:49:45.434961000 +0000
@@ -194,4 +194,18 @@
 			noop
 		}
 	}
+
+        testing_module {
+                if (1) {
+                        update reply {
+                                Reply-Message += "Foo"
+                        }
+                }
+                if (1) {
+                        update reply {
+                                Reply-Message += "Bar"
+                        }
+                        ok
+                }
+        }
 }


The idea is using a module to turn an access reject into an access reject
under certain circumstances.  It just happens to have two separate 'if'
statements (and this is what triggers the bug)

Everything is fine if you give the correct password for steve. If you give
the wrong password, something strange happens:

$ bin/radtest steve badpass localhost 1 testing123
Sending Access-Request of id 221 to 127.0.0.1 port 1812
	User-Name = "steve"
	User-Password = "badpass"
	NAS-IP-Address = 127.0.0.1
	NAS-Port = 1
rad_recv: Access-Reject packet from host 127.0.0.1 port 1812, id=221, length=25
	Reply-Message = "Foo"

You can see that only the first 'if' statement has been executed, and then
it has dropped out of the module entirely, keeping the reject status.

As a workaround, you can insert a 'noop':

--- etc/raddb/policy.conf.orig	2011-03-13 19:48:26.154961000 +0000
+++ etc/raddb/policy.conf	2011-03-13 19:57:42.294961000 +0000
@@ -194,4 +194,19 @@
 			noop
 		}
 	}
+
+        testing_module {
+                if (1) {
+                        update reply {
+                                Reply-Message += "Foo"
+                        }
+			noop
+                }
+                if (1) {
+                        update reply {
+                                Reply-Message += "Bar"
+                        }
+                        ok
+                }
+        }
 }

And then it behaves how I would have expected in the first place, without
the noop:

$ bin/radtest steve badpass localhost 1 testing123
Sending Access-Request of id 159 to 127.0.0.1 port 1812
	User-Name = "steve"
	User-Password = "badpass"
	NAS-IP-Address = 127.0.0.1
	NAS-Port = 1
rad_recv: Access-Accept packet from host 127.0.0.1 port 1812, id=159, length=81
	Service-Type = Framed-User
	Framed-Protocol = PPP
	Framed-IP-Address = 172.16.3.33
	Framed-IP-Netmask = 255.255.255.0
	Framed-Routing = Broadcast-Listen
	Filter-Id = "std.ppp"
	Framed-MTU = 1500
	Framed-Compression = Van-Jacobson-TCP-IP
	Reply-Message = "Foo"
	Reply-Message = "Bar"

Any thoughts as to what is going on here?

Thanks,

Brian.