| Age | Commit message (Collapse) | Author |
|---|
|
|
|
The function is intended to be traced on, to see abnormalities (mostly)
without producing excessive output. In the case of decode failure, the
error reason can be things like {badmatch, HugeBinary}.
Missed in commit 0058430.
|
|
As for the port number in the parent commit, a FQDN can't be arbitrarily
long, at most 255 octets. Make decode fail if it's more.
|
|
A port number is a 16-bit integer, but the regexp used to parse it in
commit 1590920 slavishly followed the RFC 6733 grammar in matching an
arbitrary number of digits. Make decode fail if it's anything more than
5, to avoid doing erlang:list_to_integer/1 on arbitrarily large lists.
Also make it fail if the resulting integer is outside of the expected
range.
|
|
To bound the length of incoming messages that will be decoded. A message
longer than the specified number of bytes is discarded. An
incoming_maxlen_exceeded counter is incremented to make note of the
occurrence.
The motivation is to prevent a sufficiently malicious peer from
generating significant load by sending long messages with many AVPs for
diameter to decode. The 24-bit message length header accomodates
(16#FFFFFF - 20) div 12 = 1398099
Unsigned32 AVPs for example, which the current record-valued decode is
too slow with in practice. A bound of 16#FFFF bytes allows for 5461
small AVPs, which is probably more than enough for the majority of
applications, but the default is the full 16#FFFFFF.
|
|
It was possible to configure the option, but doing so caused the service
to fail when starting a watchdog process:
{function_clause,
[{diameter_service,'-spawn_opts/1-lc$^0/1-0-',
[false],
[{file,"base/diameter_service.erl"},{line,846}]},
{diameter_service,start,5,
[{file,"base/diameter_service.erl"},{line,820}]},
{diameter_service,start,3,
[{file,"base/diameter_service.erl"},{line,782}]},
{diameter_service,handle_call,3,
[{file,"base/diameter_service.erl"},{line,385}]},
{gen_server,try_handle_call,4,[{file,"gen_server.erl"},{line,607}]},
{gen_server,handle_msg,5,[{file,"gen_server.erl"},{line,639}]},
{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,237}]}]}
Tests for the option in the config suite were also missing.
Bungled in commit 78b3dc6.
|
|
|
|
Required load order by ticket.
- OTP-11492, answer messages discarded
- OTP-12415, retransmission failure
- OTP-12475, grouped AVP decode
- OTP-12543, no requests after DPR
none
- OTP-12412, shutdown issues
diameter_lib
diameter_service
- OTP-12428, transport_opt() pool_size
diameter_lib
diameter_service
diameter, diameter_config
diameter_{tcp,sctp}
diameter, diameter_config
- OTP-12439, new time api in Erlang/OTP 18
diameter_lib
diameter_{config,peer,reg,service,session,stats,sync,watchdog,sctp}
- OTP-11952, service_opt() decode_string
- OTP-12589, DiameterURI encode/decode
diameter_{capx,codec,peer}
diameter_types
diameter_traffic
diameter_{service,peer_fsm}
diameter_watchdog
diameter, diameter_config
- OTP-12542, DPR with diameter:call/4
diameter_{peer_fsm,watchdog}
diameter, diameter_config
- OTP-12609, transport_opt() dpr_timeout
diameter_peer_fsm
diameter, diameter_config
|
|
|
|
* anders/diameter/dpr/OTP-12609:
Discard incoming/outgoing requests after incoming DPR
Add transport_opt() dpr_timeout
Be lenient with errors in incoming DPR
|
|
Despite claims of full backwards compatibility, the text of RFC 6733
changes the interpretation of unspecified values in a DiameterURI. In
particular, 3588 says that the default port and transport are 3868 and
sctp respectively, while 6733 says it's either 3868/tcp (aaa) or
5658/tcp (aaas). The 3588 defaults were used regardless, but now use
them only if the common dictionary is diameter_gen_base_rfc3588. The
6733 defaults are used otherwise.
This kind of change in the standard can lead to interop problems, since
a node has to know which RFC its peer is following to know that it will
properly interpret missing URI components. Encode of a URI includes all
components to avoid such confusion.
That said, note that the defaults in the diameter_uri record have *not*
been changed. This avoids breaking code that depends on them, but the
risk is that such code sends inappropriate values. The record defaults
may be changed in a future release, to force values to be explicitly
specified.
|
|
Both RFC 3588 and 6733 disallow the combination. Make its encode fail.
|
|
* anders/diameter/string_decode/OTP-11952:
Let examples override default service options
Set {restrict_connections, false} in example server
Set {string_decode, false} in examples
Test {string_decode, false} in traffic suite
Add service_opt() string_decode
Strip potentially large terms when sending outgoing Diameter messages
Improve language consistency in diameter(1)
|
|
* anders/diameter/route_record/OTP-12551:
Fix ordering of AVPs in relayed messages
|
|
To make them a bit more flexible. Can now do things like this:
server:start([{'Product-Name', "Bob"}]),
server:listen({tcp, [{capx_timeout, 2000}]})
Beware that the latter is completely different from this:
server:listen(tcp, [{capx_timeout, 2000}])
|
|
Since there's no reason to reject a client that wants to establish
multiple connections, given that diameter can handle it.
|
|
So as to do what's now recommended in diameter(1), in the grandparent
commit.
|
|
By adding string decode or not in the server or client as another
combination. Run all traffic cases in parallel: remove the sequential
tests. Common test seems unable to deal with {group, X, [parallel]}
within a group.
|
|
To control whether stringish Diameter types are decoded to string or
left as binary. The motivation is the same as in the parent commit: to
avoid large strings being copied when incoming Diameter messages are
passed between processes; or *if* in the case of messages destined for
handle_request and handle_answer callbacks, since these are decoded in
the dedicated processes that the callbacks take place in. It would be
possible to do something about other messages without requiring an
option, but disabling the decode is the most effective.
The value is a boolean(), true being the default for backwards
compatibility. Setting false causes both diameter_caps records and
decoded messages to contain binary() in relevant places that previously
had string(): diameter_app(3) callbacks need to be prepared for the
change.
The Diameter types affected are OctetString and the derived types that
can contain arbitrarily large values: OctetString, UTF8String,
DiameterIdentity, DiameterURI, IPFilterRule, and QoSFilterRule. Time and
Address are unaffected.
The DiameterURI decode has been redone using re(3), which both
simplifies and does away with a vulnerability resulting from the
conversion of arbitrary strings to atom.
The solution continues the use and abuse of the process dictionary for
encode/decode purposes, last seen in commit 0f9cdba.
|
|
Both incoming and outgoing Diameter messages pass through two or three
processes, depending on whether they're incoming or outgoing: the
transport process and corresponding peer_fsm process and (for incoming)
watchdog processes. Since terms other than binary are copied when
passing process boundaries, large terms lead to copying that can be
problematic, if frequent enough. Since only the bin and transport_data
fields of a diameter_packet record are needed by the transport process,
discard others when sending outgoing messages.
Strictly speaking, the statement that only the aforementioned fields are
needed by the transport process depends on the transport process. It's
true of those implemented by diameter (in diameter_tcp and
diameter_sctp), but an implementation that makes use of other fields is
assuming more than the documentation in diameter_transport(3) promises.
|
|
With the same motivation as in commits 5bd2d72 and b1fd629.
As in the latter, incoming DPR is the only exception.
|
|
To cause a peer connection to be closed following an outgoing DPA, in
case the peer fails to do so. It is the recipient of DPA that should
close the connection according to RFC 6733.
|
|
To avoid having the peer interpret the error as meaning the connection
shouldn't be closed, which probably does more harm than ignoring
syntactic errors in the DPR.
Note that RFC 6733 says this about incoming DPR, in 5.4 Disconnecting
Peer Connections:
Upon receipt of the message, the Disconnect-Peer-Answer message is
returned, which SHOULD contain an error if messages have recently been
forwarded, and are likely in flight, which would otherwise cause a race
condition.
The race here is presumably between answers to forwarded requests and
the outgoing DPA, but we have no handling for this: whether or not there
are pending answers is irrelevant to how DPR is answered. It's
questionable that a peer should be able to prevent disconnection in any
case: it has to be the node sending DPR that decides if it's approriate,
and the peer should take it as an indication of what's coming.
Incoming DPA is already treated leniently: the only error that's not
ignored is mismatching End-to-End and Hop-by-Hop Identifiers, since
there's no distinguishing an erroneous value from an unsolicited DPA.
This mismatch could also be ignored, which is the case for DWA for
example, but this problem is already dealt with by dpa_timeout, which
causes a connection to be closed even when the expected DPA isn't
received.
|
|
* anders/diameter/dpr/OTP-12542:
Discard CER or DWR sent with diameter:call/4
Allow DPR to be sent with diameter:call/4
Add transport_opt() dpa_timeout
Add testcase for sending DPR with diameter:call/4
|
|
* anders/diameter/dpr/OTP-12543:
Discard incoming requests after outgoing DPR
Discard outgoing requests after outgoing DPR
|
|
6.1.9 of RFC 6733 states this:
A relay or proxy agent MUST append a Route-Record AVP to all requests
forwarded.
The AVP was inserted as the head of the AVP list, not appended, since
the entire AVP list was reversed relative to the received order.
Thanks to Andrzej Trawiński.
|
|
These are requests that diameter itself sends. It's previously been
possible to send them, but answers timed out at the caller since they
were discarded in diameter_watchdog. Answers will still timeout, but now
the requests are discarded before being sent.
|
|
DPR is sent by diameter at application shutdown, service stop, or
transport removal. It has been possible to send the request with
diameter:call/4, but the answer was discarded, instead of the transport
process being terminated. This commit causes DPR to be handled in the
same way regardless of whether it's sent by diameter or by
diameter:call/4.
Note that the behaviour subsequent to DPA is unchanged. In particular,
in the connecting case, the closed connection will be reestablished
after a connect_timer expiry unless the transport is removed. The more
probable use case is the listening case, to disconnect a single peer
associated with a listening transport.
|
|
To make the default DPA timeout configurable. The timeout say how many
milliseconds to wait for DPA in response to an outgoing DPR before
terminating the transport process regardless.
|
|
That currently fails when the resulting DPA is regarded as unsolicited
in diameter_peer_fsm, causing the request to timeout at the caller.
|
|
Since there's a race between an answer being sent and the connection
being closed upon the reception of DPA that's likely to be lost, and
because of the questionability of sending messages after DPR, as
discussed in the parent commit. An exception is made for DPR so that
simultaneous DPR in both directions doesn't result in it being discarded
on both ends.
|
|
RFC 6733 isn't terribly clear about what should happen to incoming or
outgoing messages once DPR is sent and the Peer State Machine
transitions into state Closing. There's no event for this in section
5.6, Peer State Machine, and no clarification in section 5.4,
Disconnecting Peer Connections. There is a little bit of discussion in
2.1.1, SCTP Guidelines, in relation to unordered message delivery, but
the tone there is that messages might be received after DPR because of
unordered delivery, not because they were actually sent after DPR.
Discarding outgoing answers may do more harm than good, but requests are
more likely to be unexpected, as has been seen to be the case with DWR
following DPR. DPR indicates a desire to close the connection: discard
any subsequent outgoing requests.
|
|
* anders/diameter/17/time/OTP-12439:
Adapt app suite to changes in time api
|
|
* anders/diameter/time/OTP-12439:
Adapt to changes in time api
|
|
* anders/diameter/pool/OTP-12428:
Add pool_size test to config suite
|
|
Overlooked in commit 4b691d8d.
|
|
Akin to commit 85d44b58.
|
|
erlang:convert_time_resolution/3 has been renamed to convert_time_unit.
erlang:time_resolution/0 has been removed: use new time resolution
values instead.
|
|
erlang:time_resolution/0 has been removed,
erlang:convert_time_resolution/3 has been renamed convert_time_unit, and
{os,erlang}:system_time/0,1 have been added. Modify the list of ignored
undefined function calls accordingly.
|
|
* anders/diameter/grouped_decode/OTP-12475:
Allow encode of decoded diameter_avp list
Add testcases for diameter_avp decode
Fix handling of length errors on Grouped AVPs
Don't discard component diameter_avp list on Grouped AVP decode error
Fix process dictionary manipulation during message decode
|
|
* anders/diameter/17/time/OTP-12439:
Ignore undefined calls to OTP 18 time api in app suite
|
|
* anders/diameter/time/OTP-12439:
Use new time api in test suites
Use new time api in implementation
|
|
* anders/diameter/pool/OTP-12428:
Fix SCTP match blunder in suites
Be backwards compatible with diameter_sctp listener state
Add gen_tcp testcase that fails sporadically
Simplify transport suite
Remove (ancient) dead code
Don't orphan slave nodes in example suite
Refresh example code
Improve language consistency in diameter(1)
Add pool suite to test transport_opt() pool_size
Adapt tcp/sctp transport modules for pool_size > 1
Add transport_opt() pool_size
|
|
* anders/diameter/shutdown/OTP-12412:
Increase service shutdown timeout
Set shutdown = infinity for supervisor children
Monitor more efficiently at shutdown
|
|
* anders/diameter/retransmission/OTP-12415:
Fix retransmission of messages sent as header/avps list
|
|
The decode of an incoming request in a non-relay application results in
a deep list of diameter_avp records. Encoding such a list resulted in a
function_clause error in diameter_codec:pack_avp/1, which expected a
flat list. The list is only flat in the relay case, or in the absence of
AVPs of type Grouped.
This is also related to code that exists but isn't documented. It's
documented that a diameter_app(3) handle_request callback can return
{relay, Opts} to relay a request received in the relay application.
What's not documented is that it can also return {proxy|resend, Opts} in
a non-relay application, but this leads to encode failure when there are
Grouped AVPs. This shouldn't be interpreted as meaning that proxy|resend
are now supported: they aren't. The two extra terms are a historical
relic that should probably be removed. Neither are generally usable
since, for example, a proxy agent may want to modify a request before
resending it. A specific handle_request return is not needed to
implement a proxy agent. Even {relay, Opts} isn't strictly necessary.
|
|
|
|
The decode of a Grouped AVP ignored the case that extracting component
AVPs with diameter_codec:collect_avps/1 returned a tuple, in the case of
a truncated AVP header.
|
|
So that the xref testcase is independent of the release it's run on. The
code uses the new time api if available, so as to be forward compatible.
|
|
Clause matching error for specific test cases was harmless since the
subsequent clause also matched. Errors detected by the server result in
Failed-AVP being sent, which should not lead to a decode error in the
client.
|
