ssh: Change shell and exec defaults to disabled

Change the shell and exec daemon options to default to disabled
instead of enabling the Erlang shell and Erlang term evaluation
respectively. This implements the "secure by default" principle,
preventing authenticated users from executing arbitrary Erlang code
unless explicitly configured.

The new erlang_eval exec option enables Erlang term evaluation via
exec requests, replacing the previous implicit behavior.

Applications requiring these services must now explicitly enable them:

  %% Enable Erlang shell
  ssh:daemon(Port, [{shell, {shell, start, []}} | Options])

  %% Enable Erlang term evaluation via exec
  ssh:daemon(Port, [{exec, erlang_eval} | Options])

Also fix a client-side crash in ssh:shell/1 where the missing exec
key in options caused channel_cb_init_args/1 to append undefined to
the callback init args. Both erlang_eval and undefined are now
filtered out, as neither is a callback init argument.

Author: u3s

lib/kernel/test/shell_test_lib.erl

@@ -182,9 +182,10 @@ setup_tty(Config) ->
                 PrivDir = filename:join(proplists:get_value(priv_dir, Config), "nopubkey"),
                 file:make_dir(PrivDir),
                 SysDir = proplists:get_value(data_dir, Config),
-                {ok, _Sshd} = ssh:daemon(8989, [{system_dir, SysDir},
-                                    {user_dir, PrivDir},
-                                    {password, "bar"}])
+                {ok, _Sshd} = ssh:daemon(8989, [{shell, {shell, start, []}},
+                                                {system_dir, SysDir},
+                                                {user_dir, PrivDir},
+                                                {password, "bar"}])
             end),
             os:cmd(os:find_executable("tmux") ++ " new-window -n " ++ ClientName ++ " -d -- "++
                 "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null localhost -p 8989 -l foo"),

lib/ssh/doc/guides/hardening.md

@@ -303,10 +303,9 @@ _exec_ server-side service takes a string provided by the client, evaluates it
 and returns the result. The _shell_ function enables the client to open a shell
 in the shell host.
 
-Those service could - and should - be disabled when they are not needed. The
-options [exec](`t:ssh:exec_daemon_option/0`) and
-[shell](`t:ssh:shell_daemon_option/0`) are enabled per default but could be set
-to `disabled` if not needed. The same options could also install handlers for
+The options [exec](`t:ssh:exec_daemon_option/0`) and
+[shell](`t:ssh:shell_daemon_option/0`) are disabled per default.
+The same options could also install handlers for
 the string(s) passed from the client to the server.
 
 ### The id string

lib/ssh/doc/guides/introduction.md

@@ -117,12 +117,13 @@ Channels come in the following three flavors:
   as SFTP [(ssh_sftpd)](`m:ssh_sftpd`), that is built into the SSH daemon
   (server) by default, but it can be disabled. The Erlang `ssh` daemon can be
   configured to run any Erlang- implemented SSH subsystem.
-- _Shell_ \- Interactive shell. By default the Erlang daemon runs the Erlang
-  shell. The shell can be customized by providing your own read-eval-print loop.
+- _Shell_ \- Interactive shell. By default the Erlang daemon does not expose the Erlang
+  shell. It can be enabled with option `{shell, {shell, start, []}}`
+  The shell can be customized by providing your own read-eval-print loop.
   You can also provide your own Command-Line Interface (CLI) implementation, but
   that is much more work.
-- _Exec_ \- One-time remote execution of commands. See function
-  `ssh_connection:exec/4` for more information.
+- _Exec_ \- By default one-time remote execution of commands is disabled.
+  See function `ssh_connection:exec/4` for more information.
 
 ## Where to Find More Information
 

lib/ssh/doc/guides/using_ssh.md

@@ -83,7 +83,8 @@ _Step 3._ Start the Erlang `ssh` daemon:
 1> ssh:start().
 ok
 2> {ok, Sshd} = ssh:daemon(8989, [{system_dir, "/tmp/ssh_daemon"},
-                                  {user_dir, "/tmp/otptest_user/.ssh"}]).
+                                  {user_dir, "/tmp/otptest_user/.ssh"},
+                                  {shell, {shell, start, []}}]).
 {ok,<0.54.0>}
 3>
 ```

lib/ssh/src/ssh.hrl

@@ -36,8 +36,6 @@
 
 -define(DEFAULT_TRANSPORT,  {tcp, gen_tcp, tcp_closed} ).
 
--define(DEFAULT_SHELL, {shell, start, []} ).
-
 -define(DEFAULT_TIMEOUT, 5000).
 
 -define(MAX_RND_PADDING_LEN, 15).
@@ -842,6 +840,14 @@ risk.
 -type shell_daemon_option()     :: {shell, shell_spec()} .
 -doc(#{group => <<"Daemon Options">>}).
 -type shell_spec() :: mod_fun_args() | shell_fun() | disabled .
+-doc """
+The default is `disabled`.
+
+To enable the Erlang shell (the behavior from OTP versions prior to OTP @OTP-19969@):
+```
+ssh:daemon(Port, [{shell, {shell, start, []}} | Options])
+```
+""".
 -doc(#{group => <<"Daemon Options">>,
        equiv => 'shell_fun/2'/0}).
 -type shell_fun() :: 'shell_fun/1'()  | 'shell_fun/2'() .
@@ -850,7 +856,7 @@ risk.
 -type 'shell_fun/1'() :: fun((User::string()) -> pid()) .
 -doc """
 Defines the read-eval-print loop used in a daemon when a shell is requested by
-the client. The default is to use the Erlang shell: `{shell, start, []}`
+the client.
 
 See the option [`exec-option`](`t:exec_daemon_option/0`) for a description of
 how the daemon executes shell-requests and exec-requests depending on the shell-
@@ -862,7 +868,23 @@ and exec-options.
 -doc(#{group => <<"Daemon Options">>}).
 -type exec_daemon_option()      :: {exec, exec_spec()} .
 -doc(#{group => <<"Daemon Options">>}).
--type exec_spec()               :: {direct, exec_fun()} | disabled | deprecated_exec_opt().
+-type exec_spec()               :: {direct, exec_fun()} | disabled | deprecated_exec_opt() | erlang_eval.
+-doc """
+The default is `disabled`.
+
+Value `erlang_eval` enables evaluation of Erlang terms via exec requests.
+This works when the shell option is either `disabled` (no shell) or
+`{shell, start, []}` (Erlang shell). It does not work with custom shells.
+
+To restore the behavior from OTP versions prior to OTP @OTP-19969@, configure:
+```
+ssh:daemon(Port, [{shell, {shell, start, []}},
+                  {exec, erlang_eval}
+                  | Options])
+```
+
+For new code, consider using `{direct, Fun}` for more controlled exec handling.
+""".
 -doc(#{group => <<"Daemon Options">>}).
 -type exec_fun()                :: 'exec_fun/1'() | 'exec_fun/2'() | 'exec_fun/3'().
 -doc(#{group => <<"Daemon Options">>}).

lib/ssh/src/ssh_cli.erl

@@ -41,19 +41,20 @@
 
 %% state
 -record(state, {
-	  cm,
-	  channel,
-	  pty,
-          encoding,
-          deduced_encoding, % OpenSSH sometimes lies about its encodeing. This variable
-                            % is for the process of guessing the peer encoding, taylord
-                            % after the behaviour of openssh.  If it says latin1 it is so.
-                            % It there arrives characters encoded in latin1 it is so. Otherwise
-                            % assume utf8 until otherwise is proved.
-	  group,
-	  shell,
-	  exec,
-      tty
+                cm,
+                channel,
+                pty,
+                encoding,
+                %% OpenSSH sometimes lies about its encodeing. This variable
+                %% is for the process of guessing the peer encoding, taylord
+                %% after the behaviour of openssh.  If it says latin1 it is so.
+                %% It there arrives characters encoded in latin1 it is so. Otherwise
+                %% assume utf8 until otherwise is proved.
+                deduced_encoding,
+                group,
+                shell,
+                exec = erlang_eval,
+                tty
 
 	 }).
 
@@ -212,16 +213,16 @@ handle_ssh_msg({ssh_cm, ConnectionHandler,  {exec, ChannelId, WantReply, Cmd0}},
                 %% The standard I/O is directed from/to the channel ChannelId.
                 exec_direct(ConnectionHandler, ChannelId, Cmd, F, WantReply, S1);
 
-            undefined when S0#state.shell == ?DEFAULT_SHELL ; 
-                           S0#state.shell == disabled ->
-                %% Exec called and the shell is the default shell (= Erlang shell).
-                %% To be exact, eval the term as an Erlang term (but not using the
-                %% ?DEFAULT_SHELL directly). This disables banner, prompts and such.
+            erlang_eval when S0#state.shell == disabled;
+                             S0#state.shell == {shell, start, []} ->
+                %% Exec called and the shell is the Erlang shell or disabled.
+                %% To be exact, eval the term as an Erlang term
+                %% This disables banner, prompts and such.
                 %% The standard I/O is directed from/to the channel ChannelId.
                 exec_in_erlang_default_shell(ConnectionHandler, ChannelId, Cmd, WantReply, S1);
 
-            undefined ->
-                %% Exec called, but the a shell other than the default shell is defined.
+            erlang_eval ->
+                %% Exec called, but the shell is custom (not Erlang shell).
                 %% No new exec shell is defined, so don't execute!
                 %% We don't know if it is intended to use the new shell or not.
                 {"Prohibited.", ?EXEC_ERROR_STATUS, 1};

lib/ssh/src/ssh_client_channel.erl

@@ -420,7 +420,11 @@ init([Options]) ->
 
 channel_cb_init_args(Options) ->
     case proplists:get_value(exec, Options) of
+        erlang_eval ->
+            %% erlang_eval is a mode flag for ssh_cli, not a cb init arg
+            proplists:get_value(init_args, Options);
         undefined ->
+            %% exec key absent from Options (client-side ssh:shell/1)
             proplists:get_value(init_args, Options);
         Exec ->
             proplists:get_value(init_args, Options) ++ [Exec]

lib/ssh/src/ssh_connection.erl

@@ -50,7 +50,6 @@ these messages are handled by
 -include("ssh.hrl").
 -include("ssh_connect.hrl").
 -include("ssh_transport.hrl").
-
 %% API
 -export([session_channel/2, session_channel/4,
 	 exec/4, shell/2, subsystem/4, send/3, send/4, send/5, 
@@ -1497,7 +1496,8 @@ start_cli(#connection{options = Options,
         no_cli ->
             {error, cli_disabled};
         {CbModule, Args} ->
-            ssh_connection_sup:start_channel(server, ConnectionSup, self(), CbModule, ChannelId, Args, Exec, Options)
+            ssh_connection_sup:start_channel(server, ConnectionSup, self(),
+                                             CbModule, ChannelId, Args, Exec, Options)
     end.
 
 
@@ -1507,7 +1507,11 @@ start_subsystem(BinName, #connection{options = Options,
     Name = binary_to_list(BinName),
     case check_subsystem(Name, Options) of
 	{Callback, Opts} when is_atom(Callback), Callback =/= none ->
-            ssh_connection_sup:start_channel(server, ConnectionSup, self(), Callback, ChannelId, Opts, undefined, Options);
+            %% Exec is not used by subsystems; undefined is filtered
+            %% out by channel_cb_init_args/1 so it won't be appended
+            %% to the callback's init args.
+            ssh_connection_sup:start_channel(server, ConnectionSup, self(),
+                                             Callback, ChannelId, Opts, undefined, Options);
         {none, _} ->
             {error, bad_subsystem};
 	{_, _} ->

lib/ssh/src/ssh_options.erl

@@ -421,7 +421,7 @@ default(server) ->
            },
 
       shell =>
-          #{default => ?DEFAULT_SHELL,
+          #{default => disabled,
             chk => fun({M,F,A}) -> is_atom(M) andalso is_atom(F) andalso is_list(A);
                       (disabled) -> true;
                       (V) -> check_function1(V) orelse
@@ -431,9 +431,10 @@ default(server) ->
            },
 
       exec =>
-          #{default => undefined,
+          #{default => disabled,
             chk => fun({direct, V}) ->  check_function1(V) orelse check_function2(V) orelse check_function3(V);
                       (disabled) -> true;
+                      (erlang_eval) -> true; % Enable Erlang term evaluation
                       %% Compatibility (undocumented):
                       ({M,F,A}) -> is_atom(M) andalso is_atom(F) andalso is_list(A);
                       (V) -> check_function1(V) orelse check_function2(V) orelse check_function3(V)

lib/ssh/src/ssh_sftp.erl

@@ -172,6 +172,8 @@ start_channel(Cm, UserOptions0) when is_pid(Cm) ->
     PacketSize = proplists:get_value(packet_size, ChanOpts, ?XFER_PACKET_SIZE),
     case ssh_connection:session_channel(Cm, WindowSize, PacketSize, Timeout) of
 	{ok, ChannelId} ->
+            %% Exec is not used by SFTP channels; undefined is filtered
+            %% out by channel_cb_init_args/1.
             case ssh_connection_handler:start_channel(Cm, ?MODULE, ChannelId,
                                                       [Cm,ChannelId,SftpOpts], undefined) of
 		{ok, Pid} ->