commit 88d6516676cbcefb6ecdc1828cf59ba3a6e5fe7b Author: Paul Jakma Date: Sun Feb 4 17:34:34 2018 +0000 release: Quagga 1.2.3 commit cbffa53cc0454bcc4ab95d9363b13fb8c68301d4 Author: Paul Jakma Date: Sun Jan 21 17:02:32 2018 +0000 doc/security: Security announcements for 4 issues * doc/security/Quagga-2018-0543.txt: attr_endp used for NOTIFY data * doc/security/Quagga-2018-1114.txt: bgpd double free * doc/security/Quagga-2018-1550.txt: debug overrun in notify lookup tables * doc/security/Quagga-2018-1975.txt: BGP capability inf. loop commit f080b436bbddf8d28dd991c967dcac5288272522 Author: Paul Jakma Date: Sun Jan 21 17:01:18 2018 +0000 doc/security: Add a doc/security folder and template for announcements * doc/security: New folder to store Quagga security announcements, where they can be revision controlled. * doc/security/template.txt: Template for announcements commit 621d8174d4f30e11cc8fca3324eb9d7579d9c01c Author: Paul Jakma Date: Sat Jan 20 12:15:40 2018 +0000 doc: Add commit message template, suitable for commit.template * doc/commit-template.txt: Add git commit template, that can be enabled via: git config --add commit.template doc/commit-template.txt commit f1ee20c940a712c685f5435d2d5af212a8f76702 Author: Paul Jakma Date: Thu Jan 4 00:22:53 2018 +0000 bgpd: remove stream_pnt use for notify data * bgp_packet.c: (bgp_open_receive) Remove the stream_pnt introduced in c69698704806a9ac50. stream_pnt / BGP_INPUT_PNT / etc. should be avoided as much possible, and I/O put through the lib/stream checked buffer as much as possible. Not really any functional effect here given the fixed size, other than to remove something that shouldn't be copied. commit 5c2cba2d3945b302f4276174dd0f182a5d3dc501 Author: Paul Jakma Date: Wed Jan 3 23:44:30 2018 +0000 lib/privs: Remove of CAP_NET_BROADCAST forgot to decrement array count * lib/privs.c: (cap_map) Removal of Linux CAP_NET_BROADCAST from ZCAP_BIND forgot to decrement the array count in the 'num' field. Resulting in an overread of memory from zcaps2sys from zprivs_caps_init. commit 9e5251151894aefdf8e9392a2371615222119ad8 Author: Paul Jakma Date: Sat Jan 6 22:31:52 2018 +0000 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. commit ce07207c50a3d1f05d6dd49b5294282e59749787 Author: Paul Jakma Date: Sat Jan 6 21:20:51 2018 +0000 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. commit e69b535f92eafb599329bf725d9b4c6fd5d7fded Author: Paul Jakma Date: Sat Jan 6 19:52:10 2018 +0000 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. commit cc2e6770697e343f4af534114ab7e633d5beabec Author: Paul Jakma Date: Wed Jan 3 23:57:33 2018 +0000 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. commit 98bdd04e3fdd6790453e0dce13853c2472ce5d15 Author: Gerrie Roos Date: Wed Jan 17 21:16:55 2018 +0000 zebra/redistribute: Implicit withdraw needs to be explicit if update isn't sent * redistribute.{c,h}: (redistribute_add) update of redistributed route is an implicit withdraw of the old route. The RIB therefore doesn't bother deleting the old route, if doing a redistribute_add. However, if the updated route is /not/ sent to a client that received the previous route, then such a client is left with bogus state. This can happen when the new route is of a type that the client doesn't redistibute. Fix by passing in the old route, and adding an explicit delete of the old route where necessary. * zebra_rib.c: (rib_process) pass on the old route too, as per above. * redistribute_null.c: testing stub See bug #971 Modification to fix the problem at the redistribute layer instead of the RIB suggested by paul@jakma.org. commit 0bb980bbfbd0292dd69a6af0bfc7306b3ca97fd9 Author: Pier Carlo Chiodi Date: Tue Sep 19 09:25:54 2017 +0200 doc: 'match aspath' should be 'match as-path' commit adda534f95ec87206c9dfd1b3bae05221dc29730 Author: Rolf Eike Beer Date: Mon Dec 4 18:36:21 2017 +0100 bgpd: fix SIGBUS There is one test failure in the testsuite on sparc: Running ./bgpd.tests/testbgpcap.exp ... failed: testbgpcap ORF: ORF, simple, single entry, single tuple -- testbgpcap aborted! The error is a SIGBUS in bgp_capability_mp_data() because of an unaligned memory access. Use memcpy() instead of direct assignments. Compilers on platforms that support unaligned accesses should be clever enough to optimize the function call away and do the direct store, so this should not hurt there. commit 1db1b9baea511995b67a9b282d5c97e87479fe5d Author: Mathieu Jadin Date: Thu Dec 14 17:53:53 2017 +0100 bgpd: Fix mistake in NHT of connected IPv6 next-hops preventing route advertisement Since quagga-1.2.0, the Next Hop validation for directly connected peers using IPv6 does not work. In this setup, BGP updates contain two next hops: a global IPv6 address and a link-local IPv6 address (a correct behavior according to RFC 2545). This means that the length of the next hop attribute is 32 and not 16. The problem comes from the function "make_prefix()" in "bgpd/bgp_nht.c". It refuses to build a prefix structure for a route when the length of the [Anext hop attribute is different from 16, even if a valid global IPv6 address is available. The route is mistakenly considered invalid and thus, it is not installed in the routing table. Details: "make_prefix()" was not modified in quagga-1.2.0 but its interpretation was changed in commit 3dda6b3eccb9a2a88d607372c83c04c796e7daac. Before this commit, the failure of "make_prefix()" was interpreted as a successful validation of the next hop. commit 57fd1a3b237807a9eb8cf977e95a5d66d88a10e5 Author: Balaji Gurudoss Date: Tue May 2 21:50:31 2017 +0530 Updated the protocol supported list commit dcfbf4eb452d8483a73e4d487d4d93b46cee1569 Author: Paul Jakma Date: Tue Dec 5 21:09:46 2017 +0000 lib/command: make config file robust more robust and kinder to system * command.c: (config_write_file) Remove two very heavyweights sync()s and replace with an fdatasync of just the freshly writen config file data. Make the move of the new config into place more robust, by using rename instead of unlink/link. This should fix a performance issue on systems with slow storage, where the syncs were disrupting performance, see bugzilla #966. Should also be more robust. Problem diagnosed and reported by: Patrick Kuijvenhoven with an initial fix, on which this commit develops, and further work on testing. commit 8d49cd5af30f83af8010c0582561745ef09bdafe Author: Paul Jakma Date: Sun Jun 4 15:25:22 2017 +0100 doc: Bring documentation on Zserv header up to date. commit b720729eab7b531fcdecb6cefe27ae58b799360e Author: Paul Jakma Date: Wed Apr 26 13:59:40 2017 +0100 bgpd: distance comment commit fd52235386e8889244ae22eba9077db9248e05e0 Author: Borg Date: Mon Apr 24 09:51:07 2017 +0100 doc: Fix small but important logical mistake in community-list example * bgpd.texi: Comm-lists must match all communities specified. The text to accompany the example however says it is an "or" match, which is wrong and very misleading. Fix. commit 4d942820a8291a8bcb8c6c90425bc52fea1cf5ba Author: Paul Jakma Date: Thu Apr 13 17:48:55 2017 +0100 doc: document that changing bgp distance needs a hard clear of routes commit 409e437a7a8a059adda9e8fc50c91bd8801c0352 Author: Paul Jakma Date: Tue Mar 27 12:11:52 2012 +0100 bgpd: malformed attribute handling: don't pass on, and add missing notify * bgpd/bgp_attr.c: (bgp_attr_malformed) A malformed attribute should never be passed on, even if we proceed with parsing the UPDATE. The default reset case should send the NOTIFY itself, so the given subcode is used. commit cfbdd869687dc076256bce455d03b2ccf04b4a77 Author: Paul Jakma Date: Mon Apr 3 22:02:38 2017 +0100 lib/filter: change add/delete callback hooks to robustly delete * Prior band-aids were made to address use-after-frees in lib/filter with deletes, but they introduced another error. They allowed the access-list being deleted to be visible by access_list_lookup from the users' delete callback, causing deleted access-list references to not be removed, leading to different use-after-frees instead. Fix in a robust manner within the filter code. This bug was reported and debugged by matt@kontoff.com, with an initial fix, on which this commit builds. See https://bugzilla.quagga.net/show_bug.cgi?id=945. * filter.h: Change the callback hooks to take the access-list name, not the access-list reference. The name can be a weaker, more opaque ref. * filter.c: Update hooks calls to pass name. (access_list_{insert,lookup}) guard strcmp of name, as name may now potentially be NULL for access-lists in process of being deleted. (access_list_filter_delete) Transfer ownership of the access-list name to a local, so the access-list can be deleted, and the name then passed to the callback. (no_access_list_all) ditto. (no_ipv6_access_list_all) ditto. (filter_show) guard name strcmp, shouldn't be possible to see an access-list being deleted here, but better safe. * ospfd/ospf_zebra.c: (ospf_filter_update) adjust for hook change. * bgpd/bgpd.c: (peer_distribute_update) adjust for hook change, not using the arg. * ripd/ripd.c: (rip_distribute_update_all_wrapper) ditto * ripngd/ripngd.c: (ripng_distribute_update_all_wrapper) ditto. commit 6af87e10d6e7f1f99ce5416b50d352f209ffce25 Author: Paul Jakma Date: Mon Apr 3 17:45:51 2017 +0100 Revert "lib: Fix Free Pointer dereference in lib/filter.c" This reverts commit 4fdb5f401eb277fa54d80e99d241bd9b03895a6a. This introduces bugs, as callers are using the same hook for add/delete. Using a pattern of looking up the access-list by name, and updating their internal references by its result. With the access-list still active when the delete hook is called, this swaps a use-after-free or NULL deref in one hook for use-after-frees in many other places. See https://bugzilla.quagga.net/show_bug.cgi?id=945 commit 6c1ea424823e7a5039166e98a39c9d312f517019 Author: Paul Jakma Date: Mon Apr 3 17:36:47 2017 +0100 Revert "lib: call filter delete hook before freeing access list" This reverts commit 6a2e0f36b103386e57dbe3a6ee4716e809111198. This introduces bugs, as callers are using the same hook for add/delete. Using a pattern of looking up the access-list by name, and updating their internal references by its result. With the access-list still active when the delete hook is called, this swaps a NULL deref in one hook for use-after-frees in many other places. See https://bugzilla.quagga.net/show_bug.cgi?id=945 commit 0349ff4eb58e2300d77a2715d40818b8191547b3 Author: Paul Jakma Date: Wed Mar 29 11:51:59 2017 +0100 infra/buildbot: allow bots to be picked out by installed compiler. * master.cfg: add "compilers" function to return available compilers. Add "ccbots" to select the bots with given compiler tag present. workers: Change the compiler info lists from tuples to dicts, so the name can be picked out and to allow optional information. commit 97cd3684deda42f3514e6c97e6b7fa7402e24ad2 Author: Paul Jakma Date: Wed Mar 29 09:10:09 2017 +0100 infra/buildbot: Add bots, add JSON "env" config variable, poll all git branches * master.cfg: Add an "env" key to config, to store environment variables to set. Has to be stored as JSON in order to be able to have the builder pass the envs in as a property that the build steps can access later. Needed for OpenBSD, where auto* commands are wrappers that redirect to auto*- binaries based on env. Add ubuntu bot. Scan all branches in upstream git. Factor out common steps, to variables that can be re-used in addSteps. commit bc69d454344c37dcda5d5e5379af88f56ee578fd Author: Paul Jakma Date: Tue Mar 29 14:06:21 2011 +0100 lib: ptr macro arg may need brackets in some cases commit 9d035625fe5aa648d4bece430df24bcd2c04c2f2 Author: Scott Leggett Date: Thu Mar 23 12:36:15 2017 +0000 distro/systemd: add man page ref and set config file permissions * redhat/*.service: Add "Documentation=man:..." lines. Chmod and chown the config files as appropriate. * redhat/quagga.sysconfig: Make configure generated, to substitute in the configured user/groups to an Env var, for use in the service files. Note: This is partial and edited version of Scott's patch, by Paul Jakma. Stupid mistakes will be mine. Note2: Would be good to move distro agnostic files, like systemd service files, to a neutral dirctory. commit e2ad1c0d377a4a02504de45c09b5014fceeb2beb Author: Scott Leggett Date: Sat Nov 12 03:27:41 2016 +1100 doc: Fix manpage number for ospfclient. commit 2b7cf4073db9c8d3af90b44f2d25247918f5e42c Author: Scott Leggett Date: Sat Nov 12 01:58:52 2016 +1100 vtysh: Fix spelling errors in strings flagged by lintian. commit 0b8c3d6137d5fc811b34c5acb5980fe701040236 Author: Scott Leggett Date: Sat Nov 12 01:26:10 2016 +1100 doc: Tweak grammar in zebra manpage to keep lintian happy. commit 37b67cbf5c54f971beb1d559480fbe47a415d9da Author: Debian QA Group Date: Sat Nov 12 01:16:08 2016 +1100 vtysh: print error if PAM auth does not succeed commit 5e13840d7f3e7fcdf7ecff762c019bb56c88965f Author: Paul Jakma Date: Fri Mar 17 13:30:56 2017 +0000 lib/thread: get rid of the shallow-copy thread_fetch add a sane thread_main * thread.h: (thread_{fetch,call}) unexport these functions. thread_fetch has a funny "return a cloned, shallow-copied thread struct" semantics that are needless. thread_call has no users other than the usual main thread loop, which should be replaced with: (thread_main) encapsulated main thread loop. * thread.c: (thread_run) no need for this shallow-copy anymore. (thread_add_unuse) no need for a separate master argument. Update all callers to match. Setting type to THREAD_UNUSED can be done here. (thread_fetch) no need to copy the thread or add to unused _before_ running it, so return the chosen thread directly. (thread_call) This runs the thread, so add_unuse best done here, at end. (thread_main) Simple main loop for public use. * */*main.c: update to thread_main commit 671444d81b351ae72a4bc4b2d577b270c6d18f45 Author: Paul Jakma Date: Thu Mar 16 16:32:53 2017 +0000 buildbot/master: use a helper generator for make cmd string list commit a9bba93948262c0974f3b590a5793b1e78de0278 Author: Paul Jakma Date: Thu Mar 16 15:22:47 2017 +0000 buildbot/master: fix the common steps * master.cfg: Trailing commas on common steps, left from moving out from a list, had made the assignment be of tuples. All kinds of funniness ensued. Also, rpm builder, had dropped configure step in using common steps. commit 2226d4065066beed658a2cc4d1fd31a81bf73137 Author: Paul Jakma Date: Thu Mar 16 13:03:49 2017 +0000 buildbot/master: Add OBSD bot, and support for environment variable config * master.cfg: Add OpenBSD buildbot. Add an "env" key to the configuration dictionary, allowing for a JSON string of env variables, to be passed into the properties of BuildSlaves. Add a get_config_env Renderer, to allow build Steps to convert JSON env variable property to a python dictionary and give to the env= argument of some steps. commit fa8555562d4bd206b2233bcb3e602814a51f66dc Author: Paul Jakma Date: Fri Mar 31 17:23:22 2017 +0100 build: AC_EGREPP_CPP actions wrong way around, worked by accident mostly. * configure.ac: Sense of actions wrong way around. Guess it worked by accident by virtue of any non-GCC compilers being used having GCC still set, or else working with SUNPRO cflags. commit 17ab96ac9a8c5a9a5ce6ccea19cb07de6ddf740e Author: Paul Jakma Date: Wed Mar 15 15:30:41 2017 +0000 build: Work around illumos still shipping a prehistoric AWK as default * configure.ac: illumos still ships 'oawk' (old AWK) as the default 'awk'. A pre-historic version of AWK, from the mists of Unix history (AT&T System V Release 4), which Sun kept around deliberately not updating it or fixing non-critical bugs, to ensure bug-for-bug compatibility. It doesn't support the -F argument AWK has had since... slightly less deeper in the mist (e.g. SysV R4.2, shipped as 'nawk' in Solaris and illumos). Only -Fc. Joyent apparently has updated the default awk in SmartOS to nawk + extensions, and may upstream it to illumos. commit 1d4b282f79fdbc8d25404f69d2e90b4ef887de9a Author: Paul Jakma Date: Wed Mar 15 13:28:09 2017 +0000 tests: Remove DejaGNU, automake already supports tallying exit based tests * Automake already knows how to tally up exit status based tests. There's not much point wrapping such unit tests up in a whole ball of tcl and expect to scrape the output to determine the same thing and tally. Remove the DejaGNU stuff. Run the tests directly from automake, using TESTS = ... This removes the "check output doesn't differ" testcli and testcommand tests from testing, but those were testing exit status of the diff, which was succesful anyway. So would pass even if output differed and should fail. * test-cli.c: Add example usage. * test-commands.c: ditto. And add missing nodes. commit 0dde143ffcdda5f644d7025606994f67aec0b2fa Author: Paul Jakma Date: Tue Mar 14 14:20:13 2017 +0000 build: LT_INIT obsoletes AC_PROG_RANLIB commit 8d3b7b8356a5d1cc18aed0157dc7e33a0c036ce9 Author: Paul Jakma Date: Fri Mar 10 17:00:28 2017 +0000 doc: tweak CSS to Quagga colours commit 084a4ab85da677f3405038b3b3bd781fd94debb6 Author: Evgeny Uskov Date: Fri Nov 24 16:03:32 2017 +0300 bgpd: fix file descriptor leaks in vty_close In vty_close output file descriptor was not properly closed. It caused file descriptor leak each time an updated config file was saved on disk. commit c1d777f355ebcaf6cfc1b7b6fbd3c7ece2752b0e Author: Alban Browaeys Date: Mon Nov 13 14:32:31 2017 +0100 Fix wrong command persisted by vtysh Fixes invalid syntax when applying integrated configuration a file with vtysh -b. Commit d8aa4beab72cdd2c2d78f9e624fd4b704eec488f ("vtysh: Fix Quagga.conf file read in.") replaced NULL cmd argument but only in one instance of the cmd_execute_command_strict call. This patch fixes the second instance. commit a0a4e6814c250bcd5501be8206695fb8b7f49d18 Author: Eugene Bogomazov Date: Fri Oct 13 16:28:34 2017 +0300 Fix malformed AS_SEQUENCE segments for long as path