Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpcap: add timeout configuration on socket connect #1091

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pcap-int.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,8 @@ struct pcap {
get_airpcap_handle_op_t get_airpcap_handle_op;
#endif
cleanup_op_t cleanup_op;

unsigned int sock_open_timeout;
};

/*
Expand Down
31 changes: 26 additions & 5 deletions pcap-rpcap.c
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,12 @@ static int pcap_read_nocb_remote(pcap_t *p, struct pcap_pkthdr *pkt_header, u_ch
/*
* 'fp->rmt_sockdata' has always to be set before calling the select(),
* since it is cleared by the select()
*
* While not strictly necessary, it's best to include the control socket.
* It allows us to check for a connection drop as the data socket may use UDP
* and as such, is without any mean to report back any error to the client.
*/
FD_SET(pr->rmt_sockctrl, &rfds);
FD_SET(pr->rmt_sockdata, &rfds);

#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
Expand All @@ -439,8 +444,22 @@ static int pcap_read_nocb_remote(pcap_t *p, struct pcap_pkthdr *pkt_header, u_ch
}
}

/*
* In the rpcap protocol, once the capture starts, the control socket isn't
* used anymore until the capture ends.
* However, it's the only way to check for connection errors
* as the data socket may uses UDP.
*/
if (FD_ISSET(pr->rmt_sockctrl, &rfds)) {
uint8 byte;
const int nread = sock_recv(pr->rmt_sockctrl, pr->ctrl_ssl, &byte, sizeof(byte),
SOCK_MSG_PEEK | SOCK_EOF_IS_ERROR, p->errbuf, PCAP_ERRBUF_SIZE);
if (nread == -1)
return -1;
}

/* There is no data waiting, so return '0' */
if (retval == 0)
if (!FD_ISSET(pr->rmt_sockdata, &rfds))
return 0;

/*
Expand Down Expand Up @@ -1174,7 +1193,8 @@ static int pcap_startcapture_remote(pcap_t *fp)
goto error_nodiscard;

if ((sockdata = sock_open(addrinfo, SOCKOPEN_SERVER,
1 /* max 1 connection in queue */, fp->errbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
1 /* max 1 connection in queue */, fp->errbuf, PCAP_ERRBUF_SIZE,
fp->sock_open_timeout)) == INVALID_SOCKET)
goto error_nodiscard;

/* addrinfo is no longer used */
Expand Down Expand Up @@ -1297,7 +1317,8 @@ static int pcap_startcapture_remote(pcap_t *fp)
if (sock_initaddress(host, portstring, &hints, &addrinfo, fp->errbuf, PCAP_ERRBUF_SIZE) == -1)
goto error;

if ((sockdata = sock_open(addrinfo, SOCKOPEN_CLIENT, 0, fp->errbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
if ((sockdata = sock_open(addrinfo, SOCKOPEN_CLIENT, 0, fp->errbuf, PCAP_ERRBUF_SIZE,
fp->sock_open_timeout)) == INVALID_SOCKET)
goto error;

/* addrinfo is no longer used */
Expand Down Expand Up @@ -2249,7 +2270,7 @@ rpcap_setup_session(const char *source, struct pcap_rmtauth *auth,
}

if ((*sockctrlp = sock_open(addrinfo, SOCKOPEN_CLIENT, 0,
errbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
errbuf, PCAP_ERRBUF_SIZE, 0)) == INVALID_SOCKET)
{
freeaddrinfo(addrinfo);
return -1;
Expand Down Expand Up @@ -2856,7 +2877,7 @@ SOCKET pcap_remoteact_accept_ex(const char *address, const char *port, const cha
}


if ((sockmain = sock_open(addrinfo, SOCKOPEN_SERVER, 1, errbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
if ((sockmain = sock_open(addrinfo, SOCKOPEN_SERVER, 1, errbuf, PCAP_ERRBUF_SIZE, 0)) == INVALID_SOCKET)
{
freeaddrinfo(addrinfo);
return (SOCKET)-2;
Expand Down
4 changes: 2 additions & 2 deletions rpcapd/daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -2078,7 +2078,7 @@ daemon_msg_startcap_req(uint8 ver, struct daemon_slpars *pars, uint32 plen,
if (sock_initaddress(peerhost, portdata, &hints, &addrinfo, errmsgbuf, PCAP_ERRBUF_SIZE) == -1)
goto error;

if ((session->sockdata = sock_open(addrinfo, SOCKOPEN_CLIENT, 0, errmsgbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
if ((session->sockdata = sock_open(addrinfo, SOCKOPEN_CLIENT, 0, errmsgbuf, PCAP_ERRBUF_SIZE, 0)) == INVALID_SOCKET)
goto error;
}
else // Data connection is opened by the client toward the server
Expand All @@ -2089,7 +2089,7 @@ daemon_msg_startcap_req(uint8 ver, struct daemon_slpars *pars, uint32 plen,
if (sock_initaddress(NULL, "0", &hints, &addrinfo, errmsgbuf, PCAP_ERRBUF_SIZE) == -1)
goto error;

if ((session->sockdata = sock_open(addrinfo, SOCKOPEN_SERVER, 1 /* max 1 connection in queue */, errmsgbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
if ((session->sockdata = sock_open(addrinfo, SOCKOPEN_SERVER, 1 /* max 1 connection in queue */, errmsgbuf, PCAP_ERRBUF_SIZE, 0)) == INVALID_SOCKET)
goto error;

// get the complete sockaddr structure used in the data connection
Expand Down
4 changes: 2 additions & 2 deletions rpcapd/rpcapd.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ void main_startup(void)
SOCKET sock;
struct listen_sock *sock_info;

if ((sock = sock_open(tempaddrinfo, SOCKOPEN_SERVER, SOCKET_MAXCONN, errbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
if ((sock = sock_open(tempaddrinfo, SOCKOPEN_SERVER, SOCKET_MAXCONN, errbuf, PCAP_ERRBUF_SIZE, 0)) == INVALID_SOCKET)
{
switch (tempaddrinfo->ai_family)
{
Expand Down Expand Up @@ -1358,7 +1358,7 @@ main_active(void *ptr)
{
int activeclose;

if ((sockctrl = sock_open(addrinfo, SOCKOPEN_CLIENT, 0, errbuf, PCAP_ERRBUF_SIZE)) == INVALID_SOCKET)
if ((sockctrl = sock_open(addrinfo, SOCKOPEN_CLIENT, 0, errbuf, PCAP_ERRBUF_SIZE, 0)) == INVALID_SOCKET)
{
rpcapd_log(LOGPRIO_DEBUG, "%s", errbuf);

Expand Down
22 changes: 21 additions & 1 deletion sockutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ static int sock_ismcastaddr(const struct sockaddr *saddr)
* if everything is fine, INVALID_SOCKET if some errors occurred. The error message is returned
* in the 'errbuf' variable.
*/
SOCKET sock_open(struct addrinfo *addrinfo, int server, int nconn, char *errbuf, int errbuflen)
SOCKET sock_open(struct addrinfo *addrinfo, int server, int nconn, char *errbuf, int errbuflen, unsigned int timeout_sec)
{
SOCKET sock;
#if defined(SO_NOSIGPIPE) || defined(IPV6_V6ONLY) || defined(IPV6_BINDV6ONLY)
Expand Down Expand Up @@ -422,6 +422,26 @@ SOCKET sock_open(struct addrinfo *addrinfo, int server, int nconn, char *errbuf,
}
else /* we're the client */
{
/* Customize some timeouts to avoid minutes of waiting for nothing.
* Keep the defaults if unset. */
if (timeout_sec != 0) {
struct timeval timeout = { 0 };
timeout.tv_sec = timeout_sec;
if (setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (char *)&timeout, sizeof(timeout)) < 0)
{
sock_geterror("socket(): ", errbuf, errbuflen);
closesocket(sock);
return INVALID_SOCKET;
}
/* needed for the first syn */
if (setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&timeout, sizeof(timeout)) < 0)
{
sock_geterror("socket(): ", errbuf, errbuflen);
closesocket(sock);
return INVALID_SOCKET;
}
}

struct addrinfo *tempaddrinfo;
char *errbufptr;
size_t bufspaceleft;
Expand Down
2 changes: 1 addition & 1 deletion sockutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ int sock_recv(SOCKET sock, SSL *, void *buffer, size_t size, int receiveall,
char *errbuf, int errbuflen);
int sock_recv_dgram(SOCKET sock, SSL *, void *buffer, size_t size,
char *errbuf, int errbuflen);
SOCKET sock_open(struct addrinfo *addrinfo, int server, int nconn, char *errbuf, int errbuflen);
SOCKET sock_open(struct addrinfo *addrinfo, int server, int nconn, char *errbuf, int errbuflen, unsigned int timeout_sec);
int sock_close(SOCKET sock, char *errbuf, int errbuflen);

int sock_send(SOCKET sock, SSL *, const char *buffer, size_t size,
Expand Down