mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-12-03 00:53:32 +01:00
Be more strict about when to accept socks auth message
In the code as it stood, we would accept any number of socks5 username/password authentication messages, regardless of whether we had actually negotiated username/password authentication. Instead, we should only accept one, and only if we have really negotiated username/password authentication. This patch also makes some fields of socks_request_t into uint8_t, for safety.
This commit is contained in:
parent
9b3f48c074
commit
aec396d9d0
@ -1636,18 +1636,12 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
|
|||||||
unsigned char usernamelen, passlen;
|
unsigned char usernamelen, passlen;
|
||||||
struct in_addr in;
|
struct in_addr in;
|
||||||
|
|
||||||
socksver = *data;
|
if (req->socks_version == 5 && !req->got_auth) {
|
||||||
|
/* See if we have received authentication. Strictly speaking, we should
|
||||||
switch (socksver) { /* which version of socks? */
|
also check whether we actually negotiated username/password
|
||||||
|
authentication. But some broken clients will send us authentication
|
||||||
case 1: /* socks5: username/password authentication request */
|
even if we negotiated SOCKS_NO_AUTH. */
|
||||||
|
if (*data == 1) { /* username/pass version 1 */
|
||||||
if (req->socks_version != 5) {
|
|
||||||
log_warn(LD_APP,
|
|
||||||
"socks5: Received authentication attempt before "
|
|
||||||
"authentication negotiated. Rejecting.");
|
|
||||||
return -1;
|
|
||||||
}
|
|
||||||
/* Format is: authversion [1 byte] == 1
|
/* Format is: authversion [1 byte] == 1
|
||||||
usernamelen [1 byte]
|
usernamelen [1 byte]
|
||||||
username [usernamelen bytes]
|
username [usernamelen bytes]
|
||||||
@ -1669,8 +1663,19 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
|
|||||||
log_debug(LD_APP,
|
log_debug(LD_APP,
|
||||||
"socks5: Accepted username/password without checking.");
|
"socks5: Accepted username/password without checking.");
|
||||||
*drain_out = 2u + usernamelen + 1u + passlen;
|
*drain_out = 2u + usernamelen + 1u + passlen;
|
||||||
|
req->got_auth = 1;
|
||||||
return 0;
|
return 0;
|
||||||
|
} else if (req->auth_type == SOCKS_USER_PASS) {
|
||||||
|
/* unknown version byte */
|
||||||
|
log_warn(LD_APP, "Socks5 username/password version %d not recognized; "
|
||||||
|
"rejecting.", (int)*data);
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
socksver = *data;
|
||||||
|
|
||||||
|
switch (socksver) { /* which version of socks? */
|
||||||
case 5: /* socks5 */
|
case 5: /* socks5 */
|
||||||
|
|
||||||
if (req->socks_version != 5) { /* we need to negotiate a method */
|
if (req->socks_version != 5) { /* we need to negotiate a method */
|
||||||
@ -1691,7 +1696,8 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
|
|||||||
req->socks_version = 5; /* remember we've already negotiated auth */
|
req->socks_version = 5; /* remember we've already negotiated auth */
|
||||||
log_debug(LD_APP,"socks5: accepted method 0 (no authentication)");
|
log_debug(LD_APP,"socks5: accepted method 0 (no authentication)");
|
||||||
r=0;
|
r=0;
|
||||||
} else if (memchr(data+2, SOCKS_USER_PASS,nummethods)) {
|
} else if (memchr(data+2, SOCKS_USER_PASS, nummethods)) {
|
||||||
|
req->auth_type = SOCKS_USER_PASS;
|
||||||
req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass"
|
req->reply[1] = SOCKS_USER_PASS; /* tell client to use "user/pass"
|
||||||
auth method */
|
auth method */
|
||||||
req->socks_version = 5; /* remember we've already negotiated auth */
|
req->socks_version = 5; /* remember we've already negotiated auth */
|
||||||
@ -1911,7 +1917,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
|
|||||||
case 'H': /* head */
|
case 'H': /* head */
|
||||||
case 'P': /* put/post */
|
case 'P': /* put/post */
|
||||||
case 'C': /* connect */
|
case 'C': /* connect */
|
||||||
strlcpy(req->reply,
|
strlcpy((char*)req->reply,
|
||||||
"HTTP/1.0 501 Tor is not an HTTP Proxy\r\n"
|
"HTTP/1.0 501 Tor is not an HTTP Proxy\r\n"
|
||||||
"Content-Type: text/html; charset=iso-8859-1\r\n\r\n"
|
"Content-Type: text/html; charset=iso-8859-1\r\n\r\n"
|
||||||
"<html>\n"
|
"<html>\n"
|
||||||
@ -1937,7 +1943,7 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
|
|||||||
"</body>\n"
|
"</body>\n"
|
||||||
"</html>\n"
|
"</html>\n"
|
||||||
, MAX_SOCKS_REPLY_LEN);
|
, MAX_SOCKS_REPLY_LEN);
|
||||||
req->replylen = strlen(req->reply)+1;
|
req->replylen = strlen((char*)req->reply)+1;
|
||||||
/* fall through */
|
/* fall through */
|
||||||
default: /* version is not socks4 or socks5 */
|
default: /* version is not socks4 or socks5 */
|
||||||
log_warn(LD_APP,
|
log_warn(LD_APP,
|
||||||
|
12
src/or/or.h
12
src/or/or.h
@ -3088,10 +3088,15 @@ struct socks_request_t {
|
|||||||
/** Which version of SOCKS did the client use? One of "0, 4, 5" -- where
|
/** Which version of SOCKS did the client use? One of "0, 4, 5" -- where
|
||||||
* 0 means that no socks handshake ever took place, and this is just a
|
* 0 means that no socks handshake ever took place, and this is just a
|
||||||
* stub connection (e.g. see connection_ap_make_link()). */
|
* stub connection (e.g. see connection_ap_make_link()). */
|
||||||
char socks_version;
|
uint8_t socks_version;
|
||||||
int command; /**< What is this stream's goal? One from the above list. */
|
/** If using socks5 authentication, which authentication type did we
|
||||||
|
* negotiate? currently we support 0 (no authentication) and 2
|
||||||
|
* (username/password). */
|
||||||
|
uint8_t auth_type;
|
||||||
|
/** What is this stream's goal? One of the SOCKS_COMMAND_* values */
|
||||||
|
uint8_t command;
|
||||||
size_t replylen; /**< Length of <b>reply</b>. */
|
size_t replylen; /**< Length of <b>reply</b>. */
|
||||||
char reply[MAX_SOCKS_REPLY_LEN]; /**< Write an entry into this string if
|
uint8_t reply[MAX_SOCKS_REPLY_LEN]; /**< Write an entry into this string if
|
||||||
* we want to specify our own socks reply,
|
* we want to specify our own socks reply,
|
||||||
* rather than using the default socks4 or
|
* rather than using the default socks4 or
|
||||||
* socks5 socks reply. We use this for the
|
* socks5 socks reply. We use this for the
|
||||||
@ -3103,6 +3108,7 @@ struct socks_request_t {
|
|||||||
unsigned int has_finished : 1; /**< Has the SOCKS handshake finished? Used to
|
unsigned int has_finished : 1; /**< Has the SOCKS handshake finished? Used to
|
||||||
* make sure we send back a socks reply for
|
* make sure we send back a socks reply for
|
||||||
* every connection. */
|
* every connection. */
|
||||||
|
unsigned int got_auth : 1; /**< Have we received any authentication data? */
|
||||||
};
|
};
|
||||||
|
|
||||||
/********************************* circuitbuild.c **********************/
|
/********************************* circuitbuild.c **********************/
|
||||||
|
Loading…
Reference in New Issue
Block a user