Security: Rhys Kidd identified a vulnerability in the handling of

heartbeat packets.  Drop oversize heartbeat packets.

diff --git a/Changes b/Changes
index ee938a7..abe7854 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,3 +1,7 @@
+* Fri Dec 1 2006 Brendan O'Dea <bod@optus.net> 2.1.21
+- Security: Rhys Kidd identified a vulnerability in the handling of
+  heartbeat packets.  Drop oversize heartbeat packets.
+

 * Thu Aug 3 2006 Brendan O'Dea <bod@optus.net> 2.1.20
 - Fix sign problem with reporting of unknown VSAs.
 - Allow DNS servers to be specified either using the old or new
 * Thu Aug 3 2006 Brendan O'Dea <bod@optus.net> 2.1.20
 - Fix sign problem with reporting of unknown VSAs.
 - Allow DNS servers to be specified either using the old or new

diff --git a/THANKS b/THANKS
index 046a513..c39a07d 100644 (file)
--- a/THANKS
+++ b/THANKS
@@ -26,4 +26,4 @@ Jon Morby                  <jon@fido.net>
 Paul Martin                <pm@zetnet.net>
 Jonathan Yarden            <jyarden@bluegrass.net>
 Patrick Cole               <z@amused.net>
 Paul Martin                <pm@zetnet.net>
 Jonathan Yarden            <jyarden@bluegrass.net>
 Patrick Cole               <z@amused.net>

-Khaled Al Hamwi            <kh.alhamwi@gmail.com>
+Rhys Kidd                  <rhys.kidd@staff.westnet.com.au>

diff --git a/cluster.c b/cluster.c
index abe2a63..538da20 100644 (file)
--- a/cluster.c
+++ b/cluster.c
@@ -1,6 +1,6 @@
 // L2TPNS Clustering Stuff
 
 // L2TPNS Clustering Stuff
 

-char const *cvs_id_cluster = "$Id: cluster.c,v 1.51 2006/04/27 09:53:49 bodea Exp $";
+char const *cvs_id_cluster = "$Id: cluster.c,v 1.50.2.1 2006/12/02 14:09:14 bodea Exp $";

 
 #include <stdio.h>
 #include <stdlib.h>
 
 #include <stdio.h>
 #include <stdlib.h>


@@ -42,7 +42,6 @@ extern int cluster_sockfd;            // The filedescriptor for the cluster communications
 
 in_addr_t my_address = 0;              // The network address of my ethernet port.
 static int walk_session_number = 0;    // The next session to send when doing the slow table walk.
 
 in_addr_t my_address = 0;              // The network address of my ethernet port.
 static int walk_session_number = 0;    // The next session to send when doing the slow table walk.

-static int walk_bundle_number = 0;     // The next bundle to send when doing the slow table walk.

 static int walk_tunnel_number = 0;     // The next tunnel to send when doing the slow table walk.
 int forked = 0;                                // Sanity check: CLI must not diddle with heartbeat table
 
 static int walk_tunnel_number = 0;     // The next tunnel to send when doing the slow table walk.
 int forked = 0;                                // Sanity check: CLI must not diddle with heartbeat table
 


@@ -86,7 +85,6 @@ int cluster_init()
        int opt;
 
        config->cluster_undefined_sessions = MAXSESSION-1;
        int opt;
 
        config->cluster_undefined_sessions = MAXSESSION-1;

-       config->cluster_undefined_bundles = MAXBUNDLE-1;

        config->cluster_undefined_tunnels = MAXTUNNEL-1;
 
        if (!config->cluster_address)
        config->cluster_undefined_tunnels = MAXTUNNEL-1;
 
        if (!config->cluster_address)


@@ -229,7 +227,7 @@ static void cluster_uptodate(void)
        if (config->cluster_iam_uptodate)
                return;
 
        if (config->cluster_iam_uptodate)
                return;
 

-       if (config->cluster_undefined_sessions || config->cluster_undefined_tunnels || config->cluster_undefined_bundles)
+       if (config->cluster_undefined_sessions || config->cluster_undefined_tunnels)

                return;
 
        config->cluster_iam_uptodate = 1;
                return;
 
        config->cluster_iam_uptodate = 1;


@@ -522,7 +520,7 @@ void cluster_check_slaves(void)
 //
 void cluster_check_master(void)
 {
 //
 void cluster_check_master(void)
 {

-       int i, count, tcount, bcount, high_unique_id = 0;
+       int i, count, tcount, high_unique_id = 0;

        int last_free = 0;
        clockt t = TIME;
        static int probed = 0;
        int last_free = 0;
        clockt t = TIME;
        static int probed = 0;


@@ -616,19 +614,6 @@ void cluster_check_master(void)
                        config->cluster_highest_tunnelid = i;
        }
 
                        config->cluster_highest_tunnelid = i;
        }
 

-               //
-                // Go through and mark all the bundles as defined.
-                // Count the highest used bundle number as well.
-                //
-        config->cluster_highest_bundleid = 0;
-        for (i = 0, bcount = 0; i < MAXBUNDLE; ++i) {
-                if (bundle[i].state == BUNDLEUNDEF)
-                        bundle[i].state = BUNDLEFREE;
-
-                if (bundle[i].state != BUNDLEFREE && i > config->cluster_highest_bundleid)
-                        config->cluster_highest_bundleid = i;
-        }
-

                //
                // Go through and mark all the sessions as being defined.
                // reset the idle timeouts.
                //
                // Go through and mark all the sessions as being defined.
                // reset the idle timeouts.


@@ -690,11 +675,10 @@ void cluster_check_master(void)
        rebuild_address_pool();
 
                // If we're not the very first master, this is a big issue!
        rebuild_address_pool();
 
                // If we're not the very first master, this is a big issue!

-       if (count > 0)
+       if(count>0)

                LOG(0, 0, 0, "Warning: Fixed %d uninitialized sessions in becoming master!\n", count);
 
        config->cluster_undefined_sessions = 0;
                LOG(0, 0, 0, "Warning: Fixed %d uninitialized sessions in becoming master!\n", count);
 
        config->cluster_undefined_sessions = 0;

-       config->cluster_undefined_bundles = 0;

        config->cluster_undefined_tunnels = 0;
        config->cluster_iam_uptodate = 1; // assume all peers are up-to-date
 
        config->cluster_undefined_tunnels = 0;
        config->cluster_iam_uptodate = 1; // assume all peers are up-to-date
 


@@ -712,7 +696,7 @@ void cluster_check_master(void)
 // we fix it up here, and we ensure that the 'first free session'
 // pointer is valid.
 //
 // we fix it up here, and we ensure that the 'first free session'
 // pointer is valid.
 //

-static void cluster_check_sessions(int highsession, int freesession_ptr, int highbundle, int hightunnel)
+static void cluster_check_sessions(int highsession, int freesession_ptr, int hightunnel)

 {
        int i;
 
 {
        int i;
 


@@ -721,7 +705,7 @@ static void cluster_check_sessions(int highsession, int freesession_ptr, int hig
        if (config->cluster_iam_uptodate)
                return;
 
        if (config->cluster_iam_uptodate)
                return;
 

-       if (highsession > config->cluster_undefined_sessions && highbundle > config->cluster_undefined_bundles && hightunnel > config->cluster_undefined_tunnels)
+       if (highsession > config->cluster_undefined_sessions && hightunnel > config->cluster_undefined_tunnels)

                return;
 
                // Clear out defined sessions, counting the number of
                return;
 
                // Clear out defined sessions, counting the number of


@@ -737,19 +721,6 @@ static void cluster_check_sessions(int highsession, int freesession_ptr, int hig
                        ++config->cluster_undefined_sessions;
        }
 
                        ++config->cluster_undefined_sessions;
        }
 

-               // Clear out defined bundles, counting the number of
-               // undefs remaining.
-       config->cluster_undefined_bundles = 0;
-       for (i = 1 ; i < MAXBUNDLE; ++i) {
-               if (i > highbundle) {
-                       if (bundle[i].state == BUNDLEUNDEF) bundle[i].state = BUNDLEFREE; // Defined.
-                       continue;
-               }
-
-               if (bundle[i].state == BUNDLEUNDEF)
-                       ++config->cluster_undefined_bundles;
-       }
-

                // Clear out defined tunnels, counting the number of
                // undefs remaining.
        config->cluster_undefined_tunnels = 0;
                // Clear out defined tunnels, counting the number of
                // undefs remaining.
        config->cluster_undefined_tunnels = 0;


@@ -764,9 +735,9 @@ static void cluster_check_sessions(int highsession, int freesession_ptr, int hig
        }
 
 
        }
 
 

-       if (config->cluster_undefined_sessions || config->cluster_undefined_tunnels || config->cluster_undefined_bundles) {
-               LOG(2, 0, 0, "Cleared undefined sessions/bundles/tunnels. %d sess (high %d), %d bund (high %d), %d tunn (high %d)\n",
-                       config->cluster_undefined_sessions, highsession, config->cluster_undefined_bundles, highbundle, config->cluster_undefined_tunnels, hightunnel);
+       if (config->cluster_undefined_sessions || config->cluster_undefined_tunnels) {
+               LOG(2, 0, 0, "Cleared undefined sessions/tunnels. %d sess (high %d), %d tunn (high %d)\n",
+                       config->cluster_undefined_sessions, highsession, config->cluster_undefined_tunnels, hightunnel);

                return;
        }
 
                return;
        }
 


@@ -799,27 +770,6 @@ static int hb_add_type(uint8_t **p, int type, int id)
                        add_type(p, C_SESSION, id, (uint8_t *) &session[id], sizeof(sessiont));
                        break;
 
                        add_type(p, C_SESSION, id, (uint8_t *) &session[id], sizeof(sessiont));
                        break;
 

-               case C_CBUNDLE: { // Compressed C_BUNDLE
-                        uint8_t c[sizeof(bundlet) * 2]; // Bigger than worst case.
-                        uint8_t *d = (uint8_t *) &bundle[id];
-                        uint8_t *orig = d;
-                        int size;
-
-                        size = rle_compress( &d,  sizeof(bundlet), c, sizeof(c) );
-
-                                // Did we compress the full structure, and is the size actually
-                                // reduced??
-                        if ( (d - orig) == sizeof(bundlet) && size < sizeof(bundlet) ) {
-                                add_type(p, C_CBUNDLE, id, c, size);
-                                break;
-                        }
-                        // Failed to compress : Fall through.
-                }
-
-               case C_BUNDLE:
-                       add_type(p, C_BUNDLE, id, (uint8_t *) &bundle[id], sizeof(bundlet));
-                       break;
-

                case C_CTUNNEL: { // Compressed C_TUNNEL
                        uint8_t c[sizeof(tunnelt) * 2]; // Bigger than worst case.
                        uint8_t *d = (uint8_t *) &tunnel[id];
                case C_CTUNNEL: { // Compressed C_TUNNEL
                        uint8_t c[sizeof(tunnelt) * 2]; // Bigger than worst case.
                        uint8_t *d = (uint8_t *) &tunnel[id];


@@ -852,7 +802,7 @@ static int hb_add_type(uint8_t **p, int type, int id)
 //
 void cluster_heartbeat()
 {
 //
 void cluster_heartbeat()
 {

-       int i, count = 0, tcount = 0, bcount = 0;
+       int i, count = 0, tcount = 0;

        uint8_t buff[MAX_HEART_SIZE + sizeof(heartt) + sizeof(int) ];
        heartt h;
        uint8_t *p = buff;
        uint8_t buff[MAX_HEART_SIZE + sizeof(heartt) + sizeof(int) ];
        heartt h;
        uint8_t *p = buff;


@@ -873,9 +823,7 @@ void cluster_heartbeat()
        h.highsession = config->cluster_highest_sessionid;
        h.freesession = sessionfree;
        h.hightunnel = config->cluster_highest_tunnelid;
        h.highsession = config->cluster_highest_sessionid;
        h.freesession = sessionfree;
        h.hightunnel = config->cluster_highest_tunnelid;

-       h.highbundle = config->cluster_highest_bundleid;

        h.size_sess = sizeof(sessiont);         // Just in case.
        h.size_sess = sizeof(sessiont);         // Just in case.

-       h.size_bund = sizeof(bundlet);

        h.size_tunn = sizeof(tunnelt);
        h.interval = config->cluster_hb_interval;
        h.timeout  = config->cluster_hb_timeout;
        h.size_tunn = sizeof(tunnelt);
        h.interval = config->cluster_hb_interval;
        h.timeout  = config->cluster_hb_timeout;


@@ -930,22 +878,6 @@ void cluster_heartbeat()
                ++tcount;
        }
 
                ++tcount;
        }
 

-               //
-               // Fill out the packet with bundles from the bundle table...
-       while ( (p + sizeof(uint32_t) * 2 + sizeof(bundlet) ) < (buff + MAX_HEART_SIZE) ) {
-
-               if (!walk_bundle_number)        // bundle #0 isn't valid.
-                       ++walk_bundle_number;
-
-               if (bcount >= config->cluster_highest_bundleid)
-                       break;
-
-               hb_add_type(&p, C_CTUNNEL, walk_bundle_number);
-               walk_tunnel_number = (1+walk_bundle_number)%(config->cluster_highest_bundleid+1);       // +1 avoids divide by zero.
-
-               ++bcount;
-        }
-

                //
                // Did we do something wrong?
        if (p > (buff + sizeof(buff))) {        // Did we somehow manage to overun the buffer?
                //
                // Did we do something wrong?
        if (p > (buff + sizeof(buff))) {        // Did we somehow manage to overun the buffer?


@@ -955,9 +887,9 @@ void cluster_heartbeat()
        }
 
        LOG(3, 0, 0, "Sending v%d heartbeat #%d, change #%" PRIu64 " with %d changes "
        }
 
        LOG(3, 0, 0, "Sending v%d heartbeat #%d, change #%" PRIu64 " with %d changes "

-                    "(%d x-sess, %d x-bundles, %d x-tunnels, %d highsess, %d highbund, %d hightun, size %d)\n",
+                    "(%d x-sess, %d x-tunnels, %d highsess, %d hightun, size %d)\n",

            HB_VERSION, h.seq, h.table_version, config->cluster_num_changes,
            HB_VERSION, h.seq, h.table_version, config->cluster_num_changes,

-           count, bcount, tcount, config->cluster_highest_sessionid, config->cluster_highest_bundleid,
+           count, tcount, config->cluster_highest_sessionid,

            config->cluster_highest_tunnelid, (int) (p - buff));
 
        config->cluster_num_changes = 0;
            config->cluster_highest_tunnelid, (int) (p - buff));
 
        config->cluster_num_changes = 0;


@@ -1006,17 +938,6 @@ int cluster_send_session(int sid)
        return type_changed(C_CSESSION, sid);
 }
 
        return type_changed(C_CSESSION, sid);
 }
 

-// A particular bundle has been changed!
-int cluster_send_bundle(int bid)
-{
-       if (!config->cluster_iam_master) {
-               LOG(0, 0, bid, "I'm not a master, but I just tried to change a bundle!\n");
-               return -1;
-       }
-
-       return type_changed(C_CBUNDLE, bid);
-}
-

 // A particular tunnel has been changed!
 int cluster_send_tunnel(int tid)
 {
 // A particular tunnel has been changed!
 int cluster_send_tunnel(int tid)
 {


@@ -1254,31 +1175,6 @@ static int cluster_recv_session(int more, uint8_t *p)
        return 0;
 }
 
        return 0;
 }
 

-static int cluster_recv_bundle(int more, uint8_t *p)
-{
-       if (more >= MAXBUNDLE) {
-               LOG(0, 0, 0, "DANGER: Received a bundle id > MAXBUNDLE!\n");
-               return -1;
-       }
-
-       if (bundle[more].state == BUNDLEUNDEF) {
-               if (config->cluster_iam_uptodate) { // Sanity.
-                       LOG(0, 0, 0, "I thought I was uptodate but I just found an undefined bundle!\n");
-               } else {
-                       --config->cluster_undefined_bundles;
-               }
-       }
-
-       memcpy(&bundle[more], p, sizeof(bundle[more]) );
-
-       LOG(5, 0, more, "Received bundle update\n");
-
-       if (!config->cluster_iam_uptodate)
-               cluster_uptodate();     // Check to see if we're up to date.
-
-        return 0;
-}
-

 static int cluster_recv_tunnel(int more, uint8_t *p)
 {
        if (more >= MAXTUNNEL) {
 static int cluster_recv_tunnel(int more, uint8_t *p)
 {
        if (more >= MAXTUNNEL) {


@@ -1451,7 +1347,11 @@ static int cluster_process_heartbeat(uint8_t *data, int size, int more, uint8_t
                return -1; // Ignore it??
        }
 
                return -1; // Ignore it??
        }
 

-               // Ok. It's a heartbeat packet from a cluster master!
+       if (size > sizeof(past_hearts[0].data)) {
+               LOG(0, 0, 0, "Received an oversize heartbeat from %s (%d)!\n", fmtaddr(addr, 0), size);
+               return -1;
+       }
+

        if (s < sizeof(*h))
                goto shortpacket;
 
        if (s < sizeof(*h))
                goto shortpacket;
 


@@ -1565,7 +1465,7 @@ static int cluster_process_heartbeat(uint8_t *data, int size, int more, uint8_t
 
                        // Check that we don't have too many undefined sessions, and
                        // that the free session pointer is correct.
 
                        // Check that we don't have too many undefined sessions, and
                        // that the free session pointer is correct.

-       cluster_check_sessions(h->highsession, h->freesession, h->highbundle, h->hightunnel);
+       cluster_check_sessions(h->highsession, h->freesession, h->hightunnel);

 
        if (h->interval != config->cluster_hb_interval)
        {
 
        if (h->interval != config->cluster_hb_interval)
        {


@@ -1673,34 +1573,6 @@ static int cluster_process_heartbeat(uint8_t *data, int size, int more, uint8_t
                                p += sizeof(tunnel[more]);
                                s -= sizeof(tunnel[more]);
                                break;
                                p += sizeof(tunnel[more]);
                                s -= sizeof(tunnel[more]);
                                break;

-
-                       case C_CBUNDLE: { // Compressed bundle structure.
-                               uint8_t c[ sizeof(bundlet) + 2];
-                               int size;
-                               uint8_t *orig_p = p;
-
-                               size = rle_decompress((uint8_t **) &p, s, c, sizeof(c));
-                               s -= (p - orig_p);
-
-                               if (size != sizeof(bundlet) ) { // Ouch! Very very bad!
-                                       LOG(0, 0, 0, "DANGER: Received a CBUNDLE that didn't decompress correctly!\n");
-                                               // Now what? Should exit! No-longer up to date!
-                                       break;
-                               }
-
-                               cluster_recv_bundle(more, c);
-                               break;
-
-                       }
-                       case C_BUNDLE:
-                               if ( s < sizeof(bundle[more]))
-                                        goto shortpacket;
-
-                                cluster_recv_bundle(more, p);
-
-                                p += sizeof(bundle[more]);
-                                s -= sizeof(bundle[more]);
-                                break;

                        default:
                                LOG(0, 0, 0, "DANGER: I received a heartbeat element where I didn't understand the type! (%d)\n", type);
                                return -1; // can't process any more of the packet!!
                        default:
                                LOG(0, 0, 0, "DANGER: I received a heartbeat element where I didn't understand the type! (%d)\n", type);
                                return -1; // can't process any more of the packet!!


@@ -1886,13 +1758,11 @@ int cmd_show_cluster(struct cli_def *cli, char *command, char **argv, int argc)
                cli_print(cli, "Table version #  : %" PRIu64, config->cluster_table_version);
                cli_print(cli, "Next sequence number expected: %d", config->cluster_seq_number);
                cli_print(cli, "%d sessions undefined of %d", config->cluster_undefined_sessions, config->cluster_highest_sessionid);
                cli_print(cli, "Table version #  : %" PRIu64, config->cluster_table_version);
                cli_print(cli, "Next sequence number expected: %d", config->cluster_seq_number);
                cli_print(cli, "%d sessions undefined of %d", config->cluster_undefined_sessions, config->cluster_highest_sessionid);

-               cli_print(cli, "%d bundles undefined of %d", config->cluster_undefined_bundles, config->cluster_highest_bundleid);

                cli_print(cli, "%d tunnels undefined of %d", config->cluster_undefined_tunnels, config->cluster_highest_tunnelid);
        } else {
                cli_print(cli, "Table version #  : %" PRIu64, config->cluster_table_version);
                cli_print(cli, "Next heartbeat # : %d", config->cluster_seq_number);
                cli_print(cli, "Highest session  : %d", config->cluster_highest_sessionid);
                cli_print(cli, "%d tunnels undefined of %d", config->cluster_undefined_tunnels, config->cluster_highest_tunnelid);
        } else {
                cli_print(cli, "Table version #  : %" PRIu64, config->cluster_table_version);
                cli_print(cli, "Next heartbeat # : %d", config->cluster_seq_number);
                cli_print(cli, "Highest session  : %d", config->cluster_highest_sessionid);

-               cli_print(cli, "Highest bundle   : %d", config->cluster_highest_bundleid);

                cli_print(cli, "Highest tunnel   : %d", config->cluster_highest_tunnelid);
                cli_print(cli, "%d changes queued for sending", config->cluster_num_changes);
        }
                cli_print(cli, "Highest tunnel   : %d", config->cluster_highest_tunnelid);
                cli_print(cli, "%d changes queued for sending", config->cluster_num_changes);
        }

diff --git a/l2tpns.h b/l2tpns.h
index 209b38b..6c38bbf 100644 (file)
--- a/l2tpns.h
+++ b/l2tpns.h
@@ -1,5 +1,5 @@
 // L2TPNS Global Stuff
 // L2TPNS Global Stuff

-// $Id: l2tpns.h,v 1.113.2.2 2006/08/02 12:53:35 bodea Exp $
+// $Id: l2tpns.h,v 1.113.2.3 2006/12/02 14:09:14 bodea Exp $

 
 #ifndef __L2TPNS_H__
 #define __L2TPNS_H__
 
 #ifndef __L2TPNS_H__
 #define __L2TPNS_H__


@@ -14,7 +14,7 @@
 #include <sys/types.h>
 #include <libcli.h>
 
 #include <sys/types.h>
 #include <libcli.h>
 

-#define VERSION        "2.1.20"
+#define VERSION        "2.1.21"

 
 // Limits
 #define MAXTUNNEL      500             // could be up to 65535
 
 // Limits
 #define MAXTUNNEL      500             // could be up to 65535

diff --git a/l2tpns.spec b/l2tpns.spec
index 3ed4c39..4cd9ca8 100644 (file)
--- a/l2tpns.spec
+++ b/l2tpns.spec
@@ -1,6 +1,6 @@
 Summary: A high-speed clustered L2TP LNS
 Name: l2tpns
 Summary: A high-speed clustered L2TP LNS
 Name: l2tpns

-Version: 2.1.20
+Version: 2.1.21

 Release: 1
 License: GPL
 Group: System Environment/Daemons
 Release: 1
 License: GPL
 Group: System Environment/Daemons


@@ -43,5 +43,5 @@ rm -rf %{buildroot}
 %attr(644,root,root) /usr/share/man/man[58]/*
 
 %changelog
 %attr(644,root,root) /usr/share/man/man[58]/*
 
 %changelog

-* Thu Aug 3 2006 Brendan O'Dea <bod@optus.net> 2.1.20-1
-- 2.1.20 release, see /usr/share/doc/l2tpns-2.1.20/Changes
+* Fri Dec 1 2006 Brendan O'Dea <bod@optus.net> 2.1.21-1
+- 2.1.21 release, see /usr/share/doc/l2tpns-2.1.21/Changes