Changeset View
Standalone View
lib/libfetch/common.c
Show All 36 Lines | |||||
#include <sys/time.h> | #include <sys/time.h> | ||||
#include <sys/uio.h> | #include <sys/uio.h> | ||||
#include <netinet/in.h> | #include <netinet/in.h> | ||||
#include <ctype.h> | #include <ctype.h> | ||||
#include <errno.h> | #include <errno.h> | ||||
#include <fcntl.h> | #include <fcntl.h> | ||||
#include <inttypes.h> | |||||
#include <netdb.h> | #include <netdb.h> | ||||
#include <poll.h> | #include <poll.h> | ||||
#include <pwd.h> | #include <pwd.h> | ||||
#include <stdarg.h> | #include <stdarg.h> | ||||
#include <stdlib.h> | #include <stdlib.h> | ||||
#include <stdio.h> | #include <stdio.h> | ||||
#include <string.h> | #include <string.h> | ||||
#include <unistd.h> | #include <unistd.h> | ||||
▲ Show 20 Lines • Show All 256 Lines • ▼ Show 20 Lines | fetch_resolve(const char *addr, int port, int af) | ||||
} | } | ||||
return (res); | return (res); | ||||
syserr: | syserr: | ||||
fetch_syserr(); | fetch_syserr(); | ||||
return (NULL); | return (NULL); | ||||
} | } | ||||
/* | /* | ||||
* Bind a socket to a specific local address | * Bind a socket to a specific local address | ||||
*/ | */ | ||||
int | int | ||||
fetch_bind(int sd, int af, const char *addr) | fetch_bind(int sd, int af, const char *addr) | ||||
{ | { | ||||
struct addrinfo *cliai, *ai; | struct addrinfo *cliai, *ai; | ||||
int err; | int err; | ||||
if ((cliai = fetch_resolve(addr, 0, af)) == NULL) | if ((cliai = fetch_resolve(addr, 0, af)) == NULL) | ||||
return (-1); | return (-1); | ||||
for (ai = cliai; ai != NULL; ai = ai->ai_next) | for (ai = cliai; ai != NULL; ai = ai->ai_next) | ||||
if ((err = bind(sd, ai->ai_addr, ai->ai_addrlen)) == 0) | if ((err = bind(sd, ai->ai_addr, ai->ai_addrlen)) == 0) | ||||
break; | break; | ||||
if (err != 0) | if (err != 0) | ||||
fetch_syserr(); | fetch_syserr(); | ||||
freeaddrinfo(cliai); | freeaddrinfo(cliai); | ||||
return (err == 0 ? 0 : -1); | return (err == 0 ? 0 : -1); | ||||
} | } | ||||
/* | /* | ||||
* SOCKS5 connection initiation, based on RFC 1928 | |||||
* Default DNS resolution over SOCKS5 | |||||
*/ | |||||
int | |||||
fetch_socks5_init(conn_t *conn, const char *host, int port, int verbose) | |||||
{ | |||||
/* | |||||
* Size is based on largest packet prefix (4 bytes) + | |||||
* Largest FQDN (256) + one byte size (1) + | |||||
* Port (2) | |||||
*/ | |||||
unsigned char buf[BUFF_SIZE]; | |||||
lattera-gmail.com: Should this size be macro-ized? | |||||
unsigned char *ptr; | |||||
if (verbose) | |||||
fetch_info("Initializing SOCKS5 connection: %s:%d", host, port); | |||||
/* Connection initialization */ | |||||
ptr = buf; | |||||
*ptr++ = SOCKS_VERSION_5; | |||||
Done Inline ActionsMagic values should be macro-ized. lattera-gmail.com: Magic values should be macro-ized. | |||||
*ptr++ = SOCKS_CONNECTION; | |||||
*ptr++ = SOCKS_RSV; | |||||
if (fetch_write(conn, buf, 3) != 3) { | |||||
fprintf(stderr, "SOCKS5: Failed to send selection method.\n"); | |||||
goto fail; | |||||
} | |||||
/* Verify response from SOCKS5 server */ | |||||
if (fetch_read(conn, buf, 2) != 2) { | |||||
fprintf(stderr, "SOCKS5: Failed to read method.\n"); | |||||
goto fail; | |||||
} | |||||
ptr = buf; | |||||
if (ptr[0] != SOCKS_VERSION_5) { | |||||
fprintf(stderr, "SOCKS5: Currently only version 5 is implemented.\n"); | |||||
goto fail; | |||||
} | |||||
if (ptr[1] == SOCKS_NOMETHODS) { | |||||
fprintf(stderr, "SOCKS5: No acceptable methods. Disconnecting.\n"); | |||||
goto fail; | |||||
} | |||||
else if (ptr[1] != SOCKS5_NOTIMPLEMENTED) { | |||||
Done Inline ActionsMove this else up to the line above it; if we could get rid of the magic number here, that'd be great. kevans: Move this else up to the line above it; if we could get rid of the magic number here, that'd be… | |||||
fprintf(stderr, "SOCKS5: Method currently not implemented. Disconnecting.\n"); | |||||
goto fail; | |||||
} | |||||
/* Send Request */ | |||||
*ptr++ = SOCKS_VERSION_5; | |||||
Done Inline Actionsptr is already set to buf. lattera-gmail.com: `ptr` is already set to `buf`. | |||||
*ptr++ = SOCKS_CONNECTION; | |||||
*ptr++ = SOCKS_RSV; | |||||
/* Encode all targets as a hostname to avoid DNS leaks */ | |||||
*ptr++ = SOCKS_ATYP_DOMAINNAME; | |||||
if (strlen(host) > FQDN_SIZE) { | |||||
fprintf(stderr, "Hostname above 256 bytes, exiting.\n"); | |||||
Done Inline ActionsSince you're doing pointer arithmetic, strlen(host) needs to be validated to be 256 characters or less. As you modify where ptr points to, you need to ensure ptr is still within bounds prior to dereferencing it. lattera-gmail.com: Since you're doing pointer arithmetic, `strlen(host)` needs to be validated to be 256… | |||||
goto fail; | |||||
} | |||||
*ptr++ = strlen(host); | |||||
strncpy(ptr, host, strlen(host)); | |||||
ptr = ptr + strlen(host); | |||||
port = htons(port); | |||||
*ptr++ = port & 0x00ff; | |||||
*ptr++ = (port & 0xff00) >> 8; | |||||
if (fetch_write(conn, buf, ptr - buf) != ptr - buf) { | |||||
fprintf(stderr, "SOCKS5: Failed to request.\n"); | |||||
goto fail; | |||||
} | |||||
/* BND.ADDR is variable length, read the largest on non-blocking socket */ | |||||
if (!fetch_read(conn, buf, BUFF_SIZE)) { | |||||
fprintf(stderr, "SOCKS5: Failed to receive reply.\n"); | |||||
goto fail; | |||||
} | |||||
ptr = buf; | |||||
if (*ptr++ != SOCKS_VERSION_5) { | |||||
fprintf(stderr, "SOCKS5: Server responded with a non-version 5 response.\n"); | |||||
goto fail; | |||||
} | |||||
switch(*ptr++) { | |||||
case SOCKS_SUCCESS: | |||||
break; | |||||
case SOCKS_GENERAL_FAILURE: | |||||
Done Inline ActionsNo need for the breaks after goto. lattera-gmail.com: No need for the breaks after goto. | |||||
fprintf(stderr, "SOCKS5: General server failure\n"); | |||||
goto fail; | |||||
case SOCKS_CONNECTION_NOT_ALLOWED: | |||||
fprintf(stderr, "SOCKS5: Connection not allowed by ruleset.\n"); | |||||
goto fail; | |||||
case SOCKS_NETWORK_UNREACHABLE: | |||||
fprintf(stderr, "SOCKS5: Network unreachable.\n"); | |||||
goto fail; | |||||
case SOCKS_HOST_UNREACHABLE: | |||||
fprintf(stderr, "SOCKS5: Host unreachable.\n"); | |||||
goto fail; | |||||
case SOCKS_CONNECTION_REFUSED: | |||||
fprintf(stderr, "SOCKS5: Connection refused.\n"); | |||||
goto fail; | |||||
case SOCKS_TTL_EXPIRED: | |||||
fprintf(stderr, "SOCKS5: TTL expired.\n"); | |||||
goto fail; | |||||
case SOCKS_COMMAND_NOT_SUPPORTED: | |||||
fprintf(stderr, "SOCKS5: Command not supported.\n"); | |||||
goto fail; | |||||
case SOCKS_ADDRESS_NOT_SUPPORTED: | |||||
fprintf(stderr, "SOCKS5: Address type not supported.\n"); | |||||
goto fail; | |||||
default: | |||||
fprintf(stderr, "SOCKS5: Unspecified failure.\n"); | |||||
goto fail; | |||||
} | |||||
return (1); | |||||
fail: | |||||
return (0); | |||||
} | |||||
/* | |||||
* Perform SOCKS5 initialization | |||||
*/ | |||||
int | |||||
fetch_socks5_getenv(char **host, int *port) | |||||
{ | |||||
char *socks5env, *endptr, *ext; | |||||
if ((socks5env = getenv("SOCKS5_PROXY")) == NULL || *socks5env == '\0') { | |||||
Done Inline ActionsThis should read: (socks5env = getenv("SOCKS5_PROXY")) == NULL || *socks5env == '\0' (prefer explicitly comparisons as per style(9)) kevans: This should read: `(socks5env = getenv("SOCKS5_PROXY")) == NULL || *socks5env == '\0'` (prefer… | |||||
*host = NULL; | |||||
*port = -1; | |||||
return (-1); | |||||
Not Done Inline ActionsI'll leave this note here once; this function returns 0, 1, or 2 -- the difference between 1 and 2 doesn't seem to be significant from a callers' perspective, and I'm not sure this matches the prevailing style for libfetch which tends to return 0 for success, -1 for error. kevans: I'll leave this note here once; this function returns 0, 1, or 2 -- the difference between 1… | |||||
} | |||||
/* IPv6 addresses begin and end in brackets */ | |||||
if (socks5env[0] == '[') { | |||||
if (socks5env[strlen(socks5env) - 1] == ']') { | |||||
Done Inline ActionsBinary operators should have spaces around them (strlen(socks5env) - 1) kevans: Binary operators should have spaces around them (`strlen(socks5env) - 1`) | |||||
*host = strndup(socks5env, strlen(socks5env)); | |||||
if (*host == NULL) | |||||
goto fail; | |||||
*port = 1080; /* Default port as defined in RFC1928 */ | |||||
} else { | |||||
ext = strstr(socks5env, "]:"); | |||||
Done Inline ActionsMove this a line up, } else { kevans: Move this a line up, `} else {` | |||||
if (ext == NULL) { | |||||
Done Inline ActionsNeed to check for NULL return value. lattera-gmail.com: Need to check for NULL return value. | |||||
fprintf(stderr, "Bad SOCKS5_PROXY format, missing closing ']': %s\n", | |||||
Done Inline Actionsext == NULL kevans: `ext == NULL` | |||||
socks5env); | |||||
return (0); | |||||
} | |||||
ext=ext+1; | |||||
*host = strndup(socks5env, ext - socks5env); | |||||
Done Inline ActionsSee above w.r.t. binary operators and spaces around them. kevans: See above w.r.t. binary operators and spaces around them. | |||||
if (*host == NULL) | |||||
Done Inline ActionsSee above w.r.t. binary operators and spaces around them; ext and socks5env should also likely be unparenthesized here. kevans: See above w.r.t. binary operators and spaces around them; ext and socks5env should also likely… | |||||
goto fail; | |||||
*port = strtoimax(ext + 1, (char **)&endptr, 10); | |||||
if (*port == 0 && errno == EINVAL) { | |||||
Done Inline ActionsSee above w.r.t. binary operators and spaces around them. Also, why strtoimax here and below? You're given an int to work with here, so this should likely be something more like: long portconv; portconv = strtol(...); if (portconv <= 0 || portconv > INT_MAX) ... error ... *port = portconv; strtoimax and strtol are effectively the same, just with different min/max limits -- you might as well use the smallest set of limits larger than what you can handle, even if efficiency isn't all that much of a concern here. kevans: See above w.r.t. binary operators and spaces around them.
Also, why strtoimax here and below? | |||||
fprintf(stderr, "Bad SOCKS5_PROXY port: %s\n", socks5env); | |||||
Done Inline ActionsNeed to check for NULL return value. lattera-gmail.com: Need to check for NULL return value. | |||||
Not Done Inline ActionsYou must set errno = 0 before the calls to strtoimax to know for sure that this errno check is valid. Further, don't check specifically for EINVAL -- errno != 0. kevans: You must set errno = 0 before the calls to strtoimax to know for sure that this errno check is… | |||||
return (0); | |||||
} | |||||
} | |||||
} else { | |||||
ext = strrchr(socks5env, ':'); | |||||
if (ext == NULL) { | |||||
Done Inline Actionselse should move to the line above it kevans: else should move to the line above it | |||||
*host = strdup(socks5env); | |||||
*port = 1080; | |||||
Done Inline Actionsext == NULL kevans: `ext == NULL` | |||||
} else { | |||||
*host = strndup(socks5env, ext-socks5env); | |||||
if (*host == NULL) | |||||
Done Inline ActionsNeed to check for NULL return value. lattera-gmail.com: Need to check for NULL return value. | |||||
goto fail; | |||||
Done Inline ActionsMoved to the line above it kevans: Moved to the line above it | |||||
*port = strtoimax(ext + 1, (char **)&endptr, 10); | |||||
Done Inline ActionsSee above w.r.t. binary operators and spaces around them. kevans: See above w.r.t. binary operators and spaces around them. | |||||
if (*port == 0 && errno == EINVAL) { | |||||
fprintf(stderr, "Bad SOCKS5_PROXY port: %s\n", socks5env); | |||||
Done Inline ActionsNeed to check for NULL return value. lattera-gmail.com: Need to check for NULL return value. | |||||
Not Done Inline ActionsYou must set errno = 0 before the calls to strtoimax to know for sure that this errno check is valid. Further, don't check specifically for EINVAL -- errno != 0. kevans: You must set errno = 0 before the calls to strtoimax to know for sure that this errno check is… | |||||
return (0); | |||||
Done Inline ActionsSee above w.r.t. binary operators and spaces around them. kevans: See above w.r.t. binary operators and spaces around them. | |||||
} | |||||
} | |||||
} | |||||
return (2); | |||||
fail: | |||||
fprintf(stderr, "Failure to allocate memory, exiting.\n"); | |||||
return (-1); | |||||
} | |||||
/* | |||||
* Establish a TCP connection to the specified port on the specified host. | * Establish a TCP connection to the specified port on the specified host. | ||||
*/ | */ | ||||
conn_t * | conn_t * | ||||
fetch_connect(const char *host, int port, int af, int verbose) | fetch_connect(const char *host, int port, int af, int verbose) | ||||
{ | { | ||||
struct addrinfo *cais = NULL, *sais = NULL, *cai, *sai; | struct addrinfo *cais = NULL, *sais = NULL, *cai, *sai; | ||||
const char *bindaddr; | const char *bindaddr; | ||||
conn_t *conn = NULL; | conn_t *conn = NULL; | ||||
int err = 0, sd = -1; | int err = 0, sd = -1; | ||||
Done Inline ActionsThis should move down to the line with other int declarations, sorted alphabetically there. kevans: This should move down to the line with other int declarations, sorted alphabetically there. | |||||
char *sockshost; | |||||
int socksport; | |||||
DEBUGF("---> %s:%d\n", host, port); | DEBUGF("---> %s:%d\n", host, port); | ||||
/* Check if SOCKS5_PROXY env variable is set */ | |||||
if(!fetch_socks5_getenv(&sockshost, &socksport)) | |||||
goto fail; | |||||
/* Not using SOCKS5 proxy */ | |||||
if (sockshost == NULL) { | |||||
Done Inline Actionssockshost == NULL kevans: `sockshost == NULL` | |||||
/* resolve server address */ | /* resolve server address */ | ||||
if (verbose) | if (verbose) | ||||
fetch_info("resolving server address: %s:%d", host, port); | fetch_info("resolving server address: %s:%d", host, port); | ||||
if ((sais = fetch_resolve(host, port, af)) == NULL) | if ((sais = fetch_resolve(host, port, af)) == NULL) | ||||
goto fail; | goto fail; | ||||
/* resolve client address */ | /* resolve client address */ | ||||
bindaddr = getenv("FETCH_BIND_ADDRESS"); | bindaddr = getenv("FETCH_BIND_ADDRESS"); | ||||
if (bindaddr != NULL && *bindaddr != '\0') { | if (bindaddr != NULL && *bindaddr != '\0') { | ||||
if (verbose) | if (verbose) | ||||
fetch_info("resolving client address: %s", bindaddr); | fetch_info("resolving client address: %s", bindaddr); | ||||
if ((cais = fetch_resolve(bindaddr, 0, af)) == NULL) | if ((cais = fetch_resolve(bindaddr, 0, af)) == NULL) | ||||
goto fail; | goto fail; | ||||
} | } | ||||
} else { | |||||
/* resolve socks5 proxy address */ | |||||
if (verbose) | |||||
fetch_info("resolving SOCKS5 server address: %s:%d", sockshost, socksport); | |||||
if ((sais = fetch_resolve(sockshost, socksport, af)) == NULL) | |||||
goto fail; | |||||
} | |||||
/* try each server address in turn */ | /* try each server address in turn */ | ||||
for (err = 0, sai = sais; sai != NULL; sai = sai->ai_next) { | for (err = 0, sai = sais; sai != NULL; sai = sai->ai_next) { | ||||
/* open socket */ | /* open socket */ | ||||
if ((sd = socket(sai->ai_family, SOCK_STREAM, 0)) < 0) | if ((sd = socket(sai->ai_family, SOCK_STREAM, 0)) < 0) | ||||
goto syserr; | goto syserr; | ||||
/* attempt to bind to client address */ | /* attempt to bind to client address */ | ||||
for (err = 0, cai = cais; cai != NULL; cai = cai->ai_next) { | for (err = 0, cai = cais; cai != NULL; cai = cai->ai_next) { | ||||
if (cai->ai_family != sai->ai_family) | if (cai->ai_family != sai->ai_family) | ||||
continue; | continue; | ||||
Done Inline ActionsMoved up to the line above kevans: Moved up to the line above | |||||
if ((err = bind(sd, cai->ai_addr, cai->ai_addrlen)) == 0) | if ((err = bind(sd, cai->ai_addr, cai->ai_addrlen)) == 0) | ||||
break; | break; | ||||
} | } | ||||
if (err != 0) { | if (err != 0) { | ||||
if (verbose) | if (verbose) | ||||
Done Inline ActionsBraces around this single-statement block should match the prevailing style elsewhere in the file for single-statement blocks; it looks like the braces should be dropped here. kevans: Braces around this single-statement block should match the prevailing style elsewhere in the… | |||||
Done Inline ActionsThis one seems correct, no? farhan_farhan.codes: This one seems correct, no? | |||||
Not Done Inline ActionsCorrect, but stylistically inconsistent with the rest of the file- take a look ,for instance, at lines 556/557. kevans: Correct, but stylistically inconsistent with the rest of the file- take a look ,for instance… | |||||
Done Inline Actionsoh, I misunderstood your original comment. My mistake. Removing now. farhan_farhan.codes: oh, I misunderstood your original comment. My mistake. Removing now. | |||||
fetch_info("failed to bind to %s", bindaddr); | fetch_info("failed to bind to %s", bindaddr); | ||||
goto syserr; | goto syserr; | ||||
} | } | ||||
/* attempt to connect to server address */ | /* attempt to connect to server address */ | ||||
if ((err = connect(sd, sai->ai_addr, sai->ai_addrlen)) == 0) | if ((err = connect(sd, sai->ai_addr, sai->ai_addrlen)) == 0) | ||||
break; | break; | ||||
/* clean up before next attempt */ | /* clean up before next attempt */ | ||||
close(sd); | close(sd); | ||||
sd = -1; | sd = -1; | ||||
} | } | ||||
if (err != 0) { | if (err != 0) { | ||||
if (verbose) | if (verbose && sockshost == NULL) | ||||
fetch_info("failed to connect to %s:%d", host, port); | fetch_info("failed to connect to %s:%d", host, port); | ||||
else if (verbose && sockshost) | |||||
fetch_info("failed to connect to SOCKS5 server %s:%d", sockshost, socksport); | |||||
goto syserr; | goto syserr; | ||||
} | } | ||||
Done Inline ActionsThis socks5_seterr is currently blocked behind an if(verbose) -- I suspect you want to break that one out above before the verbose blocks start and set it if sockshost != NULL kevans: This socks5_seterr is currently blocked behind an if(verbose) -- I suspect you want to break… | |||||
if ((conn = fetch_reopen(sd)) == NULL) | if ((conn = fetch_reopen(sd)) == NULL) | ||||
Done Inline ActionsAdded line should go away kevans: Added line should go away | |||||
goto syserr; | goto syserr; | ||||
if (sockshost) | |||||
if (!fetch_socks5_init(conn, host, port, verbose)) | |||||
goto fail; | |||||
if (cais != NULL) | if (cais != NULL) | ||||
freeaddrinfo(cais); | freeaddrinfo(cais); | ||||
if (sais != NULL) | if (sais != NULL) | ||||
freeaddrinfo(sais); | freeaddrinfo(sais); | ||||
return (conn); | return (conn); | ||||
syserr: | syserr: | ||||
fetch_syserr(); | fetch_syserr(); | ||||
goto fail; | goto fail; | ||||
▲ Show 20 Lines • Show All 1,113 Lines • Show Last 20 Lines |
Should this size be macro-ized?