From b2fb0bbf3c7262c403f307ed933a3f1d6d2ed841 Mon Sep 17 00:00:00 2001 From: Josh Elsasser Date: Tue, 6 Feb 2007 03:24:55 +0000 Subject: [PATCH] Clean up NAT-PMP code a little. Correctly handle a NAT-PMP device mapping a different public port than requested. --- libtransmission/natpmp.c | 403 ++++++++++++++++++++------------------- libtransmission/natpmp.h | 2 +- libtransmission/shared.c | 8 +- 3 files changed, 219 insertions(+), 194 deletions(-) diff --git a/libtransmission/natpmp.c b/libtransmission/natpmp.c index 6a9a44c60..8fa857093 100644 --- a/libtransmission/natpmp.c +++ b/libtransmission/natpmp.c @@ -33,12 +33,12 @@ #define PMP_OPCODE_ADDUDP 1 #define PMP_OPCODE_ADDTCP 2 #define PMP_LIFETIME 3600 /* secs, one hour */ -#define PMP_RESPCODE_OK 0 -#define PMP_RESPCODE_BADVERS 1 -#define PMP_RESPCODE_REFUSED 2 -#define PMP_RESPCODE_NETDOWN 3 -#define PMP_RESPCODE_NOMEM 4 -#define PMP_RESPCODE_BADOPCODE 5 +#define PMP_RESULT_OK 0 +#define PMP_RESULT_BADVERS 1 +#define PMP_RESULT_REFUSED 2 +#define PMP_RESULT_NETDOWN 3 +#define PMP_RESULT_NOMEM 4 +#define PMP_RESULT_BADOPCODE 5 #define PMP_OPCODE_FROM_RESPONSE( op ) ( 0x80 ^ (op) ) #define PMP_OPCODE_TO_RESPONSE( op ) ( 0x80 | (op) ) @@ -63,8 +63,8 @@ typedef struct tr_natpmp_req_s int delay; uint64_t retry; uint64_t timeout; - int port; - tr_natpmp_uptime_t * uptime; + int askport; + int gotport; } tr_natpmp_req_t; struct tr_natpmp_s @@ -89,26 +89,35 @@ struct tr_natpmp_s int mcastfd; }; +typedef struct tr_natpmp_parse_s +{ + unsigned int tmpfail : 1; + uint32_t seconds; + uint16_t port; + uint32_t lifetime; +} +tr_natpmp_parse_t; + static int checktime( tr_natpmp_uptime_t * uptime, uint32_t seen ); static void killsock( int * fd ); static tr_natpmp_req_t * -newreq( int adding, struct in_addr addr, int port, - tr_natpmp_uptime_t * uptime ); +newreq( int adding, struct in_addr addr, int port ); +static void +killreq( tr_natpmp_req_t ** req ); +static void +resetreq( tr_natpmp_req_t * req ); static tr_tristate_t -pulsereq( tr_natpmp_req_t * req, uint64_t * renew ); +pulsereq( tr_natpmp_t * req ); +static int +sendreq( tr_natpmp_req_t * req ); static int mcastsetup(); static void mcastpulse( tr_natpmp_t * pmp ); -static void -killreq( tr_natpmp_req_t ** req ); -static int -sendrequest( int adding, int fd, int port ); static tr_tristate_t -readrequest( uint8_t * buf, int len, int adding, int port, - tr_natpmp_uptime_t * uptime, uint64_t * renew, int * tmpfail ); +parseresponse( uint8_t * buf, int len, int port, tr_natpmp_parse_t * parse ); tr_natpmp_t * tr_natpmpInit() @@ -158,7 +167,6 @@ tr_natpmpStart( tr_natpmp_t * pmp ) { pmp->mcastfd = mcastsetup(); } - /* XXX should I change state? */ } tr_lockUnlock( &pmp->lock ); @@ -183,7 +191,7 @@ tr_natpmpStop( tr_natpmp_t * pmp ) tr_dbg( "nat-pmp state add -> idle" ); if( NULL != pmp->req ) { - pmp->mappedport = pmp->req->port; + pmp->mappedport = pmp->req->gotport; killreq( &pmp->req ); pmp->state = PMP_STATE_DELETING; tr_dbg( "nat-pmp state idle -> del" ); @@ -270,7 +278,7 @@ tr_natpmpClose( tr_natpmp_t * pmp ) { /* try to send at least one delete request if we have a port mapping */ tr_natpmpStop( pmp ); - tr_natpmpPulse( pmp ); + tr_natpmpPulse( pmp, NULL ); tr_lockLock( &pmp->lock ); killreq( &pmp->req ); @@ -279,7 +287,7 @@ tr_natpmpClose( tr_natpmp_t * pmp ) } void -tr_natpmpPulse( tr_natpmp_t * pmp ) +tr_natpmpPulse( tr_natpmp_t * pmp, int * publicPort ) { tr_lockLock( &pmp->lock ); @@ -318,8 +326,7 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) } else { - pmp->req = newreq( 1, pmp->dest, pmp->newport, - &pmp->uptime ); + pmp->req = newreq( 1, pmp->dest, pmp->newport ); if( NULL == pmp->req ) { pmp->state = PMP_STATE_FAILED; @@ -329,7 +336,7 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) } if( PMP_STATE_ADDING == pmp->state ) { - switch( pulsereq( pmp->req, &pmp->renew ) ) + switch( pulsereq( pmp ) ) { case TR_NET_ERROR: if( pmp->req->nobodyhome ) @@ -341,7 +348,7 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) { pmp->state = PMP_STATE_TMPFAIL; tr_dbg( "nat-pmp state add -> err on pulse" ); - if( pmp->req->port == pmp->newport ) + if( pmp->req->askport == pmp->newport ) { pmp->newport = 0; } @@ -354,7 +361,12 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) killreq( &pmp->req ); break; case TR_NET_OK: - pmp->mappedport = pmp->req->port; + pmp->mappedport = pmp->req->gotport; + if( pmp->mappedport != pmp->newport && + pmp->newport == pmp->req->askport ) + { + pmp->newport = pmp->req->gotport; + } killreq( &pmp->req ); pmp->state = PMP_STATE_MAPPED; pmp->mapped = 1; @@ -372,8 +384,7 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) if( NULL == pmp->req ) { assert( 0 < pmp->mappedport ); - pmp->req = newreq( 0, pmp->dest, pmp->mappedport, - &pmp->uptime ); + pmp->req = newreq( 0, pmp->dest, pmp->mappedport ); if( NULL == pmp->req ) { pmp->state = PMP_STATE_FAILED; @@ -382,7 +393,7 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) } if( PMP_STATE_DELETING == pmp->state ) { - switch( pulsereq( pmp->req, &pmp->renew ) ) + switch( pulsereq( pmp ) ) { case TR_NET_ERROR: if( pmp->req->nobodyhome ) @@ -407,8 +418,9 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) break; case TR_NET_OK: tr_dbg( "nat-pmp state del -> idle with port %i", - pmp->req->port); - tr_inf( "nat-pmp unmapped port %i", pmp->req->port ); + pmp->req->askport); + tr_inf( "nat-pmp unmapped port %i", + pmp->req->askport ); pmp->mapped = 0; pmp->mappedport = -1; killreq( &pmp->req ); @@ -444,6 +456,11 @@ tr_natpmpPulse( tr_natpmp_t * pmp ) } } + if( NULL != publicPort ) + { + *publicPort = -1; + } + tr_lockUnlock( &pmp->lock ); } @@ -482,76 +499,84 @@ killsock( int * fd ) } static tr_natpmp_req_t * -newreq( int adding, struct in_addr addr, int port, - tr_natpmp_uptime_t * uptime ) +newreq( int adding, struct in_addr addr, int port ) { tr_natpmp_req_t * ret; - uint64_t now; ret = calloc( 1, sizeof( *ret ) ); if( NULL == ret ) { - goto err; + return NULL; } - ret->fd = -1; + ret->fd = tr_netOpenUDP( addr, htons( PMP_PORT ), 1 ); if( 0 > ret->fd ) { - goto err; - } - if( sendrequest( adding, ret->fd, port ) ) - { - goto err; + free( ret ); + return NULL; } - now = tr_date(); ret->adding = adding; - ret->delay = PMP_INITIAL_DELAY; - ret->retry = now + PMP_INITIAL_DELAY; - ret->timeout = now + PMP_TOTAL_DELAY; - ret->port = port; - ret->uptime = uptime; + ret->askport = port; + ret->gotport = port; + resetreq( ret ); + if( sendreq( ret ) ) + { + killreq( &ret ); + return NULL; + } return ret; +} - err: - if( NULL != ret ) +static void +killreq( tr_natpmp_req_t ** req ) +{ + if( NULL != *req ) { - killsock( &ret->fd ); + killsock( &(*req)->fd ); + free( *req ); + *req = NULL; } - free( ret ); +} - return NULL; +static void +resetreq( tr_natpmp_req_t * req ) +{ + uint64_t now; + + now = tr_date(); + req->delay = PMP_INITIAL_DELAY; + req->retry = now; + req->timeout = now + PMP_TOTAL_DELAY; } static tr_tristate_t -pulsereq( tr_natpmp_req_t * req, uint64_t * renew ) +pulsereq( tr_natpmp_t * pmp ) { + tr_natpmp_req_t * req = pmp->req; struct sockaddr_in sin; uint8_t buf[16]; - int res, tmpfail; + int res; uint64_t now; tr_tristate_t ret; + tr_natpmp_parse_t parse; now = tr_date(); - + /* check for timeout */ if( now >= req->timeout ) { tr_dbg( "nat-pmp request timed out" ); req->nobodyhome = 1; return TR_NET_ERROR; } - - if( now >= req->retry ) + /* send another request if it's been long enough */ + if( now >= req->retry && sendreq( req ) ) { - if( sendrequest( req->adding, req->fd, req->port ) ) - { - return TR_NET_ERROR; - } - req->delay *= 2; - req->timeout = now + req->delay; + return TR_NET_ERROR; } + /* check for incoming packets */ res = tr_netRecvFrom( req->fd, buf, sizeof( buf ), &sin ); if( TR_NET_BLOCK & res ) { @@ -571,14 +596,68 @@ pulsereq( tr_natpmp_req_t * req, uint64_t * renew ) return TR_NET_ERROR; } + /* parse the packet */ tr_dbg( "nat-pmp read %i byte response", res ); + ret = parseresponse( buf, res, req->askport, &parse ); + req->tmpfail = parse.tmpfail; + /* check for device reset */ + if( checktime( &pmp->uptime, parse.seconds ) ) + { + pmp->renew = 0; + tr_inf( "detected nat-pmp device reset" ); + resetreq( req ); + ret = TR_NET_WAIT; + } + if( TR_NET_OK == ret && req->adding ) + { + if( req->askport != parse.port ) + { + tr_dbg( "nat-pmp received %i for public port instead of %i", + parse.port, req->askport ); + req->gotport = parse.port; + } + tr_dbg( "nat-pmp set renew to half of %u", parse.lifetime ); + pmp->renew = now + ( parse.lifetime / 2 * 1000 ); + } - ret = readrequest( buf, res, req->adding, req->port, req->uptime, renew, - &tmpfail ); - req->tmpfail = ( tmpfail ? 1 : 0 ); return ret; } +static int +sendreq( tr_natpmp_req_t * req ) +{ + uint8_t buf[12]; + int res; + + bzero( buf, sizeof( buf ) ); + buf[0] = PMP_VERSION; + buf[1] = PMP_OPCODE_ADDTCP; + PMP_TOBUF16( buf + 4, req->askport ); + if( req->adding ) + { + PMP_TOBUF16( buf + 6, req->askport ); + PMP_TOBUF32( buf + 8, PMP_LIFETIME ); + } + + res = tr_netSend( req->fd, buf, sizeof( buf ) ); + if( TR_NET_CLOSE & res && EHOSTUNREACH == errno ) + { + res = TR_NET_BLOCK; + } + if( TR_NET_CLOSE & res ) + { + tr_err( "failed to send nat-pmp request (%s)", strerror( errno ) ); + return 1; + } + else if( !( TR_NET_BLOCK & res ) ) + { + /* XXX is it all right to assume the entire thing is written? */ + req->retry = tr_date() + req->delay; + req->delay *= 2; + } + return 0; +} + static int mcastsetup() { @@ -604,6 +683,7 @@ mcastpulse( tr_natpmp_t * pmp ) uint8_t buf[16]; int res; char dbgstr[INET_ADDRSTRLEN]; + tr_natpmp_parse_t parse; res = tr_netRecvFrom( pmp->mcastfd, buf, sizeof( buf ), &sin ); if( TR_NET_BLOCK & res ) @@ -627,162 +707,101 @@ mcastpulse( tr_natpmp_t * pmp ) return; } - if( TR_NET_OK == readrequest( buf, res, 0, -1, &pmp->uptime, &pmp->renew, NULL ) && - PMP_STATE_NOBODYHOME == pmp->state ) + if( TR_NET_OK == parseresponse( buf, res, -1, &parse ) ) { - tr_dbg( "nat-pmp state notfound -> idle" ); - pmp->state = PMP_STATE_IDLE; + if( checktime( &pmp->uptime, parse.seconds ) ) + { + pmp->renew = 0; + tr_inf( "detected nat-pmp device reset" ); + if( NULL != pmp->req ) + { + resetreq( pmp->req ); + } + } + if( PMP_STATE_NOBODYHOME == pmp->state ) + { + tr_dbg( "nat-pmp state notfound -> idle" ); + pmp->state = PMP_STATE_IDLE; + } } } -static void -killreq( tr_natpmp_req_t ** req ) -{ - if( NULL != *req ) - { - killsock( &(*req)->fd ); - free( *req ); - *req = NULL; - } -} - -static int -sendrequest( int adding, int fd, int port ) -{ - uint8_t buf[12]; - int res; - - buf[0] = PMP_VERSION; - buf[1] = PMP_OPCODE_ADDTCP; - buf[2] = 0; - buf[3] = 0; - PMP_TOBUF16( buf + 4, port ); - if( adding ) - { - PMP_TOBUF16( buf + 6, port ); - PMP_TOBUF32( buf + 8, PMP_LIFETIME ); - } - else - { - PMP_TOBUF16( buf + 6, 0 ); - PMP_TOBUF32( buf + 8, 0 ); - } - - res = tr_netSend( fd, buf, sizeof( buf ) ); - if( TR_NET_CLOSE & res && EHOSTUNREACH == errno ) - { - res = TR_NET_BLOCK; - } - /* XXX is it all right to assume the entire thing is written? */ - - /* XXX I should handle blocking here */ - - return ( ( TR_NET_CLOSE | TR_NET_BLOCK ) & res ? 1 : 0 ); -} - static tr_tristate_t -readrequest( uint8_t * buf, int len, int adding, int port, - tr_natpmp_uptime_t * uptime, uint64_t * renew, int * tmpfail ) +parseresponse( uint8_t * buf, int len, int port, tr_natpmp_parse_t * parse ) { - uint8_t version, opcode, wantedopcode; - uint16_t rescode, privport, pubport; - uint32_t seconds, lifetime; + int version, respopcode, opcode, wantedopcode, rescode, privport; - assert( !adding || NULL != tmpfail ); - if( NULL != tmpfail ) - { - *tmpfail = 0; - } - if( 4 > len ) - { - tr_err( "read truncated %i byte nat-pmp response packet", len ); - return TR_NET_ERROR; - } - version = buf[0]; - opcode = buf[1]; - rescode = PMP_FROMBUF16( buf + 2 ); - wantedopcode = ( 0 < port ? PMP_OPCODE_ADDTCP : PMP_OPCODE_GETIP ); - - if( !PMP_OPCODE_IS_RESPONSE( opcode ) ) - { - tr_dbg( "nat-pmp ignoring request packet" ); - return TR_NET_WAIT; - } - opcode = PMP_OPCODE_FROM_RESPONSE( opcode ); - - if( PMP_VERSION != version ) - { - tr_err( "bad nat-pmp version %hhu", buf[0] ); - return TR_NET_ERROR; - } - if( wantedopcode != opcode ) - { - tr_err( "bad nat-pmp opcode %hhu", opcode ); - return TR_NET_ERROR; - } - switch( rescode ) - { - case PMP_RESPCODE_OK: - break; - case PMP_RESPCODE_REFUSED: - case PMP_RESPCODE_NETDOWN: - case PMP_RESPCODE_NOMEM: - if( NULL != tmpfail ) - { - *tmpfail = 1; - } - /* fallthrough */ - default: - tr_err( "bad nat-pmp result code %hu", rescode ); - return TR_NET_ERROR; - } + bzero( parse, sizeof( *parse ) ); if( 8 > len ) { tr_err( "read truncated %i byte nat-pmp response packet", len ); return TR_NET_ERROR; } - seconds = PMP_FROMBUF32( buf + 4 ); - if( checktime( uptime, seconds ) ) + /* parse the first 8 bytes: version, opcode, and result code */ + version = buf[0]; + respopcode = buf[1]; + opcode = PMP_OPCODE_FROM_RESPONSE( respopcode ); + wantedopcode = ( 0 < port ? PMP_OPCODE_ADDTCP : PMP_OPCODE_GETIP ); + rescode = PMP_FROMBUF16( buf + 2 ); + + if( PMP_VERSION != version ) { - *renew = 0; - tr_inf( "detected nat-pmp device reset" ); - /* XXX should reset retry counter here */ + tr_err( "unknown nat-pmp version %hhu", buf[0] ); + return TR_NET_ERROR; + } + if( !PMP_OPCODE_IS_RESPONSE( respopcode ) ) + { + tr_dbg( "nat-pmp ignoring request packet" ); return TR_NET_WAIT; } - - if( 0 <= port ) + if( wantedopcode != opcode ) + { + tr_err( "unknown nat-pmp opcode %hhu", opcode ); + return TR_NET_ERROR; + } + + switch( rescode ) + { + case PMP_RESULT_OK: + break; + case PMP_RESULT_REFUSED: + tr_err( "nat-pmp mapping failed: refused/unauthorized/disabled" ); + parse->tmpfail = 1; + return TR_NET_ERROR; + case PMP_RESULT_NETDOWN: + tr_err( "nat-pmp mapping failed: network down" ); + parse->tmpfail = 1; + return TR_NET_ERROR; + case PMP_RESULT_NOMEM: + tr_err( "nat-pmp mapping refused: insufficient resources" ); + parse->tmpfail = 1; + return TR_NET_ERROR; + default: + tr_err( "nat-pmp mapping refused: unknown result code: %hu", + rescode ); + return TR_NET_ERROR; + } + + parse->seconds = PMP_FROMBUF32( buf + 4 ); + if( PMP_OPCODE_ADDTCP == opcode ) { - assert( PMP_OPCODE_ADDTCP == wantedopcode ); if( 16 > len ) { tr_err( "read truncated %i byte nat-pmp response packet", len ); return TR_NET_ERROR; } - privport = PMP_FROMBUF16( buf + 8 ); - pubport = PMP_FROMBUF16( buf + 10 ); - lifetime = PMP_FROMBUF32( buf + 12 ); + privport = PMP_FROMBUF16( buf + 8 ); + parse->port = PMP_FROMBUF16( buf + 10 ); + parse->lifetime = PMP_FROMBUF32( buf + 12 ); if( port != privport ) { - /* private port doesn't match, ignore it */ tr_dbg( "nat-pmp ignoring message for port %i, expected port %i", privport, port ); return TR_NET_WAIT; } - - if( adding ) - { - if( port != pubport ) - { - *tmpfail = 1; - /* XXX should just start announcing the pub port we're given */ - return TR_NET_ERROR; - } - tr_dbg( "nat-pmp set renew to half of %u", lifetime ); - *renew = tr_date() + ( lifetime / 2 * 1000 ); - } } return TR_NET_OK; diff --git a/libtransmission/natpmp.h b/libtransmission/natpmp.h index f7abc0d9d..4e367ce8a 100644 --- a/libtransmission/natpmp.h +++ b/libtransmission/natpmp.h @@ -32,7 +32,7 @@ void tr_natpmpStart( tr_natpmp_t * ); void tr_natpmpStop( tr_natpmp_t * ); int tr_natpmpStatus( tr_natpmp_t * ); void tr_natpmpForwardPort( tr_natpmp_t *, int ); -void tr_natpmpPulse( tr_natpmp_t * ); +void tr_natpmpPulse( tr_natpmp_t *, int * ); void tr_natpmpClose( tr_natpmp_t * ); #endif diff --git a/libtransmission/shared.c b/libtransmission/shared.c index ac101da3d..7c4fd22cb 100644 --- a/libtransmission/shared.c +++ b/libtransmission/shared.c @@ -258,6 +258,7 @@ static void SharedLoop( void * _s ) { tr_shared_t * s = _s; uint64_t date1, date2, lastchoke = 0; + int newPort; tr_sharedLock( s ); @@ -266,7 +267,12 @@ static void SharedLoop( void * _s ) date1 = tr_date(); /* NAT-PMP and UPnP pulses */ - tr_natpmpPulse( s->natpmp ); + newPort = -1; + tr_natpmpPulse( s->natpmp, &newPort ); + if( 0 < newPort && newPort != s->publicPort ) + { + SetPublicPort( s, newPort ); + } tr_upnpPulse( s->upnp ); /* Handle incoming connections */