From 9e265625a1ac8aafbe2812c67de7ddbbf1793a0e Mon Sep 17 00:00:00 2001 From: Erik Ekman Date: Mon, 16 Jun 2014 21:12:49 +0200 Subject: [PATCH] Fix authentication bypass bug The client could bypass the password check by continuing after getting error from the server and guessing the network parameters. The server would still accept the rest of the setup and also network traffic. Add checks for normal and raw mode that user has authenticated before allowing any other communication. Problem found by Oscar Reparaz. Backported to iodine 0.6 branch. --- CHANGELOG | 5 ++++- src/iodined.c | 46 ++++++++++++++++++++++++++++++++++------------ src/user.c | 8 +++++++- src/user.h | 2 ++ tests/user.c | 9 +++++++++ 5 files changed, 56 insertions(+), 14 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 888beac..ab01d78 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,7 +5,10 @@ iodine - http://code.kryo.se/iodine CHANGES: -2010-02-13: 0.6.0-rc1 "Hotspotify" +2014-06-17: 0.6.0 + - Fix authentication bypass vulnerability; found by Oscar Reparaz. + +2010-02-06: 0.6.0-rc1 "Hotspotify" - Fixed tunnel not working on Windows. - Any device name is now supported on Windows, fixes #47. - Multiple installed TAP32 interfaces are now supported, fixes #46. diff --git a/src/iodined.c b/src/iodined.c index 3681084..b05ff57 100644 --- a/src/iodined.c +++ b/src/iodined.c @@ -116,6 +116,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) { @@ -142,6 +143,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) { @@ -780,8 +795,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; @@ -810,7 +827,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 */ } @@ -846,8 +863,8 @@ 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 */ } @@ -888,7 +905,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 */ } @@ -1016,7 +1033,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 */ } @@ -1051,7 +1068,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 */ } @@ -1084,7 +1101,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 */ } @@ -1214,7 +1231,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 */ } @@ -1785,10 +1802,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) { @@ -1813,15 +1831,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); @@ -1843,9 +1864,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 dfe9c36..959cb1a 100644 --- a/src/user.c +++ b/src/user.c @@ -78,6 +78,8 @@ init_users(in_addr_t my_ip, int netbits) users[i].disabled = 0; created_users++; } + users[i].authenticated = 0; + users[i].authenticated_raw = 0; users[i].active = 0; /* Rest is reset on login ('V' packet) */ } @@ -119,7 +121,9 @@ find_user_by_ip(uint32_t ip) ret = -1; for (i = 0; i < USERS; 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; @@ -171,6 +175,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 51a6092..e0c60e9 100644 --- a/src/user.h +++ b/src/user.h @@ -36,6 +36,8 @@ struct 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 afd61ca..ae35285 100644 --- a/tests/user.c +++ b/tests/user.c @@ -92,6 +92,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); } @@ -135,7 +140,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++) {