Skip to content

Commit

Permalink
add test for acct_unique policy
Browse files Browse the repository at this point in the history
and re-add commas in between fields.  Which makes it harder for
malicious actors to mangle multiple fields together.

The real solution is to make md5() take varargs, and then do the MD5
separately for each argument.  And to pass the values as raw
value-boxes, instead of as printable strings
  • Loading branch information
alandekok committed Dec 11, 2023
1 parent 08d2b8e commit 1c3794a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
7 changes: 4 additions & 3 deletions raddb/policy.d/accounting
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ class_value_prefix = 'ai:'
# result in the string `192.0.2.1`, but will instead be
# represented internally as 32-bits of binary data `c0000201`.
# The MD5 hash of those inputs will then be different.
# We fix this issue by using `%string(..)` to convert the
# inputs to MD5 into printable string form.
#
# We fix this issue by converting the MD5 inputs into printable
# string form.
#
# Similarly, the output of `%md5(..)` is binary safe, and is
# therefore a binary blob. We therefore convert the output
Expand Down Expand Up @@ -67,7 +68,7 @@ acct_unique {
# is not included
#
else {
&request.Acct-Unique-Session-Id := %hex(%md5("%{User-Name}%{Acct-Multi-Session-ID}%{Acct-Session-ID}%{&NAS-IPv6-Address || &NAS-IP-Address}%{NAS-Identifier}%{NAS-Port-ID}%{NAS-Port}"))
&request.Acct-Unique-Session-Id := %hex(%md5("%{User-Name},%{Acct-Multi-Session-ID},%{Acct-Session-ID},%{&NAS-IPv6-Address || &NAS-IP-Address},%{NAS-Identifier},%{NAS-Port-ID},%{NAS-Port}"))
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/tests/keywords/acct_unique
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#
# PRE: hex
#
# Test the acct_unique policy
#
#
&Acct-Unique-Session-Id := %hex(%md5("%{User-Name},%{Acct-Multi-Session-ID},%{Acct-Session-ID},%{&NAS-IPv6-Address || &NAS-IP-Address},%{NAS-Identifier},%{NAS-Port-ID},%{NAS-Port}"))

if &Acct-Unique-Session-Id != "159dccf021583d7413b0114a090529ca" {
test_fail
}

success

0 comments on commit 1c3794a

Please sign in to comment.