quagga: update to version 1.1.1 (#541)
authorJosef Schlehofer <pepe.schlehofer@gmail.com>
Sun, 26 Jan 2020 22:13:23 +0000 (23:13 +0100)
committerMoritz Warning <moritzwarning@web.de>
Mon, 27 Jan 2020 22:58:39 +0000 (23:58 +0100)
Makefile changes:
- Use HTTPS everywhere
- Reorder some things to have it in sync with other packages
- Fixed SPDX License Identifier
- Added PKG_LICENSE_FILES
- For checksum use SHA256

Refreshed patches

Fixes CVEs:
- CVE-2017-5495
- CVE-2017-16227

Fixes security issues:
- Quagga-2018-0543: attr_endp used for NOTIFY data
https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-0543.txt
- Quagga-2018-1114: bgpd double free
https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-1114.txt
- Quagga-2018-1550: debug overrun in notify lookup tables
https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-1550.txt
- Quagga-2018-1975: BGP capability inf. loop
https://gogs.quagga.net/Quagga/quagga/src/master/doc/security/Quagga-2018-1975.txt

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
quagga/Makefile
quagga/patches/001-bgpd-Fix-AS_PATH-size-calculation-for-long-paths.patch [new file with mode: 0644]
quagga/patches/002-Quagga-2018-0543.patch [new file with mode: 0644]
quagga/patches/003-Quagga-2018-1114.patch [new file with mode: 0644]
quagga/patches/004-Quagga-2018-1550.patch [new file with mode: 0644]
quagga/patches/005-Quagga-2018-1975.patch [new file with mode: 0644]
quagga/patches/150-no-cross-fs-link.patch

index e2751f4ed91ed18c534952f0096d2f9bfde51a01..91fee84c4ea2ef3f1cfe833e7b299a5be7aa0fb6 100644 (file)
@@ -8,12 +8,21 @@
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=quagga
-PKG_VERSION:=1.1.0
+PKG_VERSION:=1.1.1
 PKG_RELEASE:=1
-PKG_HASH:=f7a43a9c59bfd3722002210530b2553c8d5cc05bfea5acd56d4f102b9f55dc63
 
 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
 PKG_SOURCE_URL:=@SAVANNAH/quagga/
+PKG_HASH:=b5a94e5bdad3062e04595a5692b8cc435f0a85102f75dfdca0a06d093b4ef63f
+
+PKG_MAINTAINER:=Vasilis Tsiligiannis <acinonyx@openwrt.gr>
+PKG_LICENSE:=GPL-2.0-or-later
+PKG_LICENSE_FILES:=COPYING
+
+PKG_BUILD_PARALLEL:=1
+PKG_FIXUP:=autoreconf
+PKG_INSTALL:=1
+
 PKG_CONFIG_DEPENDS:= \
        CONFIG_IPV6 \
        CONFIG_PACKAGE_quagga-watchquagga \
@@ -26,10 +35,6 @@ PKG_CONFIG_DEPENDS:= \
        CONFIG_PACKAGE_quagga-ripd \
        CONFIG_PACKAGE_quagga-ripngd \
        CONFIG_PACKAGE_quagga-vtysh
-PKG_BUILD_PARALLEL:=1
-PKG_FIXUP:=autoreconf
-PKG_INSTALL:=1
-PKG_LICENSE:=GPL-2.0
 
 include $(INCLUDE_DIR)/package.mk
 
@@ -37,10 +42,9 @@ define Package/quagga/Default
   SECTION:=net
   CATEGORY:=Network
   SUBMENU:=Routing and Redirection
-  DEPENDS:=quagga
   TITLE:=The Quagga Software Routing Suite
-  URL:=http://www.quagga.net
-  MAINTAINER:=Vasilis Tsiligiannis <acinonyx@openwrt.gr>
+  URL:=https://www.quagga.net
+  DEPENDS:=quagga
 endef
 
 define Package/quagga
diff --git a/quagga/patches/001-bgpd-Fix-AS_PATH-size-calculation-for-long-paths.patch b/quagga/patches/001-bgpd-Fix-AS_PATH-size-calculation-for-long-paths.patch
new file mode 100644 (file)
index 0000000..3ecbc0e
--- /dev/null
@@ -0,0 +1,28 @@
+From: Andreas Jaggi <aj@open.ch>
+Date: Mon, 2 Oct 2017 19:38:43 +0530
+Subject: bgpd: Fix AS_PATH size calculation for long paths
+Origin: http://git.savannah.gnu.org/cgit/quagga.git/commit?id=7a42b78be9a4108d98833069a88e6fddb9285008
+Bug-Debian: https://bugs.debian.org/879474
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-16227
+
+If you have an AS_PATH with more entries than
+what can be written into a single AS_SEGMENT_MAX
+it needs to be broken up.  The code that noticed
+that the AS_PATH needs to be broken up was not
+correctly calculating the size of the resulting
+message.  This patch addresses this issue.
+---
+ bgpd/bgp_aspath.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+--- a/bgpd/bgp_aspath.c
++++ b/bgpd/bgp_aspath.c
+@@ -903,7 +903,7 @@ aspath_put (struct stream *s, struct asp
+               assegment_header_put (s, seg->type, AS_SEGMENT_MAX);
+               assegment_data_put (s, seg->as, AS_SEGMENT_MAX, use32bit);
+               written += AS_SEGMENT_MAX;
+-              bytes += ASSEGMENT_SIZE (written, use32bit);
++              bytes += ASSEGMENT_SIZE (AS_SEGMENT_MAX, use32bit);
+             }
+           
+           /* write the final segment, probably is also the first */
diff --git a/quagga/patches/002-Quagga-2018-0543.patch b/quagga/patches/002-Quagga-2018-0543.patch
new file mode 100644 (file)
index 0000000..1f77143
--- /dev/null
@@ -0,0 +1,58 @@
+From cc2e6770697e343f4af534114ab7e633d5beabec Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Wed, 3 Jan 2018 23:57:33 +0000
+Subject: bgpd/security: invalid attr length sends NOTIFY with data overrun
+
+Security issue: Quagga-2018-0543
+
+See: https://www.quagga.net/security/Quagga-2018-0543.txt
+
+* bgpd/bgp_attr.c: (bgp_attr_parse) An invalid attribute length is correctly
+  checked, and a NOTIFY prepared.  The NOTIFY can include the incorrect
+  received data with the NOTIFY, for debug purposes.  Commit
+  c69698704806a9ac5 modified the code to do that just, and also send the
+  malformed attr with the NOTIFY.  However, the invalid attribute length was
+  used as the length of the data to send back.
+
+  The result is a read past the end of data, which is then written to the
+  NOTIFY message and sent to the peer.
+
+  A configured BGP peer can use this bug to read up to 64 KiB of memory from
+  the bgpd process, or crash the process if the invalid read is caught by
+  some means (unmapped page and SEGV, or other mechanism) resulting in a DoS.
+
+  This bug _ought_ /not/ be exploitable by anything other than the connected
+  BGP peer, assuming the underlying TCP transport is secure.  For no BGP
+  peer should send on an UPDATE with this attribute.  Quagga will not, as
+  Quagga always validates the attr header length, regardless of type.
+
+  However, it is possible that there are BGP implementations that do not
+  check lengths on some attributes (e.g.  optional/transitive ones of a type
+  they do not recognise), and might pass such malformed attrs on.  If such
+  implementations exists and are common, then this bug might be triggerable
+  by BGP speakers further hops away.  Those peers will not receive the
+  NOTIFY (unless they sit on a shared medium), however they might then be
+  able to trigger a DoS.
+
+  Fix: use the valid bound to calculate the length.
+
+--- a/bgpd/bgp_attr.c
++++ b/bgpd/bgp_attr.c
+@@ -2079,6 +2079,8 @@ bgp_attr_parse (struct peer *peer, struc
+   memset (seen, 0, BGP_ATTR_BITMAP_SIZE);
+   /* End pointer of BGP attribute. */
++  assert (size <= stream_get_size (BGP_INPUT (peer)));
++  assert (size <= stream_get_endp (BGP_INPUT (peer)));
+   endp = BGP_INPUT_PNT (peer) + size;
+   
+   /* Get attributes to the end of attribute length. */
+@@ -2160,7 +2162,7 @@ bgp_attr_parse (struct peer *peer, struc
+           bgp_notify_send_with_data (peer,
+                                      BGP_NOTIFY_UPDATE_ERR,
+                                      BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+-                                     startp, attr_endp - startp);
++                                     startp, endp - startp);
+         return BGP_ATTR_PARSE_ERROR;
+       }
+       
diff --git a/quagga/patches/003-Quagga-2018-1114.patch b/quagga/patches/003-Quagga-2018-1114.patch
new file mode 100644 (file)
index 0000000..a4b91c5
--- /dev/null
@@ -0,0 +1,99 @@
+From e69b535f92eafb599329bf725d9b4c6fd5d7fded Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Sat, 6 Jan 2018 19:52:10 +0000
+Subject: bgpd/security: Fix double free of unknown attribute
+
+Security issue: Quagga-2018-1114
+See: https://www.quagga.net/security/Quagga-2018-1114.txt
+
+It is possible for bgpd to double-free an unknown attribute. This can happen
+via bgp_update_receive receiving an UPDATE with an invalid unknown attribute.
+bgp_update_receive then will call bgp_attr_unintern_sub and bgp_attr_flush,
+and the latter may try free an already freed unknown attr.
+
+* bgpd/bgp_attr.c: (transit_unintern) Take a pointer to the caller's storage
+  for the (struct transit *), so that transit_unintern can NULL out the
+  caller's reference if the (struct transit) is freed.
+  (cluster_unintern) By inspection, appears to have a similar issue.
+  (bgp_attr_unintern_sub) adjust for above.
+
+--- a/bgpd/bgp_attr.c
++++ b/bgpd/bgp_attr.c
+@@ -186,15 +186,17 @@ cluster_intern (struct cluster_list *clu
+ }
+ void
+-cluster_unintern (struct cluster_list *cluster)
++cluster_unintern (struct cluster_list **cluster)
+ {
+-  if (cluster->refcnt)
+-    cluster->refcnt--;
++  struct cluster_list *c = *cluster;
++  if (c->refcnt)
++    c->refcnt--;
+-  if (cluster->refcnt == 0)
++  if (c->refcnt == 0)
+     {
+-      hash_release (cluster_hash, cluster);
+-      cluster_free (cluster);
++      hash_release (cluster_hash, c);
++      cluster_free (c);
++      *cluster = NULL;
+     }
+ }
+@@ -344,15 +346,18 @@ transit_intern (struct transit *transit)
+ }
+ void
+-transit_unintern (struct transit *transit)
++transit_unintern (struct transit **transit)
+ {
+-  if (transit->refcnt)
+-    transit->refcnt--;
++  struct transit *t = *transit;
++  
++  if (t->refcnt)
++    t->refcnt--;
+-  if (transit->refcnt == 0)
++  if (t->refcnt == 0)
+     {
+-      hash_release (transit_hash, transit);
+-      transit_free (transit);
++      hash_release (transit_hash, t);
++      transit_free (t);
++      *transit = NULL;
+     }
+ }
+@@ -793,11 +798,11 @@ bgp_attr_unintern_sub (struct attr *attr
+       UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES));
+       
+       if (attr->extra->cluster)
+-        cluster_unintern (attr->extra->cluster);
++        cluster_unintern (&attr->extra->cluster);
+       UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST));
+       
+       if (attr->extra->transit)
+-        transit_unintern (attr->extra->transit);
++        transit_unintern (&attr->extra->transit);
+     }
+ }
+--- a/bgpd/bgp_attr.h
++++ b/bgpd/bgp_attr.h
+@@ -184,10 +184,10 @@ extern unsigned long int attr_unknown_co
+ /* Cluster list prototypes. */
+ extern int cluster_loop_check (struct cluster_list *, struct in_addr);
+-extern void cluster_unintern (struct cluster_list *);
++extern void cluster_unintern (struct cluster_list **);
+ /* Transit attribute prototypes. */
+-void transit_unintern (struct transit *);
++void transit_unintern (struct transit **);
+ /* Below exported for unit-test purposes only */
+ struct bgp_attr_parser_args {
diff --git a/quagga/patches/004-Quagga-2018-1550.patch b/quagga/patches/004-Quagga-2018-1550.patch
new file mode 100644 (file)
index 0000000..4bd3da8
--- /dev/null
@@ -0,0 +1,104 @@
+From 9e5251151894aefdf8e9392a2371615222119ad8 Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Sat, 6 Jan 2018 22:31:52 +0000
+Subject: bgpd/security: debug print of received NOTIFY data can over-read msg
+ array
+
+Security issue: Quagga-2018-1550
+See: https://www.quagga.net/security/Quagga-2018-1550.txt
+
+* bgpd/bgp_debug.c: (struct message) Nearly every one of the NOTIFY
+  code/subcode message arrays has their corresponding size variables off
+  by one, as most have 1 as first index.
+
+  This means (bgp_notify_print) can cause mes_lookup to overread the (struct
+  message) by 1 pointer value if given an unknown index.
+
+  Fix the bgp_notify_..._msg_max variables to use the compiler to calculate
+  the correct sizes.
+
+--- a/bgpd/bgp_debug.c
++++ b/bgpd/bgp_debug.c
+@@ -29,6 +29,7 @@ Software Foundation, Inc., 59 Temple Pla
+ #include "log.h"
+ #include "sockunion.h"
+ #include "filter.h"
++#include "memory.h"
+ #include "bgpd/bgpd.h"
+ #include "bgpd/bgp_aspath.h"
+@@ -73,7 +74,8 @@ const struct message bgp_status_msg[] =
+   { Clearing,    "Clearing"    },
+   { Deleted,     "Deleted"     },
+ };
+-const int bgp_status_msg_max = BGP_STATUS_MAX;
++#define BGP_DEBUG_MSG_MAX(msg) const int msg ## _max = array_size (msg)
++BGP_DEBUG_MSG_MAX (bgp_status_msg);
+ /* BGP message type string. */
+ const char *bgp_type_str[] =
+@@ -84,7 +86,8 @@ const char *bgp_type_str[] =
+   "NOTIFICATION",
+   "KEEPALIVE",
+   "ROUTE-REFRESH",
+-  "CAPABILITY"
++  "CAPABILITY",
++  NULL,
+ };
+ /* message for BGP-4 Notify */
+@@ -98,15 +101,15 @@ static const struct message bgp_notify_m
+   { BGP_NOTIFY_CEASE, "Cease"},
+   { BGP_NOTIFY_CAPABILITY_ERR, "CAPABILITY Message Error"},
+ };
+-static const int bgp_notify_msg_max = BGP_NOTIFY_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_msg);
+ static const struct message bgp_notify_head_msg[] = 
+ {
+   { BGP_NOTIFY_HEADER_NOT_SYNC, "/Connection Not Synchronized"},
+   { BGP_NOTIFY_HEADER_BAD_MESLEN, "/Bad Message Length"},
+-  { BGP_NOTIFY_HEADER_BAD_MESTYPE, "/Bad Message Type"}
++  { BGP_NOTIFY_HEADER_BAD_MESTYPE, "/Bad Message Type"},
+ };
+-static const int bgp_notify_head_msg_max = BGP_NOTIFY_HEADER_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_head_msg);
+ static const struct message bgp_notify_open_msg[] = 
+ {
+@@ -119,7 +122,7 @@ static const struct message bgp_notify_o
+   { BGP_NOTIFY_OPEN_UNACEP_HOLDTIME, "/Unacceptable Hold Time"}, 
+   { BGP_NOTIFY_OPEN_UNSUP_CAPBL, "/Unsupported Capability"},
+ };
+-static const int bgp_notify_open_msg_max = BGP_NOTIFY_OPEN_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_open_msg);
+ static const struct message bgp_notify_update_msg[] = 
+ {
+@@ -136,7 +139,7 @@ static const struct message bgp_notify_u
+   { BGP_NOTIFY_UPDATE_INVAL_NETWORK, "/Invalid Network Field"},
+   { BGP_NOTIFY_UPDATE_MAL_AS_PATH, "/Malformed AS_PATH"},
+ };
+-static const int bgp_notify_update_msg_max = BGP_NOTIFY_UPDATE_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_update_msg);
+ static const struct message bgp_notify_cease_msg[] =
+ {
+@@ -150,7 +153,7 @@ static const struct message bgp_notify_c
+   { BGP_NOTIFY_CEASE_COLLISION_RESOLUTION, "/Connection collision resolution"},
+   { BGP_NOTIFY_CEASE_OUT_OF_RESOURCE, "/Out of Resource"},
+ };
+-static const int bgp_notify_cease_msg_max = BGP_NOTIFY_CEASE_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_cease_msg);
+ static const struct message bgp_notify_capability_msg[] = 
+ {
+@@ -159,7 +162,7 @@ static const struct message bgp_notify_c
+   { BGP_NOTIFY_CAPABILITY_INVALID_LENGTH, "/Invalid Capability Length"},
+   { BGP_NOTIFY_CAPABILITY_MALFORMED_CODE, "/Malformed Capability Value"},
+ };
+-static const int bgp_notify_capability_msg_max = BGP_NOTIFY_CAPABILITY_MAX;
++BGP_DEBUG_MSG_MAX (bgp_notify_capability_msg);
+ /* Origin strings. */
+ const char *bgp_origin_str[] = {"i","e","?"};
diff --git a/quagga/patches/005-Quagga-2018-1975.patch b/quagga/patches/005-Quagga-2018-1975.patch
new file mode 100644 (file)
index 0000000..5e0c6ee
--- /dev/null
@@ -0,0 +1,32 @@
+From ce07207c50a3d1f05d6dd49b5294282e59749787 Mon Sep 17 00:00:00 2001
+From: Paul Jakma <paul@jakma.org>
+Date: Sat, 6 Jan 2018 21:20:51 +0000
+Subject: bgpd/security: fix infinite loop on certain invalid OPEN messages
+
+Security issue: Quagga-2018-1975
+See: https://www.quagga.net/security/Quagga-2018-1975.txt
+
+* bgpd/bgp_packet.c: (bgp_capability_msg_parse) capability parser can infinite
+  loop due to checks that issue 'continue' without bumping the input
+  pointer.
+
+--- a/bgpd/bgp_packet.c
++++ b/bgpd/bgp_packet.c
+@@ -2251,7 +2251,8 @@ bgp_capability_msg_parse (struct peer *p
+   end = pnt + length;
+-  while (pnt < end)
++  /* XXX: Streamify this */
++  for (; pnt < end; pnt += hdr->length + 3)
+     {      
+       /* We need at least action, capability code and capability length. */
+       if (pnt + 3 > end)
+@@ -2339,7 +2340,6 @@ bgp_capability_msg_parse (struct peer *p
+           zlog_warn ("%s unrecognized capability code: %d - ignored",
+                      peer->host, hdr->code);
+         }
+-      pnt += hdr->length + 3;
+     }
+   return 0;
+ }
index 84451033f937ca7e4876c4e508bc6c5ac1b881ff..18078bc8871a138a2a9b52b4d5097c3025a68b36 100644 (file)
@@ -1,6 +1,6 @@
 --- a/lib/command.c
 +++ b/lib/command.c
-@@ -3198,6 +3198,13 @@ DEFUN (config_write_file,
+@@ -3200,6 +3200,13 @@ DEFUN (config_write_file,
                 VTY_NEWLINE);
          goto finished;
        }
@@ -14,7 +14,7 @@
    if (link (config_file, config_file_sav) != 0)
      {
        vty_out (vty, "Can't backup old configuration file %s.%s", config_file_sav,
-@@ -3211,7 +3218,23 @@ DEFUN (config_write_file,
+@@ -3213,7 +3220,23 @@ DEFUN (config_write_file,
                VTY_NEWLINE);
        goto finished;
      }