diff --git a/CHANGELOG b/CHANGELOG index 01173a1..9f3f967 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -26,6 +26,7 @@ master: - Do not let sockets be inherited by sub-processes, fixes #99. - Add unspecified RR type (called PRIVATE; id 65399, in private use range). For servers with RFC3597 support. Fixes #97. + - Fix authentication bypass vulnerability; found by Oscar Reparaz. 2010-02-06: 0.6.0-rc1 "Hotspotify" - Fixed tunnel not working on Windows. diff --git a/src/iodined.c b/src/iodined.c index c14f1a7..cdfc920 100644 --- a/src/iodined.c +++ b/src/iodined.c @@ -174,6 +174,7 @@ syslog(int a, const char *str, ...) } #endif +/* This will not check that user has passed login challenge */ static int check_user_and_ip(int userid, struct query *q) { @@ -200,6 +201,20 @@ check_user_and_ip(int userid, struct query *q) return memcmp(&(users[userid].host), &(tempin->sin_addr), sizeof(struct in_addr)); } +/* This checks that user has passed normal (non-raw) login challenge */ +static int +check_authenticated_user_and_ip(int userid, struct query *q) +{ + int res = check_user_and_ip(userid, q); + if (res) + return res; + + if (!users[userid].authenticated) + return 1; + + return 0; +} + static void send_raw(int fd, char *buf, int buflen, int user, int cmd, struct query *q) { @@ -836,8 +851,10 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) login_calculate(logindata, 16, password, users[userid].seed); if (read >= 18 && (memcmp(logindata, unpacked+1, 16) == 0)) { - /* Login ok, send ip/mtu/netmask info */ + /* Store login ok */ + users[userid].authenticated = 1; + /* Send ip/mtu/netmask info */ tempip.s_addr = my_ip; tmp[0] = strdup(inet_ntoa(tempip)); tempip.s_addr = users[userid].tun_ip; @@ -866,7 +883,7 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) char reply[5]; userid = b32_8to5(in[1]); - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { write_dns(dns_fd, q, "BADIP", 5, 'T'); return; /* illegal id */ } @@ -903,7 +920,7 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) userid = b32_8to5(in[1]); - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { write_dns(dns_fd, q, "BADIP", 5, 'T'); return; /* illegal id */ } @@ -944,7 +961,7 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) userid = b32_8to5(in[1]); - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { write_dns(dns_fd, q, "BADIP", 5, 'T'); return; /* illegal id */ } @@ -1072,7 +1089,7 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) /* Downstream fragsize probe packet */ userid = (b32_8to5(in[1]) >> 1) & 15; - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { write_dns(dns_fd, q, "BADIP", 5, 'T'); return; /* illegal id */ } @@ -1107,7 +1124,7 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) /* Downstream fragsize packet */ userid = unpacked[0]; - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { write_dns(dns_fd, q, "BADIP", 5, 'T'); return; /* illegal id */ } @@ -1140,7 +1157,7 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) /* Ping packet, store userid */ userid = unpacked[0]; - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { write_dns(dns_fd, q, "BADIP", 5, 'T'); return; /* illegal id */ } @@ -1270,7 +1287,7 @@ handle_null_request(int tun_fd, int dns_fd, struct query *q, int domain_len) userid = code; /* Check user and sending ip number */ - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { write_dns(dns_fd, q, "BADIP", 5, 'T'); return; /* illegal id */ } @@ -1847,10 +1864,11 @@ handle_raw_login(char *packet, int len, struct query *q, int fd, int userid) if (len < 16) return; - /* can't use check_user_and_ip() since IP address will be different, + /* can't use check_authenticated_user_and_ip() since IP address will be different, so duplicate here except IP address */ if (userid < 0 || userid >= created_users) return; if (!users[userid].active || users[userid].disabled) return; + if (!users[userid].authenticated) return; if (users[userid].last_pkt + 60 < time(NULL)) return; if (debug >= 1) { @@ -1875,15 +1893,18 @@ handle_raw_login(char *packet, int len, struct query *q, int fd, int userid) user_set_conn_type(userid, CONN_RAW_UDP); login_calculate(myhash, 16, password, users[userid].seed - 1); send_raw(fd, myhash, 16, userid, RAW_HDR_CMD_LOGIN, q); + + users[userid].authenticated_raw = 1; } } static void handle_raw_data(char *packet, int len, struct query *q, int dns_fd, int tun_fd, int userid) { - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { return; } + if (!users[userid].authenticated_raw) return; /* Update query and time info for user */ users[userid].last_pkt = time(NULL); @@ -1905,9 +1926,10 @@ handle_raw_data(char *packet, int len, struct query *q, int dns_fd, int tun_fd, static void handle_raw_ping(struct query *q, int dns_fd, int userid) { - if (check_user_and_ip(userid, q) != 0) { + if (check_authenticated_user_and_ip(userid, q) != 0) { return; } + if (!users[userid].authenticated_raw) return; /* Update query and time info for user */ users[userid].last_pkt = time(NULL); diff --git a/src/user.c b/src/user.c index 67821a4..0dc95db 100644 --- a/src/user.c +++ b/src/user.c @@ -75,6 +75,8 @@ init_users(in_addr_t my_ip, int netbits) users[i].tun_ip = ip; net.s_addr = ip; users[i].disabled = 0; + users[i].authenticated = 0; + users[i].authenticated_raw = 0; users[i].active = 0; /* Rest is reset on login ('V' packet) */ } @@ -116,7 +118,9 @@ find_user_by_ip(uint32_t ip) ret = -1; for (i = 0; i < usercount; i++) { - if (users[i].active && !users[i].disabled && + if (users[i].active && + users[i].authenticated && + !users[i].disabled && users[i].last_pkt + 60 > time(NULL) && ip == users[i].tun_ip) { ret = i; @@ -168,6 +172,8 @@ find_available_user() /* Not used at all or not used in one minute */ if ((!users[i].active || users[i].last_pkt + 60 < time(NULL)) && !users[i].disabled) { users[i].active = 1; + users[i].authenticated = 0; + users[i].authenticated_raw = 0; users[i].last_pkt = time(NULL); users[i].fragsize = 4096; users[i].conn = CONN_DNS_NULL; diff --git a/src/user.h b/src/user.h index 00d883b..76fc54b 100644 --- a/src/user.h +++ b/src/user.h @@ -37,6 +37,8 @@ struct tun_user { char id; int active; + int authenticated; + int authenticated_raw; int disabled; time_t last_pkt; int seed; diff --git a/tests/user.c b/tests/user.c index 6534317..2e4f36d 100644 --- a/tests/user.c +++ b/tests/user.c @@ -94,6 +94,11 @@ START_TEST(test_find_user_by_ip) users[0].last_pkt = time(NULL); + testip = (unsigned int) inet_addr("127.0.0.2"); + fail_unless(find_user_by_ip(testip) == -1); + + users[0].authenticated = 1; + testip = (unsigned int) inet_addr("127.0.0.2"); fail_unless(find_user_by_ip(testip) == 0); } @@ -137,7 +142,11 @@ START_TEST(test_find_available_user) init_users(ip, 27); for (i = 0; i < USERS; i++) { + users[i].authenticated = 1; + users[i].authenticated_raw = 1; fail_unless(find_available_user() == i); + fail_if(users[i].authenticated); + fail_if(users[i].authenticated_raw); } for (i = 0; i < USERS; i++) {