Feature #2451
Adding functionality for RADIUS relay of Class AVP to accounting server
Description
Added an optional feature to the eap-radius plugin:
"eap-radius.accounting_relay_class" (default no)
If enabled, then the first Class AVP in incoming RADIUS Access-Accept messages will be relayed to the accounting server as a part of consequential Accounting-Request start/interim/stop messages. (Ref: RFC 2865 section 5.25)
The idea/usage of this functionality is that a RADIUS server granting access to strongSwan can generate and send along a unique Class AVP that will be relayed by the RADIUS client (e.g. strongSwan) to the accaunting server, hence creating an easy way on the RADIUS server side to uniquely correlate authorisation sessions with accounting sessions.
Note: Requires the eap-radius.accounting option enabled.
Associated revisions
History
#1 Updated by Tobias Brunner over 3 years ago
- Status changed from New to Feedback
Thanks for the suggestion and patch.
I suppose we could use the existing eap-radius.class_group functionality for this. But I guess technically this use of the attribute is rather the opposite (i.e. not a group identifier but a unique client identifier). So that might be confusing.
But I'd still prefer to use parts of that functionality for this instead of duplicating the parsing and adding new members to the ike_sa_t
class. I did that in the 2451-radius-accounting-class branch. The conversion to identification_t
might not be ideal in case the attribute does not contain a string. So that might have to be changed to chunk_t*
(requiring some work in auth_cfg_t
as there are currently no rules that store chunk_t
pointers) if it turns out to be a problem. Since all received attributes are stored we could also send all of them, but I guess there usually won't be more than one?
Not sure if making this configurable is required (or is there a downside to forwarding the attribute, besides the increased message size?).
#2 Updated by Thomas Strangert over 3 years ago
Thank you for your fast response and feedback :)
I've had a look at the code in the new branch you created and compared it with what I submitted. With that, plus your comments above, I have the following comments:
- My code / patch aimed to have minimal impact on existing code and to be functionally separated/backwards compatible and/or simply because I figured that you would prefer to have "atomic" additions. For instance, I avoided to interleave my stuff into the existing
process_class()
, instead creating a similarprocess_class_relay()
function that kicked in only when my proposed option switch eap-radius.accounting_relay_class was enabled.
- I wasn't fully aware of the
auth_cfg_t
functionality (in particular the usage ofget_auth_cfg()
andadd()
methods) and what implications it would have if I started toadd()
my stuff with it. This was why I chose to create a variable and aset()/get()
to theike_sa_t
class instead. As long asauth_cfg_t
keeps its data/has the same lifespan asike_sa_t
then I have no opinion on where to store theClass
AVPs.
- Regarding the lifespan/how to keep and update the
Class
AVPs: It is important that theClass
AVPs sent out to the accounting server are the same as the ones received in the most recentAccess-Accept
message (and also in the same order!). Theoretically, anAccess-Accept
message could be a follow-up re-authentication with updatedClass
data or even withoutClass
AVPs, in either case consequentAccounting-Request
messages shouldn't continue relaying initial/oldClass
data.
- I would strongly advice to have my proposed separate option switch
eap-radius.accounting_relay_class
(default no) enabling the relay function and not to bundle it with the existingeap-radius.class_group
. There might be accounting servers in service out there that have support for incomingClass
relay, but presently doesn't get that stimuli from strongSwan clients and there's no telling how those would react if they suddenly do! Also, I am not aware of how theclass_group
functionality as it exists in strongSwan will react if the RADIUS authorisation server starts to pump seemingly random, binary data inClass
AVPs, that are really meant only to be interpreted in an accounting server?
- Hence: for the intents and purposes of RFC 2865 section 5.25, the
Class
AVP data contains 1-253 arbitrary binary octets, so no C-code string handling implied or guaranteed.chunk_t/memcpy()
handling a must!
#3 Updated by Tobias Brunner over 3 years ago
- My code / patch aimed to have minimal impact on existing code and to be functionally separated/backwards compatible and/or simply because I figured that you would prefer to have "atomic" additions. For instance, I avoided to interleave my stuff into the existing
process_class()
, instead creating a similarprocess_class_relay()
function that kicked in only when my proposed option switch eap-radius.accounting_relay_class was enabled.
It always depends :) But the existing code in process_class()
already does exactly the same thing you wanted (store the received attribute(s) somewhere).
- I wasn't fully aware of the
auth_cfg_t
functionality (in particular the usage ofget_auth_cfg()
andadd()
methods) and what implications it would have if I started toadd()
my stuff with it. This was why I chose to create a variable and aset()/get()
to theike_sa_t
class instead. As long asauth_cfg_t
keeps its data/has the same lifespan asike_sa_t
then I have no opinion on where to store theClass
AVPs.
I try to avoid adding new members to major classes, in particular for features only a minority of users will use. And, yes, the lifespan of the auth data is the same as that of the IKE_SAs (unless the option charon.flush_auth_cfg
is enabled). Another option would be to store them in the eap-radius plugin (in a hashtable mapping IKE_SAs to Class
attributes).
- Regarding the lifespan/how to keep and update the
Class
AVPs: It is important that theClass
AVPs sent out to the accounting server are the same as the ones received in the most recentAccess-Accept
message (and also in the same order!). Theoretically, anAccess-Accept
message could be a follow-up re-authentication with updatedClass
data or even withoutClass
AVPs, in either case consequentAccounting-Request
messages shouldn't continue relaying initial/oldClass
data.
I don't think that's an issue here. There is no EAP/RADIUS reauthentication with IKEv2 (a new IKE_SA has to be created to reauthenticate). So there won't be any update of these attributes. The only thing that could happen is using multiple authentication rounds, but using EAP for more than one is very uncommon (if at all possible). The order of rules in auth_cfg_t
is maintained when enumerating them (however, only the first attribute is currently used).
- I would strongly advice to have my proposed separate option switch
eap-radius.accounting_relay_class
(default no) enabling the relay function and not to bundle it with the existingeap-radius.class_group
. There might be accounting servers in service out there that have support for incomingClass
relay, but presently doesn't get that stimuli from strongSwan clients and there's no telling how those would react if they suddenly do! Also, I am not aware of how theclass_group
functionality as it exists in strongSwan will react if the RADIUS authorisation server starts to pump seemingly random, binary data inClass
AVPs, that are really meant only to be interpreted in an accounting server?
It's not bundled with that other option at all. But sure, we can add a separate option to enable this.
- Hence: for the intents and purposes of RFC 2865 section 5.25, the
Class
AVP data contains 1-253 arbitrary binary octets, so no C-code string handling implied or guaranteed.chunk_t/memcpy()
handling a must!
Well, that RFC is 17 years old. So do people currently use random binary data here, or are these generally hex/base64 strings?
#4 Updated by Tobias Brunner over 3 years ago
I pushed a version that stores the attributes in the eap-radius plugin (and has an option to enable it) to the 2451-radius-accounting-class-ht branch.
#5 Updated by Thomas Strangert over 3 years ago
Hi,
Built and tested (to a limited extent) the 2451-radius-accounting-class-ht
branch - OK!
Did not test sending multiple Class
to see if those were relayed (and also in order).
I'm happy with the patch, you may merge the branch to master/HEAD now.
#6 Updated by Tobias Brunner about 3 years ago
- Status changed from Feedback to Closed
- Assignee set to Tobias Brunner
- Target version set to 5.6.1
- Resolution set to Fixed
Thanks for testing. Merged to master.
eap-radius: Optionally send Class attributes in RADIUS accounting messages
If enabled, add the RADIUS Class attributes received in Access-Accept messages
to RADIUS accounting messages as suggested by RFC 2865 section 5.25.
Fixes #2451.