Merge branch 'hans/ssh/check_dirs/OTP-12788'

* hans/ssh/check_dirs/OTP-12788:
  ssh: Check that user_dir and system_dir exists and are readable

2 files changed, 102 insertions, 4 deletions

diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl
index 4a07473f74..18951c8c89 100644
--- a/lib/ssh/src/ssh.erl
+++ b/lib/ssh/src/ssh.erl
@@ -24,6 +24,7 @@
 -include("ssh.hrl").
 -include("ssh_connect.hrl").
 -include_lib("public_key/include/public_key.hrl").
+-include_lib("kernel/include/file.hrl").
 
 -export([start/0, start/1, stop/0, connect/3, connect/4, close/1, connection_info/2,
 	 channel_info/3,
@@ -389,9 +390,9 @@ handle_option([Opt | Rest], SocketOptions, SshOptions) ->
 handle_ssh_option({minimal_remote_max_packet_size, Value} = Opt) when is_integer(Value), Value >=0 ->
     Opt;
 handle_ssh_option({system_dir, Value} = Opt) when is_list(Value) ->
-    Opt;
+    check_dir(Opt);
 handle_ssh_option({user_dir, Value} = Opt) when is_list(Value) ->
-    Opt;
+    check_dir(Opt);
 handle_ssh_option({user_dir_fun, Value} = Opt) when is_function(Value) ->
     Opt;
 handle_ssh_option({silently_accept_hosts, Value} = Opt) when is_boolean(Value) ->
@@ -581,4 +582,31 @@ handle_ip(Inet) -> %% Default to ipv4
 		    [inet | Inet]
 	    end
     end.
-	
+
+check_dir({_,Dir} = Opt) ->
+    case directory_exist_readable(Dir) of
+	ok ->
+	    Opt;
+	{error,Error} ->
+	    throw({error, {eoptions,{Opt,Error}}})
+    end.
+
+directory_exist_readable(Dir) ->
+    case file:read_file_info(Dir) of
+	{ok, #file_info{type = directory,
+			access = Access}} ->
+	    case Access of
+		read -> ok;
+		read_write -> ok;
+		_ -> {error, eacces}
+	    end;
+
+	{ok, #file_info{}}->
+	    {error, enotdir};
+
+	{error, Error} ->
+	    {error, Error}
+    end.
+		
+		    
+
diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl
index 4b72c57cca..406c0c071e 100644
--- a/lib/ssh/test/ssh_basic_SUITE.erl
+++ b/lib/ssh/test/ssh_basic_SUITE.erl
@@ -23,6 +23,7 @@
 
 -include_lib("common_test/include/ct.hrl").
 -include_lib("kernel/include/inet.hrl").
+-include_lib("kernel/include/file.hrl").
 
 %% Note: This directive should only be used in test suites.
 -compile(export_all).
@@ -49,6 +50,7 @@ all() ->
      daemon_already_started,
      server_password_option,
      server_userpassword_option,
+     {group, dir_options},
      double_close,
      ssh_connect_timeout,
      ssh_connect_arg4_timeout,
@@ -83,7 +85,9 @@ groups() ->
 			    max_sessions_ssh_connect_sequential,
 			    max_sessions_sftp_start_channel_parallel,
 			    max_sessions_sftp_start_channel_sequential
-			   ]}
+			   ]},
+     {dir_options, [], [user_dir_option,
+			system_dir_option]}
     ].
 
 
@@ -134,6 +138,30 @@ init_per_group(internal_error, Config) ->
     ssh_test_lib:setup_dsa(DataDir, PrivDir),
     file:delete(filename:join(PrivDir, "system/ssh_host_dsa_key")),
     Config;
+init_per_group(dir_options, Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    %% Make unreadable dir:
+    Dir_unreadable = filename:join(PrivDir, "unread"),
+    ok = file:make_dir(Dir_unreadable),
+    {ok,F1} = file:read_file_info(Dir_unreadable),
+    ok = file:write_file_info(Dir_unreadable, 
+			      F1#file_info{mode = F1#file_info.mode band (bnot 8#00444)}),
+    %% Make readable file:
+    File_readable = filename:join(PrivDir, "file"),
+    ok = file:write_file(File_readable, <<>>),
+    %% Check:
+    case {file:read_file_info(Dir_unreadable), 
+	  file:read_file_info(File_readable)} of
+	{{ok, #file_info{type=directory, access=Md}},
+	 {ok, #file_info{type=regular, access=Mf}}} when Md=/=read, Md=/=read_write ->
+	    %% Save:
+	    [{unreadable_dir, Dir_unreadable},
+	     {readable_file, File_readable} 
+	     | Config];
+	X ->
+	    ct:log("#file_info : ~p",[X]),
+	    {skip, "File or dir mode settings failed"}
+    end;
 init_per_group(_, Config) ->
     Config.
 
@@ -652,6 +680,48 @@ server_userpassword_option(Config) when is_list(Config) ->
     ssh:stop_daemon(Pid).
 
 %%--------------------------------------------------------------------
+system_dir_option(Config) ->
+    DirUnread = proplists:get_value(unreadable_dir,Config),
+    FileRead = proplists:get_value(readable_file,Config),
+
+    case ssh_test_lib:daemon([{system_dir, DirUnread}]) of
+	{error,{eoptions,{{system_dir,DirUnread},eacces}}} ->
+	    ok;
+	{Pid1,_Host1,Port1} when is_pid(Pid1),is_integer(Port1) ->
+	    ssh:stop_daemon(Pid1),
+	    ct:fail("Didn't detect that dir is unreadable", [])
+	end,
+    
+    case ssh_test_lib:daemon([{system_dir, FileRead}]) of
+	{error,{eoptions,{{system_dir,FileRead},enotdir}}} ->
+	    ok;
+	{Pid2,_Host2,Port2} when is_pid(Pid2),is_integer(Port2) ->
+	    ssh:stop_daemon(Pid2),
+	    ct:fail("Didn't detect that option is a plain file", [])
+    end.
+
+
+user_dir_option(Config) ->
+    DirUnread = proplists:get_value(unreadable_dir,Config),
+    FileRead = proplists:get_value(readable_file,Config),
+    %% Any port will do (beware, implementation knowledge!):
+    Port = 65535,
+
+    case ssh:connect("localhost", Port, [{user_dir, DirUnread}]) of
+	{error,{eoptions,{{user_dir,DirUnread},eacces}}} ->
+	    ok;
+	{error,econnrefused} ->
+	    ct:fail("Didn't detect that dir is unreadable", [])
+    end,
+
+    case ssh:connect("localhost", Port, [{user_dir, FileRead}]) of
+	{error,{eoptions,{{user_dir,FileRead},enotdir}}} ->
+	    ok;
+	{error,econnrefused} ->
+	    ct:fail("Didn't detect that option is a plain file", [])
+    end.
+
+%%--------------------------------------------------------------------
 ssh_msg_debug_fun_option_client() ->
     [{doc, "validate client that uses the 'ssh_msg_debug_fun' option"}].
 ssh_msg_debug_fun_option_client(Config) ->