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

Use UPnP to punch holes in NATs #142

Open
wants to merge 4 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions clients/zctl/zctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ main(int argc,
if (code)
fprintf (stderr, "%s: %s: %s\n",
argv[0], error_message (code), ssline);
ZClosePort();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Is this just to close the UPnP session before we exit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, otherwise the UPnP port mapping will persist on the router until you reboot the router.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really making me want to have a proper zephyr_ctx that we can manage the lifecycle with. It's a bit annoying we can't easily test for these sorts of port leaks. Not something to address now, of course.

exit((code != 0));
}

Expand All @@ -194,6 +195,7 @@ main(int argc,
#else
run_command(argc-1, argv+1);
#endif
ZClosePort();
exit(0);
}

Expand Down
4 changes: 3 additions & 1 deletion clients/zstat/zstat.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ const char *hm_head[] = {
"Looking for a new server:",
"Time running:",
"Size:",
"Machine type:"
"Machine type:",
"External IP:",
"UPnP IGD Root URL:",
};
#define HM_SIZE (sizeof(hm_head) / sizeof (char *))
const char *srv_head[] = {
Expand Down
7 changes: 6 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,12 @@ if test "x$with_ares" != "xno"; then
AC_MSG_ERROR(libcares not found)))
fi
AC_SUBST(ARES_LIBS)


PKG_CHECK_MODULES([UPNP], [miniupnpc], [AC_DEFINE([HAVE_UPNP], [1], [Use UPnP])], [])
CFLAGS="$UPNP_CFLAGS $CFLAGS"
LIBS="$UPNP_LIBS $LIBS"
LIBZEPHYR_LIBS="$LIBZEPHYR_LIBS $UPNP_LIBS"

AC_PROG_GCC_TRADITIONAL
AC_FUNC_VPRINTF
AC_FUNC_GETPGRP
Expand Down
7 changes: 7 additions & 0 deletions h/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ extern int __subscriptions_next;

extern int __Zephyr_port; /* Port number */
extern struct in_addr __My_addr;
extern struct in_addr __My_addr_internal;
extern int __UPnP_active;
extern char* __UPnP_rooturl;
extern int __Zephyr_fd;
extern int __Q_CompleteLength;
extern struct sockaddr_in __HM_addr;
Expand Down Expand Up @@ -205,4 +208,8 @@ Code_t ZFormatAuthenticNotice(ZNotice_t*, char*, int, int*, C_Block);
#define Z_tktprinc(tkt) ((tkt)->client)
#endif

void Z_InitUPnP_ZHM();
void Z_InitUPnP();
void Z_CloseUPnP();

#endif /* __INTERNAL_H__ */
2 changes: 1 addition & 1 deletion lib/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ OBJS = zephyr_err.lo ZAsyncLocate.lo ZCkAuth.lo ZCkIfNot.lo ZClosePort.lo \
ZSendPkt.lo ZSendRaw.lo ZSendRLst.lo ZSetDest.lo ZSetFD.lo ZSetSrv.lo \
ZSubs.lo ZVariables.lo ZWait4Not.lo Zinternal.lo ZMakeZcode.lo \
ZReadZcode.lo ZCkZAut.lo quad_cksum.lo charset.lo ZExpnRlm.lo \
ZDumpSession.lo
ZDumpSession.lo ZUPnP.lo

.SUFFIXES: .lo

Expand Down
15 changes: 10 additions & 5 deletions lib/ZClosePort.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,16 @@ static const char rcsid_ZClosePort_c[] = "$Id$";
Code_t
ZClosePort(void)
{
if (__Zephyr_fd >= 0 && __Zephyr_open)
(void) close(__Zephyr_fd);
if (__Zephyr_fd >= 0 && __Zephyr_open) {
(void) close(__Zephyr_fd);

__Zephyr_fd = -1;
__Zephyr_open = 0;
#ifdef Z_DEBUG
Z_debug_stderr("ZClosePort() closed port %d", ntohs(__Zephyr_port));
#endif
}
Z_CloseUPnP();
__Zephyr_fd = -1;
__Zephyr_open = 0;

return (ZERR_NONE);
return (ZERR_NONE);
}
48 changes: 43 additions & 5 deletions lib/ZInit.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@ static const char rcsid_ZInitialize_c[] =

#include <internal.h>

#include <arpa/inet.h>
#include <sys/socket.h>
#ifdef HAVE_KRB4
#include <krb_err.h>
#endif
#ifdef HAVE_KRB5
#include <krb5.h>
#endif
#if defined(__APPLE__) && defined(__MACH__)
#include "nwi/network_state_information_priv.h"
#endif

#ifndef INADDR_NONE
#define INADDR_NONE 0xffffffff
Expand Down Expand Up @@ -55,6 +59,8 @@ ZInitialize(void)
int s;
Code_t code;
ZNotice_t notice;
int nf;
char* mp;
#ifdef HAVE_KRB5
char **krealms = NULL;
#else
Expand Down Expand Up @@ -112,6 +118,8 @@ ZInitialize(void)
code will fall back to something which might not be "right",
but this is is ok, since none of the servers call krb_rd_req. */

__My_addr_internal.s_addr = INADDR_NONE;
__My_addr.s_addr = INADDR_NONE;
servaddr.s_addr = INADDR_NONE;
if (! __Zephyr_server) {
if ((code = ZOpenPort(NULL)) != ZERR_NONE)
Expand Down Expand Up @@ -143,6 +151,21 @@ ZInitialize(void)
if (hostent && hostent->h_addrtype == AF_INET)
memcpy(&servaddr, hostent->h_addr, sizeof(servaddr));

// Field 10 contains our external IP.
mp = notice.z_message;
*(notice.z_message+notice.z_message_len-1) = 0;
for (nf=0; mp<notice.z_message+notice.z_message_len && nf<10; nf++) {
mp += strlen(mp)+1;
}
if (nf==10 && mp<notice.z_message+notice.z_message_len) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not have a kConstant or CONSTANT for this 10?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I know of.

inet_aton(mp, &__My_addr);
}
// Field 11 contains the IGD root URL (if UPnP is enabled)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's an IGD root URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internet Gateway Device. It's the UPnP endpoint that you have to do UDP-HTTP to... (!)

mp += strlen(mp)+1;
if (mp<notice.z_message+notice.z_message_len) {
__UPnP_rooturl = strdup(mp);
}

ZFreeNotice(&notice);
}

Expand Down Expand Up @@ -177,7 +200,6 @@ ZInitialize(void)
#endif
#endif

__My_addr.s_addr = INADDR_NONE;
if (servaddr.s_addr != INADDR_NONE) {
/* Try to get the local interface address by connecting a UDP
* socket to the server address and getting the local address.
Expand All @@ -192,25 +214,41 @@ ZInitialize(void)
if (connect(s, (struct sockaddr *) &sin, sizeof(sin)) == 0
&& getsockname(s, (struct sockaddr *) &sin, &sinsize) == 0
&& sin.sin_addr.s_addr != 0)
memcpy(&__My_addr, &sin.sin_addr, sizeof(__My_addr));
memcpy(&__My_addr_internal, &sin.sin_addr, sizeof(__My_addr_internal));
close(s);
}
}
if (__My_addr.s_addr == INADDR_NONE) {
#if defined(__APPLE__) && defined(__MACH__)
if (__My_addr_internal.s_addr == INADDR_NONE) {
nwi_state_t state = nwi_state_copy();
nwi_ifstate_t ifstate = nwi_state_get_first_ifstate(state, AF_INET);
if (ifstate != NULL) {
memcpy(&__My_addr_internal, &ifstate->iaddr, sizeof(__My_addr_internal));
}
if (state != NULL) {
nwi_state_release(state);
}
}
#endif
if (__My_addr_internal.s_addr == INADDR_NONE) {
/* We couldn't figure out the local interface address by the
* above method. Try by resolving the local hostname. (This
* is a pretty broken thing to do, and unfortunately what we
* always do on server machines.) */
if (gethostname(hostname, sizeof(hostname)) == 0) {
hostent = gethostbyname(hostname);
if (hostent && hostent->h_addrtype == AF_INET)
memcpy(&__My_addr, hostent->h_addr, sizeof(__My_addr));
memcpy(&__My_addr_internal, hostent->h_addr, sizeof(__My_addr_internal));
}
}
/* If the above methods failed, zero out __My_addr so things will
* sort of kind of work. */
if (__My_addr_internal.s_addr == INADDR_NONE)
__My_addr_internal.s_addr = 0;

/* If ZHM didn't give us an external address, use the internal one */
if (__My_addr.s_addr == INADDR_NONE)
__My_addr.s_addr = 0;
__My_addr = __My_addr_internal;

/* Get the sender so we can cache it */
(void) ZGetSender();
Expand Down
2 changes: 2 additions & 0 deletions lib/ZLocations.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ Z_SendLocation(char *class,
if (!location_info_set)
ZInitLocationInfo(NULL, NULL);

Z_InitUPnP();

memset((char *)&notice, 0, sizeof(notice));
notice.z_kind = ACKED;
notice.z_port = (u_short) ((wg_port == -1) ? 0 : wg_port);
Expand Down
6 changes: 5 additions & 1 deletion lib/ZOpenPort.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,12 @@ ZOpenPort(u_short *port)
__Zephyr_port = bindin.sin_port;
__Zephyr_open = 1;

#ifdef Z_DEBUG
Z_debug_stderr("ZOpenPort() opened port %d", ntohs(__Zephyr_port));
#endif

if (port)
*port = bindin.sin_port;
*port = __Zephyr_port;

return ZERR_NONE;
}
2 changes: 2 additions & 0 deletions lib/ZRetSubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ Z_RetSubs(register ZNotice_t *notice,
if ((retval = ZOpenPort((u_short *)0)) != ZERR_NONE)
return (retval);

Z_InitUPnP();

notice->z_kind = ACKED;
notice->z_port = __Zephyr_port;
notice->z_class = ZEPHYR_CTL_CLASS;
Expand Down
8 changes: 8 additions & 0 deletions lib/ZSubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@ ZSubscriptions(register ZSubscription_t *sublist,
int size, start, numok;
Z_AuthProc cert_routine;

if (ZGetFD() < 0) {
if ((retval = ZOpenPort((u_short *)0)) != ZERR_NONE) {
return (retval);
}
}

Z_InitUPnP();

/* nitems = 0 means cancel all subscriptions; still need to allocate a */
/* array for one item so we can cancel, however. */

Expand Down
85 changes: 85 additions & 0 deletions lib/ZUPnP.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
#include <internal.h>

#ifdef HAVE_UPNP
#include <arpa/inet.h>
#include <miniupnpc/miniupnpc.h>
#include <miniupnpc/upnpcommands.h>

static struct UPNPUrls __UPnP_urls = {.controlURL = NULL};
static struct IGDdatas __UPnP_data;
static int __UPnP_attempted = 0;
quentinmit marked this conversation as resolved.
Show resolved Hide resolved
static const char* __UPnP_name = "Zephyr Client";

void Z_InitUPnP_ZHM() {
struct UPNPDev * devlist;
int upnperror = 0;
if (__UPnP_active) {
// tried to initialize twice
return;
}
devlist = upnpDiscover(
2000,
NULL/*multicast interface*/,
NULL/*minissdpd socket path*/,
UPNP_LOCAL_PORT_ANY/*sameport*/,
0/*ipv6*/,
2/*TTL*/,
&upnperror);
if (devlist) {
int igdfound = UPNP_GetValidIGD(devlist, &__UPnP_urls, &__UPnP_data, NULL, 0);
if (igdfound) {
__UPnP_rooturl = __UPnP_urls.rootdescURL;
char extIpAddr[16];
if (UPNP_GetExternalIPAddress(__UPnP_urls.controlURL, __UPnP_data.first.servicetype, extIpAddr) == 0) {
struct in_addr ext_addr;
if (inet_aton(extIpAddr, &ext_addr)) {
__My_addr = ext_addr;
__UPnP_active = 1;
}
}
}
freeUPNPDevlist(devlist);
}
__UPnP_name = "Zephyr Host Manager";
Z_InitUPnP();
}

void Z_InitUPnP() {
if (__UPnP_attempted) {
return;
}
__UPnP_attempted = 1;
if (__UPnP_rooturl && !__UPnP_active) {
__UPnP_active = UPNP_GetIGDFromUrl(__UPnP_rooturl, &__UPnP_urls, &__UPnP_data, NULL, 0);
}
if (__UPnP_active) {
char port_str[16];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 16? Port are only 0..65535, so shouldn't [8] be sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied verbatim from the library.

snprintf(port_str, 16, "%d", ntohs(__Zephyr_port));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error checking to make sure snprintf didn't overflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is this.

int ret = UPNP_AddPortMapping(__UPnP_urls.controlURL,
__UPnP_data.first.servicetype,
port_str,
port_str,
inet_ntoa(__My_addr_internal),
__UPnP_name, "UDP", NULL, NULL);
// TODO: Handle error 718 (ConflictInMappingEntry) by choosing a new random port.
}
}

void Z_CloseUPnP() {
if (__UPnP_active && __UPnP_attempted) {
__UPnP_attempted = 0;
char port_str[16];
snprintf(port_str, 16, "%d", ntohs(__Zephyr_port));
UPNP_DeletePortMapping(__UPnP_urls.controlURL,
__UPnP_data.first.servicetype,
port_str,
"UDP",
NULL);
}
}
#else
// Noop.
void Z_InitUPnP_ZHM() {};
void Z_InitUPnP() {}
void Z_CloseUPnP() {}
#endif
15 changes: 9 additions & 6 deletions lib/Zinternal.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ static const char copyright[] =
int __Zephyr_fd = -1;
int __Zephyr_open;
int __Zephyr_port = -1;
struct in_addr __My_addr;
struct in_addr __My_addr = {.s_addr = INADDR_NONE};
struct in_addr __My_addr_internal = {.s_addr = INADDR_NONE};
int __UPnP_active = 0;
char* __UPnP_rooturl = NULL;
int __Q_CompleteLength;
int __Q_Size;
struct _Z_InputQ *__Q_Head, *__Q_Tail;
Expand Down Expand Up @@ -651,12 +654,12 @@ Z_FormatHeader(ZNotice_t *notice,
if (!notice->z_sender)
notice->z_sender = ZGetSender();

if (ZGetFD() < 0) {
retval = ZOpenPort((u_short *)0);
if (retval != ZERR_NONE)
return (retval);
}
if (notice->z_port == 0) {
if (ZGetFD() < 0) {
retval = ZOpenPort((u_short *)0);
if (retval != ZERR_NONE)
return (retval);
}
notice->z_port = __Zephyr_port;
}

Expand Down