From 00f66063d455a0c402d3e2de578e690afad360d2 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 22 Jun 2012 20:42:05 +0930 Subject: [PATCH 01/14] The number of clients we have is not a warning. --- monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index 13ca8564..277a3c4e 100644 --- a/monitor.c +++ b/monitor.c @@ -371,7 +371,7 @@ monitor_new_socket(int s) { c->state = MONITOR_STATE_COMMAND; monitor_socket_count++; WRITE_STR(s,"\nMONITOR:You are talking to servald\n"); - WHYF("Got %d clients", monitor_socket_count); + INFOF("Got %d clients", monitor_socket_count); } fcntl(monitor_named_socket,F_SETFL, From 510711586fbb03e1ac76200a6e5ab7c29e971856 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 22 Jun 2012 21:47:30 +0930 Subject: [PATCH 02/14] Check the socket isn't too long before copying it to prevent a seg fault. --- overlay_mdp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/overlay_mdp.c b/overlay_mdp.c index 4fd525b8..5e50b286 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -1206,6 +1206,9 @@ int overlay_mdp_client_init() overlay_mdp_client_socket_path_len=strlen(overlay_mdp_client_socket_path)+1; if(debug&DEBUG_IO) DEBUGF("MDP client socket name='%s'",overlay_mdp_client_socket_path); } + if (overlay_mdp_client_socket_path_len > 104 - 1) + FATALF("MDP socket path too long (%d > %d)", overlay_mdp_client_socket_path_len, 104 - 1); + bcopy(overlay_mdp_client_socket_path,name.sun_path, overlay_mdp_client_socket_path_len); unlink(name.sun_path); From c4f19f8a82be9ecce5fdf5aef7b852de1a281d0a Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 00:38:47 +0930 Subject: [PATCH 03/14] Factor out code to fill in a sockaddr_un to a single function and remove overlay_mdp.c's duplicate socket handling (which was incomplete). All platforms except Android use normal sockets (although this is easy to change and the choice is keyed off a single define). This gives consistent handling to unix domain sockets which were previously a mishmash where some were always normal, some always abstract and some chose (but in different ways). It also gives consistent error checking. Doesn't pass tests yet though. --- Android.mk | 2 +- Makefile.in | 4 +- monitor-cli.c | 14 +- monitor.c | 65 +++---- overlay_mdp.c | 484 ++++++++++++++++++++++-------------------------- overlay_route.c | 2 +- serval.h | 10 +- socket.c | 104 +++++++++++ socket.h | 5 + vomp.c | 34 ++-- 10 files changed, 385 insertions(+), 339 deletions(-) create mode 100644 socket.c create mode 100644 socket.h diff --git a/Android.mk b/Android.mk index eb6a0577..c3c443b2 100644 --- a/Android.mk +++ b/Android.mk @@ -63,7 +63,7 @@ SERVALD_LOCAL_CFLAGS = \ -DHAVE_STRING_H=1 -DHAVE_ARPA_INET_H=1 -DHAVE_SYS_SOCKET_H=1 \ -DHAVE_SYS_MMAN_H=1 -DHAVE_SYS_TIME_H=1 -DHAVE_POLL_H=1 -DHAVE_NETDB_H=1 \ -DHAVE_JNI_H=1 -DHAVE_STRUCT_UCRED=1 -DHAVE_CRYPTO_SIGN_NACL_GE25519_H=1 \ - -DBYTE_ORDER=_BYTE_ORDER -DHAVE_LINUX_STRUCT_UCRED \ + -DBYTE_ORDER=_BYTE_ORDER -DHAVE_LINUX_STRUCT_UCRED -DUSE_ABSTRACT_NAMESPACE -I$(NACL_INC) \ -I$(SQLITE3_INC) diff --git a/Makefile.in b/Makefile.in index 11e43b8d..e37c7e53 100644 --- a/Makefile.in +++ b/Makefile.in @@ -51,7 +51,8 @@ SRCS= main.c \ audiodevices.c \ audio_msm_g1.c \ audio_alsa.c \ - audio_reflector.c + audio_reflector.c \ + socket.c HAVE_VOIPTEST= @HAVE_VOIPTEST@ ifeq ($(HAVE_VOIPTEST), 1) @@ -76,6 +77,7 @@ CFLAGS+=-Wall -Wno-unused-value #CFLAGS+=-Wunreachable-code #CFLAGS+=-O0 CFLAGS+=-DDO_TIMING_CHECKS +#CFLAGS+=-DUSE_ABSTRACT_NAMESPACE DEFS= @DEFS@ diff --git a/monitor-cli.c b/monitor-cli.c index 4a0cf3e3..ca2cdf51 100644 --- a/monitor-cli.c +++ b/monitor-cli.c @@ -25,6 +25,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #include #include "serval.h" +#include "socket.h" char cmd[1024]; int cmdLen=0; @@ -63,7 +64,8 @@ int app_monitor_cli(int argc, const char *const *argv, struct command_line_optio const char *sid=NULL; cli_arg(argc, argv, o, "sid", &sid, NULL, ""); struct sockaddr_un addr; - + socklen_t len; + if (!strcasecmp(sid,"reflect")) { pipeAudio=1; reflectAudio=1; sid=""; @@ -75,13 +77,9 @@ int app_monitor_cli(int argc, const char *const *argv, struct command_line_optio } memset(&addr, 0, sizeof(addr)); - addr.sun_family = AF_UNIX; - addr.sun_path[0]=0; - snprintf(&addr.sun_path[1],100,"%s", - confValueGet("monitor.socket",DEFAULT_MONITOR_SOCKET_NAME)); - int len = 1+strlen(&addr.sun_path[1]) + sizeof(addr.sun_family); - char *p=(char *)&addr; - printf("last char='%c' %02x\n",p[len-1],p[len-1]); + socket_setname(&addr, confValueGet("monitor.socket", DEFAULT_MONITOR_SOCKET_NAME), &len); + + INFOF("socket path is \'%s\'", addr.sun_path); if (connect(fd, (struct sockaddr*)&addr, len) == -1) { perror("connect"); diff --git a/monitor.c b/monitor.c index 277a3c4e..4f035ecd 100644 --- a/monitor.c +++ b/monitor.c @@ -24,6 +24,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ #include "serval.h" +#include "socket.h" #include "rhizome.h" #include @@ -62,71 +63,49 @@ int monitor_process_command(int index,char *cmd); int monitor_process_data(int index); static void monitor_new_socket(int s); -int monitor_named_socket=-1; -int monitor_setup_sockets() -{ - struct sockaddr_un name; - int len; +int monitor_named_socket = -1; +int +monitor_setup_sockets(void) { + int reuseP, send_buffer_size; - bzero(&name, sizeof(name)); - name.sun_family = AF_UNIX; - - if (monitor_named_socket!=-1) + if (monitor_named_socket != -1) return 0; /* ignore SIGPIPE so that we don't explode */ signal(SIGPIPE, SIG_IGN); - if ((monitor_named_socket = socket(AF_UNIX, SOCK_STREAM, 0))==-1) { - WHY_perror("socket"); - goto error; - } -#ifdef linux - /* Use abstract namespace as Android has no writable FS which supports sockets. - Abstract namespace is just plain better, anyway, as no dead files end up - hanging around. */ - name.sun_path[0]=0; - /* XXX: 104 comes from OSX sys/un.h - no #define (note Linux has UNIX_PATH_MAX and it's 108(!)) */ - snprintf(&name.sun_path[1],104-2, - confValueGet("monitor.socket",DEFAULT_MONITOR_SOCKET_NAME)); - /* Doesn't include trailing nul */ - len = 1+strlen(&name.sun_path[1]) + sizeof(name.sun_family); -#else - snprintf(name.sun_path,104-1,"%s/%s", - serval_instancepath(), - confValueGet("monitor.socket",DEFAULT_MONITOR_SOCKET_NAME)); - unlink(name.sun_path); - /* Includes trailing nul */ - len = 1+strlen(name.sun_path) + sizeof(name.sun_family); -#endif - - if(bind(monitor_named_socket, (struct sockaddr *)&name, len)==-1) { + if ((monitor_named_socket = socket_bind(confValueGet("monitor.socket", DEFAULT_MONITOR_SOCKET_NAME))) == -1) { WHY_perror("bind"); goto error; } - if(listen(monitor_named_socket,MAX_MONITOR_SOCKETS)==-1) { + + if (listen(monitor_named_socket,MAX_MONITOR_SOCKETS) == -1) { WHY_perror("listen"); goto error; } - int reuseP=1; - if(setsockopt(monitor_named_socket, SOL_SOCKET, SO_REUSEADDR, - &reuseP, sizeof(reuseP)) < 0) { + reuseP = 1; + if (setsockopt(monitor_named_socket, SOL_SOCKET, SO_REUSEADDR, + &reuseP, sizeof(reuseP)) < 0) { WHY_perror("setsockopt"); - WHY("Could not indicate reuse addresses. Not necessarily a problem (yet)"); + goto error; } - int send_buffer_size=64*1024; - if(setsockopt(monitor_named_socket, SOL_SOCKET, SO_RCVBUF, - &send_buffer_size, sizeof(send_buffer_size))==-1) + send_buffer_size = 64 * 1024; + if (setsockopt(monitor_named_socket, SOL_SOCKET, SO_RCVBUF, + &send_buffer_size, sizeof(send_buffer_size)) == -1) { WHY_perror("setsockopt"); - if (debug&(DEBUG_IO|DEBUG_VERBOSE_IO)) WHY("Monitor server socket setup"); + goto error; + } + + if (debug & (DEBUG_IO | DEBUG_VERBOSE_IO)) WHY("Monitor server socket setup"); return 0; error: close(monitor_named_socket); - monitor_named_socket=-1; + monitor_named_socket = -1; + return -1; } diff --git a/overlay_mdp.c b/overlay_mdp.c index 5e50b286..e0c7ea27 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -17,98 +17,46 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ #include "serval.h" +#include "socket.h" #include -int mdp_abstract_socket=-1; -int mdp_named_socket=-1; +int mdp_socket = -1; int overlay_mdp_setup_sockets() { - struct sockaddr_un name; - int len; + struct sockaddr_un name; + socklen_t len; + int reuseP, send_buffer_size; - name.sun_family = AF_UNIX; - -#ifndef HAVE_LINUX_IF_H - /* Abstrack name space (i.e., non-file represented) unix domain sockets are a - linux-only thing. */ - mdp_abstract_socket = -1; -#else - if (mdp_abstract_socket==-1) { - /* Abstract name space unix sockets is a special Linux thing, which is - convenient for us because Android is Linux, but does not have a shared - writable path that is on a UFS partition, so we cannot use traditional - named unix domain sockets. So the abstract name space gives us a solution. */ - name.sun_path[0]=0; - /* XXX The 100 should be replaced with the actual maximum allowed. - Apparently POSIX requires it to be at least 100, but I would still feel - more comfortable with using the appropriate constant. */ - snprintf(&name.sun_path[1],100, - confValueGet("mdp.socket",DEFAULT_MDP_SOCKET_NAME)); - len = 1+strlen(&name.sun_path[1]) + sizeof(name.sun_family); - - mdp_abstract_socket = socket(AF_UNIX, SOCK_DGRAM, 0); - if (mdp_abstract_socket>-1) { - int dud=0; - int reuseP=1; - if(setsockopt( mdp_abstract_socket, SOL_SOCKET, SO_REUSEADDR, - &reuseP, sizeof(reuseP)) < 0) - { - WARN_perror("setsockopt"); - WARN("Could not indicate reuse addresses. Not necessarily a problem (yet)"); - } - int r=bind(mdp_abstract_socket, (struct sockaddr *)&name, len); - if (r) { - WARN_perror("bind"); - dud=1; - r=0; - WARN("bind() of abstract name space socket failed (not an error on non-linux systems"); - } - if (dud) { - close(mdp_abstract_socket); - mdp_abstract_socket=-1; - WHY("Could not open abstract name-space socket (only a problem on Linux)."); - } - - int send_buffer_size=64*1024; - int res = setsockopt(mdp_abstract_socket, SOL_SOCKET, SO_SNDBUF, - &send_buffer_size, sizeof(send_buffer_size)); - } - } -#endif - if (mdp_named_socket==-1) { - if (!form_serval_instance_path(&name.sun_path[0], 100, "mdp.socket")) { - return WHY("Cannot construct name of unix domain socket."); - } - unlink(&name.sun_path[0]); - len = 0+strlen(&name.sun_path[0]) + sizeof(name.sun_family)+1; - mdp_named_socket = socket(AF_UNIX, SOCK_DGRAM, 0); - if (mdp_named_socket>-1) { - int dud=0; - int reuseP=1; - if(setsockopt( mdp_named_socket, SOL_SOCKET, SO_REUSEADDR, - &reuseP, sizeof(reuseP)) < 0) - { - WARN_perror("setsockopt"); - WARN("Could not indicate reuse addresses. Not necessarily a problem (yet)"); - } - int r=bind(mdp_named_socket, (struct sockaddr *)&name, len); - if (r) { dud=1; r=0; WHY("bind() of named unix domain socket failed"); } - if (dud) { - close(mdp_named_socket); - mdp_named_socket=-1; - WHY("Could not open named unix domain socket."); - } + if (mdp_socket != -1) + return 0; - int send_buffer_size=64*1024; - int res = setsockopt(mdp_named_socket, SOL_SOCKET, SO_RCVBUF, - &send_buffer_size, sizeof(send_buffer_size)); - if (res) - WHY_perror("setsockopt"); - } + socket_setname(&name, confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME), &len); + + mdp_socket = socket(AF_UNIX, SOCK_DGRAM, 0); + reuseP = 1; + if(setsockopt(mdp_socket, SOL_SOCKET, SO_REUSEADDR, + &reuseP, sizeof(reuseP)) < 0) { + WARN_perror("setsockopt"); + goto error; + } + if (bind(mdp_socket, (struct sockaddr *)&name, len) == -1) { + WARN_perror("bind"); + goto error; + } + + send_buffer_size = 64 * 1024; + if (setsockopt(mdp_socket, SOL_SOCKET, SO_SNDBUF, + &send_buffer_size, sizeof(send_buffer_size))) { + WHY_perror("setsockopt"); + goto error; } return 0; - + + error: + close(mdp_socket); + mdp_socket = -1; + return -1; } int overlay_mdp_get_fds(struct pollfd *fds,int *fdcount,int fdmax) @@ -116,29 +64,24 @@ int overlay_mdp_get_fds(struct pollfd *fds,int *fdcount,int fdmax) /* Make sure sockets are open */ overlay_mdp_setup_sockets(); - if ((*fdcount)>=fdmax) return -1; - if (mdp_abstract_socket>-1) - { - if (debug&DEBUG_IO) { - fprintf(stderr,"MDP abstract name space socket is poll() slot #%d (fd %d)\n", - *fdcount,mdp_abstract_socket); - } - fds[*fdcount].fd=mdp_abstract_socket; - fds[*fdcount].events=POLLIN; - (*fdcount)++; - } - if ((*fdcount)>=fdmax) return -1; - if (mdp_named_socket>-1) - { - if (debug&DEBUG_IO) { - fprintf(stderr,"MDP named unix domain socket is poll() slot #%d (fd %d)\n", - *fdcount,mdp_named_socket); - } - fds[*fdcount].fd=mdp_named_socket; - fds[*fdcount].events=POLLIN; - (*fdcount)++; - } + /* XXX: Should assert IMO DOC 2012/06/23 */ + if (*fdcount >= fdmax) + return -1; + if (mdp_socket == -1) + return 0; + + if (debug & DEBUG_IO) + INFOF("MDP abstract name space socket is poll() slot #%d (fd %d)", + *fdcount, mdp_socket); + + fds[*fdcount].fd = mdp_socket; + fds[*fdcount].events=POLLIN; + (*fdcount)++; + + /* XXX: as above */ + if (*fdcount >= fdmax) + return -1; return 0; } @@ -482,7 +425,7 @@ int overlay_saw_mdp_frame(int interface, overlay_mdp_frame *mdp,long long now) addr.sun_family=AF_UNIX; errno=0; int len=overlay_mdp_relevant_bytes(mdp); - int r=sendto(mdp_named_socket,mdp,len,0,(struct sockaddr*)&addr,sizeof(addr)); + int r=sendto(mdp_socket,mdp,len,0,(struct sockaddr*)&addr,sizeof(addr)); if (r==overlay_mdp_relevant_bytes(mdp)) { return 0; } @@ -710,7 +653,7 @@ int overlay_mdp_dispatch(overlay_mdp_frame *mdp,int userGeneratedFrameP, if (overlay_mdp_sanitytest_sourceaddr(&mdp->out.src,userGeneratedFrameP, recvaddr,recvaddrlen)) return overlay_mdp_reply_error - (mdp_named_socket, + (mdp_socket, (struct sockaddr_un *)recvaddr, recvaddrlen,8, "Source address is invalid (you must bind to a source address before" @@ -735,7 +678,7 @@ int overlay_mdp_dispatch(overlay_mdp_frame *mdp,int userGeneratedFrameP, NaCl cryptobox keys can be used for signing. */ if (broadcast) { if (!(mdp->packetTypeAndFlags&MDP_NOCRYPT)) - return overlay_mdp_reply_error(mdp_named_socket, + return overlay_mdp_reply_error(mdp_socket, recvaddr,recvaddrlen,5, "Broadcast packets cannot be encrypted "); } @@ -956,116 +899,118 @@ int overlay_mdp_poll() struct sockaddr *recvaddr=(struct sockaddr *)&recvaddrbuffer[0]; socklen_t recvaddrlen=sizeof(recvaddrbuffer); struct sockaddr_un *recvaddr_un=NULL; + + /* XXX: Should call overlay_mdp_setup_sockets? DOC 2012/06/23 */ + if (mdp_socket == -1) + return 0; + + ttl=-1; + bzero((void *)recvaddrbuffer,sizeof(recvaddrbuffer)); + fcntl(mdp_socket, F_SETFL, + fcntl(mdp_socket, F_GETFL, NULL)|O_NONBLOCK); + int len = recvwithttl(mdp_socket,buffer,sizeof(buffer),&ttl, + recvaddr,&recvaddrlen); + recvaddr_un=(struct sockaddr_un *)recvaddr; - if (mdp_named_socket>-1) { - ttl=-1; - bzero((void *)recvaddrbuffer,sizeof(recvaddrbuffer)); - fcntl(mdp_named_socket, F_SETFL, - fcntl(mdp_named_socket, F_GETFL, NULL)|O_NONBLOCK); - int len = recvwithttl(mdp_named_socket,buffer,sizeof(buffer),&ttl, - recvaddr,&recvaddrlen); - recvaddr_un=(struct sockaddr_un *)recvaddr; - - if (len>0) { + if (len>0) { /* Look at overlay_mdp_frame we have received */ overlay_mdp_frame *mdp=(overlay_mdp_frame *)&buffer[0]; switch(mdp->packetTypeAndFlags&MDP_TYPE_MASK) { - case MDP_GOODBYE: - return overlay_mdp_releasebindings(recvaddr_un,recvaddrlen); - case MDP_VOMPEVENT: - return vomp_mdp_event(mdp,recvaddr_un,recvaddrlen); - case MDP_NODEINFO: - return overlay_route_node_info(mdp,recvaddr_un,recvaddrlen); - case MDP_GETADDRS: - { - overlay_mdp_frame mdpreply; + case MDP_GOODBYE: + return overlay_mdp_releasebindings(recvaddr_un,recvaddrlen); + case MDP_VOMPEVENT: + return vomp_mdp_event(mdp,recvaddr_un,recvaddrlen); + case MDP_NODEINFO: + return overlay_route_node_info(mdp,recvaddr_un,recvaddrlen); + case MDP_GETADDRS: + { + overlay_mdp_frame mdpreply; - /* Work out which SIDs to get ... */ - int sid_num=mdp->addrlist.first_sid; - int max_sid=mdp->addrlist.last_sid; - int max_sids=mdp->addrlist.frame_sid_count; - /* ... and constrain list for sanity */ - if (sid_num<0) sid_num=0; - if (max_sids>MDP_MAX_SID_REQUEST) max_sids=MDP_MAX_SID_REQUEST; - if (max_sids<0) max_sids=0; + /* Work out which SIDs to get ... */ + int sid_num=mdp->addrlist.first_sid; + int max_sid=mdp->addrlist.last_sid; + int max_sids=mdp->addrlist.frame_sid_count; + /* ... and constrain list for sanity */ + if (sid_num<0) sid_num=0; + if (max_sids>MDP_MAX_SID_REQUEST) max_sids=MDP_MAX_SID_REQUEST; + if (max_sids<0) max_sids=0; - /* Prepare reply packet */ - mdpreply.packetTypeAndFlags=MDP_ADDRLIST; - mdpreply.addrlist.first_sid=sid_num; - mdpreply.addrlist.last_sid=max_sid; - mdpreply.addrlist.frame_sid_count=max_sids; + /* Prepare reply packet */ + mdpreply.packetTypeAndFlags=MDP_ADDRLIST; + mdpreply.addrlist.first_sid=sid_num; + mdpreply.addrlist.last_sid=max_sid; + mdpreply.addrlist.frame_sid_count=max_sids; - /* Populate with SIDs */ - int i=0; - int count=0; - if (mdp->addrlist.selfP) { - /* from self */ - int cn=0,in=0,kp=0; - while(keyring_next_identity(keyring,&cn,&in,&kp)) { - if (count>=sid_num&&(icontexts[cn]->identities[in] - ->keypairs[kp]->public_key, - mdpreply.addrlist.sids[i++],SID_SIZE); - in++; kp=0; - count++; - if (i>=max_sids) break; - } - } else { - /* from peer list */ - int bin,slot; - i=0; - count=0; - for(bin=0;bin=sid_num)&&(iaddrlist.selfP) { + /* from self */ + int cn=0,in=0,kp=0; + while(keyring_next_identity(keyring,&cn,&in,&kp)) { + if (count>=sid_num&&(icontexts[cn]->identities[in] + ->keypairs[kp]->public_key, + mdpreply.addrlist.sids[i++],SID_SIZE); + in++; kp=0; + count++; + if (i>=max_sids) break; } - count++; - } + } else { + /* from peer list */ + int bin,slot; + i=0; + count=0; + for(bin=0;bin=sid_num)&&(ipacketTypeAndFlags=MDP_ERROR; - mdp->error.error=2; - snprintf(mdp->error.message,128,"Illegal request type. Clients may use only MDP_TX or MDP_BIND."); - int len=4+4+strlen(mdp->error.message)+1; - errno=0; - /* We ignore the result of the following, because it is just sending an - error message back to the client. If this fails, where would we report - the error to? My point exactly. */ - sendto(mdp_named_socket,mdp,len,0,(struct sockaddr *)recvaddr,recvaddrlen); + break; + case MDP_TX: /* Send payload (and don't treat it as system privileged) */ + return overlay_mdp_dispatch(mdp,1,(struct sockaddr_un*)recvaddr,recvaddrlen); + break; + case MDP_BIND: /* Bind to port */ + return overlay_mdp_process_bind_request(mdp_socket,mdp, + recvaddr_un,recvaddrlen); + break; + default: + /* Client is not allowed to send any other frame type */ + WHY("Illegal frame type."); + mdp->packetTypeAndFlags=MDP_ERROR; + mdp->error.error=2; + snprintf(mdp->error.message,128,"Illegal request type. Clients may use only MDP_TX or MDP_BIND."); + int len=4+4+strlen(mdp->error.message)+1; + errno=0; + /* We ignore the result of the following, because it is just sending an + error message back to the client. If this fails, where would we report + the error to? My point exactly. */ + sendto(mdp_socket,mdp,len,0,(struct sockaddr *)recvaddr,recvaddrlen); } - } - - fcntl(mdp_named_socket, F_SETFL, - fcntl(mdp_named_socket, F_GETFL, NULL)&(~O_NONBLOCK)); } + fcntl(mdp_socket, F_SETFL, + fcntl(mdp_socket, F_GETFL, NULL)&(~O_NONBLOCK)); + return 0; } @@ -1116,8 +1061,10 @@ int overlay_mdp_relevant_bytes(overlay_mdp_frame *mdp) int mdp_client_socket=-1; int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms) { - int len=4; - + int len; + socklen_t socklen; + struct sockaddr_un name; + if (mdp_client_socket==-1) if (overlay_mdp_client_init() != 0) return -1; @@ -1126,17 +1073,15 @@ int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms) memory contents. */ len=overlay_mdp_relevant_bytes(mdp); if (len<0) return WHY("MDP frame invalid (could not compute length)"); - - /* Construct name of socket to send to. */ - struct sockaddr_un name; - name.sun_family = AF_UNIX; - if (!FORM_SERVAL_INSTANCE_PATH(name.sun_path, "mdp.socket")) - return -1; - + + socket_setname(&name, confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME), &socklen); + fcntl(mdp_client_socket, F_SETFL, fcntl(mdp_client_socket, F_GETFL, NULL)|O_NONBLOCK); + int result=sendto(mdp_client_socket, mdp, len, 0, - (struct sockaddr *)&name, sizeof(struct sockaddr_un)); + (struct sockaddr *)&name, socklen); + if (result<0) { mdp->packetTypeAndFlags=MDP_ERROR; mdp->error.error=1; @@ -1177,72 +1122,77 @@ int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms) } } -char overlay_mdp_client_socket_path[1024]; -int overlay_mdp_client_socket_path_len=-1; +char overlay_mdp_client_socket_path[1024] = { 0 }; int overlay_mdp_client_init() { - if (mdp_client_socket==-1) { - /* Open socket to MDP server (thus connection is always local) */ - if (0) WHY("Use of abstract name space socket for Linux not implemented"); - - mdp_client_socket = socket(AF_UNIX, SOCK_DGRAM, 0); - if (mdp_client_socket < 0) { - WHY_perror("socket"); - return WHY("Could not open socket to MDP server"); - } - - /* We must bind to a temporary file name */ - struct sockaddr_un name; - unsigned int random_value; - if (urandombytes((unsigned char *)&random_value,sizeof(int))) - return WHY("urandombytes() failed"); - name.sun_family = AF_UNIX; - if (overlay_mdp_client_socket_path_len==-1) { - char fmt[1024]; - if (!FORM_SERVAL_INSTANCE_PATH(fmt, "mdp-client-%d-%08x.socket")) - return WHY("Could not form MDP client socket name"); - snprintf(overlay_mdp_client_socket_path,1024,fmt,getpid(),random_value); - overlay_mdp_client_socket_path_len=strlen(overlay_mdp_client_socket_path)+1; - if(debug&DEBUG_IO) DEBUGF("MDP client socket name='%s'",overlay_mdp_client_socket_path); - } - if (overlay_mdp_client_socket_path_len > 104 - 1) - FATALF("MDP socket path too long (%d > %d)", overlay_mdp_client_socket_path_len, 104 - 1); - - bcopy(overlay_mdp_client_socket_path,name.sun_path, - overlay_mdp_client_socket_path_len); - unlink(name.sun_path); - int len = 1 + strlen(name.sun_path) + sizeof(name.sun_family) + 1; - int r=bind(mdp_client_socket, (struct sockaddr *)&name, len); - if (r) { - WHY_perror("bind"); - return WHY("Could not bind MDP client socket to file name"); - } - - int send_buffer_size=128*1024; - int res = setsockopt(mdp_client_socket, SOL_SOCKET, SO_RCVBUF, - &send_buffer_size, sizeof(send_buffer_size)); - if (res) WHYF("setsockopt() failed: errno=%d",errno); + struct sockaddr_un name; + unsigned int random_value; + int send_buffer_size; + socklen_t len; + + if (mdp_client_socket != -1) + return 0; + + /* Open socket to MDP server (thus connection is always local) */ + mdp_client_socket = socket(AF_UNIX, SOCK_DGRAM, 0); + if (mdp_client_socket < 0) { + WHY_perror("socket"); + goto error; + } + /* We must bind to a temporary file name */ + if (urandombytes((unsigned char *)&random_value,sizeof(int))) { + WHY("urandombytes() failed"); + goto error; } + if (overlay_mdp_client_socket_path[0] == 0) { + snprintf(overlay_mdp_client_socket_path, sizeof(overlay_mdp_client_socket_path), + "mdp-client-%d-%08x.socket", getpid(), random_value); + if(debug & DEBUG_IO) DEBUGF("MDP client socket name='%s'",overlay_mdp_client_socket_path); + } + + socket_setname(&name, overlay_mdp_client_socket_path, &len); + + if (bind(mdp_client_socket, (struct sockaddr *)&name, len) == -1) { + WHY_perror("bind"); + goto error; + } + + send_buffer_size = 128 * 1024; + if (setsockopt(mdp_client_socket, SOL_SOCKET, SO_RCVBUF, + &send_buffer_size, sizeof(send_buffer_size))) { + WHY_perror("setsockopt"); + goto error; + } + return 0; + + error: + close(mdp_client_socket); + mdp_client_socket = -1; + return -1; } -int overlay_mdp_client_done() -{ - if (mdp_client_socket!=-1) { +int +overlay_mdp_client_done(void) { + overlay_mdp_frame mdp; + + if (mdp_client_socket != -1) { /* Tell MDP server to release all our bindings */ - overlay_mdp_frame mdp; mdp.packetTypeAndFlags=MDP_GOODBYE; overlay_mdp_send(&mdp,0,0); } - if (overlay_mdp_client_socket_path_len>-1) - unlink(overlay_mdp_client_socket_path); - if (mdp_client_socket!=-1) + if (overlay_mdp_client_socket_path[0] != 0) + socket_done(overlay_mdp_client_socket_path); + + if (mdp_client_socket != -1) close(mdp_client_socket); - mdp_client_socket=-1; + + mdp_client_socket = -1; + return 0; } @@ -1282,6 +1232,7 @@ int overlay_mdp_recv(overlay_mdp_frame *mdp,int *ttl) /* Null terminate received address so that the stat() call below can succeed */ if (recvaddrlen<1024) recvaddrbuffer[recvaddrlen]=0; if (len>0) { +#ifndef USE_ABSTRACT_NAMESPACE /* Make sure recvaddr matches who we sent it to */ if (strncmp(mdp_socket_name, recvaddr_un->sun_path, sizeof(recvaddr_un->sun_path))) { /* Okay, reply was PROBABLY not from the server, but on OSX if the path @@ -1294,6 +1245,7 @@ int overlay_mdp_recv(overlay_mdp_frame *mdp,int *ttl) if ((sb1.st_ino!=sb2.st_ino)||(sb1.st_dev!=sb2.st_dev)) return WHY("Reply did not come from server"); } +#endif int expected_len = overlay_mdp_relevant_bytes(mdp); diff --git a/overlay_route.c b/overlay_route.c index 5a47a8b2..8e1f97e2 100644 --- a/overlay_route.c +++ b/overlay_route.c @@ -1498,7 +1498,7 @@ int overlay_route_node_info(overlay_mdp_frame *mdp, } } - return overlay_mdp_reply(mdp_named_socket,addr,addrlen,mdp); + return overlay_mdp_reply(mdp_socket,addr,addrlen,mdp); return 0; } diff --git a/serval.h b/serval.h index f04d9047..ce930d6d 100755 --- a/serval.h +++ b/serval.h @@ -1166,8 +1166,7 @@ int overlay_mdp_poll(); int overlay_mdp_reply_error(int sock, struct sockaddr_un *recvaddr,int recvaddrlen, int error_number,char *message); -extern int mdp_abstract_socket; -extern int mdp_named_socket; +extern int mdp_socket; typedef struct sockaddr_mdp { @@ -1539,5 +1538,12 @@ extern int sigIoFlag; void sigPipeHandler(int signal); void sigIoHandler(int signal); +#ifdef USE_ABSTRACT_NAMESPACE +/* Long ones for abstract name space */ #define DEFAULT_MONITOR_SOCKET_NAME "org.servalproject.servald.monitor.socket" #define DEFAULT_MDP_SOCKET_NAME "org.servalproject.servald.mdp.socket" +#else +/* Short ones elsewhere */ +#define DEFAULT_MONITOR_SOCKET_NAME "monitor.socket" +#define DEFAULT_MDP_SOCKET_NAME "mdp.socket" +#endif diff --git a/socket.c b/socket.c new file mode 100644 index 00000000..13ca5c79 --- /dev/null +++ b/socket.c @@ -0,0 +1,104 @@ +/* + Copyright (C) 2012 Daniel O'Connor, Serval Project. + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License + as published by the Free Software Foundation; either version 2 + of the License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +*/ + +#ifdef HAVE_NETINET_IN_H +#include +#endif +#include +#include +#include +#ifdef HAVE_SYS_SOCKET_H +#include +#endif +#include +#include + +#include "serval.h" +#include "socket.h" + +/* Create a socket and bind it to name in the abstract namespace for android or + * $SERVALINSTANCE_PATH/name for everything else. + * + * Use abstract namespace as Android has no writable FS which supports sockets. + * Don't use it for anything else because it makes testing harder (as we can't run + * more than one servald on a given system. +*/ +int +socket_bind(const char *name) { + int s, oerrno; + struct sockaddr_un sockname; + socklen_t len; + + if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) + return -1; + + socket_setname(&sockname, name, &len); + unlink(sockname.sun_path); + + if (bind(s, (struct sockaddr *)&sockname, len) == -1) { + oerrno = errno; + close(s); + errno = oerrno; + return -1; + } + + return s; +} + +/* Set sockname to name handling abstract name space sockets etc. */ +void +socket_setname(struct sockaddr_un *sockname, const char *name, socklen_t *len) { + bzero(sockname, sizeof(*sockname)); + sockname->sun_family = AF_UNIX; + +#ifdef USE_ABSTRACT_NAMESPACE + sockname->sun_path[0] = 0; + /* Note: -2 here not -1 because sprintf will put the trailling nul in */ + *len = snprintf(sockname->sun_path + 1, sizeof(sockname->sun_path) - 2, "%s", name); + if (*len > sizeof(sockname->sun_path) - 2) + FATALF("Socket path too long (%d > %d)", *len, sizeof(sockname->sun_path) - 2); + + /* Doesn't include trailing nul */ + *len = 1 + strlen(sockname->sun_path + 1) + sizeof(sockname->sun_family); +#else + *len = snprintf(sockname->sun_path, sizeof(sockname->sun_path) - 1, "%s/%s", + serval_instancepath(), name); + if (*len > sizeof(sockname->sun_path) - 1) + FATALF("Socket path too long (%d > %d)", *len, sizeof(sockname->sun_path) - 1); + +#ifdef SUN_LEN + *len = SUN_LEN(sockname); +#else + /* Includes trailing nul */ + *len = 1 + strlen(sockname->sun_path) + sizeof(sockname->sun_family); +#endif +#endif +} + +/* Cleanup socket opened by socket_bind */ +void +socket_done(const char *name) { +#ifndef USE_ABSTRACT_NAMESPACE + struct sockaddr_un sockname; + socklen_t len; + + socket_setname(&sockname, name, &len); + unlink(sockname.sun_path); + +#endif +} diff --git a/socket.h b/socket.h new file mode 100644 index 00000000..df18259b --- /dev/null +++ b/socket.h @@ -0,0 +1,5 @@ +int socket_bind(const char *name); +void socket_setname(struct sockaddr_un *sockname, const char *name, socklen_t *len); +void socket_done(const char *name); + + diff --git a/vomp.c b/vomp.c index 4c745617..178d13f8 100644 --- a/vomp.c +++ b/vomp.c @@ -321,7 +321,7 @@ int vomp_send_status(vomp_call_state *call,int flags,overlay_mdp_frame *arg) long long now=overlay_gettime_ms(); for(i=0;i=now) { - overlay_mdp_reply(mdp_named_socket, + overlay_mdp_reply(mdp_socket, vomp_interested_usocks[i], vomp_interested_usock_lengths[i], &mdp); @@ -474,7 +474,7 @@ int vomp_mdp_event(overlay_mdp_frame *mdp, if (!memcmp(recvaddr->sun_path, vomp_interested_usocks[i],recvaddrlen)) /* found it -- so we are already monitoring this one */ - return overlay_mdp_reply_error(mdp_named_socket,recvaddr,recvaddrlen, + return overlay_mdp_reply_error(mdp_socket,recvaddr,recvaddrlen, 0,"Success"); if (vomp_interested_expiries[i]=VOMP_MAX_CALLS) return overlay_mdp_reply_error - (mdp_named_socket,recvaddr,recvaddrlen,4004, + (mdp_socket,recvaddr,recvaddrlen,4004, "All call slots in use"); int slot=vomp_call_count++; vomp_call_state *call=&vomp_call_states[slot]; @@ -618,7 +618,7 @@ int vomp_mdp_event(overlay_mdp_frame *mdp, { if (urandombytes((unsigned char *)&call->local.session,sizeof(int))) return overlay_mdp_reply_error - (mdp_named_socket,recvaddr,recvaddrlen,4005, + (mdp_socket,recvaddr,recvaddrlen,4005, "Insufficient entropy"); call->local.session&=VOMP_SESSION_MASK; printf("session=0x%08x\n",call->local.session); @@ -638,7 +638,7 @@ int vomp_mdp_event(overlay_mdp_frame *mdp, WHY("sending MDP reply back"); dump("recvaddr",(unsigned char *)recvaddr,recvaddrlen); int result= overlay_mdp_reply_error - (mdp_named_socket,recvaddr,recvaddrlen,0, "Success"); + (mdp_socket,recvaddr,recvaddrlen,0, "Success"); if (result) WHY("Failed to send MDP reply"); return result; } @@ -650,12 +650,12 @@ int vomp_mdp_event(overlay_mdp_frame *mdp, =vomp_find_call_by_session(mdp->vompevent.call_session_token); if (!call) return overlay_mdp_reply_error - (mdp_named_socket,recvaddr,recvaddrlen,4006, + (mdp_socket,recvaddr,recvaddrlen,4006, "No such call"); if (call->local.state==VOMP_STATE_INCALL) vomp_call_stop_audio(call); call->local.state=VOMP_STATE_CALLENDED; monitor_call_status(call); - overlay_mdp_reply_error(mdp_named_socket, + overlay_mdp_reply_error(mdp_socket, recvaddr,recvaddrlen,0,"Success"); return vomp_send_status(call,VOMP_TELLREMOTE|VOMP_TELLINTERESTED,NULL); } @@ -667,7 +667,7 @@ int vomp_mdp_event(overlay_mdp_frame *mdp, =vomp_find_call_by_session(mdp->vompevent.call_session_token); if (!call) return overlay_mdp_reply_error - (mdp_named_socket,recvaddr,recvaddrlen,4006, + (mdp_socket,recvaddr,recvaddrlen,4006, "No such call"); if (call->local.state==VOMP_STATE_RINGINGIN) { call->local.state=VOMP_STATE_INCALL; @@ -675,11 +675,11 @@ int vomp_mdp_event(overlay_mdp_frame *mdp, call->ringing=0; /* state machine does job of starting audio stream, just tell everyone about the changed state. */ - overlay_mdp_reply_error(mdp_named_socket, + overlay_mdp_reply_error(mdp_socket, recvaddr,recvaddrlen,0,"Success"); return vomp_send_status(call,VOMP_TELLREMOTE|VOMP_TELLINTERESTED,NULL); } else { - overlay_mdp_reply_error(mdp_named_socket, + overlay_mdp_reply_error(mdp_socket, recvaddr,recvaddrlen,4009, "Call is not RINGINGIN, so cannot be picked up"); } @@ -699,7 +699,7 @@ int vomp_mdp_event(overlay_mdp_frame *mdp, break; default: /* didn't understand it, so respond with an error */ - return overlay_mdp_reply_error(mdp_named_socket, + return overlay_mdp_reply_error(mdp_socket, recvaddr,recvaddrlen,4001, "Invalid VOMPEVENT request (use DIAL,HANGUP,CALLREJECT,AUDIOSTREAMING,REGISTERINTERST,WITHDRAWINTERST only)"); From 523c9ad49f0ef272ae5dfa3cd92a4e32ebbdb58e Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 00:40:54 +0930 Subject: [PATCH 04/14] Push debug knobs into a separate makefile so I don't have to run configure when I change them. --- Makefile.dbg | 4 ++++ Makefile.in | 5 +---- 2 files changed, 5 insertions(+), 4 deletions(-) create mode 100644 Makefile.dbg diff --git a/Makefile.dbg b/Makefile.dbg new file mode 100644 index 00000000..79b0c106 --- /dev/null +++ b/Makefile.dbg @@ -0,0 +1,4 @@ +#CFLAGS+=-Wunreachable-code +#CFLAGS+=-O0 +#CFLAGS+=-DDO_TIMING_CHECKS +#CFLAGS+=-DUSE_ABSTRACT_NAMESPACE diff --git a/Makefile.in b/Makefile.in index e37c7e53..5abf8ca5 100644 --- a/Makefile.in +++ b/Makefile.in @@ -74,10 +74,7 @@ LDFLAGS=@LDFLAGS@ @PORTAUDIO_LIBS@ @SRC_LIBS@ @SPANDSP_LIBS@ @CODEC2_LIBS@ @PTHR CFLAGS= @CPPFLAGS@ @CFLAGS@ @PORTAUDIO_CFLAGS@ @SRC_CFLAGS@ @SPANDSP_CFLAGS@ @PTHREAD_CFLAGS@ $(VOIPTEST_CFLAGS) CFLAGS+=-Wall -Wno-unused-value -#CFLAGS+=-Wunreachable-code -#CFLAGS+=-O0 -CFLAGS+=-DDO_TIMING_CHECKS -#CFLAGS+=-DUSE_ABSTRACT_NAMESPACE +-include Makefile.dbg DEFS= @DEFS@ From aac5a4c275032a7387c0ba1c10907b9ab096d644 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 13:41:49 +0930 Subject: [PATCH 05/14] Set c-basic-offset to 2 for all files. --- .dir-locals.el | 1 + 1 file changed, 1 insertion(+) create mode 100644 .dir-locals.el diff --git a/.dir-locals.el b/.dir-locals.el new file mode 100644 index 00000000..fe1458c4 --- /dev/null +++ b/.dir-locals.el @@ -0,0 +1 @@ +((c-mode . ((c-basic-offset . 2)))) \ No newline at end of file From 4861ff92529cb5ea729cfec6452347aeaf33062b Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 14:24:58 +0930 Subject: [PATCH 06/14] Call a function to cleanup various sockets properly. --- monitor.c | 6 ++++++ overlay_mdp.c | 7 ++++++- serval.h | 2 ++ server.c | 11 ++++------- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/monitor.c b/monitor.c index 4f035ecd..65bd368c 100644 --- a/monitor.c +++ b/monitor.c @@ -109,6 +109,12 @@ monitor_setup_sockets(void) { return -1; } +void +monitor_cleanup_sockets(void) { + socket_done(confValueGet("monitor.socket", DEFAULT_MONITOR_SOCKET_NAME)); +} + + int monitor_get_fds(struct pollfd *fds,int *fdcount,int fdmax) { /* Make sure sockets are open */ diff --git a/overlay_mdp.c b/overlay_mdp.c index e0c7ea27..32a77a1e 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -1175,10 +1175,15 @@ int overlay_mdp_client_init() return -1; } +void +overlay_mdp_server_done(void) { + socket_done(confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME)); +} + int overlay_mdp_client_done(void) { overlay_mdp_frame mdp; - + if (mdp_client_socket != -1) { /* Tell MDP server to release all our bindings */ mdp.packetTypeAndFlags=MDP_GOODBYE; diff --git a/serval.h b/serval.h index ce930d6d..1c41c116 100755 --- a/serval.h +++ b/serval.h @@ -1317,6 +1317,7 @@ int keyring_mapping_request(keyring_file *k,overlay_mdp_frame *req); extern int mdp_client_socket; int overlay_mdp_client_init(); int overlay_mdp_client_done(); +void overlay_mdp_server_done(); int overlay_mdp_client_poll(long long timeout_ms); int overlay_mdp_recv(overlay_mdp_frame *mdp,int *ttl); int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms); @@ -1481,6 +1482,7 @@ int app_monitor_cli(int argc, const char *const *argv, struct command_line_optio int monitor_get_fds(struct pollfd *fds,int *fdcount,int fdmax); int monitor_setup_sockets(); +void monitor_cleanup_sockets(void); int monitor_poll(); int monitor_get_fds(struct pollfd *fds,int *fdcount,int fdmax); int monitor_call_status(vomp_call_state *call); diff --git a/server.c b/server.c index 17234599..49e3394e 100644 --- a/server.c +++ b/server.c @@ -289,13 +289,10 @@ void serverCleanUp() char filename[1024]; if (FORM_SERVAL_INSTANCE_PATH(filename, PIDFILE_NAME)) unlink(filename); - if (mdp_client_socket==-1) { - if (FORM_SERVAL_INSTANCE_PATH(filename, "mdp.socket")) { - unlink(filename); - } - } else { - overlay_mdp_client_done(); - } + + overlay_mdp_server_done(); + overlay_mdp_client_done(); + monitor_cleanup_sockets(); } static void signame(char *buf, size_t len, int signal) From 719d1fe72cc25ecbd73f00189595d5d1936141a4 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 14:25:58 +0930 Subject: [PATCH 07/14] Add option to set SO_REUSEADDR to socket_bind() and use it. --- monitor.c | 2 +- overlay_mdp.c | 23 ++++++----------------- socket.c | 13 +++++++++++-- socket.h | 2 +- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/monitor.c b/monitor.c index 65bd368c..6c48ecf9 100644 --- a/monitor.c +++ b/monitor.c @@ -74,7 +74,7 @@ monitor_setup_sockets(void) { /* ignore SIGPIPE so that we don't explode */ signal(SIGPIPE, SIG_IGN); - if ((monitor_named_socket = socket_bind(confValueGet("monitor.socket", DEFAULT_MONITOR_SOCKET_NAME))) == -1) { + if ((monitor_named_socket = socket_bind(confValueGet("monitor.socket", DEFAULT_MONITOR_SOCKET_NAME), 0)) == -1) { WHY_perror("bind"); goto error; } diff --git a/overlay_mdp.c b/overlay_mdp.c index 32a77a1e..e4093bd9 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -21,27 +21,16 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. #include int mdp_socket = -1; -int overlay_mdp_setup_sockets() -{ - struct sockaddr_un name; - socklen_t len; - int reuseP, send_buffer_size; +int +overlay_mdp_setup_sockets(void) { + int send_buffer_size; if (mdp_socket != -1) return 0; - socket_setname(&name, confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME), &len); - - mdp_socket = socket(AF_UNIX, SOCK_DGRAM, 0); - reuseP = 1; - if(setsockopt(mdp_socket, SOL_SOCKET, SO_REUSEADDR, - &reuseP, sizeof(reuseP)) < 0) { - WARN_perror("setsockopt"); - goto error; - } - if (bind(mdp_socket, (struct sockaddr *)&name, len) == -1) { - WARN_perror("bind"); - goto error; + if ((mdp_socket = socket_bind(confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME), 1)) == -1) { + WHY_perror("socket_bind"); + goto error; } send_buffer_size = 64 * 1024; diff --git a/socket.c b/socket.c index 13ca5c79..f9086a40 100644 --- a/socket.c +++ b/socket.c @@ -39,14 +39,23 @@ * more than one servald on a given system. */ int -socket_bind(const char *name) { - int s, oerrno; +socket_bind(const char *name, int reuse) { + int s, oerrno, reuseP; struct sockaddr_un sockname; socklen_t len; if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) return -1; + if (reuse) { + reuseP = 1; + if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR, + &reuseP, sizeof(reuseP)) < 0) { + close(s); + return -1; + } + } + socket_setname(&sockname, name, &len); unlink(sockname.sun_path); diff --git a/socket.h b/socket.h index df18259b..0b86e70d 100644 --- a/socket.h +++ b/socket.h @@ -1,4 +1,4 @@ -int socket_bind(const char *name); +int socket_bind(const char *name, int reuse); void socket_setname(struct sockaddr_un *sockname, const char *name, socklen_t *len); void socket_done(const char *name); From 626ddee8da2bc5b0538e32f17497be9f0a7e571b Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 14:26:43 +0930 Subject: [PATCH 08/14] Change monitor.socket and mdp.socket prefs to be a suffix which is applied to the instance directory (normal) or org.servalproject.servald (abstract) --- serval.h | 10 ++++------ socket.c | 3 ++- tests/dnaprotocol | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/serval.h b/serval.h index 1c41c116..371a6b89 100755 --- a/serval.h +++ b/serval.h @@ -1541,11 +1541,9 @@ void sigPipeHandler(int signal); void sigIoHandler(int signal); #ifdef USE_ABSTRACT_NAMESPACE -/* Long ones for abstract name space */ -#define DEFAULT_MONITOR_SOCKET_NAME "org.servalproject.servald.monitor.socket" -#define DEFAULT_MDP_SOCKET_NAME "org.servalproject.servald.mdp.socket" -#else -/* Short ones elsewhere */ +#define DEFAULT_ABSTRACT_PREFIX "org.servalproject.servald" +#endif + #define DEFAULT_MONITOR_SOCKET_NAME "monitor.socket" #define DEFAULT_MDP_SOCKET_NAME "mdp.socket" -#endif + diff --git a/socket.c b/socket.c index f9086a40..2e6d36ef 100644 --- a/socket.c +++ b/socket.c @@ -78,7 +78,8 @@ socket_setname(struct sockaddr_un *sockname, const char *name, socklen_t *len) { #ifdef USE_ABSTRACT_NAMESPACE sockname->sun_path[0] = 0; /* Note: -2 here not -1 because sprintf will put the trailling nul in */ - *len = snprintf(sockname->sun_path + 1, sizeof(sockname->sun_path) - 2, "%s", name); + *len = snprintf(sockname->sun_path + 1, sizeof(sockname->sun_path) - 2, "%s.%s", + DEFAULT_ABSTRACT_PREFIX, name); if (*len > sizeof(sockname->sun_path) - 2) FATALF("Socket path too long (%d > %d)", *len, sizeof(sockname->sun_path) - 2); diff --git a/tests/dnaprotocol b/tests/dnaprotocol index 6a07befd..d8194cab 100755 --- a/tests/dnaprotocol +++ b/tests/dnaprotocol @@ -37,8 +37,8 @@ setup_servald_instance() { push_instance set_instance "$1" executeOk_servald config set interfaces "+>$2" - executeOk_servald config set monitor.socket "org.servalproject.servald.monitor.socket.$1" - executeOk_servald config set mdp.socket "org.servalproject.servald.mdp.socket.$1" + executeOk_servald config set monitor.socket "monitor.socket.$1" + executeOk_servald config set mdp.socket "mdp.socket.$1" executeOk_servald keyring add assert [ -e "$SERVALINSTANCE_PATH/serval.keyring" ] executeOk_servald keyring list From 38bf7c3a67cf12263c39d11fd9f5e9c60da0b41a Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 14:32:26 +0930 Subject: [PATCH 09/14] Don't hand roll socket name generation. Don't check if it's abstract (since the paths start with nul it was a NOP previously anyway). --- overlay_mdp.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/overlay_mdp.c b/overlay_mdp.c index e4093bd9..2762cde4 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -1210,12 +1210,16 @@ int overlay_mdp_client_poll(long long timeout_ms) int overlay_mdp_recv(overlay_mdp_frame *mdp,int *ttl) { - char mdp_socket_name[101]; - if (!FORM_SERVAL_INSTANCE_PATH(mdp_socket_name, "mdp.socket")) - return WHY("Could not find mdp socket"); + struct sockaddr_un mdpname; + socklen_t mdplen; + + socket_setname(&mdpname, confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME), &mdplen); + /* Check if reply available */ fcntl(mdp_client_socket, F_SETFL, fcntl(mdp_client_socket, F_GETFL, NULL)|O_NONBLOCK); + /* XXX: Why not just pass in a struct sockaddr_un? DOC 2012/06/24 */ unsigned char recvaddrbuffer[1024]; + bzero(recvaddrbuffer, sizeof(recvaddrbuffer)); struct sockaddr *recvaddr=(struct sockaddr *)recvaddrbuffer; unsigned int recvaddrlen=sizeof(recvaddrbuffer); struct sockaddr_un *recvaddr_un; @@ -1223,24 +1227,23 @@ int overlay_mdp_recv(overlay_mdp_frame *mdp,int *ttl) int len = recvwithttl(mdp_client_socket,(unsigned char *)mdp, sizeof(overlay_mdp_frame),ttl,recvaddr,&recvaddrlen); recvaddr_un=(struct sockaddr_un *)recvaddr; - /* Null terminate received address so that the stat() call below can succeed */ - if (recvaddrlen<1024) recvaddrbuffer[recvaddrlen]=0; + if (len>0) { -#ifndef USE_ABSTRACT_NAMESPACE /* Make sure recvaddr matches who we sent it to */ - if (strncmp(mdp_socket_name, recvaddr_un->sun_path, sizeof(recvaddr_un->sun_path))) { +#ifndef USE_ABSTRACT_NAMESPACE + if (strncmp(mdpname.sun_path, recvaddr_un->sun_path, sizeof(recvaddr_un->sun_path))) { /* Okay, reply was PROBABLY not from the server, but on OSX if the path has a symlink in it, it is resolved in the reply path, but might not be in the request path (mdp_socket_name), thus we need to stat() and compare inode numbers etc */ struct stat sb1,sb2; - if (stat(mdp_socket_name,&sb1)) return WHY("stat(mdp_socket_name) failed, so could not verify that reply came from MDP server"); + if (stat(mdpname.sun_path,&sb1)) return WHY("stat(mdp_socket_name) failed, so could not verify that reply came from MDP server"); if (stat(recvaddr_un->sun_path,&sb2)) return WHY("stat(ra->sun_path) failed, so could not verify that reply came from MDP server"); if ((sb1.st_ino!=sb2.st_ino)||(sb1.st_dev!=sb2.st_dev)) return WHY("Reply did not come from server"); } #endif - + /* XXX: Should check abstract namespace socket - DOC 2012/06/24 */ int expected_len = overlay_mdp_relevant_bytes(mdp); if (len < expected_len){ From 4f94f8701aae6abd82d8521aa4db321c2d65ee7c Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 14:41:42 +0930 Subject: [PATCH 10/14] Add socket.h to HDRS --- Makefile.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile.in b/Makefile.in index 5abf8ca5..a8adafe4 100644 --- a/Makefile.in +++ b/Makefile.in @@ -68,7 +68,8 @@ HDRS= fifo.h \ serval.h \ strbuf.h \ sha2.h \ - sqlite-amalgamation-3070900/sqlite3.h + sqlite-amalgamation-3070900/sqlite3.h \ + socket.h LDFLAGS=@LDFLAGS@ @PORTAUDIO_LIBS@ @SRC_LIBS@ @SPANDSP_LIBS@ @CODEC2_LIBS@ @PTHREAD_LIBS@ From 79d5563344d020814e384e53f1338c21e63dc8f0 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 14:44:43 +0930 Subject: [PATCH 11/14] Fix 719d1fe72cc25ecbd73f00189595d5d1936141a4 (SOCK_DGRAM vs SOCK_STREAM) --- monitor.c | 3 ++- overlay_mdp.c | 2 +- socket.c | 4 ++-- socket.h | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/monitor.c b/monitor.c index 6c48ecf9..0cb23646 100644 --- a/monitor.c +++ b/monitor.c @@ -74,7 +74,8 @@ monitor_setup_sockets(void) { /* ignore SIGPIPE so that we don't explode */ signal(SIGPIPE, SIG_IGN); - if ((monitor_named_socket = socket_bind(confValueGet("monitor.socket", DEFAULT_MONITOR_SOCKET_NAME), 0)) == -1) { + if ((monitor_named_socket = socket_bind(confValueGet("monitor.socket", DEFAULT_MONITOR_SOCKET_NAME), + SOCK_STREAM, 0)) == -1) { WHY_perror("bind"); goto error; } diff --git a/overlay_mdp.c b/overlay_mdp.c index 2762cde4..7f325320 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -28,7 +28,7 @@ overlay_mdp_setup_sockets(void) { if (mdp_socket != -1) return 0; - if ((mdp_socket = socket_bind(confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME), 1)) == -1) { + if ((mdp_socket = socket_bind(confValueGet("mdp.socket", DEFAULT_MDP_SOCKET_NAME), SOCK_DGRAM, 1)) == -1) { WHY_perror("socket_bind"); goto error; } diff --git a/socket.c b/socket.c index 2e6d36ef..0038b3a3 100644 --- a/socket.c +++ b/socket.c @@ -39,12 +39,12 @@ * more than one servald on a given system. */ int -socket_bind(const char *name, int reuse) { +socket_bind(const char *name, int type, int reuse) { int s, oerrno, reuseP; struct sockaddr_un sockname; socklen_t len; - if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) + if ((s = socket(AF_UNIX, type, 0)) == -1) return -1; if (reuse) { diff --git a/socket.h b/socket.h index 0b86e70d..7d3fdb0d 100644 --- a/socket.h +++ b/socket.h @@ -1,4 +1,4 @@ -int socket_bind(const char *name, int reuse); +int socket_bind(const char *name, int type, int reuse); void socket_setname(struct sockaddr_un *sockname, const char *name, socklen_t *len); void socket_done(const char *name); From fbc0a9b0b0f608fb44f359a3f69b0ec40e649247 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 17:22:26 +0930 Subject: [PATCH 12/14] Whitespace --- socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/socket.c b/socket.c index 0038b3a3..eb56ae61 100644 --- a/socket.c +++ b/socket.c @@ -40,7 +40,7 @@ */ int socket_bind(const char *name, int type, int reuse) { - int s, oerrno, reuseP; + int s, oerrno, reuseP; struct sockaddr_un sockname; socklen_t len; From 04986bf4939efe1c1ab8c7f89fa64eec83f0d801 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 17:27:50 +0930 Subject: [PATCH 13/14] Comment out & convert a printf to INFOF. --- overlay_mdp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/overlay_mdp.c b/overlay_mdp.c index 7f325320..ba8cb738 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -409,7 +409,7 @@ int overlay_saw_mdp_frame(int interface, overlay_mdp_frame *mdp,long long now) } if (match>-1) { struct sockaddr_un addr; - printf("unix domain socket '%s'\n",mdp_bindings_sockets[match]); + //INFOF("unix domain socket '%s'\n",mdp_bindings_sockets[match]); bcopy(mdp_bindings_sockets[match],&addr.sun_path[0],mdp_bindings_socket_name_lengths[match]); addr.sun_family=AF_UNIX; errno=0; From 15eb6c5705aad64a4c3aab413bc2bcc20d9282c7 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Sun, 24 Jun 2012 17:32:47 +0930 Subject: [PATCH 14/14] Convert printf/fprintf to INFOF/WHYF --- overlay_mdp.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/overlay_mdp.c b/overlay_mdp.c index ba8cb738..3b0613f7 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -409,7 +409,7 @@ int overlay_saw_mdp_frame(int interface, overlay_mdp_frame *mdp,long long now) } if (match>-1) { struct sockaddr_un addr; - //INFOF("unix domain socket '%s'\n",mdp_bindings_sockets[match]); + //INFOF("unix domain socket '%s'",mdp_bindings_sockets[match]); bcopy(mdp_bindings_sockets[match],&addr.sun_path[0],mdp_bindings_socket_name_lengths[match]); addr.sun_family=AF_UNIX; errno=0; @@ -420,7 +420,7 @@ int overlay_saw_mdp_frame(int interface, overlay_mdp_frame *mdp,long long now) } if (errno==ENOENT) { /* far-end of socket has died, so drop binding */ - printf("Closing dead MDP client '%s'\n",mdp_bindings_sockets[match]); + INFOF("Closing dead MDP client '%s'",mdp_bindings_sockets[match]); overlay_mdp_releasebindings(&addr,mdp_bindings_socket_name_lengths[match]); } WHY_perror("sendto(e)"); @@ -620,10 +620,10 @@ int overlay_mdp_sanitytest_sourceaddr(sockaddr_mdp *src,int userGeneratedFrameP, } } - printf("addr=%s port=%u (0x%x)\n", - overlay_render_sid(src->sid),src->port,src->port); - if (recvaddr) printf("recvaddr='%s'\n", - recvaddr->sun_path); + INFOF("addr=%s port=%u (0x%x)", + overlay_render_sid(src->sid),src->port,src->port); + if (recvaddr) INFOF("recvaddr='%s'", + recvaddr->sun_path); return WHY("No such socket binding:unix domain socket tuple exists -- someone might be trying to spoof someone else's connection"); } @@ -751,8 +751,8 @@ int overlay_mdp_dispatch(overlay_mdp_frame *mdp,int userGeneratedFrameP, dump("nonce",nonce,crypto_box_curve25519xsalsa20poly1305_NONCEBYTES); dump("plain text",&plain[16],cipher_len-16); dump("cipher text",cipher_text,cipher_len-16); - printf("frame->payload->length=%d,cipher_len-16=%d,cipher_offset=%d\n", - frame->payload->length,cipher_len-16,cipher_offset); + INFOF("frame->payload->length=%d,cipher_len-16=%d,cipher_offset=%d", + frame->payload->length,cipher_len-16,cipher_offset); dump("frame",&frame->payload->bytes[0], frame->payload->length); } @@ -1266,10 +1266,10 @@ int overlay_mdp_bind(unsigned char *localaddr,int port) int result=overlay_mdp_send(&mdp,MDP_AWAITREPLY,5000); if (result) { if (mdp.packetTypeAndFlags==MDP_ERROR) - fprintf(stderr,"Could not bind to MDP port %d: error=%d, message='%s'\n", - port,mdp.error.error,mdp.error.message); + WHYF("Could not bind to MDP port %d: error=%d, message='%s'", + port,mdp.error.error,mdp.error.message); else - fprintf(stderr,"Could not bind to MDP port %d (no reason given)\n",port); + WHYF("Could not bind to MDP port %d (no reason given)",port); return -1; } return 0; @@ -1288,12 +1288,12 @@ int overlay_mdp_getmyaddr(int index,unsigned char *sid) if (result) { if (a.packetTypeAndFlags==MDP_ERROR) { - fprintf(stderr,"Could not get list of local MDP addresses\n"); - fprintf(stderr," MDP Server error #%d: '%s'\n", - a.error.error,a.error.message); + WHYF("Could not get list of local MDP addresses"); + WHYF(" MDP Server error #%d: '%s'", + a.error.error,a.error.message); } else - fprintf(stderr,"Could not get list of local MDP addresses\n"); + WARNF("Could not get list of local MDP addresses"); return WHY("Failed to get local address list"); } if ((a.packetTypeAndFlags&MDP_TYPE_MASK)!=MDP_ADDRLIST)