commit 9a6de8ce32c36d04fd96e48194230dfcdc1132cf Author: Zack Weinberg zackw@panix.com Date: Sun Apr 1 14:23:17 2012 -0700
transmit_room() now takes 3 arguments and the chopper produces exactly what is asked for. As a side-effect this made HTTP reliable under automated test. --- src/protocol/chop.cc | 106 ++++++++++++++++++++++++++--------------- src/steg.h | 66 +++++++++++++++----------- src/steg/embed.cc | 5 ++- src/steg/http.cc | 124 ++++++++++++++++++++----------------------------- src/steg/nosteg.cc | 4 +- src/steg/nosteg_rr.cc | 4 +- 6 files changed, 165 insertions(+), 144 deletions(-)
diff --git a/src/protocol/chop.cc b/src/protocol/chop.cc index a46d0ac..ad8ac9f 100644 --- a/src/protocol/chop.cc +++ b/src/protocol/chop.cc @@ -73,6 +73,8 @@ const size_t SECTION_LEN = UINT16_MAX; const size_t MIN_BLOCK_SIZE = HEADER_LEN + TRAILER_LEN; const size_t MAX_BLOCK_SIZE = MIN_BLOCK_SIZE + SECTION_LEN*2;
+const size_t HANDSHAKE_LEN = sizeof(uint32_t); + enum opcode_t { op_DAT = 0, // Pass data section along to upstream @@ -729,16 +731,18 @@ chop_circuit_t::send_special(opcode_t f, struct evbuffer *payload) size_t blocksize; log_assert(d <= SECTION_LEN); chop_conn_t *conn = pick_connection(d, &blocksize); + size_t lo = MIN_BLOCK_SIZE + ((conn && !conn->sent_handshake) + ? HANDSHAKE_LEN : 0);
- if (!conn || (blocksize - MIN_BLOCK_SIZE < d)) { + if (!conn || (blocksize - lo < d)) { log_warn("no usable connection for special block " "(opcode %02x, need %lu bytes, have %lu)", - (unsigned int)f, (unsigned long)(d + MIN_BLOCK_SIZE), + (unsigned int)f, (unsigned long)(d + lo), (unsigned long)blocksize); return -1; }
- return send_targeted(conn, d, (blocksize - MIN_BLOCK_SIZE) - d, f, payload); + return send_targeted(conn, d, (blocksize - lo) - d, f, payload); }
int @@ -749,33 +753,47 @@ chop_circuit_t::send_targeted(chop_conn_t *conn) avail = SECTION_LEN; avail += MIN_BLOCK_SIZE;
- size_t room = conn->steg->transmit_room(); - if (room < MIN_BLOCK_SIZE) { - log_warn(conn, "send() called without enough transmit room " - "(have %lu, need %lu)", (unsigned long)room, - (unsigned long)MIN_BLOCK_SIZE); - return -1; + // If we have any data to transmit, ensure we do not send a block + // that contains no data at all. + size_t lo = MIN_BLOCK_SIZE + (avail == MIN_BLOCK_SIZE ? 0 : 1); + + // If this connection has not yet sent a handshake, it will need to. + size_t hi = MAX_BLOCK_SIZE; + if (!conn->sent_handshake) { + lo += HANDSHAKE_LEN; + hi += HANDSHAKE_LEN; + avail += HANDSHAKE_LEN; } - log_debug(conn, "offers %lu bytes (%s)", (unsigned long)room, - conn->steg->cfg()->name());
- if (room < avail) - avail = room; + size_t room = conn->steg->transmit_room(avail, lo, hi); + if (room == 0) + log_abort(conn, "must send but cannot send"); + if (room < lo || room >= hi) + log_abort(conn, "steg size request (%lu) out of range [%lu, %lu]", + (unsigned long)room, (unsigned long)lo, (unsigned long)hi);
- return send_targeted(conn, avail); + log_debug(conn, "requests %lu bytes (%s)", (unsigned long)room, + conn->steg->cfg()->name()); + + return send_targeted(conn, room); }
int chop_circuit_t::send_targeted(chop_conn_t *conn, size_t blocksize) { - log_assert(blocksize >= MIN_BLOCK_SIZE && blocksize <= MAX_BLOCK_SIZE); + size_t lo = MIN_BLOCK_SIZE, hi = MAX_BLOCK_SIZE; + if (!conn->sent_handshake) { + lo += HANDSHAKE_LEN; + hi += HANDSHAKE_LEN; + } + log_assert(blocksize >= lo && blocksize <= hi);
struct evbuffer *xmit_pending = bufferevent_get_input(up_buffer); size_t avail = evbuffer_get_length(xmit_pending); opcode_t op = op_DAT;
- if (avail > blocksize - MIN_BLOCK_SIZE) - avail = blocksize - MIN_BLOCK_SIZE; + if (avail > blocksize - lo) + avail = blocksize - lo; else if (avail > SECTION_LEN) avail = SECTION_LEN; else if (upstream_eof && !sent_fin) @@ -783,7 +801,7 @@ chop_circuit_t::send_targeted(chop_conn_t *conn, size_t blocksize) // this direction; mark it as such op = op_FIN;
- return send_targeted(conn, avail, (blocksize - MIN_BLOCK_SIZE) - avail, + return send_targeted(conn, avail, (blocksize - lo) - avail, op, xmit_pending); }
@@ -868,6 +886,10 @@ chop_circuit_t::pick_connection(size_t desired, size_t *blocksize)
desired += MIN_BLOCK_SIZE;
+ // If we have any data to transmit, ensure we do not send a block + // that contains no data at all. + size_t lo = MIN_BLOCK_SIZE + (desired == MIN_BLOCK_SIZE ? 0 : 1); + log_debug(this, "target block size %lu bytes", (unsigned long)desired);
// Find the best fit for the desired transmission from all the @@ -880,18 +902,25 @@ chop_circuit_t::pick_connection(size_t desired, size_t *blocksize) continue; }
- size_t room = conn->steg->transmit_room(); - - if (room <= MIN_BLOCK_SIZE) - room = 0; + size_t shake = conn->sent_handshake ? 0 : HANDSHAKE_LEN; + size_t room = conn->steg->transmit_room(desired + shake, lo + shake, + MAX_BLOCK_SIZE + shake); + if (room == 0) { + log_debug(conn, "offers 0 bytes (%s)", + conn->steg->cfg()->name()); + continue; + }
- if (room > MAX_BLOCK_SIZE) - room = MAX_BLOCK_SIZE; + if (room < lo + shake || room >= MAX_BLOCK_SIZE + shake) + log_abort(conn, "steg size request (%lu) out of range [%lu, %lu]", + (unsigned long)room, + (unsigned long)(lo + shake), + (unsigned long)(MAX_BLOCK_SIZE + shake));
log_debug(conn, "offers %lu bytes (%s)", (unsigned long)room, conn->steg->cfg()->name());
- if (room >= desired) { + if (room >= desired + shake) { if (room < minabove) { minabove = room; targabove = conn; @@ -915,7 +944,7 @@ chop_circuit_t::pick_connection(size_t desired, size_t *blocksize) // their initial values, so we'll return NULL and set blocksize to 0, // which callers know how to handle. if (targabove) { - *blocksize = desired; + *blocksize = minabove; return targabove; } else { *blocksize = maxbelow; @@ -1086,7 +1115,7 @@ chop_conn_t::send(struct evbuffer *block) upstream ? '+' : '-', upstream ? upstream->circuit_id : 0); if (evbuffer_prepend(block, (void *)&upstream->circuit_id, - sizeof(upstream->circuit_id))) { + HANDSHAKE_LEN)) { log_warn(this, "failed to prepend handshake to first block"); return -1; } @@ -1332,29 +1361,28 @@ chop_conn_t::send() } else { log_debug(this, "must send (no upstream)");
- size_t room = steg->transmit_room(); - if (room < MIN_BLOCK_SIZE) { - log_warn(this, "send() called without enough transmit room " - "(have %lu, need %lu)", (unsigned long)room, - (unsigned long)MIN_BLOCK_SIZE); - conn_do_flush(this); - return; - } + size_t room = steg->transmit_room(MIN_BLOCK_SIZE, MIN_BLOCK_SIZE, + MAX_BLOCK_SIZE); + if (room < MIN_BLOCK_SIZE || room >= MAX_BLOCK_SIZE) + log_abort(this, "steg size request (%lu) out of range [%lu, %lu]", + (unsigned long)room, + (unsigned long)MIN_BLOCK_SIZE, + (unsigned long)MAX_BLOCK_SIZE);
// Since we have no upstream, we can't encrypt anything; instead, // generate random bytes and feed them straight to steg_transmit. struct evbuffer *chaff = evbuffer_new(); struct evbuffer_iovec v; - if (!chaff || evbuffer_reserve_space(chaff, MIN_BLOCK_SIZE, &v, 1) != 1 || - v.iov_len < MIN_BLOCK_SIZE) { + if (!chaff || evbuffer_reserve_space(chaff, room, &v, 1) != 1 || + v.iov_len < room) { log_warn(this, "memory allocation failed"); if (chaff) evbuffer_free(chaff); conn_do_flush(this); return; } - v.iov_len = MIN_BLOCK_SIZE; - rng_bytes((uint8_t *)v.iov_base, MIN_BLOCK_SIZE); + v.iov_len = room; + rng_bytes((uint8_t *)v.iov_base, room); if (evbuffer_commit_space(chaff, &v, 1)) { log_warn(this, "evbuffer_commit_space failed"); if (chaff) diff --git a/src/steg.h b/src/steg.h index a044afb..ad6b829 100644 --- a/src/steg.h +++ b/src/steg.h @@ -52,11 +52,22 @@ struct steg_t /** Return the steg_config_t from which this steg_t was created. */ virtual steg_config_t *cfg() = 0;
- /** Report the maximum number of bytes that could be transmitted on - your connection at this time. You must be prepared to handle a - subsequent request to transmit any _smaller_ number of bytes on - this connection. */ - virtual size_t transmit_room() = 0; + /** The protocol using this steg module would like to transmit PREF + bytes on your connection. Return an adjusted number of bytes; + you may adjust down to indicate that you cannot transmit all of + the available data, or up to indicate that it should be padded. + + Returning zero indicates that your connection cannot transmit at + all right now; if you do this, transmit() will not be called. + Returning any nonzero value indicates that you want to transmit + exactly that number of bytes. The protocol may or may not call + transmit() after you return a nonzero value, but if it does, it + will provide the number of bytes you requested. + + If you return a nonzero value, it MUST be greater than or equal + to MIN, and less than or equal to MAX. PREF is guaranteed to be + in this range already. */ + virtual size_t transmit_room(size_t pref, size_t min, size_t max) = 0;
/** Consume all of the data in SOURCE, disguise it, and write it to the outbound buffer for your connection. Return 0 on success, -1 @@ -94,32 +105,33 @@ steg_config_t *steg_new(const char *name, config_t *cfg);
/* Macros for use in defining steg modules. */
-#define STEG_DEFINE_MODULE(mod) \ - /* new_ dispatchers */ \ - static steg_config_t *mod##_new(config_t *cfg) \ - { return new mod##_steg_config_t(cfg); } \ - \ - /* canned methods */ \ - const char *mod##_steg_config_t::name() { return #mod; } \ - \ - /* module object */ \ - extern const steg_module s_mod_##mod = { \ - #mod, mod##_new \ +#define STEG_DEFINE_MODULE(mod) \ + /* new_ dispatchers */ \ + static steg_config_t *mod##_new(config_t *cfg) \ + { return new mod##_steg_config_t(cfg); } \ + \ + /* canned methods */ \ + const char *mod##_steg_config_t::name() \ + { return #mod; } \ + \ + /* module object */ \ + extern const steg_module s_mod_##mod = { \ + #mod, mod##_new \ } /* deliberate absence of semicolon */
-#define STEG_CONFIG_DECLARE_METHODS(mod) \ - mod##_steg_config_t(config_t *cfg); \ - virtual ~mod##_steg_config_t(); \ - virtual const char *name(); \ - virtual steg_t *steg_create(conn_t *conn) \ +#define STEG_CONFIG_DECLARE_METHODS(mod) \ + mod##_steg_config_t(config_t *); \ + virtual ~mod##_steg_config_t(); \ + virtual const char *name(); \ + virtual steg_t *steg_create(conn_t *) \ /* deliberate absence of semicolon */
-#define STEG_DECLARE_METHODS(mod) \ - virtual ~mod##_steg_t(); \ - virtual steg_config_t *cfg(); \ - virtual size_t transmit_room(); \ - virtual int transmit(struct evbuffer *source); \ - virtual int receive(struct evbuffer *dest) \ +#define STEG_DECLARE_METHODS(mod) \ + virtual ~mod##_steg_t(); \ + virtual steg_config_t *cfg(); \ + virtual size_t transmit_room(size_t, size_t, size_t); \ + virtual int transmit(struct evbuffer *); \ + virtual int receive(struct evbuffer *) \ /* deliberate absence of semicolon */
#endif diff --git a/src/steg/embed.cc b/src/steg/embed.cc index f24c210..d301620 100644 --- a/src/steg/embed.cc +++ b/src/steg/embed.cc @@ -163,7 +163,7 @@ embed_steg_t::cfg() }
size_t -embed_steg_t::transmit_room() +embed_steg_t::transmit_room(size_t, size_t lo, size_t hi) { if (is_finished() || !is_outgoing()) return 0;
@@ -173,6 +173,9 @@ embed_steg_t::transmit_room() // 2 bytes for data length, 4 bytes for the index of a new trace size_t room = get_pkt_size() - 2; if (cur_pkt == 0) room -= 4; + + if (room < lo) room = lo; + if (room > hi) room = hi; return room; }
diff --git a/src/steg/http.cc b/src/steg/http.cc index 77ea859..a4fd790 100644 --- a/src/steg/http.cc +++ b/src/steg/http.cc @@ -36,6 +36,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "connections.h" #include "protocol.h" #include "steg.h" +#include "rng.h" + #include "payloads.h" #include "cookies.h" #include "swfSteg.h" @@ -184,64 +186,59 @@ http_steg_t::cfg() return config; }
-size_t -http_steg_t::transmit_room() +static size_t +clamp(size_t val, size_t lo, size_t hi) { - unsigned int mjc; + if (val < lo) return lo; + if (val > hi) return hi; + return val; +}
+size_t +http_steg_t::transmit_room(size_t pref, size_t lo, size_t hi) +{ if (have_transmitted) /* can't send any more on this connection */ return 0;
- if (config->is_clientside) { - return (MIN_COOKIE_SIZE + rand() % (MAX_COOKIE_SIZE - MIN_COOKIE_SIZE)) / 4; + // MIN_COOKIE_SIZE and MAX_COOKIE_SIZE are *after* base64'ing + if (lo < MIN_COOKIE_SIZE*3/4) + lo = MIN_COOKIE_SIZE*3/4; + + if (hi > MAX_COOKIE_SIZE*3/4) + hi = MAX_COOKIE_SIZE*3/4; } else { - if (!have_received) return 0;
switch (type) { - case HTTP_CONTENT_SWF: - return 1024; + if (hi >= 1024) + hi = 1024; + break;
case HTTP_CONTENT_JAVASCRIPT: - mjc = config->pl.max_JS_capacity / 2; - if (mjc > 1024) { - // it should be 1024 + ...., but seems like we need to be a little bit smaller (chopper bug?) - int rval = 512 + rand()%(mjc - 1024); - // fprintf(stderr, "returning rval %d, mjc %d\n", rval, mjc); - return rval; - } - log_warn("js capacity too small\n"); - exit(-1); + if (hi >= config->pl.max_JS_capacity / 2) + hi = config->pl.max_JS_capacity / 2; + break;
case HTTP_CONTENT_HTML: - mjc = config->pl.max_HTML_capacity / 2; - if (mjc > 1024) { - // it should be 1024 + ...., but seems like we need to be a little bit smaller (chopper bug?) - int rval = 512 + rand()%(mjc - 1024); - // fprintf(stderr, "returning rval %d, mjc %d\n", rval, mjc); - return rval; - } - log_warn("js capacity too small\n"); - exit(-1); + if (hi >= config->pl.max_HTML_capacity / 2) + hi = config->pl.max_HTML_capacity / 2; + break;
case HTTP_CONTENT_PDF: - // return 1024 + rand()%(get_max_PDF_capacity() - 1024) - return PDF_MIN_AVAIL_SIZE; + if (hi >= PDF_MIN_AVAIL_SIZE) + hi = PDF_MIN_AVAIL_SIZE; + break; } - - return SIZE_MAX; } -} - - - -
+ log_assert(hi >= lo); + return clamp(pref + rng_range_geom(hi - lo, 8), lo, hi); +}
int lookup_peer_name_from_ip(const char* p_ip, char* p_name) { @@ -346,7 +343,7 @@ http_client_cookie_transmit (http_steg_t *s, struct evbuffer *source, // substitute / with _, + with ., = with - that maybe inserted anywhere in the middle sanitize_b64(data2, len);
- + cookie_len = gen_b64_cookie_field(cookiebuf, data2, len); cookiebuf[cookie_len] = 0;
@@ -356,7 +353,10 @@ http_client_cookie_transmit (http_steg_t *s, struct evbuffer *source, log_debug("cookie generation failed\n"); return -1; } - + log_debug(conn, "cookie input %ld encoded %d final %d", + sbuflen, len, cookie_len); + log_debug(conn, "cookie encoded: %s", data2); + log_debug(conn, "cookie final: %s", cookiebuf);
// add uri field rval = evbuffer_add(dest, buf, strstr(buf, "\r\n") - buf + 2); @@ -657,28 +657,27 @@ http_server_receive(http_steg_t *s, conn_t *conn, struct evbuffer *dest, struct char *p; char *pend;
- char outbuf[MAX_COOKIE_SIZE]; + char outbuf[MAX_COOKIE_SIZE * 3/2]; char outbuf2[MAX_COOKIE_SIZE]; int sofar = 0; //int cookie_mode = 0;
if (s2.pos == -1) { - log_debug("Did not find end of request %d", (int) evbuffer_get_length(source)); - // evbuffer_dump(source, stderr); + log_debug(conn, "Did not find end of request %d", + (int) evbuffer_get_length(source)); return RECV_INCOMPLETE; }
- log_debug("SERVER received request header of length %d", (int)s2.pos); + log_debug(conn, "SERVER received request header of length %d", (int)s2.pos);
data = (char*) evbuffer_pullup(source, s2.pos+4);
if (data == NULL) { - log_debug("SERVER evbuffer_pullup fails"); + log_debug(conn, "SERVER evbuffer_pullup fails"); return RECV_BAD; }
- data[s2.pos+3] = 0;
type = find_uri_type((char *)data, s2.pos+4); @@ -691,19 +690,14 @@ http_server_receive(http_steg_t *s, conn_t *conn, struct evbuffer *dest, struct p = data + sizeof "GET /" -1;
pend = strstr(p, "\r\n"); - - if (pend == NULL || (pend - p > MAX_COOKIE_SIZE)) { - fprintf(stderr, "incorrect cookie recovery \n"); - exit(-1); - - } - - + log_assert(pend); + if (pend - p > MAX_COOKIE_SIZE * 3/2) + log_abort(conn, "cookie too big: %lu (max %lu)", + (unsigned long)(pend - p), (unsigned long)MAX_COOKIE_SIZE);
bzero(outbuf, sizeof(outbuf)); int cookielen = unwrap_b64_cookie((char*) p, (char*) outbuf, pend - p);
- desanitize_b64(outbuf, cookielen); outbuf[cookielen] = '\n'; bzero(outbuf2, sizeof(outbuf2)); @@ -711,25 +705,19 @@ http_server_receive(http_steg_t *s, conn_t *conn, struct evbuffer *dest, struct base64::decoder D; sofar = D.decode(outbuf, cookielen+1, outbuf2);
+ if (sofar <= 0) + log_warn(conn, "base64 decode failed\n");
- if (sofar <= 0) - log_warn("decode failed\n"); - - - if (sofar >= MAX_COOKIE_SIZE) { - log_warn("cookie buffer overflow????\n"); - exit(-1); - } - + if (sofar >= MAX_COOKIE_SIZE) + log_abort(conn, "cookie decode buffer overflow\n");
if (evbuffer_add(dest, outbuf2, sofar)) { - log_debug("Failed to transfer buffer"); + log_debug(conn, "Failed to transfer buffer"); return RECV_BAD; } evbuffer_drain(source, s2.pos + sizeof("\r\n\r\n") - 1); } while (evbuffer_get_length(source));
- s->have_received = 1; s->type = type;
@@ -737,16 +725,6 @@ http_server_receive(http_steg_t *s, conn_t *conn, struct evbuffer *dest, struct return RECV_GOOD; }
- - - - - - - - - - int http_steg_t::receive(struct evbuffer *dest) { diff --git a/src/steg/nosteg.cc b/src/steg/nosteg.cc index df80c89..328a1d6 100644 --- a/src/steg/nosteg.cc +++ b/src/steg/nosteg.cc @@ -87,9 +87,9 @@ nosteg_steg_t::cfg() }
size_t -nosteg_steg_t::transmit_room() +nosteg_steg_t::transmit_room(size_t pref, size_t, size_t) { - return SIZE_MAX; + return pref; }
int diff --git a/src/steg/nosteg_rr.cc b/src/steg/nosteg_rr.cc index 8b316bb..adde6c8 100644 --- a/src/steg/nosteg_rr.cc +++ b/src/steg/nosteg_rr.cc @@ -93,9 +93,9 @@ nosteg_rr_steg_t::cfg() }
size_t -nosteg_rr_steg_t::transmit_room() +nosteg_rr_steg_t::transmit_room(size_t pref, size_t, size_t) { - return can_transmit ? SIZE_MAX : 0; + return can_transmit ? pref : 0; }
int
tor-commits@lists.torproject.org