[tor-commits] [tor/master] Refactor fetch_from_buf_socks() to be greedy
nickm at torproject.org
nickm at torproject.org
Wed Jul 13 16:13:22 UTC 2011
commit 05c424f4b830e98595b33397b58504462dbda8db
Author: Nick Mathewson <nickm at torproject.org>
Date: Wed Jun 29 17:44:29 2011 -0400
Refactor fetch_from_buf_socks() to be greedy
Previously, fetch_from_buf_socks() might return 0 if there was still
data on the buffer and a subsequent call to fetch_from_buf_socks()
would return 1. This was making some of the socks5 unit tests
harder to write, and could potentially have caused misbehavior with
some overly verbose SOCKS implementations. Now,
fetch_from_buf_socks() does as much processing as it can, and
returns 0 only if it really needs more data. This brings it into
line with the evbuffer socks implementation.
---
src/or/buffers.c | 13 ++++++-------
src/or/connection_edge.c | 23 ++++++++++-------------
src/test/test.c | 23 ++++++++---------------
3 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/src/or/buffers.c b/src/or/buffers.c
index 44a492a..1310b42 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1544,9 +1544,7 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req,
else if (n_drain > 0)
buf_remove_from_front(buf, n_drain);
- } while (res == 0 && buf->head &&
- buf->datalen > buf->head->datalen &&
- want_length < buf->head->datalen);
+ } while (res == 0 && buf->head && want_length < buf->datalen);
return res;
}
@@ -1690,6 +1688,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
}
*drain_out = 2u + usernamelen + 1u + passlen;
req->got_auth = 1;
+ *want_length_out = 7; /* Minimal socks5 sommand. */
return 0;
} else if (req->auth_type == SOCKS_USER_PASS) {
/* unknown version byte */
@@ -1749,8 +1748,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
}
/* we know the method; read in the request */
log_debug(LD_APP,"socks5: checking request");
- if (datalen < 8) {/* basic info plus >=2 for addr plus 2 for port */
- *want_length_out = 8;
+ if (datalen < 7) {/* basic info plus >=1 for addr plus 2 for port */
+ *want_length_out = 7;
return 0; /* not yet */
}
req->command = (unsigned char) *(data+1);
@@ -1891,7 +1890,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
return -1;
}
log_debug(LD_APP,"socks4: Username not here yet.");
- *want_length_out = datalen+1024; /* ???? */
+ *want_length_out = datalen+1024; /* More than we need, but safe */
return 0;
}
authend = next;
@@ -1919,7 +1918,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
return -1;
}
log_debug(LD_APP,"socks4: Destaddr not all here yet.");
- *want_length_out = datalen + 1024;
+ *want_length_out = datalen + 1024; /* More than we need, but safe */
return 0;
}
if (MAX_SOCKS_ADDR_LEN <= next-startaddr) {
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 4e988bd..bff73d4 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1883,6 +1883,7 @@ connection_ap_handshake_process_socks(edge_connection_t *conn)
socks_request_t *socks;
int sockshere;
or_options_t *options = get_options();
+ int had_reply = 0;
tor_assert(conn);
tor_assert(conn->_base.type == CONN_TYPE_AP);
@@ -1900,22 +1901,18 @@ connection_ap_handshake_process_socks(edge_connection_t *conn)
sockshere = fetch_from_buf_socks(conn->_base.inbuf, socks,
options->TestSocks, options->SafeSocks);
};
+
+ if (socks->replylen) {
+ had_reply = 1;
+ connection_write_to_buf(socks->reply, socks->replylen, TO_CONN(conn));
+ socks->replylen = 0;
+ }
+
if (sockshere == 0) {
- if (socks->replylen) {
- connection_write_to_buf(socks->reply, socks->replylen, TO_CONN(conn));
- /* zero it out so we can do another round of negotiation */
- socks->replylen = 0;
- } else {
- log_debug(LD_APP,"socks handshake not all here yet.");
- }
+ log_debug(LD_APP,"socks handshake not all here yet.");
return 0;
} else if (sockshere == -1) {
- if (socks->replylen) { /* we should send reply back */
- log_debug(LD_APP,"reply is already set for us. Using it.");
- connection_ap_handshake_socks_reply(conn, socks->reply, socks->replylen,
- END_STREAM_REASON_SOCKSPROTOCOL);
-
- } else {
+ if (!had_reply) {
log_warn(LD_APP,"Fetching socks handshake failed. Closing.");
connection_ap_handshake_socks_reply(conn, NULL, 0,
END_STREAM_REASON_SOCKSPROTOCOL);
diff --git a/src/test/test.c b/src/test/test.c
index 64916ea..01fb30c 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -382,8 +382,6 @@ test_socks_5_supported_commands(void *ptr)
ADD_DATA(buf, "\x05\x01\x00");
ADD_DATA(buf, "\x05\x01\x00\x03\x0Etorproject.org\x11\x11");
test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
- get_options()->SafeSocks), 0);
- test_eq(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
get_options()->SafeSocks), 1);
test_eq(5, socks->socks_version);
@@ -400,8 +398,6 @@ test_socks_5_supported_commands(void *ptr)
ADD_DATA(buf, "\x05\x01\x00");
ADD_DATA(buf, "\x05\xF0\x00\x03\x0Etorproject.org\x01\x02");
test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
- get_options()->SafeSocks) == 0);
- test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
get_options()->SafeSocks) == 1);
test_eq(5, socks->socks_version);
test_eq(2, socks->replylen);
@@ -416,8 +412,6 @@ test_socks_5_supported_commands(void *ptr)
ADD_DATA(buf, "\x05\x01\x00");
ADD_DATA(buf, "\x05\xF1\x00\x01\x02\x02\x02\x05\x01\x03");
test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
- get_options()->SafeSocks) == 0);
- test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
get_options()->SafeSocks) == 1);
test_eq(5, socks->socks_version);
test_eq(2, socks->replylen);
@@ -528,24 +522,23 @@ test_socks_5_authenticate_with_data(void *ptr)
/* SOCKS 5 Send username/password */
/* SOCKS 5 Send CONNECT [01] to IP address 2.2.2.2:4369 */
- ADD_DATA(buf, "\x01\x02me\x02me\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11");
- test_assert(!fetch_from_buf_socks(buf, socks,
+ ADD_DATA(buf, "\x01\x02me\x03you\x05\x01\x00\x01\x02\x02\x02\x02\x11\x11");
+ test_assert(fetch_from_buf_socks(buf, socks,
get_options()->TestSocks,
- get_options()->SafeSocks));
- test_eq(5, socks->socks_version);
- test_eq(2, socks->replylen);
- test_eq(5, socks->reply[0]);
- test_eq(0, socks->reply[1]);
-
- test_assert(fetch_from_buf_socks(buf, socks, get_options()->TestSocks,
get_options()->SafeSocks) == 1);
test_eq(5, socks->socks_version);
test_eq(2, socks->replylen);
test_eq(5, socks->reply[0]);
test_eq(0, socks->reply[1]);
+
test_streq("2.2.2.2", socks->address);
test_eq(4369, socks->port);
+ test_eq(2, socks->usernamelen);
+ test_eq(3, socks->passwordlen);
+ test_memeq("me", socks->username, 2);
+ test_memeq("you", socks->password, 3);
+
done:
;
}
More information about the tor-commits
mailing list