commit 13e30cbac02e24cee9615e35d11fc71ab59fb8d1 Author: Zack Weinberg zackw@panix.com Date: Mon Jul 18 14:42:19 2011 -0700

Switch from C-library assert to an obfs_assert/abort that go through the logging system --- src/crypt.c | 5 +-- src/network.c | 19 ++++++------- src/protocol.c | 34 ++++++++++++------------ src/protocols/dummy.c | 1 - src/protocols/obfs2.c | 3 +- src/sha256.c | 4 +- src/socks.c | 15 +++++------ src/util.c | 66 ++++++++++++++++++++++++++++++++---------------- src/util.h | 20 +++++++++++++++ 9 files changed, 102 insertions(+), 65 deletions(-)

diff --git a/src/crypt.c b/src/crypt.c index 722fc00..9984e24 100644 --- a/src/crypt.c +++ b/src/crypt.c @@ -6,7 +6,6 @@ #include "crypt.h" #include "util.h"

-#include <assert.h> #include <fcntl.h> #include <limits.h> #include <stdlib.h> @@ -160,7 +159,7 @@ crypt_new(const uchar *key, size_t keylen) { crypt_t *k;

- assert(keylen == AES_BLOCK_SIZE); + obfs_assert(keylen == AES_BLOCK_SIZE); k = xzalloc(sizeof(crypt_t)); AES_set_encrypt_key(key, AES_BLOCK_SIZE * CHAR_BIT, &k->key);

@@ -173,7 +172,7 @@ crypt_new(const uchar *key, size_t keylen) void crypt_set_iv(crypt_t *key, const uchar *iv, size_t ivlen) { - assert(ivlen == sizeof(key->ivec)); + obfs_assert(ivlen == sizeof(key->ivec)); memcpy(key->ivec, iv, ivlen); }

diff --git a/src/network.c b/src/network.c index fb770c9..fcbb45c 100644 --- a/src/network.c +++ b/src/network.c @@ -10,7 +10,6 @@ #include "socks.h" #include "protocol.h"

-#include <assert.h> #include <errno.h> #include <stdlib.h> #include <string.h> @@ -96,7 +95,7 @@ close_all_connections(void) conn_t *conn = DOWNCAST(conn_t, dll_node, conn_list.head); conn_free(conn); /* removes it */ } - assert(!n_connections); + obfs_assert(!n_connections); } /** This function spawns a listener configured according to the @@ -226,7 +225,7 @@ simple_listener_cb(struct evconnlistener *evcl, bufferevent_setcb(conn->input, plaintext_read_cb, NULL, input_event_cb, conn); } else { - assert(conn->mode == LSN_SOCKS_CLIENT); + obfs_assert(conn->mode == LSN_SOCKS_CLIENT); bufferevent_setcb(conn->input, socks_read_cb, NULL, input_event_cb, conn); } @@ -303,7 +302,7 @@ conn_free(conn_t *conn) memset(conn, 0x99, sizeof(conn_t)); free(conn);

- assert(n_connections>=0); + obfs_assert(n_connections>=0); log_debug("Connection destroyed. " "We currently have %d connections!", n_connections);

@@ -336,19 +335,19 @@ socks_read_cb(struct bufferevent *bev, void *arg) conn_t *conn = arg; //struct bufferevent *other; enum socks_ret socks_ret; - assert(bev == conn->input); /* socks must be on the initial bufferevent */ + obfs_assert(bev == conn->input); /* socks must be on the initial bufferevent */

do { enum socks_status_t status = socks_state_get_status(conn->socks_state); if (status == ST_SENT_REPLY) { /* We shouldn't be here. */ - assert(0); + obfs_abort(); } else if (status == ST_HAVE_ADDR) { int af, r, port; const char *addr=NULL; r = socks_state_get_address(conn->socks_state, &af, &addr, &port); - assert(r==0); + obfs_assert(r==0); r = bufferevent_socket_connect_hostname(conn->output, get_evdns_base(), af, addr, port); @@ -461,7 +460,7 @@ static void input_event_cb(struct bufferevent *bev, short what, void *arg) { conn_t *conn = arg; - assert(bev == conn->input); + obfs_assert(bev == conn->input);

if (what & (BEV_EVENT_EOF|BEV_EVENT_ERROR)) { log_warn("Got error: %s", @@ -478,7 +477,7 @@ static void output_event_cb(struct bufferevent *bev, short what, void *arg) { conn_t *conn = arg; - assert(bev == conn->output); + obfs_assert(bev == conn->output);

/** If we got the BEV_EVENT_ERROR flag *AND* we are in socks mode @@ -532,7 +531,7 @@ output_event_cb(struct bufferevent *bev, short what, void *arg) struct sockaddr_storage ss; struct sockaddr *sa = (struct sockaddr*)&ss; socklen_t slen = sizeof(&ss); - assert(conn->socks_state); + obfs_assert(conn->socks_state); if (getpeername(bufferevent_getfd(bev), sa, &slen) == 0) { /* Figure out where we actually connected to so that we can tell the * socks client */ diff --git a/src/protocol.c b/src/protocol.c index 9aa1f8b..4473a94 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -5,8 +5,8 @@ #include "protocol.h" #include "protocols/obfs2.h" #include "protocols/dummy.h" +#include "util.h"

-#include <assert.h> #include <stdlib.h> #include <string.h>

@@ -50,7 +50,7 @@ proto_params_init(int n_options, const char *const *options) void proto_params_free(protocol_params_t *params) { - assert(params); + obfs_assert(params);

if (params->target_address) free(params->target_address); @@ -70,9 +70,9 @@ proto_params_free(protocol_params_t *params) struct protocol_t * proto_create(protocol_params_t *params) { - assert(params); - assert(params->vtable); - assert(params->vtable->create); + obfs_assert(params); + obfs_assert(params->vtable); + obfs_assert(params->vtable->create); return params->vtable->create(params); }

@@ -82,9 +82,9 @@ proto_create(protocol_params_t *params) */ int proto_handshake(struct protocol_t *proto, void *buf) { - assert(proto); - assert(proto->vtable); - assert(proto->vtable->handshake); + obfs_assert(proto); + obfs_assert(proto->vtable); + obfs_assert(proto->vtable->handshake); return proto->vtable->handshake(proto, buf); }

@@ -93,9 +93,9 @@ proto_handshake(struct protocol_t *proto, void *buf) { */ int proto_send(struct protocol_t *proto, void *source, void *dest) { - assert(proto); - assert(proto->vtable); - assert(proto->vtable->send); + obfs_assert(proto); + obfs_assert(proto->vtable); + obfs_assert(proto->vtable->send); return proto->vtable->send(proto, source, dest); }

@@ -104,9 +104,9 @@ proto_send(struct protocol_t *proto, void *source, void *dest) { */ enum recv_ret proto_recv(struct protocol_t *proto, void *source, void *dest) { - assert(proto); - assert(proto->vtable); - assert(proto->vtable->recv); + obfs_assert(proto); + obfs_assert(proto->vtable); + obfs_assert(proto->vtable->recv); return proto->vtable->recv(proto, source, dest); }

@@ -116,8 +116,8 @@ proto_recv(struct protocol_t *proto, void *source, void *dest) { */ void proto_destroy(struct protocol_t *proto) { - assert(proto); - assert(proto->vtable); - assert(proto->vtable->destroy); + obfs_assert(proto); + obfs_assert(proto->vtable); + obfs_assert(proto->vtable->destroy); proto->vtable->destroy(proto); } diff --git a/src/protocols/dummy.c b/src/protocols/dummy.c index 61cdf2b..b1493ce 100644 --- a/src/protocols/dummy.c +++ b/src/protocols/dummy.c @@ -6,7 +6,6 @@ #include "../protocol.h" #include "../util.h"

-#include <assert.h> #include <stdlib.h> #include <string.h>

diff --git a/src/protocols/obfs2.c b/src/protocols/obfs2.c index c42d1c4..5acd09e 100644 --- a/src/protocols/obfs2.c +++ b/src/protocols/obfs2.c @@ -7,7 +7,6 @@

#include "../util.h"

-#include <assert.h> #include <stdlib.h> #include <string.h>

@@ -322,7 +321,7 @@ obfs2_handshake(struct protocol_t *s, struct evbuffer *buf) SEED | E_PAD_KEY( UINT32(MAGIC_VALUE) | UINT32(PADLEN) | WR(PADLEN) ) */

- assert(sizeof(magic) == 4); + obfs_assert(sizeof(magic) == 4);

/* generate padlen */ if (random_bytes((uchar*)&plength, 4) < 0) diff --git a/src/sha256.c b/src/sha256.c index ff2cf21..d303f5f 100644 --- a/src/sha256.c +++ b/src/sha256.c @@ -5,7 +5,7 @@ have a SHA256. */

#include "sha256.h" -#include <assert.h> +#include "util.h" #include <string.h> #include <arpa/inet.h> /* for htonl/ntohl */

@@ -23,7 +23,7 @@ get_uint32(const void *ptr) memcpy(&val, ptr, 4); return val; } -#define LTC_ARGCHK(x) assert((x)) +#define LTC_ARGCHK(x) obfs_assert(x)

#define CRYPT_OK 0 #define CRYPT_NOP -1 diff --git a/src/socks.c b/src/socks.c index b67455f..6bd708f 100644 --- a/src/socks.c +++ b/src/socks.c @@ -7,7 +7,6 @@

#include "util.h"

-#include <assert.h> #include <errno.h> #include <stdlib.h> #include <string.h> @@ -189,19 +188,19 @@ socks5_handle_request(struct evbuffer *source, struct parsereq *parsereq) goto err;

if (evbuffer_remove(source, destaddr, addrlen) != addrlen) - assert(0); + obfs_abort();

if (evbuffer_remove(source, (char *)&destport, 2) != 2) - assert(0); + obfs_abort();

destaddr[addrlen] = '\0';

if (af == AF_UNSPEC) { - assert(addrlen < sizeof(parsereq->addr)); + obfs_assert(addrlen < sizeof(parsereq->addr)); memcpy(parsereq->addr, destaddr, addrlen+1); } else { char a[16]; - assert(addrlen <= 16); + obfs_assert(addrlen <= 16); memcpy(a, destaddr, addrlen); if (evutil_inet_ntop(af, destaddr, parsereq->addr, sizeof(parsereq->addr)) == NULL) @@ -331,7 +330,7 @@ socks5_handle_negotiation(struct evbuffer *source, }

if (evbuffer_remove(source, methods, nmethods) < 0) - assert(0); + obfs_abort();

for (found_noauth=0, i=0; i<nmethods ; i++) { if (methods[i] == SOCKS5_METHOD_NOAUTH) { @@ -477,7 +476,7 @@ handle_socks(struct evbuffer *source, struct evbuffer *dest, }

/* ST_SENT_REPLY connections shouldn't be here! */ - assert(socks_state->state != ST_SENT_REPLY && + obfs_assert(socks_state->state != ST_SENT_REPLY && socks_state->state != ST_HAVE_ADDR);

if (socks_state->version == 0) { @@ -527,7 +526,7 @@ handle_socks(struct evbuffer *source, struct evbuffer *dest, return SOCKS_CMD_NOT_CONNECT; } else if (r == SOCKS_BROKEN) goto broken; - assert(0); + obfs_abort(); } break; default: diff --git a/src/util.c b/src/util.c index ce25375..231c3a8 100644 --- a/src/util.c +++ b/src/util.c @@ -4,7 +4,6 @@

#include "util.h"

-#include <assert.h> #include <errno.h> #include <fcntl.h> #include <limits.h> @@ -28,8 +27,7 @@ static void ATTR_NORETURN die_oom(void) { - log_warn("Memory allocation failed: %s",strerror(errno)); - exit(1); + log_error("Memory allocation failed: %s",strerror(errno)); }

void * @@ -37,7 +35,7 @@ xmalloc(size_t size) { void *result;

- assert(size < SIZE_T_CEILING); + obfs_assert(size < SIZE_T_CEILING);

/* Some malloc() implementations return NULL when the input argument is zero. We don't bother detecting whether the implementation we're @@ -57,7 +55,7 @@ void * xrealloc(void *ptr, size_t size) { void *result; - assert (size < SIZE_T_CEILING); + obfs_assert (size < SIZE_T_CEILING); if (size == 0) size = 1;

@@ -217,8 +215,8 @@ obfs_vsnprintf(char *str, size_t size, const char *format, va_list args) static void dll_insert_after(dll_t *list, dll_node_t *node, dll_node_t *new_node) { - assert(node); - assert(new_node); + obfs_assert(node); + obfs_assert(new_node);

if (!list) return; @@ -238,8 +236,8 @@ dll_insert_after(dll_t *list, dll_node_t *node, dll_node_t *new_node) static void dll_insert_before(dll_t *list, dll_node_t *node, dll_node_t *new_node) { - assert(node); - assert(new_node); + obfs_assert(node); + obfs_assert(new_node);

if (!list) return; @@ -266,7 +264,7 @@ dll_init(dll_t *list) static void dll_insert_beginning(dll_t *list, dll_node_t *node) { - assert(node); + obfs_assert(node);

if (!list) return; @@ -288,8 +286,8 @@ dll_insert_beginning(dll_t *list, dll_node_t *node) int dll_append(dll_t *list, dll_node_t *node) { - assert(list); - assert(node); + obfs_assert(list); + obfs_assert(node);

if (!list->tail) dll_insert_beginning(list, node); @@ -306,7 +304,7 @@ dll_append(dll_t *list, dll_node_t *node) void dll_remove(dll_t *list, dll_node_t *node) { - assert(node); + obfs_assert(node);

if (!list) return; @@ -326,6 +324,10 @@ dll_remove(dll_t *list, dll_node_t *node) off tor. It's basicaly a stripped down version of tor's logging system. Thank you tor. */

+/* Note: obfs_assert and obfs_abort cannot be used anywhere in the + logging system, as they will recurse into the logging system and + cause an infinite loop. We use plain old abort(3) instead. */ + /* Size of maximum log entry, including newline and NULL byte. */ #define MAX_LOG_ENTRY 1024 /* String to append when a log entry doesn't fit in MAX_LOG_ENTRY. */ @@ -335,6 +337,7 @@ dll_remove(dll_t *list, dll_node_t *node)

/** Logging severities */

+#define LOG_SEV_ERR 4 #define LOG_SEV_WARN 3 #define LOG_SEV_INFO 2 #define LOG_SEV_DEBUG 1 @@ -351,11 +354,12 @@ static const char * sev_to_string(int severity) { switch (severity) { + case LOG_SEV_ERR: return "error"; case LOG_SEV_WARN: return "warn"; case LOG_SEV_INFO: return "info"; case LOG_SEV_DEBUG: return "debug"; default: - assert(0); return "UNKNOWN"; + abort(); } }

@@ -364,6 +368,8 @@ sev_to_string(int severity) static int string_to_sev(const char *string) { + if (!strcasecmp(string, "error")) + return LOG_SEV_ERR; if (!strcasecmp(string, "warn")) return LOG_SEV_WARN; else if (!strcasecmp(string, "info")) @@ -381,7 +387,8 @@ string_to_sev(const char *string) static int sev_is_valid(int severity) { - return (severity == LOG_SEV_WARN || + return (severity == LOG_SEV_ERR || + severity == LOG_SEV_WARN || severity == LOG_SEV_INFO || severity == LOG_SEV_DEBUG); } @@ -473,7 +480,8 @@ log_set_min_severity(const char* sev_string) static void logv(int severity, const char *format, va_list ap) { - assert(sev_is_valid(severity)); + if (!sev_is_valid(severity)) + abort();

if (logging_method == LOG_METHOD_NULL) return; @@ -500,7 +508,8 @@ logv(int severity, const char *format, va_list ap) size_t offset = buflen-TRUNCATED_STR_LEN; r = obfs_snprintf(buf+offset, TRUNCATED_STR_LEN+1, "%s", TRUNCATED_STR); - if (r < 0) assert(0); + if (r < 0) + abort(); } n = buflen; } else @@ -512,24 +521,26 @@ logv(int severity, const char *format, va_list ap) if (logging_method == LOG_METHOD_STDOUT) fprintf(stdout, "%s", buf); else if (logging_method == LOG_METHOD_FILE) { - assert(logging_logfile); + if (!logging_logfile) + abort(); if (write(logging_logfile, buf, strlen(buf)) < 0) - printf("%s(): Terrible write() error!!!\n", __func__); + abort(); } else - assert(0); + abort(); }

/**** Public logging API. ****/

void -log_info(const char *format, ...) +log_error(const char *format, ...) { va_list ap; va_start(ap,format);

- logv(LOG_SEV_INFO, format, ap); + logv(LOG_SEV_ERR, format, ap);

va_end(ap); + exit(1); }

void @@ -544,6 +555,17 @@ log_warn(const char *format, ...) }

void +log_info(const char *format, ...) +{ + va_list ap; + va_start(ap,format); + + logv(LOG_SEV_INFO, format, ap); + + va_end(ap); +} + +void log_debug(const char *format, ...) { va_list ap; diff --git a/src/util.h b/src/util.h index 0693930..8e5dd77 100644 --- a/src/util.h +++ b/src/util.h @@ -98,6 +98,10 @@ void close_obfsproxy_logfile(void);

/** The actual log-emitting functions */

+/** Fatal errors: the program cannot continue and will exit. */ +void log_error(const char *format, ...) + ATTR_PRINTF_1 ATTR_NORETURN; + /** Warn-level severity: for messages that only appear when something has gone wrong. */ void log_warn(const char *format, ...) @@ -113,4 +117,20 @@ void log_info(const char *format, ...) void log_debug(const char *format, ...) ATTR_PRINTF_1;

+/** Assertion checking. We don't ever compile assertions out, and we + want precise control over the error messages, so we use our own + assertion macros. */ +#define obfs_assert(expr) \ + do { \ + if (!(expr)) \ + log_error("assertion failure at %s:%d: %s", \ + __FILE__, __LINE__, #expr); \ + } while (0) + +#define obfs_abort() \ + do { \ + log_error("aborted at %s:%d", __FILE__, __LINE__); \ + } while (0) + + #endif