diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ ### Fixed ### Security +## [0.9.5] - 2019-11-18 +### Added +- Support for LIST-STATUS and LIST-EXTENDED LIST responses +### Fixed +- Client buffer handling via guam-0.9.2-stalling-client-buffer-and-split-command-handling patch + ## [0.9.4] - 2018-02-19 ### Fixed - Support empty lines from the client diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -19,4 +19,4 @@ test: test.spec all - $(REBAR) ct --cover --readable --allow_user_terms + $(REBAR) ct --cover --readable=true --allow_user_terms diff --git a/apps/kolab_guam/src/kolab_guam.app.src b/apps/kolab_guam/src/kolab_guam.app.src --- a/apps/kolab_guam/src/kolab_guam.app.src +++ b/apps/kolab_guam/src/kolab_guam.app.src @@ -2,7 +2,7 @@ {application, kolab_guam, [ {description, "IMAP session proxy"}, - {vsn, "0.9.4"}, + {vsn, "0.9.5"}, {registered, []}, {applications, [ kernel, diff --git a/apps/kolab_guam/src/kolab_guam_session.erl b/apps/kolab_guam/src/kolab_guam_session.erl --- a/apps/kolab_guam/src/kolab_guam_session.erl +++ b/apps/kolab_guam/src/kolab_guam_session.erl @@ -201,36 +201,51 @@ BufferThisData = case PostAction of perform_passthrough -> + %lager:info("sending (no buffer): ~s", [ModifiedData]), eimap:passthrough_data(ImapSession, ModifiedData), <<>>; buffer_data -> - Data + % Originally Aaron uses Data here, but later on this buffer is assumed to be + % already decoded, so we do have to use PreprocessData here, I think. + case binary:matches(PreprocessData, <<"\r\n">>) of + [] -> + %lager:info("buffering: ~s", [PreprocessData]), + PreprocessData; + List -> + {FoundPos, _} = lists:last(List), + % I would like to have some binary:match for the last instead of the + % first occurrence; but I'm really inexperienced in erlang so I don't + % know how to solve this efficiently, so I'm using binary:matches with + % using the last element only + SplitPos = FoundPos + 2, + eimap:passthrough_data(ImapSession, binary:part(PreprocessData, 0, SplitPos)), + %lager:info("sending first part: ~s", [binary:part(PreprocessData, 0, SplitPos)] ), + %lager:info("buffering second part: ~s", [binary:part(PreprocessData, SplitPos, size(PreprocessData)-SplitPos)]), + binary:part(PreprocessData, SplitPos, size(PreprocessData)-SplitPos) + end end, { TLS, Socket, Inflator, Deflator, NewUndecidedRules, NewActiveRules, BufferThisData, NewSplitCommand, NewSplitResetTrigger } end, set_socket_active(TLSActive, CurrentSocket), - PrevBuffered = State#state.buffered_client_data, + %buffered_client_data is already in DataToBuffer via preprocess_client_data { noreply, State#state{ rules_deciding = CurrentUndecidedRules, rules_active = CurrentActiveRules, socket = CurrentSocket, client_tls_active = TLSActive, inflator = CurrentInflator, deflator = CurrentDeflator, - buffered_client_data = <>, + buffered_client_data = <>, current_command_split = SplitCommand, command_split_reset_trigger = SplitResetTrigger } }. preprocess_client_data(undefined, Data, #state{ buffered_client_data = Buffered }) -> <>; preprocess_client_data(Z, Data, #state{ buffered_client_data = Buffered }) -> - Inflated = joined(zlib:inflate(Z, Data), <<>>), + Inflated = iolist_to_binary(zlib:inflate(Z, Data)), <>. postprocess_server_data(undefined, Data) -> %% we aren't compressing so there is nothing to do Data; postprocess_server_data(Z, Data) -> - joined(zlib:deflate(Z, Data, sync), <<>>). - -joined([], Binary) -> Binary; -joined([H|Rest], Binary) -> joined(Rest, <>). + iolist_to_binary(zlib:deflate(Z, Data, sync)). init_rules(RuleConfig) -> init_rule(RuleConfig, []). init_rule([], Acc) -> Acc; @@ -261,7 +276,7 @@ apply_next_rule_serverside(ImapSession, ModifiedData, [{ Module, ModifiedRuleState } | ActiveRulesAcc], ActiveRules). apply_ruleset_clientside(_ImapSession, _Socket, ClientData, _CurrentCommandSplit, [], []) -> - { ClientData, [], [], [], [], perform_passthrough }; + { ClientData, undefined, [], [], [], perform_passthrough }; apply_ruleset_clientside(ImapSession, Socket, ClientData, CurrentCommandSplit, UndecidedRules, CurrentlyActiveRules) -> { PostAction, SplitCommand, SplitResetTrigger } = case CurrentCommandSplit of diff --git a/apps/kolab_guam/src/rules/kolab_guam_rule_filter_groupware.erl b/apps/kolab_guam/src/rules/kolab_guam_rule_filter_groupware.erl --- a/apps/kolab_guam/src/rules/kolab_guam_rule_filter_groupware.erl +++ b/apps/kolab_guam/src/rules/kolab_guam_rule_filter_groupware.erl @@ -102,6 +102,7 @@ filter_folder(State, <<"* LIST ", Details/binary>> = Response, Acc) -> { filter_on_details(State, Response, Acc, Details), true }; filter_folder(State, <<"* XLIST ", Details/binary>> = Response, Acc) -> { filter_on_details(State, Response, Acc, Details), true }; filter_folder(State, <<"* LSUB ", Details/binary>> = Response, Acc) -> { filter_on_details(State, Response, Acc, Details), true }; +filter_folder(State, <<"* STATUS ", Details/binary>> = Response, Acc) -> { filter_on_details(State, Response, Acc, Details), true }; filter_folder(#state{ tag = Tag }, Response, Acc) -> HasMore = case byte_size(Tag) =< byte_size(Response) of @@ -115,25 +116,28 @@ { add_response(Response, Acc), HasMore }. filter_on_details(#state{ blacklist = Blacklist }, Response, Acc, Details) -> - %% first determine if we have a quoted item or a non-quoted item and start from there - DetailsSize = byte_size(Details), - { Quoted, Start } = case binary:at(Details, DetailsSize - 1) of $" -> { quoted, DetailsSize - 2 }; _ -> { unquoted, DetailsSize - 1 } end, - Folder = find_folder_name(Details, Quoted, Start, Start, binary:at(Details, Start)), - %io:format("COMPARING ~p ??? ~p~n", [Folder, in_blacklist(Folder, Blacklist)]), + %% Remove "*" and extract response command name + { _, Start, _ } = pop_token(Response), %% asterisk + { Cmd, _, _ } = pop_token(Start), %% command + + %% Extract folder name + Suffix = + case Cmd =:= <<"STATUS">> of + true -> Details; + _ -> + { Pos, _Length } = binary:match(Details, [<<")">>], []), + { _Delimiter, Rest, _} = pop_token(binary:part(Details, Pos + 2, byte_size(Details) - Pos - 2)), + Rest + end, + { Folder, _, _ } = pop_token(list_to_binary([Suffix, <<"\r\n">>])), + + %% Check the folder in blacklist + %% io:format("COMPARING ~p ??? ~p~n", [Folder, in_blacklist(Folder, Blacklist)]), case in_blacklist(Folder, Blacklist) of true -> Acc; _ -> add_response(Response, Acc) end. -find_folder_name(Details, quoted, End, Start, $") -> - binary:part(Details, Start + 1, End - Start); -find_folder_name(Details, unquoted, End, Start, $ ) -> - binary:part(Details, Start + 1, End - Start); -find_folder_name(Details, _Quoted, _End, 0, _) -> - Details; -find_folder_name(Details, Quoted, End, Start, _) -> - find_folder_name(Details, Quoted, End, Start - 1, binary:at(Details, Start - 1)). - add_response(Response, <<>>) -> Response; add_response(Response, Acc) -> <>. @@ -147,3 +151,72 @@ _ -> in_blacklist(Folder, List) end end. + +%% pop_token from https://github.com/MainframeHQ/switchboard/blob/master/src/imap.erl (BSD Lic.) +%% with some small changes by Aleksander Machniak +pop_token(Data) -> + pop_token(Data, none). + +pop_token(<<>>, State) -> + {none, <<>>, State}; + +%% Consume hanging spaces +pop_token(<<" ", Rest/binary>>, none) -> + pop_token(Rest, none); + +%% \r\n +pop_token(<<$\r, $\n, Rest/binary>>, none) -> + {crlf, Rest, none}; + +%% NIL +pop_token(<<"NIL", Rest/binary>>, none) -> + {nil, Rest, none}; + +%% ( | ) | [ | ] +pop_token(<<$(, Rest/binary>>, none) -> + {'(', Rest, none}; +pop_token(<<$), Rest/binary>>, none) -> + {')', Rest, none}; + + +%% Atom +pop_token(<> = Data, {atom, AtomAcc}) when + C =:= 32; C =:= 40; C =:= 41; C =:= $(; C =:= $) -> + {AtomAcc, Data, none}; +pop_token(<<$\r, $\n, _/binary>> = Data, {atom, AtomAcc}) -> + {AtomAcc, Data, none}; +pop_token(<>, none) when C >= 35, C < 123 -> + pop_token(Rest, {atom, <>}); +pop_token(<>, {atom, AtomAcc}) when C >= 35, C < 123 -> + pop_token(Rest, {atom, <>}); + +%% Literal Strings +pop_token(<<${, Rest/binary>>, none) -> + pop_token(Rest, {literal, <<>>}); +pop_token(<<$}, $\r, $\n, Rest/binary>>, {literal, ByteAcc}) -> + pop_token(Rest, {literal, binary_to_integer(ByteAcc), <<>>}); +pop_token(<>, {literal, ByteAcc}) when D >= 48, D < 58 -> + pop_token(Rest, {literal, <>}); +pop_token(Binary, {literal, Bytes, LiteralAcc}) when is_integer(Bytes) -> + case Binary of + <> -> + {<>, Rest, none}; + _ -> + %% If the binary is too short, accumulate it in the state + pop_token(<<>>, {literal, Bytes - size(Binary), <>}) + end; + +%% Quoted Strings +pop_token(<<$", Rest/binary>>, none) -> + pop_token(Rest, {quoted, <<>>}); +pop_token(<<$\\, C, Rest/binary>>, {quoted, Acc}) -> + pop_token(Rest, {quoted, <>}); +pop_token(<<$", Rest/binary>>, {quoted, Acc}) -> + {Acc, Rest, none}; +pop_token(<<$\r, $\n, _>>, {quoted, _}) -> + throw({error, crlf_in_quoted}); +pop_token(<>, {quoted, Acc}) -> + pop_token(Rest, {quoted, <>}); + +pop_token(Binary, _) -> + {none, Binary, none}. diff --git a/apps/kolab_guam/test/kolab_guam_rules_SUITE.erl b/apps/kolab_guam/test/kolab_guam_rules_SUITE.erl --- a/apps/kolab_guam/test/kolab_guam_rules_SUITE.erl +++ b/apps/kolab_guam/test/kolab_guam_rules_SUITE.erl @@ -94,7 +94,7 @@ kolab_guam_rule_filter_groupware_responsefiltering_multipacket_test(_TestConfig) -> %% Data to be fed into the test, one tuple per iteration - %% Tuple format: { [ input_packets ] = complete_message, correct_output } + %% Tuple format: { [] = folder_blacklist, [] = input_data_packets, correct_output } Data = [ { [ @@ -110,7 +110,7 @@ ], [ <<"* LIST (\\Noinferiors \\Subscribed) \"/\" INBOX\r\n* LIST (\\Subscribed) \"/\" Archive\r\n* LIST (\\Subscribed \\HasChildren) \"/\" Calendar\r\n* LIST (\\Subscribed) \"/\" \"Calendar/Personal Calendar\"\r\n* LIST (\\Subscribed) \"/\" Configuration\r\n* LIST (\\Subscribed \\HasChildren) \"/\" Contacts\r\n* LIST (\\Subscribed) \"/\" \"Contacts/Personal Contacts\"\r\n* LIST (\\Subscribed) \"/\" Drafts\r\n* LIST (\\Subscribed) \"/\" Files\r\n* LIST (\\Subscribed) \"/\" Journal\r\n* LIST (\\Subscribed)">>, - <<"\"/\" Notes\r\n* LIST (\\Subscribed) \"/\" Sent\r\n* LIST (\\Subscribed) \"/\" Spam\r\n* LIST (\\Subscribed) \"/\" Tasks\r\n* LIST (\\Subscribed) \"/\" Trash\r\n7 OK Completed (0.000 secs 15 calls)\r\n">> + <<" \"/\" Notes\r\n* LIST (\\Subscribed) \"/\" Sent\r\n* LIST (\\Subscribed) \"/\" Spam\r\n* LIST (\\Subscribed) \"/\" Tasks\r\n* LIST (\\Subscribed) \"/\" Trash\r\n7 OK Completed (0.000 secs 15 calls)\r\n">> ], <<"* LIST (\\Noinferiors \\Subscribed) \"/\" INBOX\r\n* LIST (\\Subscribed) \"/\" Archive\r\n* LIST (\\Subscribed) \"/\" Drafts\r\n* LIST (\\Subscribed) \"/\" Sent\r\n* LIST (\\Subscribed) \"/\" Spam\r\n* LIST (\\Subscribed) \"/\" Trash\r\n7 OK Completed (0.000 secs 15 calls)\r\n">> }, @@ -124,6 +124,69 @@ <<"7 OK Completed (0.000 secs 15 calls)\r\n">> ], <<"7 OK Completed (0.000 secs 15 calls)\r\n">> + }, + %Test folder names with square brackes + { + [ + {<<"[Calendar]">>,<<"[Calendar]/">>} + ], + [ + <<"* LSUB () \".\" INBOX\r\n* LSUB () \".\" \"[Stuff].Sent Mail.Sent\"\r\n* LSUB () \".\" [Calendar]\r\nwgku OK Lsub completed (0.001 + 0.000 secs).\r\n">> + ], + <<"* LSUB () \".\" INBOX\r\n* LSUB () \".\" \"[Stuff].Sent Mail.Sent\"\r\nwgku OK Lsub completed (0.001 + 0.000 secs).\r\n">> + }, + %Split CR and LF + { + [ + {<<"Calendar">>, <<"Calendar/">>} + ], + [ + <<"* LIST (\\Noinferiors \\Subscribed) \"/\" INBOX\r">>, + <<"\n* LIST (\\Subscribed \\HasChildren) \"/\" Calendar\r">>, + <<"\n7 OK Completed (0.000 secs 15 calls)\r\n">> + ], + <<"* LIST (\\Noinferiors \\Subscribed) \"/\" INBOX\r\n7 OK Completed (0.000 secs 15 calls)\r\n">> + }, + %Numeric folders + { + [ + ], + [ + <<"* LIST () \"/\" 2017\r\n">>, + <<"7 OK Completed (0.000 secs 15 calls)\r\n">> + + ], + <<"* LIST () \"/\" 2017\r\n7 OK Completed (0.000 secs 15 calls)\r\n">> + }, + %LIST-EXTENDED + LIST-STATUS response (as used by evolution) + { + [ + {<<"Calendar">>, <<"Calendar/">>}, + {<<"Calendar/Calendar2/">>, <<"Calendar/Calendar2/">>} + ], + [ + <<"* LIST (\\Noinferiors \\Subscribed) \"/\" INBOX\r\n">>, + <<"* STATUS \"INBOX\" (MESSAGES 17 UNSEEN 16)\r\n">>, + <<"* LIST (\\Subscribed \\HasChildren) \"/\" Calendar (CHILDINFO (\"SUBSCRIBED\"))\r\n">>, + <<"* STATUS \"Calendar\" (MESSAGES 17 UNSEEN 16)\r\n">>, + <<"* LIST (\\Subscribed \\HasNoChildren) \"/\" Calendar/Calendar2\r\n">>, + <<"* STATUS \"Calendar/Calendar2\" (MESSAGES 17 UNSEEN 16)\r\n">>, + <<"7 OK Completed (0.000 secs 15 calls)\r\n">> + ], + <<"* LIST (\\Noinferiors \\Subscribed) \"/\" INBOX\r\n* STATUS \"INBOX\" (MESSAGES 17 UNSEEN 16)\r\n7 OK Completed (0.000 secs 15 calls)\r\n">> + }, + %String literals + %Filtering on string literals is broken, it will attempt to filter on {8}Calendar + { + [ + %{<<"Calendar">>, <<"Calendar/">>} + ], + [ + %<<"* LIST (\Subscribed) \"/\" {8}Calendar\r\n">>, + <<"* LIST (\Subscribed) \"/\" {6}\r\n">>, + <<"Folder\r\n">> + ], + <<"* LIST (\Subscribed) \"/\" {6}\r\nFolder\r\n">> } ], %% setup boilerplate