Add patch 0001-Fix-authentication-bypass-bug.patch
authorgregor herrmann <gregoa@debian.org>
Tue, 17 Jun 2014 17:32:50 +0000 (19:32 +0200)
committergregor herrmann <gregoa@debian.org>
Tue, 17 Jun 2014 17:32:50 +0000 (19:32 +0200)
from upstream's iodine-0.6.0 branch.

This fixes a security problem where the client could bypass the password
check by continuing after getting an error from the server and guessing the
network parameters and the server would still accept the rest of the setup
and also network traffic. The patch adds checks for normal and raw mode that
user has authenticated before allowing any other communication.

Thanks: Salvatore Bonaccorso for the bug report, and Erik Ekman for backporting the fix super fast.
Closes: #751834
debian/patches/0001-Fix-authentication-bypass-bug.patch [new file with mode: 0644]
debian/patches/series

diff --git a/debian/patches/0001-Fix-authentication-bypass-bug.patch b/debian/patches/0001-Fix-authentication-bypass-bug.patch
new file mode 100644 (file)
index 0000000..03c5958
--- /dev/null
@@ -0,0 +1,258 @@
+From 9e265625a1ac8aafbe2812c67de7ddbbf1793a0e Mon Sep 17 00:00:00 2001
+From: Erik Ekman <erik@kryo.se>
+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(-)
+
+--- 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.
+--- 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 que
+       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_
+                       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_
+               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_
+               }
+               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_
+               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_
+               /* 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_
+               /* 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_
+               /* 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_
+               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,
+       
+       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,
+               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, s
+ 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);
+--- 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;
+--- a/src/user.h
++++ b/src/user.h
+@@ -39,6 +39,8 @@
+ struct _user {
+       char id;
+       int active;
++      int authenticated;
++      int authenticated_raw;
+       int disabled;
+       time_t last_pkt;
+       int seed;
+--- a/tests/user.c
++++ b/tests/user.c
+@@ -93,6 +93,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);
+ }
+ END_TEST
+@@ -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++) {
index c26f738f2a2f21920607d35a90ea24685c7ef4b3..73e433fe911bcc77304b5b8f3cdf982ea9e5b962 100644 (file)
@@ -7,3 +7,4 @@ cmdline-r-u.patch
 0001-man-iodine.8-add-note-about-sharing-port-dnsport.patch
 verbose-build.patch
 test-libs.patch
+0001-Fix-authentication-bypass-bug.patch