From 1ff4783ab8b1bdb32ced6072eb193896b429d115 Mon Sep 17 00:00:00 2001 From: Patrik Nyblom Date: Wed, 25 Aug 2010 18:04:56 +0200 Subject: Fix anomalies in epmd not yet reported as security issues Use erts_(v)snprintf to ensure no buffer overruns in debug printouts. Disallow everything except port and name requests from remote nodes. Disallow kill command even from localhost if alive nodes exist. -relaxed_command_check when starting epmd returns the possibility to kill this epmd when nodes are alive (from localhost). Disallow stop command completely except if -relaxed_command_check is given when epmd was started. Environment variable ERL_EPMD_RELAXED_COMMAND_CHECK can be set to always get -relaxed_command_check. --- erts/epmd/src/Makefile.in | 25 +++++-- erts/epmd/src/epmd.c | 48 ++++++++++-- erts/epmd/src/epmd_int.h | 8 +- erts/epmd/src/epmd_srv.c | 51 +++++++++++++ erts/epmd/test/epmd_SUITE.erl | 166 ++++++++++++++++++++++++++++++++++++------ 5 files changed, 262 insertions(+), 36 deletions(-) diff --git a/erts/epmd/src/Makefile.in b/erts/epmd/src/Makefile.in index 498756b468..70a54fe46e 100644 --- a/erts/epmd/src/Makefile.in +++ b/erts/epmd/src/Makefile.in @@ -1,7 +1,7 @@ # # %CopyrightBegin% # -# Copyright Ericsson AB 1998-2009. All Rights Reserved. +# Copyright Ericsson AB 1998-2010. All Rights Reserved. # # The contents of this file are subject to the Erlang Public License, # Version 1.1, (the "License"); you may not use this file except in @@ -49,15 +49,27 @@ include ../epmd.mk BINDIR = $(ERL_TOP)/bin/$(TARGET) OBJDIR = $(ERL_TOP)/erts/obj$(TYPEMARKER)/$(TARGET) +ERTS_INCL = -I$(ERL_TOP)/erts/include \ + -I$(ERL_TOP)/erts/include/$(TARGET) \ + -I$(ERL_TOP)/erts/include/internal \ + -I$(ERL_TOP)/erts/include/internal/$(TARGET) + +# On windows we always need reentrant libraries. +ifeq ($(TARGET),win32) +ERTS_INTERNAL_LIBS=-L../../lib/internal/$(TARGET) -lerts_internal_r$(ERTS_LIB_TYPEMARKER) @ERTS_INTERNAL_X_LIBS@ +else +ERTS_INTERNAL_LIBS=-L../../lib/internal/$(TARGET) -lerts_internal$(ERTS_LIB_TYPEMARKER) @ERTS_INTERNAL_X_LIBS@ -lm +endif CC = @CC@ WFLAGS = @WFLAGS@ -CFLAGS = @CFLAGS@ @DEFS@ $(TYPE_FLAGS) $(WFLAGS) +CFLAGS = @CFLAGS@ @DEFS@ $(TYPE_FLAGS) $(WFLAGS) $(ERTS_INCL) LD = @LD@ -LIBS = @LIBS@ +LIBS = @LIBS@ $(ERTS_INTERNAL_LIBS) LDFLAGS = @LDFLAGS@ + # ---------------------------------------------------- # Release directory specification # ---------------------------------------------------- @@ -90,7 +102,7 @@ EPMD_OBJS = $(OBJDIR)/epmd.o \ #--------------------------------- -all: $(BINDIR)/$(EPMD) +all: erts_lib $(BINDIR)/$(EPMD) docs: @@ -109,9 +121,12 @@ clean: $(BINDIR)/$(EPMD): $(EPMD_OBJS) $(PURIFY) $(LD) $(LDFLAGS) -o $@ $(EPMD_OBJS) $(LIBS) -$(OBJDIR)/%.o: %.c +$(OBJDIR)/%.o: %.c epmd.h epmd_int.h $(CC) $(CFLAGS) $(EPMD_FLAGS) -o $@ -c $< +erts_lib: + cd $(ERL_TOP)/erts/lib_src && $(MAKE) $(TYPE) + include $(ERL_TOP)/make/otp_release_targets.mk release_spec: all diff --git a/erts/epmd/src/epmd.c b/erts/epmd/src/epmd.c index 9c2ce065bb..f195fdac8c 100644 --- a/erts/epmd/src/epmd.c +++ b/erts/epmd/src/epmd.c @@ -23,6 +23,7 @@ #endif #include "epmd.h" /* Renamed from 'epmd_r4.h' */ #include "epmd_int.h" +#include "erl_printf.h" #ifdef HAVE_STDLIB_H # include @@ -33,6 +34,7 @@ static void usage(EpmdVars *); static void run_daemon(EpmdVars*); static int get_port_no(void); +static int check_relaxed(void); #ifdef __WIN32__ static int has_console(void); #endif @@ -161,6 +163,7 @@ int main(int argc, char** argv) g->silent = 0; g->is_daemon = 0; + g->brutal_kill = check_relaxed(); g->packet_timeout = CLOSE_TIMEOUT; /* Default timeout */ g->delay_accept = 0; g->delay_write = 0; @@ -196,6 +199,9 @@ int main(int argc, char** argv) } else if (strcmp(argv[0], "-daemon") == 0) { g->is_daemon = 1; argv++; argc--; + } else if (strcmp(argv[0], "-relaxed_command_check") == 0) { + g->brutal_kill = 1; + argv++; argc--; } else if (strcmp(argv[0], "-kill") == 0) { if (argc == 1) kill_epmd(g); @@ -387,10 +393,10 @@ static void run_daemon(EpmdVars *g) static void usage(EpmdVars *g) { - fprintf(stderr, "usage: epmd [-d|-debug] [DbgExtra...] [-port No] [-daemon]\n"); + fprintf(stderr, "usage: epmd [-d|-debug] [DbgExtra...] [-port No] [-daemon] [-allow_brutal_kill]\n"); fprintf(stderr, " [-d|-debug] [-port No] [-names|-kill|-stop name]\n\n"); - fprintf(stderr, "See the Erlang epmd manual page for info about the usage.\n"); - fprintf(stderr, "The -port and DbgExtra options are\n\n"); + fprintf(stderr, "See the Erlang epmd manual page for info about the usage.\n\n"); + fprintf(stderr, "Regular options\n"); fprintf(stderr, " -port No\n"); fprintf(stderr, " Let epmd listen to another port than default %d\n", EPMD_PORT_NO); @@ -401,6 +407,14 @@ static void usage(EpmdVars *g) fprintf(stderr, " the number of saved used node names to 5.\n\n"); fprintf(stderr, " If you give more than one debug flag you may\n"); fprintf(stderr, " get more debugging information.\n\n"); + fprintf(stderr, " -daemon\n"); + fprintf(stderr, " Start epmd detached (as a daemon)\n"); + fprintf(stderr, " -relaxed_command_check\n"); + fprintf(stderr, " Allow this instance of epmd to be killed with\n"); + fprintf(stderr, " epmd -kill even if there " + "are registered nodes.\n"); + fprintf(stderr, " Also allows forced unregister (epmd -stop).\n"); + fprintf(stderr, "\nDbgExtra options\n"); fprintf(stderr, " -packet_timout Seconds\n"); fprintf(stderr, " Set the number of seconds a connection can be\n"); fprintf(stderr, " inactive before epmd times out and closes the\n"); @@ -413,6 +427,18 @@ static void usage(EpmdVars *g) fprintf(stderr, " -delay_write Seconds\n"); fprintf(stderr, " Also a simulation of a busy server. Inserts\n"); fprintf(stderr, " a delay before a reply is sent.\n"); + fprintf(stderr, "\nInteractive options\n"); + fprintf(stderr, " -names\n"); + fprintf(stderr, " List names registered with the currently " + "running epmd\n"); + fprintf(stderr, " -kill\n"); + fprintf(stderr, " Kill the currently runniing epmd\n"); + fprintf(stderr, " (only allowed if -names show empty database or\n"); + fprintf(stderr, " -allow_brutal_kill was given when epmd was started).\n"); + fprintf(stderr, " -stop Name\n"); + fprintf(stderr, " Forcibly unregisters a name with epmd\n"); + fprintf(stderr, " (only allowed if -allow_brutal_kill was given when \n"); + fprintf(stderr, " epmd was started).\n"); epmd_cleanup_exit(g,1); } @@ -432,20 +458,20 @@ static void usage(EpmdVars *g) * args... Arguments to print out according to the format * */ - +#define DEBUG_BUFFER_SIZE 2048 static void dbg_gen_printf(int onsyslog,int perr,int from_level, EpmdVars *g,const char *format, va_list args) { time_t now; char *timestr; - char buf[2048]; + char buf[DEBUG_BUFFER_SIZE]; if (g->is_daemon) { #ifndef NO_SYSLOG if (onsyslog) { - vsprintf(buf, format, args); + erts_vsnprintf(buf, DEBUG_BUFFER_SIZE, format, args); syslog(LOG_ERR,"epmd: %s",buf); } #endif @@ -456,9 +482,10 @@ static void dbg_gen_printf(int onsyslog,int perr,int from_level, time(&now); timestr = (char *)ctime(&now); - sprintf(buf, "epmd: %.*s: ", (int) strlen(timestr)-1, timestr); + erts_snprintf(buf, DEBUG_BUFFER_SIZE, "epmd: %.*s: ", + (int) strlen(timestr)-1, timestr); len = strlen(buf); - vsprintf(buf + len, format, args); + erts_vsnprintf(buf + len, DEBUG_BUFFER_SIZE - len, format, args); if (perr == 1) perror(buf); else @@ -545,4 +572,9 @@ static int get_port_no(void) char* port_str = getenv("ERL_EPMD_PORT"); return (port_str != NULL) ? atoi(port_str) : EPMD_PORT_NO; } +static int check_relaxed(void) +{ + char* port_str = getenv("ERL_EPMD_RELAXED_COMMAND_CHECK"); + return (port_str != NULL) ? 1 : 0; +} diff --git a/erts/epmd/src/epmd_int.h b/erts/epmd/src/epmd_int.h index d69d8c93ef..c2558d52a1 100644 --- a/erts/epmd/src/epmd_int.h +++ b/erts/epmd/src/epmd_int.h @@ -246,8 +246,10 @@ typedef struct { int fd; /* File descriptor */ - unsigned open:1; /* TRUE if open */ - unsigned keep:1; /* Don't close when sent reply */ + unsigned char open; /* TRUE if open */ + unsigned char keep; /* Don't close when sent reply */ + unsigned char local_peer; /* The peer of this connection is via + loopback interface */ unsigned got; /* # of bytes we have got */ unsigned want; /* Number of bytes we want */ char *buf; /* The remaining buffer */ @@ -287,6 +289,7 @@ typedef struct { int debug; int silent; int is_daemon; + int brutal_kill; unsigned packet_timeout; unsigned delay_accept; unsigned delay_write; @@ -308,6 +311,7 @@ void epmd_call(EpmdVars*,int); void run(EpmdVars*); void epmd_cleanup_exit(EpmdVars*, int); int epmd_conn_close(EpmdVars*,Connection*); +void stop_cli(EpmdVars *g, char *name); #ifdef DONT_USE_MAIN int start_epmd(char *,char *,char *,char *,char *,char *,char *,char *,char *,char *); diff --git a/erts/epmd/src/epmd_srv.c b/erts/epmd/src/epmd_srv.c index 9e470e41b0..3ba8a93fbd 100644 --- a/erts/epmd/src/epmd_srv.c +++ b/erts/epmd/src/epmd_srv.c @@ -386,6 +386,10 @@ static void do_request(g, fd, s, buf, bsize) { case EPMD_ALIVE2_REQ: dbg_printf(g,1,"** got ALIVE2_REQ"); + if (!s->local_peer) { + dbg_printf(g,0,"ALIVE2_REQ from non local address"); + return; + } /* The packet has the format "axxyyyyyy" where xx is port, given in network byte order, and yyyyyy is symname, possibly null @@ -555,6 +559,10 @@ static void do_request(g, fd, s, buf, bsize) case EPMD_DUMP_REQ: dbg_printf(g,1,"** got DUMP_REQ"); + if (!s->local_peer) { + dbg_printf(g,0,"DUMP_REQ from non local address"); + return; + } { Node *node; @@ -604,7 +612,19 @@ static void do_request(g, fd, s, buf, bsize) break; case EPMD_KILL_REQ: + if (!s->local_peer) { + dbg_printf(g,0,"KILL_REQ from non local address"); + return; + } dbg_printf(g,1,"** got KILL_REQ"); + + if (!g->brutal_kill && (g->nodes.reg != NULL)) { + dbg_printf(g,0,"Disallowed KILL_REQ, live nodes"); + if (reply(g, fd,"NO",2) != 2) + dbg_printf(g,0,"failed to send reply to KILL_REQ"); + return; + } + if (reply(g, fd,"OK",2) != 2) dbg_printf(g,0,"failed to send reply to KILL_REQ"); dbg_tty_printf(g,1,"epmd killed"); @@ -614,6 +634,15 @@ static void do_request(g, fd, s, buf, bsize) case EPMD_STOP_REQ: dbg_printf(g,1,"** got STOP_REQ"); + if (!s->local_peer) { + dbg_printf(g,0,"STOP_REQ from non local address"); + return; + } + if (!g->brutal_kill) { + dbg_printf(g,0,"Disallowed STOP_REQ, no relaxed_command_check"); + return; + } + if (bsize <= 1 ) { dbg_printf(g,0,"packet too small for request STOP_REQ (%d)",bsize); @@ -701,6 +730,14 @@ static int conn_open(EpmdVars *g,int fd) for (i = 0; i < g->max_conn; i++) { if (g->conn[i].open == EPMD_FALSE) { + struct sockaddr_in si; +#ifdef HAVE_SOCKLEN_T + socklen_t st; +#else + int st; +#endif + st = sizeof(si); + g->active_conn++; s = &g->conn[i]; @@ -710,6 +747,20 @@ static int conn_open(EpmdVars *g,int fd) s->fd = fd; s->open = EPMD_TRUE; s->keep = EPMD_FALSE; + + /* Determine if connection is from localhost */ + if (getpeername(s->fd,(struct sockaddr*) &si,&st) || + st < sizeof(si)) { + /* Failure to get peername is regarder as non local host */ + s->local_peer = EPMD_FALSE; + } else { + s->local_peer = + ((((unsigned) ntohl(si.sin_addr.s_addr)) & 0xFF000000U) == + 0x7F000000U); /* Only 127.x.x.x allowed, no false positives */ + } + dbg_tty_printf(g,2,(s->local_peer) ? "Local peer connected" : + "Non-local peer connected"); + s->want = 0; /* Currently unknown */ s->got = 0; s->mod_time = current_time(g); /* Note activity */ diff --git a/erts/epmd/test/epmd_SUITE.erl b/erts/epmd/test/epmd_SUITE.erl index 5bcbe96b31..51da934ac4 100644 --- a/erts/epmd/test/epmd_SUITE.erl +++ b/erts/epmd/test/epmd_SUITE.erl @@ -65,7 +65,10 @@ returns_valid_empty_extra/1, returns_valid_populated_extra_with_nulls/1, buffer_overrun_1/1, - buffer_overrun_2/1 + buffer_overrun_2/1, + no_nonlocal_register/1, + no_nonlocal_kill/1, + no_live_killing/1 ]). @@ -125,8 +128,12 @@ all(suite) -> returns_valid_empty_extra, returns_valid_populated_extra_with_nulls, - buffer_overrun_1, - buffer_overrun_2 + buffer_overrun_1, + buffer_overrun_2, + + no_nonlocal_register, + no_nonlocal_kill, + no_live_killing ]. %% @@ -421,7 +428,7 @@ unregister_others_name_1(doc) -> unregister_others_name_1(suite) -> []; unregister_others_name_1(Config) when is_list(Config) -> - ?line ok = epmdrun(), + ?line ok = epmdrun("-relaxed_command_check"), ?line {ok,RSock} = register_node("foo"), ?line {ok,Sock} = connect(), M = [?EPMD_STOP_REQ,"foo"], @@ -438,7 +445,7 @@ unregister_others_name_2(doc) -> unregister_others_name_2(suite) -> []; unregister_others_name_2(Config) when is_list(Config) -> - ?line ok = epmdrun(), + ?line ok = epmdrun("-relaxed_command_check"), ?line {ok,Sock} = connect(), M = [?EPMD_STOP_REQ,"xxx42"], ?line ok = send(Sock,[size16(M),M]), @@ -710,6 +717,7 @@ returns_valid_populated_extra_with_nulls(Config) when is_list(Config) -> ?line ok = close(Sock), ok. +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% buffer_overrun_1(suite) -> []; @@ -762,6 +770,111 @@ alltrue([true|T]) -> alltrue(T); alltrue([_|_]) -> false. +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +no_nonlocal_register(suite) -> + []; +no_nonlocal_register(doc) -> + ["Ensure that we cannot register throug a nonlocal connection"]; +no_nonlocal_register(Config) when is_list(Config) -> + ?line ok = epmdrun(), + ?line {ok,Ifs} = inet:getiflist(), + ?line Addr0 = [ inet:ifget(I, [addr]) || I <- Ifs ], + ?line Addr1 = [ A || {ok,[{addr,A}]} <- Addr0], + ?line Addr = lists:filter(fun({127,_,_,_}) -> + false; + (_) -> + true + end,Addr1), + %% Now we should have all non loopback interface addresses, + %% none should accept a alive2 registration. + ?line Res = lists:map(fun(Ad={A1,A2,A3,A4}) -> + try + Name = "gurka_"++ + integer_to_list(A1)++"_"++ + integer_to_list(A2)++"_"++ + integer_to_list(A3)++"_"++ + integer_to_list(A4), + Bname = list_to_binary(Name), + NameS = byte_size(Bname), + ?line Bin= <<$x:8,4747:16,$M:8,0:8,5:16, + 5:16,NameS:16,Bname/binary, + 0:16>>, + ?line S = size(Bin), + {ok, E} = connect(Ad), + gen_tcp:send(E,[<>,Bin]), + closed = recv(E,1), + gen_tcp:close(E), + true + catch + _:_ -> + false + end + end, Addr), + erlang:display(Res), + ?line true = alltrue(Res), + ok. + +no_nonlocal_kill(suite) -> + []; +no_nonlocal_kill(doc) -> + ["Ensure that we cannot kill through nonlocal connection"]; +no_nonlocal_kill(Config) when is_list(Config) -> + ?line ok = epmdrun(), + ?line {ok,Ifs} = inet:getiflist(), + ?line Addr0 = [ inet:ifget(I, [addr]) || I <- Ifs ], + ?line Addr1 = [ A || {ok,[{addr,A}]} <- Addr0], + ?line Addr = lists:filter(fun({127,_,_,_}) -> + false; + (_) -> + true + end,Addr1), + %% Now we should have all non loopback interface addresses, + %% none should accept a alive2 registration. + ?line Res = lists:map(fun(Ad) -> + try + {ok, E} = connect(Ad), + M = [?EPMD_KILL_REQ], + send(E, [size16(M), M]), + closed = recv(E,2), + gen_tcp:close(E), + sleep(?MEDIUM_PAUSE), + {ok, E2} = connect(Ad), + gen_tcp:close(E2), + true + catch + _:_ -> + false + end + end, Addr), + erlang:display(Res), + ?line true = alltrue(Res), + ok. +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +no_live_killing(doc) -> + ["Dont allow killing with live nodes or any unregistering w/o -relaxed_command_check"]; +no_live_killing(suite) -> + []; +no_live_killing(Config) when is_list(Config) -> + ?line ok = epmdrun(), + ?line {ok,RSock} = register_node("foo"), + ?line {ok,Sock} = connect(), + ?line M = [?EPMD_KILL_REQ], + ?line ok = send(Sock,[size16(M),M]), + ?line {ok,"NO"} = recv(Sock,2), + ?line close(Sock), + ?line {ok,Sock2} = connect(), + ?line M2 = [?EPMD_STOP_REQ,"foo"], + ?line ok = send(Sock2,[size16(M2),M2]), + ?line closed = recv(Sock2,1), + ?line close(Sock2), + ?line close(RSock), + ?line sleep(?MEDIUM_PAUSE), + ?line {ok,Sock3} = connect(), + ?line M3 = [?EPMD_KILL_REQ], + ?line ok = send(Sock3,[size16(M3),M3]), + ?line {ok,"OK"} = recv(Sock3,2), + ?line close(Sock3), + ok. %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% % Terminate all tests with killing epmd. @@ -784,16 +897,24 @@ cleanup() -> % Normal debug start of epmd epmdrun() -> + epmdrun([]). +epmdrun(Args) -> case os:find_executable(epmd) of false -> {error, {could_not_find_epmd_in_path}}; Path -> - epmdrun(Path) + epmdrun(Path,Args) end. -epmdrun(Epmd) -> +epmdrun(Epmd,Args0) -> %% test_server:format("epmdrun() => Epmd = ~p",[Epmd]), - osrun(Epmd ++ " " ?EPMDARGS " -port " ++ integer_to_list(?PORT)). + Args = case Args0 of + [] -> + []; + O -> + " "++O + end, + osrun(Epmd ++ Args ++ " " ?EPMDARGS " -port " ++ integer_to_list(?PORT)). %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% @@ -810,16 +931,19 @@ osrun(Cmd) -> % Passive mode is the default connect() -> - connect(?PORT, passive). + connect("localhost",?PORT, passive). + +connect(Addr) -> + connect(Addr,?PORT, passive). connect_active() -> - connect(?PORT, active). + connect("localhost",?PORT, active). % Try a few times before giving up -connect(Port, Mode) -> - case connect_repeat(?CONN_RETRY, Port, Mode) of +connect(Addr, Port, Mode) -> + case connect_repeat(Addr, ?CONN_RETRY, Port, Mode) of {ok,Sock} -> {ok,Sock}; {error,timeout} -> @@ -836,25 +960,25 @@ connect(Port, Mode) -> % Try a few times before giving up. Pause a small time between % each try. -connect_repeat(1, Port, Mode) -> - connect_mode(Port, Mode); -connect_repeat(Retry, Port, Mode) -> - case connect_mode(Port, Mode) of +connect_repeat(Addr, 1, Port, Mode) -> + connect_mode(Addr,Port, Mode); +connect_repeat(Addr,Retry, Port, Mode) -> + case connect_mode(Addr,Port, Mode) of {ok,Sock} -> {ok,Sock}; {error,Reason} -> test_server:format("connect: error: ~w~n",[Reason]), timer:sleep(?CONN_SLEEP), - connect_repeat(Retry - 1, Port, Mode); + connect_repeat(Addr, Retry - 1, Port, Mode); Any -> test_server:format("connect: unknown message: ~w~n",[Any]), exit(1) end. -connect_mode(Port, active) -> - gen_tcp:connect("localhost", Port, [{packet, 0}], ?CONN_TIMEOUT); -connect_mode(Port, passive) -> - gen_tcp:connect("localhost", Port, [{packet, 0}, {active, false}], +connect_mode(Addr,Port, active) -> + gen_tcp:connect(Addr, Port, [{packet, 0}], ?CONN_TIMEOUT); +connect_mode(Addr, Port, passive) -> + gen_tcp:connect(Addr, Port, [{packet, 0}, {active, false}], ?CONN_TIMEOUT). -- cgit v1.2.3