From ee45dae161bca294fb2feffacee079e190a57ce4 Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Wed, 4 Mar 2026 10:29:27 +0100 Subject: [PATCH 1/2] Fix scramble encryption, Message-Authenticator HMAC, and optional client field - packet/1: pre-generate random authenticator for 'request' cmd before encoding attributes, so that scramble encryption (User-Password) and Message-Authenticator HMAC computation both use the same authenticator value that will be written into the packet header - encode_body/2 for 'request': use pre-generated authenticator from packet/1 if present, instead of always generating a new one - encode_message_authenticator/2: fix typo 'reqid' -> 'req_id' in pattern match, so Message-Authenticator HMAC is actually computed when msg_hmac=true - request/4: make 'client' field optional in NAS map (fall back to <<>>), since NAS maps often only carry 'secret' and omitting 'client' caused a function_clause crash on the server side when decoding incoming packets --- src/eradius_req.erl | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/eradius_req.erl b/src/eradius_req.erl index 94cfb1ef..0256dc08 100644 --- a/src/eradius_req.erl +++ b/src/eradius_req.erl @@ -156,8 +156,15 @@ packet(#{req_id := _, cmd := _, authenticator := _, body := Body, secret := _} = when is_binary(Body) -> %% body must be fully prepared encode_body(Req, Body); -packet(#{req_id := _, cmd := _, secret := _, attrs := Attrs, eap_msg := EAPmsg} = Req) +packet(#{req_id := _, cmd := Cmd, secret := _, attrs := Attrs, eap_msg := EAPmsg} = Req0) when is_list(Attrs) -> + %% For 'request' cmd, the authenticator is random. Pre-generate it so that + %% scramble encryption (User-Password) and Message-Authenticator HMAC can + %% both use the same value before encode_body writes it into the packet header. + Req = case Cmd of + request -> Req0#{authenticator => random_authenticator()}; + _ -> Req0 + end, Body0 = encode_attributes(Req, Attrs, <<>>), Body1 = encode_eap_message(EAPmsg, Body0), Body = encode_message_authenticator(Req, Body1), @@ -218,7 +225,8 @@ new(Command, MetricsCallback) -spec request(binary(), binary(), eradius_server:client(), 'undefined' | metrics_callback()) -> req() | no_return(). request(<> = Header, Body, - #{secret := Secret, client := ClientId}, MetricsCallback) -> + #{secret := Secret} = NAS, MetricsCallback) -> + ClientId = maps:get(client, NAS, <<>>), Command = decode_command(Cmd), Req = new(Command, MetricsCallback), mk_req(Command, ReqId, Len, Authenticator, Header, Body, @@ -321,7 +329,9 @@ mk_req(_, _, Len, _, _, Body, _) encode_body(#{req_id := ReqId, cmd := Cmd} = Req, Body) when Cmd =:= request -> - Authenticator = random_authenticator(), + %% Use pre-generated authenticator if present (set by packet/1 for scramble + %% encryption), otherwise generate a fresh one. + Authenticator = maps:get(authenticator, Req, random_authenticator()), Packet = <<(encode_command(Cmd)):8, ReqId:8, (byte_size(Body) + 20):16, Authenticator:16/binary, Body/binary>>, {Packet, Req#{is_valid := true, request_authenticator => Authenticator}}; @@ -357,7 +367,7 @@ encode_command(discack) -> ?RDisconnect_Ack; encode_command(discnak) -> ?RDisconnect_Nak. -spec encode_message_authenticator(req(), binary()) -> binary(). -encode_message_authenticator(#{reqid := ReqId, cmd := Cmd, +encode_message_authenticator(#{req_id := ReqId, cmd := Cmd, authenticator := Authenticator, secret := Secret, msg_hmac := true}, Body) -> From 8ba4c84e38cb8393ca65488f64e373d5ad41dbae Mon Sep 17 00:00:00 2001 From: Andreas Schultz Date: Wed, 4 Mar 2026 10:29:33 +0100 Subject: [PATCH 2/2] Fix reconfigure server list update and start_client/1 return value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - handle_call({reconfigure, ...}): also update #state.servers when reconfiguring, so that new servers are actually used for sending requests (previously only config was updated, leaving servers empty → no_active_servers) - start_client/1: return the client manager pid instead of the supervisor pid, so callers can pass it directly to eradius_client:send_request/3,4 (previously the supervisor pid was returned, causing send_request to crash with {wanna_send,...} arriving at the supervisor gen_server) --- src/eradius_client_mngr.erl | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/eradius_client_mngr.erl b/src/eradius_client_mngr.erl index 52bab28b..4b42d96c 100644 --- a/src/eradius_client_mngr.erl +++ b/src/eradius_client_mngr.erl @@ -119,10 +119,16 @@ %%%========================================================================= %% @doc Start a new RADIUS client that is managed by the eradius applications supervisor tree. +%% Returns the client manager pid (usable with eradius_client:send_request/3,4). -spec start_client(client_opts()) -> {ok, pid()} | {error, supervisor:startchild_err()}. start_client(Opts) -> - eradius_client_top_sup:start_client([Opts]). + case eradius_client_top_sup:start_client([Opts]) of + {ok, SupPid} -> + client_mngr_pid(SupPid); + Error -> + Error + end. %% @doc Start a new, named RADIUS client that is managed by the eradius applications supervisor tree. -spec start_client(gen_server:server_name(), client_opts()) -> @@ -261,8 +267,9 @@ handle_call({failed, _Peer}, _From, State) -> %% @private handle_call({reconfigure, Opts}, _From, #state{config = OConfig} = State0) -> case client_config(maps:merge(OConfig, Opts)) of - {ok, Config} -> - State = reconfigure_address(Config, State0#state{config = Config}), + {ok, #{servers := Servers} = Config} -> + State1 = State0#state{config = Config, servers = Servers}, + State = reconfigure_address(Config, State1), {reply, ok, State}; {error, _} = Error -> @@ -509,3 +516,11 @@ find_socket_process(PortIdx, Sockets, #state{owner = Owner, config = Config}) -> Socket -> {Socket, Sockets} end. + +client_mngr_pid(SupPid) -> + case lists:keyfind(eradius_client_mngr, 1, supervisor:which_children(SupPid)) of + {eradius_client_mngr, Pid, worker, _} when is_pid(Pid) -> + {ok, Pid}; + _ -> + {error, not_started} + end.