Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssh: Add bannerfun to the server role #9149

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ssh: Add bannerfun to the server role #9149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fffdce8
ssh: Add bannerfun to the server role
alexandrejbr Dec 5, 2024
f60efc4
ssh: bannerfun works with openssh
alexandrejbr Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions lib/ssh/src/ssh.hrl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,11 +1143,16 @@ in the User's Guide chapter.

- **`failfun`** - Provides a fun to implement your own logging when a user fails
to authenticate.

- **`bannerfun`** - Provides a fun to implement the construction of a banner
text that is sent at the beginning of the user authentication. The banner will
not be sent if the function does not return a binary.
""".
-doc(#{title => <<"Daemon Options">>}).
-type callbacks_daemon_options() ::
{failfun, fun((User::string(), Peer::{inet:ip_address(), inet:port_number()}, Reason::term()) -> _)}
| {connectfun, fun((User::string(), Peer::{inet:ip_address(), inet:port_number()}, Method::string()) ->_)} .
| {connectfun, fun((User::string(), Peer::{inet:ip_address(), inet:port_number()}, Method::string()) ->_)}
| {bannerfun, fun((User::string()) -> binary())}.
alexandrejbr marked this conversation as resolved.
Show resolved Hide resolved

-doc(#{title => <<"Other data types">>}).
-type opaque_daemon_options() ::
Expand Down Expand Up @@ -1246,7 +1251,8 @@ in the User's Guide chapter.
userauth_preference,
available_host_keys,
pwdfun_user_state,
authenticated = false
authenticated = false,
userauth_banner_sent = false
}).

-record(alg,
Expand Down
47 changes: 36 additions & 11 deletions lib/ssh/src/ssh_fsm_userauth_server.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,22 @@ callback_mode() ->
%%---- userauth request to server
handle_event(internal,
Msg = #ssh_msg_userauth_request{service = ServiceName,
method = Method},
method = Method,
user = User},
StateName = {userauth,server},
D0 = #data{ssh_params=Ssh0}) ->

D0) ->
D1 = maybe_send_banner(D0, User),
#data{ssh_params=Ssh0} = D1,
case {ServiceName, Ssh0#ssh.service, Method} of
{"ssh-connection", "ssh-connection", "none"} ->
%% Probably the very first userauth_request but we deny unauthorized login
%% However, we *may* accept unauthorized login if instructed so
case ssh_auth:handle_userauth_request(Msg, Ssh0#ssh.session_id, Ssh0) of
{not_authorized, _, {Reply,Ssh}} ->
D = ssh_connection_handler:send_msg(Reply, D0#data{ssh_params = Ssh}),
D = ssh_connection_handler:send_msg(Reply, D1#data{ssh_params = Ssh}),
{keep_state, D};
{authorized, User, {Reply, Ssh1}} ->
D = connected_state(Reply, Ssh1, User, Method, D0),
D = connected_state(Reply, Ssh1, User, Method, D1),
{next_state, {connected,server}, D,
[set_max_initial_idle_timeout(D),
{change_callback_module,ssh_connection_handler}
Expand All @@ -87,18 +89,18 @@ handle_event(internal,
%% Yepp! we support this method
case ssh_auth:handle_userauth_request(Msg, Ssh0#ssh.session_id, Ssh0) of
{authorized, User, {Reply, Ssh1}} ->
D = connected_state(Reply, Ssh1, User, Method, D0),
D = connected_state(Reply, Ssh1, User, Method, D1),
{next_state, {connected,server}, D,
[set_max_initial_idle_timeout(D),
{change_callback_module,ssh_connection_handler}
]};
{not_authorized, {User, Reason}, {Reply, Ssh}} when Method == "keyboard-interactive" ->
retry_fun(User, Reason, D0),
D = ssh_connection_handler:send_msg(Reply, D0#data{ssh_params = Ssh}),
retry_fun(User, Reason, D1),
D = ssh_connection_handler:send_msg(Reply, D1#data{ssh_params = Ssh}),
{next_state, {userauth_keyboard_interactive,server}, D};
{not_authorized, {User, Reason}, {Reply, Ssh}} ->
retry_fun(User, Reason, D0),
D = ssh_connection_handler:send_msg(Reply, D0#data{ssh_params = Ssh}),
retry_fun(User, Reason, D1),
D = ssh_connection_handler:send_msg(Reply, D1#data{ssh_params = Ssh}),
{keep_state, D}
end;
false ->
Expand All @@ -116,7 +118,7 @@ handle_event(internal,
{Shutdown, D} =
?send_disconnect(?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE,
io_lib:format("Unknown service: ~p",[ServiceName]),
StateName, D0),
StateName, D1),
{stop, Shutdown, D}
end;

Expand Down Expand Up @@ -213,3 +215,26 @@ retry_fun(User, Reason, #data{ssh_params = #ssh{opts = Opts,
ok
end.

maybe_send_banner(D0 = #data{ssh_params = #ssh{userauth_banner_sent = false} = Ssh}, User) ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use maybe to simplify code? something like this:

maybe_send_banner(D0 = #data{ssh_params = #ssh{userauth_banner_sent = false} = Ssh}, User) ->
    Opts = Ssh#ssh.opts,
    BannerFun = maps:get(bannerfun, Opts, undefined),
    maybe
        true ?= is_function(BannerFun, 1),
        BannerTxt = BannerFun(User),
        true ?= is_binary(BannerTxt) andalso byte_size(BannerTxt)>0, % Ignore bad banner texts
        Banner = #ssh_msg_userauth_banner{message = BannerText,
                                          language = <<>>},
        D = D0#data{ssh_params = Ssh#ssh{userauth_banner_sent = true}},
        ssh_connection_handler:send_msg(Banner, D)
    else
        _ -> D0
    end;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that if you want. Other option is to have the default bannerfun being fun(V) -> <<>> end

Then the implementation can be something like this:

maybe_send_banner(D0 = #data{ssh_params = #ssh{userauth_banner_sent = false} = Ssh}, User) ->
    BannerFun = ?GET_OPT(bannerfun, Ssh#ssh.opts),
    case BannerFun(User) of
        BannerText when is_binary(BannerText), byte_size(BannerText) > 0 ->
            Banner = #ssh_msg_userauth_banner{message = BannerText,
                                              language = <<>>},
            D = D0#data{ssh_params = Ssh#ssh{userauth_banner_sent = true}},
            ssh_connection_handler:send_msg(Banner, D);
        _ ->
            D0
    end;
maybe_send_banner(D, _) ->
    D.

Which one do you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your proposal more :-)

Opts = Ssh#ssh.opts,
BannerText = case maps:get(bannerfun, Opts, undefined) of
undefined ->
<<>>;
BannerFun when is_function(BannerFun, 1) ->
%% Ignore bad banner texts
case BannerFun(User) of
B when is_binary(B) -> B;
_ -> <<>>
end
end,
case BannerText of
<<>> ->
D0;
BannerText ->
Banner = #ssh_msg_userauth_banner{message = BannerText,
language = <<>>},
D = D0#data{ssh_params = Ssh#ssh{userauth_banner_sent = true}},
ssh_connection_handler:send_msg(Banner, D)
end;
maybe_send_banner(D, _) ->
D.
6 changes: 6 additions & 0 deletions lib/ssh/src/ssh_options.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,12 @@ default(server) ->
class => user_option
},

bannerfun =>
#{default => undefined,
chk => fun(V) -> check_function1(V) end,
class => user_option
},

%%%%% Undocumented
infofun =>
#{default => fun(_,_,_) -> void end,
Expand Down
45 changes: 44 additions & 1 deletion lib/ssh/test/ssh_options_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
auth_none/1,
connectfun_disconnectfun_client/1,
disconnectfun_option_client/1,
disconnectfun_option_server/1,
disconnectfun_option_server/1,
bannerfun_server/1,
id_string_no_opt_client/1,
id_string_no_opt_server/1,
id_string_own_string_client/1,
Expand Down Expand Up @@ -114,6 +115,7 @@ suite() ->

all() ->
[connectfun_disconnectfun_server,
bannerfun_server,
connectfun_disconnectfun_client,
server_password_option,
server_userpassword_option,
Expand Down Expand Up @@ -778,6 +780,47 @@ connectfun_disconnectfun_server(Config) ->
{fail, "No connectfun action"}
end.

%%--------------------------------------------------------------------
bannerfun_server(Config) ->
u3s marked this conversation as resolved.
Show resolved Hide resolved
UserDir = proplists:get_value(user_dir, Config),
SysDir = proplists:get_value(data_dir, Config),

Parent = self(),
Ref = make_ref(),
BannerFun = fun(U) -> Parent ! {banner,Ref,U}, list_to_binary(U) end,

{Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
{user_dir, UserDir},
{password, "morot"},
{failfun, fun ssh_test_lib:failfun/2},
{bannerfun, BannerFun}]),
ConnectionRef =
ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
{user, "foo"},
{password, "morot"},
{user_dir, UserDir},
{user_interaction, false}]),
receive
{banner,Ref,U} ->
"foo" = U,
%% Make sure no second banner is sent
receive
{banner,Ref,U} ->
ssh:close(ConnectionRef),
ssh:stop_daemon(Pid),
{fail, "More than 1 banner sent"}
after 2000 ->
ssh:close(ConnectionRef),
ssh:stop_daemon(Pid)
end
after 10000 ->
receive
X -> ct:log("received ~p",[X])
after 0 -> ok
end,
{fail, "No bannerfun action"}
end.

%%--------------------------------------------------------------------
connectfun_disconnectfun_client(Config) ->
UserDir = proplists:get_value(user_dir, Config),
Expand Down
6 changes: 5 additions & 1 deletion lib/ssh/test/ssh_to_openssh_SUITE.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,12 @@ eserver_oclient_renegotiate_helper1(Config) ->
SystemDir = proplists:get_value(data_dir, Config),
PrivDir = proplists:get_value(priv_dir, Config),

%% Having the erlang sending a banner is accepted by openssh
BannerFun = fun(_U) -> <<"Banner to the client">> end,

{Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir},
{failfun, fun ssh_test_lib:failfun/2}]),
{failfun, fun ssh_test_lib:failfun/2},
{bannerfun, BannerFun}]),
ct:sleep(500),

RenegLimitK = 3,
Expand Down
Loading