aaa4j / aaa4j-radius

Java library for building RADIUS clients and RADIUS servers
https://aaa4j.org
Apache License 2.0
27 stars 11 forks source link

Request Authenticator does not seem to be correctly implemented for RFC 2866 (Accounting) #11

Closed ramberg closed 3 months ago

ramberg commented 1 year ago

Hi Thomas,

Request Authenticator seems to be implemented correctly for Access-Request, according to RFC 2865 the value MUST be changed each time a new Identifier is used. I think this is sufficiently covered by

byte[] authenticatorBytes = new byte[16];
randomProvider.nextBytes(authenticatorBytes);

but according to RFC 2866, the Request Authenticator for Accounting-Request shall contain a 16-octet MD5 hash value calculated according to the method described in "Request Authenticator" above (RFC 2866).

I used your package to write a high throughput / high parallel test application to test a new configuration of our radius server (not your package). Thank you so much btw for this awesome package. I needed to adapt org.aaa4j.radius.core.packet.PacketCodec#encodeRequest for our radius server to accept "your" accounting requests. I just added this dirty piece of code to the end of the method, and that did the trick:

        if (request instanceof AccountingRequest) {
            Arrays.fill(bytes, 4, 4 + 16, (byte) 0x00);

            MessageDigest md5Instance = getMd5Instance();
            md5Instance.update(bytes);
            md5Instance.update(secret);
            byte[] sign = md5Instance.digest();

            System.arraycopy(sign, 0, bytes, 4, 16);
        }

I know this is dirty and kinda ugly, and I basically just disabled all unit test for now :). Currently I'm only using a tiny fraction of your code, just to generate Access-Requests, parse Access-Responses and generating Accounting-Requests. If I find the time, I will create a fork and make a contribution incl. unit tests, but right now I just wanted to let you know, in case you find the time yourself.

Thanks again for this package, it saved so much of my time.

tsyd commented 3 months ago

Hello Ronny,

Thank you for pointing out this issue regarding the Accounting-Request packet authenticator. This is now fixed in v0.3.1.