eap-radius: Change trigger for Accounting Start messages for IKEv1

Some clients won't do Mode Config or XAuth during reauthentication.
Because Start messages previously were triggered by TRANSACTION exchanges
none were sent for new SAs of such clients, while Stop messages were still
sent for the old SAs when they were destroyed.  This resulted in an
incorrect state on the RADIUS server.

Since 31be582399 the assign_vips() event is also triggered during
reauthentication if the client does not do a Mode Config exchange.
So instead of waiting for a TRANSACTION exchange we trigger the Start
message when a virtual IP is assigned to a client.

With this the charon.plugins.eap-radius.accounting_requires_vip option
would not have any effect for IKEv1 anymore.  However, it previously also
only worked if the client did an XAuth exchange, which is probably
rarely used without virtual IPs, so this might not be much of a
regression.

Fixes #937.

diff --git a/conf/plugins/eap-radius.opt b/conf/plugins/eap-radius.opt
index 2a6786dd97289ea2fc606d87f8b38cb912b0507a..7d511155bf637047129bb630fc18b05ecfa5dce3 100644 (file)
--- a/conf/plugins/eap-radius.opt
+++ b/conf/plugins/eap-radius.opt
@@ -11,7 +11,7 @@ charon.plugins.eap-radius.accounting_interval = 0
 
 charon.plugins.eap-radius.accounting_requires_vip = no
        If enabled, accounting is disabled unless an IKE_SA has at least one
-       virtual IP.
+       virtual IP. Only for IKEv2, for IKEv1 a virtual IP is strictly necessary.
 
 charon.plugins.eap-radius.class_group = no
        Use class attributes in Access-Accept messages as group membership

diff --git a/src/libcharon/plugins/eap_radius/eap_radius_accounting.c b/src/libcharon/plugins/eap_radius/eap_radius_accounting.c
index cef19305c532e7db0ab38f460329998dba7c98f3..7a76fdd07311bcce1be343db3b88d6bc92eec146 100644 (file)
--- a/src/libcharon/plugins/eap_radius/eap_radius_accounting.c
+++ b/src/libcharon/plugins/eap_radius/eap_radius_accounting.c
@@ -860,11 +860,6 @@ METHOD(listener_t, message_hook, bool,
        if (plain && ike_sa->get_state(ike_sa) == IKE_ESTABLISHED &&
                !incoming && !message->get_request(message))
        {
-               if (ike_sa->get_version(ike_sa) == IKEV1 &&
-                       message->get_exchange_type(message) == TRANSACTION)
-               {
-                       send_start(this, ike_sa);
-               }
                if (ike_sa->get_version(ike_sa) == IKEV2 &&
                        message->get_exchange_type(message) == IKE_AUTH)
                {
@@ -874,6 +869,17 @@ METHOD(listener_t, message_hook, bool,
        return TRUE;
 }
 
+METHOD(listener_t, assign_vips, bool,
+       private_eap_radius_accounting_t *this, ike_sa_t *ike_sa, bool assign)
+{
+       /* start accounting as soon as the virtual IP is set */
+       if (assign && ike_sa->get_version(ike_sa) == IKEV1)
+       {
+               send_start(this, ike_sa);
+       }
+       return TRUE;
+}
+
 METHOD(listener_t, ike_rekey, bool,
        private_eap_radius_accounting_t *this, ike_sa_t *old, ike_sa_t *new)
 {
@@ -1003,6 +1009,7 @@ eap_radius_accounting_t *eap_radius_accounting_create()
                                .ike_updown = _ike_updown,
                                .ike_rekey = _ike_rekey,
                                .message = _message_hook,
+                               .assign_vips = _assign_vips,
                                .child_updown = _child_updown,
                                .child_rekey = _child_rekey,
                                .children_migrate = _children_migrate,