processone / ejabberd

Robust, Ubiquitous and Massively Scalable Messaging Platform (XMPP, MQTT, SIP Server)
https://www.process-one.net/en/ejabberd/
Other
6.03k stars 1.5k forks source link

Allow to specify user, group and mode of UNIX sockets #4059

Closed sideeffect42 closed 1 year ago

sideeffect42 commented 1 year ago

Is your feature request related to a problem? Please describe.

I would like to have some functionality of ejabberd to be fronted by a reverse proxy. Since the reverse proxy and ejabberd are running on the same machine it would be practical if the reverse proxy could talk to ejabberd via a UNIX socket.

Unfortunately, the sockets created by the listen option are owned by the ejabberd service user and have mode 0755 (umask 0022). As a result (on Linux) only the ejabberd user can connect to the socket.

Describe the solution you'd like

I would like there to be configuration options to set the ownership and mode of the socket which is created.

Describe alternatives you've considered

Possible alternative solutions:

set the umask: setting the umask before starting ejabberd alters the sockets' mode, but it will also affect all the other files ejabberd creates. For the reverse proxy to be able to connect to the socket I would need to umask 0000 which seems like a bad idea.

update ownership/mode after starting ejabberd: theoretically possible but somewhat fiddly. Patching init script, maybe?

badlop commented 1 year ago

The ejabberd source code just calls Erlang/OTP function gen_tcp:listen providing the file path, and looking at the erlang documentation, there's no way to modify the permissions you mention. See listen_option and local_address.

update ownership/mode after starting ejabberd: theoretically possible but somewhat fiddly.

In this sense, I wrote this dirty and experimental patch that updates the file permissions after creating it. It requires some testing...

Example configuration in ejabberd.yml:

unix_socket_domain_details:
  mode: 775
  owner: 117
  group: 135

Patch against ejabberd git:

```diff diff --git a/src/ejabberd_listener.erl b/src/ejabberd_listener.erl index 8b02e3847..036c736fe 100644 --- a/src/ejabberd_listener.erl +++ b/src/ejabberd_listener.erl @@ -200,11 +200,29 @@ listen_tcp(Port, SockOpts) -> {keepalive, true} | ExtraOpts]), case Res of {ok, ListenSocket} -> + set_unix_socket_domain_file_details(Port), {ok, ListenSocket}; {error, _} = Err -> Err end. +set_unix_socket_domain_file_details(<<"unix:", Path/binary>>) -> + Usd = ejabberd_option:unix_socket_domain_details(), + case maps:get(mode, Usd, undefined) of + undefined -> ok; + Mode -> ok = file:change_mode(Path, list_to_integer(integer_to_list(Mode), 8)) + end, + case maps:get(owner, Usd, undefined) of + undefined -> ok; + Owner -> ok = file:change_owner(Path, Owner) + end, + case maps:get(group, Usd, undefined) of + undefined -> ok; + Group -> ok = file:change_group(Path, Group) + end; +set_unix_socket_domain_file_details(_Port) -> + ok. + -spec split_opts(transport(), opts()) -> {opts(), [gen_tcp:option()]}. split_opts(Transport, Opts) -> maps:fold( diff --git a/src/ejabberd_option.erl b/src/ejabberd_option.erl index 81f4bab7f..c0cf54e44 100644 --- a/src/ejabberd_option.erl +++ b/src/ejabberd_option.erl @@ -160,6 +160,7 @@ -export([sql_type/0, sql_type/1]). -export([sql_username/0, sql_username/1]). -export([trusted_proxies/0]). +-export([unix_socket_domain_details/0]). -export([use_cache/0, use_cache/1]). -export([validate_stream/0]). -export([version/0]). @@ -1080,6 +1081,10 @@ sql_username(Host) -> trusted_proxies() -> ejabberd_config:get_option({trusted_proxies, global}). +-spec unix_socket_domain_details() -> #{'group'=>non_neg_integer(), 'mode'=>non_neg_integer(), 'owner'=>non_neg_integer()}. +unix_socket_domain_details() -> + ejabberd_config:get_option({unix_socket_domain_details, global}). + -spec use_cache() -> boolean(). use_cache() -> use_cache(global). diff --git a/src/ejabberd_options.erl b/src/ejabberd_options.erl index 06087921d..96152d7e5 100644 --- a/src/ejabberd_options.erl +++ b/src/ejabberd_options.erl @@ -426,6 +426,12 @@ opt_type(sql_prepared_statements) -> econf:bool(); opt_type(trusted_proxies) -> econf:either(all, econf:list(econf:ip_mask())); +opt_type(unix_socket_domain_details) -> + econf:options( + #{group => econf:non_neg_int(), + owner => econf:non_neg_int(), + mode => econf:non_neg_int()}, + [unique, {return, map}]); opt_type(use_cache) -> econf:bool(); opt_type(validate_stream) -> @@ -709,6 +715,7 @@ options() -> {sql_username, <<"ejabberd">>}, {sql_prepared_statements, true}, {trusted_proxies, []}, + {unix_socket_domain_details, #{}}, {validate_stream, false}, {websocket_origin, []}, {websocket_ping_interval, timer:seconds(60)}, @@ -782,6 +789,7 @@ globals() -> sm_cache_missed, sm_cache_size, trusted_proxies, + unix_socket_domain_details, validate_stream, version, websocket_origin, ```
Neustradamus commented 1 year ago

@sideeffect42: Have you tried the @badlop patch?

sideeffect42 commented 1 year ago

I have just given @badlop's patch a spin on a fresh build of 23.04.

It seems to work like what I would expect. My thoughts on the current implementation:

badlop commented 1 year ago

Great ideas!. Here is a new patch with several improvements:

Configure like this:

listen:
  -
    port: "unix://tmp/asd/socket"
    unix_socket:
      mode: "0775"
      owner: 117
      group: 135

New patch:

```diff diff --git a/src/ejabberd_listener.erl b/src/ejabberd_listener.erl index 8b02e3847..b9309889d 100644 --- a/src/ejabberd_listener.erl +++ b/src/ejabberd_listener.erl @@ -149,6 +149,7 @@ init({Port, _, udp} = EndPoint, Module, Opts, SockOpts) -> init({Port, _, tcp} = EndPoint, Module, Opts, SockOpts) -> case listen_tcp(Port, SockOpts) of {ok, ListenSocket} -> + set_unix_socket_domain_file_details(Port, Opts), case inet:sockname(ListenSocket) of {ok, {Addr, Port1}} -> proc_lib:init_ack({ok, self()}), @@ -205,6 +206,37 @@ listen_tcp(Port, SockOpts) -> Err end. +set_unix_socket_domain_file_details(<<"unix:", Path/binary>>, Opts) -> + Usd = maps:get(unix_socket, Opts), + case maps:get(mode, Usd, undefined) of + undefined -> ok; + Mode -> ok = file:change_mode(Path, Mode) + end, + case maps:get(owner, Usd, undefined) of + undefined -> ok; + Owner -> + try + ok = file:change_owner(Path, Owner) + catch + error:{badmatch, {error, eperm}} -> + ?ERROR_MSG("Error trying to set owner ~p for socket ~p", [Owner, Path]), + throw({error_setting_socket_owner, Owner, Path}) + end + end, + case maps:get(group, Usd, undefined) of + undefined -> ok; + Group -> + try + ok = file:change_group(Path, Group) + catch + error:{badmatch, {error, eperm}} -> + ?ERROR_MSG("Error trying to set group ~p for socket ~p", [Group, Path]), + throw({error_setting_socket_group, Group, Path}) + end + end; +set_unix_socket_domain_file_details(_Port, _Opts) -> + ok. + -spec split_opts(transport(), opts()) -> {opts(), [gen_tcp:option()]}. split_opts(Transport, Opts) -> maps:fold( @@ -699,6 +731,12 @@ listen_opt_type(shaper) -> econf:shaper(); listen_opt_type(access) -> econf:acl(); +listen_opt_type(unix_socket) -> + econf:options( + #{group => econf:non_neg_int(), + owner => econf:non_neg_int(), + mode => econf:octal()}, + [unique, {return, map}]); listen_opt_type(use_proxy_protocol) -> econf:bool(). @@ -709,5 +747,6 @@ listen_options() -> {accept_interval, 0}, {send_timeout, 15000}, {backlog, 5}, + {unix_socket, #{}}, {use_proxy_protocol, false}, {supervisor, true}]. ```
prefiks commented 1 year ago

I also didn't found any code that allow mapping from user -> id (there is no mention of getpwuid in erlang source code that is usually used for that), and i also not found a way to change umask for drivers so not sure if we will be able to solve those two points

badlop commented 1 year ago

BTW, I noticed that I only added the relevant code for TCP listeners, I guess it should be added to UDP too, right?

As it is there is a short time frame in which an attacker might connect to the socket before its permissions are set correctly.

Just to be sure, could you check that possibility exists? For this debugging, you can add the erlang code: timer:sleep(30000), that will add a delay of 30 seconds, so that you can attempt to break into the server before the proper permissions are set. For example:

set_unix_socket_domain_file_details(<<"unix:", Path/binary>>, Opts) ->
    timer:sleep(30000),
    Usd = maps:get(unix_socket, Opts),
    ...
sideeffect42 commented 1 year ago

BTW, I noticed that I only added the relevant code for TCP listeners, I guess it should be added to UDP too, right?

I'm not sure, but having it is better than needing it.

As it is there is a short time frame in which an attacker might connect to the socket before its permissions are set correctly.

Just to be sure, could you check that possibility exists? For this debugging, you can add the erlang code: timer:sleep(30000), that will add a delay of 30 seconds, so that you can attempt to break into the server before the proper permissions are set.

On my system I can "acquire API output" using ths following test script and config:

#!/bin/sh
set -x

prefix=/usr/local
ejabberdctl=${prefix}/sbin/ejabberdctl
socket_path=/tmp/ejabberd-admin.sock
user=ejabberd

EJABBERD_CONFIG_PATH=${PWD}/ejabberd.test.yml
export EJABBERD_CONFIG_PATH

"${ejabberdctl}" stop
"${ejabberdctl}" stopped

(umask 0000 && HOME=$(eval "cd ~${user}" && pwd -P) capsh --iab=^cap_chown,^cap_setpcap,^cap_net_bind_service --user="${user}" --addamb=cap_chown,cap_net_bind_service -- -c "${ejabberdctl} start")

su -s /bin/bash nobody -c "x='';\
curl --silent --parallel --unix-socket ${socket_path} http://127.0.0.1/api/registered_vhosts\$x{1..39999}\
"

ejabberd.test.yml:

fqdn: xmpp.example.net
language: en
hosts:
  - example.net
listen:
  -
    port: unix:/tmp/ejabberd-admin.sock
    transport: tcp
    module: ejabberd_http
    unix_socket:
      #owner: 0
      #group: 0
      mode: '0700'
    request_handlers:
      /api: mod_http_api
api_permissions:
  "admin access":
    who: all
    what:
      - "*"
      - "!stop"
      - "!start"

It might not work on every try but I was having pretty good luck even without adding a timer:sleep().

badlop commented 1 year ago

Thanks! Right, your script produces the problem many times. And adding the sleep it's 100% reproducible.

I guess the most simple solution would be to tell the admin to set the socket path in a directory where only the proper users and groups can view the directory content. That way, even if Erlang doesn't set the proper permissions, nobody can read the files there...

But it would be better if ejabberd does it automatically:

  1. instruct ejabberd to create a restricted directory,
  2. then create the socket file provisionally in that directory where nobody can access it,
  3. set the desired permissions to the file
  4. and then move the file to the definitive well-known path.

This patch implements that idea. The provisional path for a given socket file is deterministic, and there's a DEBUG line that prints it, so you can point the script to that path.

With that patch, the script doesn't get access to the API, even when adding a sleep in ejabberd to allow curl access the provisional file.

```diff diff --git a/src/ejabberd_listener.erl b/src/ejabberd_listener.erl index fcfe44ab3..26fddb8c8 100644 --- a/src/ejabberd_listener.erl +++ b/src/ejabberd_listener.erl @@ -108,6 +108,7 @@ init({Port, _, udp} = EndPoint, Module, Opts, SockOpts) -> {Port2, ExtraOpts} = case Port of <<"unix:", Path/binary>> -> SO = lists:keydelete(ip, 1, SockOpts), + setup_provisional_udsocket_dir(Path), file:delete(Path), {0, [{ip, {local, Path}} | SO]}; _ -> @@ -119,6 +120,7 @@ init({Port, _, udp} = EndPoint, Module, Opts, SockOpts) -> {reuseaddr, true} | ExtraOpts2]) of {ok, Socket} -> + set_definitive_udsocket(Port, Opts), case inet:sockname(Socket) of {ok, {Addr, Port1}} -> proc_lib:init_ack({ok, self()}), @@ -149,6 +151,7 @@ init({Port, _, udp} = EndPoint, Module, Opts, SockOpts) -> init({Port, _, tcp} = EndPoint, Module, Opts, SockOpts) -> case listen_tcp(Port, SockOpts) of {ok, ListenSocket} -> + set_definitive_udsocket(Port, Opts), case inet:sockname(ListenSocket) of {ok, {Addr, Port1}} -> proc_lib:init_ack({ok, self()}), @@ -186,8 +189,10 @@ listen_tcp(Port, SockOpts) -> {Port2, ExtraOpts} = case Port of <<"unix:", Path/binary>> -> SO = lists:keydelete(ip, 1, SockOpts), + Prov = setup_provisional_udsocket_dir(Path), file:delete(Path), - {0, [{ip, {local, Path}} | SO]}; + file:delete(Prov), + {0, [{ip, {local, Prov}} | SO]}; _ -> {Port, SockOpts} end, @@ -205,6 +210,72 @@ listen_tcp(Port, SockOpts) -> Err end. +%%% +%%% Unix Domain Socket utility functions +%%% + +setup_provisional_udsocket_dir(DefinitivePath) -> + ProvisionalPath = get_provisional_udsocket_path(DefinitivePath), + SocketDir = filename:dirname(ProvisionalPath), + file:make_dir(SocketDir), + file:change_mode(SocketDir, 8#00700), + ?DEBUG("Creating a Unix Domain Socket provisional file at ~ts for the definitive path ~s", + [ProvisionalPath, DefinitivePath]), + ProvisionalPath. + +get_provisional_udsocket_path(Path) -> + MnesiaDir = mnesia:system_info(directory), + SocketDir = filename:join(MnesiaDir, "socket"), + PathBase64 = misc:term_to_base64(Path), + PathBuild = filename:join(SocketDir, PathBase64), + %% Shorthen the path, a long path produces a crash when opening the socket. + binary:part(PathBuild, {0, erlang:min(107, byte_size(PathBuild))}). + +get_definitive_udsocket_path(<<"unix", _>> = Unix) -> + Unix; +get_definitive_udsocket_path(ProvisionalPath) -> + PathBase64 = filename:basename(ProvisionalPath), + {term, Path} = misc:base64_to_term(PathBase64), + Path. + +set_definitive_udsocket(<<"unix:", Path/binary>>, Opts) -> + Prov = get_provisional_udsocket_path(Path), + timer:sleep(5000), + Usd = maps:get(unix_socket, Opts), + case maps:get(mode, Usd, undefined) of + undefined -> ok; + Mode -> ok = file:change_mode(Prov, Mode) + end, + case maps:get(owner, Usd, undefined) of + undefined -> ok; + Owner -> + try + ok = file:change_owner(Prov, Owner) + catch + error:{badmatch, {error, eperm}} -> + ?ERROR_MSG("Error trying to set owner ~p for socket ~p", [Owner, Prov]), + throw({error_setting_socket_owner, Owner, Prov}) + end + end, + case maps:get(group, Usd, undefined) of + undefined -> ok; + Group -> + try + ok = file:change_group(Prov, Group) + catch + error:{badmatch, {error, eperm}} -> + ?ERROR_MSG("Error trying to set group ~p for socket ~p", [Group, Prov]), + throw({error_setting_socket_group, Group, Prov}) + end + end, + file:rename(Prov, Path); +set_definitive_udsocket(_Port, _Opts) -> + ok. + +%%% +%%% +%%% + -spec split_opts(transport(), opts()) -> {opts(), [gen_tcp:option()]}. split_opts(Transport, Opts) -> maps:fold( @@ -493,8 +564,11 @@ format_error(Reason) -> -spec format_endpoint(endpoint()) -> string(). format_endpoint({Port, IP, _Transport}) -> case Port of + <<"unix:", _/binary>> -> + Port; Unix when is_binary(Unix) -> - <<"unix:", Unix/binary>>; + Def = get_definitive_udsocket_path(Unix), + <<"unix:", Def/binary>>; _ -> IPStr = case tuple_size(IP) of 4 -> inet:ntoa(IP); @@ -699,6 +773,12 @@ listen_opt_type(shaper) -> econf:shaper(); listen_opt_type(access) -> econf:acl(); +listen_opt_type(unix_socket) -> + econf:options( + #{group => econf:non_neg_int(), + owner => econf:non_neg_int(), + mode => econf:octal()}, + [unique, {return, map}]); listen_opt_type(use_proxy_protocol) -> econf:bool(). @@ -709,5 +789,6 @@ listen_options() -> {accept_interval, 0}, {send_timeout, 15000}, {backlog, 128}, + {unix_socket, #{}}, {use_proxy_protocol, false}, {supervisor, true}]. ```