don't add Event-Timestamp in code

it should be a policy.  And ignore Acct-Delay-Time if its value
is unreasonable

diff --git a/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc b/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc
index a79c5ce4d76dc325ce9dfaff48e6056b02bb8d69..1ef9c50af3bc0adb5a92f0fe6c5a0e32eec2e438 100644 (file)
--- a/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc
+++ b/doc/antora/modules/reference/pages/raddb/sites-available/default.adoc
@@ -1542,11 +1542,16 @@ The server is supposed to conclude that the start time was
 "Acct-Delay-Time" seconds in the past.
 
 If there's no Event-Timestamp, then we create one, using
-Acct-Delay-Time as an offset if it exists.
+Acct-Delay-Time as an offset if it exists.  BUT we only do
+this if Acct-Delay-Time exists, and has a reasonable value.
 
 ```
        if (!Event-Timestamp) {
-               Event-Timestamp := %time.request() - %{Acct-Delay-Time || 0}
+               Event-Timestamp := %time.request()
+
+               if (Acct-Delay-Time && (Acct-Delay-Time < 86400 * 7)) {
+                       Event-Timestamp -= Acct-Delay-Time
+               }
        }
 
 ```

diff --git a/raddb/sites-available/default b/raddb/sites-available/default
index 800d29d7ed4585ae82d98e611e65f284898faf0a..d58c3b944d3ea783c38ddbed846500d4d38de88d 100644 (file)
--- a/raddb/sites-available/default
+++ b/raddb/sites-available/default
@@ -1357,10 +1357,15 @@ recv Accounting-Request {
        #  "Acct-Delay-Time" seconds in the past.
        #
        #  If there's no Event-Timestamp, then we create one, using
-       #  Acct-Delay-Time as an offset if it exists.
+       #  Acct-Delay-Time as an offset if it exists.  BUT we only do
+       #  this if Acct-Delay-Time exists, and has a reasonable value.
        #
        if (!Event-Timestamp) {
-               Event-Timestamp := %time.request() - %{Acct-Delay-Time || 0}
+               Event-Timestamp := %time.request()
+
+               if (Acct-Delay-Time && (Acct-Delay-Time < 86400 * 7)) {
+                       Event-Timestamp -= Acct-Delay-Time
+               }
        }
 
        #

diff --git a/src/process/radius/base.c b/src/process/radius/base.c
index 8b4ab2240e3fd7eaf299457993016655747e70f7..b3978fb7843621eb9bb286dc1315cc227c8044d5 100644 (file)
--- a/src/process/radius/base.c
+++ b/src/process/radius/base.c
@@ -66,7 +66,6 @@ static fr_dict_attr_t const *attr_user_name;
 static fr_dict_attr_t const *attr_user_password;
 static fr_dict_attr_t const *attr_original_packet_code;
 static fr_dict_attr_t const *attr_error_cause;
-static fr_dict_attr_t const *attr_event_timestamp;
 
 extern fr_dict_attr_autoload_t process_radius_dict_attr[];
 fr_dict_attr_autoload_t process_radius_dict_attr[] = {
@@ -85,8 +84,6 @@ fr_dict_attr_autoload_t process_radius_dict_attr[] = {
        { .out = &attr_original_packet_code, .name = "Extended-Attribute-1.Original-Packet-Code", .type = FR_TYPE_UINT32, .dict = &dict_radius },
        { .out = &attr_error_cause, .name = "Error-Cause", .type = FR_TYPE_UINT32, .dict = &dict_radius },
 
-       { .out = &attr_event_timestamp, .name = "Event-Timestamp", .type = FR_TYPE_DATE, .dict = &dict_radius },
-
        DICT_AUTOLOAD_TERMINATOR
 };
 
@@ -588,37 +585,8 @@ RESUME(access_challenge)
  */
 RECV(accounting_request)
 {
-       fr_pair_t                       *acct_delay, *event_timestamp;
-
        radius_request_pairs_store(request, mctx->rctx);
 
-       /*
-        *      Acct-Delay-Time is horrific.  Its existence in a packet means that any retransmissions can't
-        *      be retransmissions!  Instead, we have to send a brand new packet each time.  This rewriting is
-        *      expensive, causes ID churn, over-allocation of IDs, and makes it more difficult to discover
-        *      end-to-end failures.
-        *
-        *      As a result, we delete Acct-Delay-Time, and replace it with Event-Timestamp.
-        */
-       event_timestamp = fr_pair_find_by_da(&request->request_pairs, NULL, attr_event_timestamp);
-       if (!event_timestamp) {
-               MEM(event_timestamp = fr_pair_afrom_da(request->request_ctx, attr_event_timestamp));
-               fr_pair_append(&request->request_pairs, event_timestamp);
-               event_timestamp->vp_date = fr_time_to_unix_time(request->packet->timestamp);
-
-               acct_delay = fr_pair_find_by_da(&request->request_pairs, NULL, attr_event_timestamp);
-               if (acct_delay) {
-                       if (acct_delay->vp_uint32 < ((365 * 86400))) {
-                               event_timestamp->vp_date = fr_unix_time_sub_time_delta(event_timestamp->vp_date, fr_time_delta_from_sec(acct_delay->vp_uint32));
-
-                               RDEBUG("Accounting-Request packet contains %pP.  Creating %pP",
-                                      acct_delay, event_timestamp);
-                       }
-               } else {
-                       RDEBUG("Accounting-Request packet is missing Event-Timestamp.  Adding it to packet as %pP.", event_timestamp);
-               }
-       }
-
        return CALL_RECV(generic);
 }