From fbb8586af3dd74b52ecdfa74d758f14cb82683fd Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 26 Sep 2011 00:21:08 +0200 Subject: Fix and clarify relay behaviour Leave it up to a handle_request callback to decide whether or not to filter the peer from which the incoming request was sent. Reply with 3002 (DIAMETER_UNABLE_TO_DELIVER) on anything but an answer from the peer. --- lib/diameter/doc/src/diameter_app.xml | 40 ++++++++------- lib/diameter/src/app/diameter_service.erl | 83 +++++++++++++------------------ 2 files changed, 55 insertions(+), 68 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/doc/src/diameter_app.xml b/lib/diameter/doc/src/diameter_app.xml index fc359b9d1d..a3d9a8eeac 100644 --- a/lib/diameter/doc/src/diameter_app.xml +++ b/lib/diameter/doc/src/diameter_app.xml @@ -467,11 +467,11 @@ callback returned false.

Packet = packet() SvcName = term() Peer = peer() -Action = Reply | {relay, Opts} | discard | {eval, Action, ContF} +Action = Reply | {relay, Opts} | discard | {eval, Action, PostF} Reply = {reply, message()} | {protocol_error, 3000..3999} Opts = diameter:call_opts() -ContF = diameter:evaluable() +PostF = diameter:evaluable()

@@ -559,26 +559,28 @@ will cause the request process in question to fail.

{relay, Opts}

-Relay a request to another peer. -The appropriate Route-Record AVP will be added to the relayed request -by diameter and pick_peer/4 -and prepare_request/3 -callback will take place just as if pick_peer/4 +and subsequent callbacks take place just as if diameter:call/4 had been called explicitly. -However, returning a relay tuple also causes the End-to-End -Identifier to be preserved in the header of the relayed request as -required by RFC 3588.

+The End-to-End Identifier of the incoming request is preserved in the +header of the relayed request.

-The returned Opts should not specify detach and -the handle_answer/4 -callback following from a relayed request must return its first +The returned Opts should not specify detach. +A subsequent handle_answer/4 +callback for the relayed request must return its first argument, the diameter_packet record containing the answer message. Note that the extra option can be specified to supply arguments -that can distinguish the relay case from others if so desired, -although the form of the request message may be sufficient.

+that can distinguish the relay case from others if so desired. +Any other return value (for example, from a +handle_error/4 callback) +causes the request to be answered with 3002 (DIAMETER_UNABLE_TO_DELIVER).

 discard @@ -587,18 +589,18 @@ although the form of the request message may be sufficient.

Discard the request.

-{eval, Action, ContF} +{eval, Action, PostF}

Handle the request as if Action has been returned and then -evaluate ContF in the request process.

+evaluate PostF in the request process.

-Note that diameter will respond to protocol errors in an incoming -request without invoking handle_request/3.

+Note that protocol errors detected by diameter will result in an +answer message without handle_request/3 being invoked.

 diff --git a/lib/diameter/src/app/diameter_service.erl b/lib/diameter/src/app/diameter_service.erl index 63b0649dc4..b7b9ac1ef4 100644 --- a/lib/diameter/src/app/diameter_service.erl +++ b/lib/diameter/src/app/diameter_service.erl @@ -1685,9 +1685,9 @@ recv_request({Id, Alias}, T, TPid, Apps, Caps, Pkt) -> %% DIAMETER_APPLICATION_UNSUPPORTED 3007 %% A request was sent for an application that is not supported. -recv_request(false, {_, OH, OR}, TPid, _, _, Pkt) -> - ?LOG({error, application}, Pkt), - reply(answer_message({OH, OR, 3007}, collect_avps(Pkt)), ?BASE, TPid, Pkt). +recv_request(false, T, TPid, _, _, Pkt) -> + As = collect_avps(Pkt), + protocol_error(3007, T, TPid, Pkt#diameter_packet{avps = As}). collect_avps(Pkt) -> case diameter_codec:collect_avps(Pkt) of @@ -1706,13 +1706,9 @@ collect_avps(Pkt) -> %% set to an unrecognized value, or that is inconsistent with the %% AVP's definition. %% -recv_request({_, OH, OR}, {TPid, _}, _, #diameter_packet{errors = [Bs | _], - bin = Bin, - avps = Avps} - = Pkt) +recv_request(T, {TPid, _}, _, #diameter_packet{errors = [Bs | _]} = Pkt) when is_bitstring(Bs) -> - ?LOG({error, invalid_avp_bits}, Bin), - reply(answer_message({OH, OR, 3009}, Avps), ?BASE, TPid, Pkt); + protocol_error(3009, T, TPid, Pkt); %% Either we support this application but don't recognize the command %% or we're a relay and the command isn't proxiable. @@ -1722,18 +1718,15 @@ recv_request({_, OH, OR}, {TPid, _}, _, #diameter_packet{errors = [Bs | _], %% recognize or support. This MUST be used when a Diameter node %% receives an experimental command that it does not understand. %% -recv_request({_, OH, OR}, +recv_request(T, {TPid, _}, #diameter_app{id = Id}, #diameter_packet{header = #diameter_header{is_proxiable = P}, - msg = M, - avps = Avps, - bin = Bin} + msg = M} = Pkt) when ?APP_ID_RELAY /= Id, undefined == M; ?APP_ID_RELAY == Id, not P -> - ?LOG({error, command_unsupported}, Bin), - reply(answer_message({OH, OR, 3001}, Avps), ?BASE, TPid, Pkt); + protocol_error(3001, T, TPid, Pkt); %% Error bit was set on a request. %% @@ -1742,15 +1735,12 @@ recv_request({_, OH, OR}, %% either set to an invalid combination, or to a value that is %% inconsistent with the command code's definition. %% -recv_request({_, OH, OR}, +recv_request(T, {TPid, _}, _, - #diameter_packet{header = #diameter_header{is_error = true}, - avps = Avps, - bin = Bin} + #diameter_packet{header = #diameter_header{is_error = true}} = Pkt) -> - ?LOG({error, error_bit}, Bin), - reply(answer_message({OH, OR, 3008}, Avps), ?BASE, TPid, Pkt); + protocol_error(3008, T, TPid, Pkt); %% A message in a locally supported application or a proxiable message %% in the relay application. Don't distinguish between the two since @@ -1878,7 +1868,7 @@ resend(true, _, _, T, {TPid, _}, Pkt) -> %% Route-Record loop resend(false, Opts, App, - {SvcName, _, _}, + {SvcName, _, _} = T, {TPid, #diameter_caps{origin_host = {_, OH}}}, #diameter_packet{header = Hdr0, avps = Avps} @@ -1887,46 +1877,41 @@ resend(false, Seq = diameter_session:sequence(), Hdr = Hdr0#diameter_header{hop_by_hop_id = Seq}, Msg = [Hdr, Route | Avps], - %% Filter sender as ineligible receiver. - reply(call(SvcName, App, Msg, [{filter, {neg, {host, OH}}} | Opts]), - TPid, - Pkt). + resend(call(SvcName, App, Msg, Opts), T, TPid, Pkt). %% The incoming request is relayed with the addition of a -%% Route-Record. Note the requirement on the return from call/4. -%% This places a requirement on the values returned by the -%% handle_answer and handle_error callbacks of the application module -%% in question. +%% Route-Record. Note the requirement on the return from call/4 below, +%% which places a requirement on the value returned by the +%% handle_answer callback of the application module in question. +%% +%% Note that there's nothing stopping the request from being relayed +%% back to the sender. A pick_peer callback may want to avoid this but +%% a smart peer might recognize the potential loop and choose another +%% route. A less smart one will probably just relay the request back +%% again and force us to detect the loop. A pick_peer that wants to +%% avoid this can specify filter to avoid the possibility. +%% Eg. {neg, {host, OH} where #diameter_caps{origin_host = {OH, _}}. %% %% RFC 6.3 says that a relay agent does not modify Origin-Host but %% says nothing about a proxy. Assume it should behave the same way. -%% reply/3 +%% resend/4 %% %% Relay a reply to a relayed request. %% Answer from the peer: reset the hop by hop identifier and send. -reply(#diameter_packet{bin = B} - = Pkt, - TPid, - #diameter_packet{header = #diameter_header{hop_by_hop_id = Id}, - transport_data = TD}) -> +resend(#diameter_packet{bin = B} + = Pkt, + _, + TPid, + #diameter_packet{header = #diameter_header{hop_by_hop_id = Id}, + transport_data = TD}) -> send(TPid, Pkt#diameter_packet{bin = diameter_codec:hop_by_hop_id(Id, B), transport_data = TD}); %% TODO: counters -%% Not. Ignoring the error feels harsh but there is no appropriate -%% Result-Code for a protocol error (which this isn't really anyway) -%% and the RFC doesn't provide any guidance how to act. A weakness -%% here is that we don't deal well with a decode error: the request -%% will simply timeout on the peer's end. Better would be to just send -%% the answer (with modified hop by hop identifier) on regardless, at -%% least in the relay case in which there's no examination of the -%% answer. In the proxy case it's not clear that the callback won't -%% examine the answer. Just be quiet here since a decode error causes -%% the request process to crash (or not depending on the error and -%% config and/or handle_answer callback). -reply(_, _, _) -> - ok. +%% Or not: DIAMETER_UNABLE_TO_DELIVER. +resend(_, T, TPid, Pkt) -> + protocol_error(3002, T, TPid, Pkt). %% is_loop/4 %% -- cgit v1.2.3