Skip to content

Commit

Permalink
fix de/encode of Protocol Configuration Options (PCO)
Browse files Browse the repository at this point in the history
During the PCO processing the container protocol type would
be invalidated when a non PPP option was found. Any later PPP
option would then fail to be handled, cause a crash.

Add a PCO generator to the proper test to catch this problem.
  • Loading branch information
RoadRunnr committed Nov 29, 2018
1 parent c40dc18 commit f036d98
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
8 changes: 4 additions & 4 deletions src/gtp_packet.erl
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ decode_protocol_opts(<<Id:16, Length:8, Data:Length/bytes, Next/binary>>, Protoc
when Protocol == 0, Id >= 16#8000, Id < 16#FF00 ->
Opt = decode_protocol_ppp_opt(Id, Data),
decode_protocol_opts(Next, Protocol, [Opt | Opts]);
decode_protocol_opts(<<Id:16, Length:8, Data:Length/bytes, Next/binary>>, _Protocol, Opts) ->
decode_protocol_opts(Next, -1, [{Id, Data} | Opts]).
decode_protocol_opts(<<Id:16, Length:8, Data:Length/bytes, Next/binary>>, Protocol, Opts) ->
decode_protocol_opts(Next, Protocol, [{Id, Data} | Opts]).

decode_array_of_seq_no(Array) ->
[X || <<X:16/integer>> <= Array].
Expand Down Expand Up @@ -577,9 +577,9 @@ encode_protocol_config_opts({Protocol, Opts}) ->

encode_protocol_opts(_Protocol, [], Opts) ->
Opts;
encode_protocol_opts(_Protocol, [{Id, Data} | T], Opts)
encode_protocol_opts(Protocol, [{Id, Data} | T], Opts)
when Id < 16#8000; Id >= 16#FF00 ->
encode_protocol_opts(-1, T, <<Opts/binary, Id:16, (size(Data)):8, Data/binary>>);
encode_protocol_opts(Protocol, T, <<Opts/binary, Id:16, (size(Data)):8, Data/binary>>);
encode_protocol_opts(Protocol, [Opt | T], Opts)
when Protocol == 0 ->
{Id, Data} = encode_protocol_ppp_opt(Opt),
Expand Down
28 changes: 20 additions & 8 deletions test/property_test/gtplib_prop.erl
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ enc_dec_prop(_Config) ->
numtests(1000,
?FORALL(Msg, msg_gen(),
begin
ct:pal("Msg: ~p", [Msg]),
%% ct:pal("Msg: ~p", [Msg]),
Enc = gtp_packet:encode(Msg),
?equal(Enc, gtp_packet:encode(gtp_packet:decode(Enc)))
end)).
Expand Down Expand Up @@ -433,7 +433,7 @@ v1_simple_ie() ->
%% gen_mm_context_umts_and_used_cipher(),
gen_pdp_context(),
gen_access_point_name(),
%% gen_protocol_configuration_options(),
gen_protocol_configuration_options(),
gen_gsn_address(),
gen_ms_international_pstn_isdn_number(),
gen_quality_of_service_profile(),
Expand Down Expand Up @@ -533,7 +533,7 @@ v2_simple_ie() ->
gen_v2_mobile_equipment_identity(),
gen_v2_msisdn(),
gen_v2_indication(),
%% gen_v2_protocol_configuration_options(),
gen_v2_protocol_configuration_options(),
gen_v2_pdn_address_allocation(),
gen_v2_bearer_level_quality_of_service(),
gen_v2_flow_quality_of_service(),
Expand Down Expand Up @@ -660,7 +660,7 @@ prime_ie() ->
?LET(I, integer(1,10), vector(I, prime_simple_ie()))).

put_ie(IE, IEs) ->
ct:pal("IE: ~p", [IE]),
%% ct:pal("IE: ~p", [IE]),
Key = {element(1, IE), element(2, IE)},
IEs#{Key => IE}.

Expand Down Expand Up @@ -1049,11 +1049,24 @@ gen_access_point_name() ->
apn = apn()
}.

gen_random_list_of_pco() ->
list(oneof(
[{ipcp,'CP-Configure-Request',0,
[{ms_dns1,<<0,0,0,0>>},{ms_dns2,<<0,0,0,0>>}]},
{13, <<>>},
{65280, <<19,1,132>>},
{12, <<>>},
{10, <<>>},
{16, <<>>},
{13, ip4_address()},
{ipcp,'CP-Configure-Nak',0,
[{ms_dns1, ip4_address()},{ms_dns2, ip4_address()}]},
{5,<<2>>}])).

gen_protocol_configuration_options() ->
%% TODO: proper generator...
#protocol_configuration_options{
instance = instance(),
config = []
config = {0, gen_random_list_of_pco()}
}.

gen_gsn_address() ->
Expand Down Expand Up @@ -1691,10 +1704,9 @@ gen_v2_indication() ->
}.

gen_v2_protocol_configuration_options() ->
%% TODO: proper generator...
#v2_protocol_configuration_options{
instance = instance(),
config = []
config = {0, gen_random_list_of_pco()}
}.

gen_v2_pdn_address_allocation() ->
Expand Down

0 comments on commit f036d98

Please sign in to comment.