Home - Waterfall Grid T-Grid Console Builders Recent Builds Buildslaves Changesources - JSON API - About

Console View

Legend:   Passed Failed Warnings Failed Again Running Exception Offline No data

lib: Avoid re-definition of IPPROTO_IP with some versions of Linux
* Starting from linux-4.11 [commit
  bcb41c6bced1ee778d23c53a6b4807fb08cf5540], linux/mroute.h includes
  linux/in.h , that makes gcc roar a lot of things like "error:
  redeclaration of enumerator 'IPPROTO_IP'" when compiling quagga-1.2.2

  lib/zebra.h includes sys/capability.h first, then includes
  netinet/in.h . In sys/capability.h, it includes linux/xattr.h, and
  that includes linux/libc-compat.h . Since at that time netinet/in.h is
  not included yet, _NETINET_IN_H is not defined, causing libc-compat.h
  set __UAPI_DEF_IN_IPPROTO to 1. Then, a include of netinet/in.h
  defines IPPROTO_IP. Later a include of linux/mroute.h includes
  linux/in.h. Because __UAPI_DEF_IN_IPPROTO is set to non zero,
  IPPROTO_IP is redeclared.

* lib/zebra.h: Move the privs/capabilities include block to after the
  network block.
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.
Paul Jakma
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.
Paul Jakma
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
Paul Jakma
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.
Scott Leggett
doc: Fix manpage number for ospfclient.
Paul Jakma
release: Quagga 1.2.4
Brown paper bag release.
Paul Jakma
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 <patrick.kuijvenhoven@gmail.com>

  with an initial fix, on which this commit develops, and further work on
Paul Jakma
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

* 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
* filter.c: Update hooks calls to pass name.
  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
* ripd/ripd.c: (rip_distribute_update_all_wrapper) ditto
* ripngd/ripngd.c: (ripng_distribute_update_all_wrapper) ditto.
Paul Jakma
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
Paul Jakma
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.
Paul Jakma
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
Paul Jakma
lib: ptr macro arg may need brackets in some cases
Paul Jakma
bgpd: distance comment
Rolf Eike Beer
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.
Ilya Shipitsin
ripd/rip_snmp.c: Remove not needed check
rn cannot be null here

issue detected by cppcheck:

[ripd/rip_snmp.c:208] -> [ripd/rip_snmp.c:207]: (warning) Either the condition
'if(rn&&!strncmp(i->name,ifp->name,INTERFACE_NAMSIZ))' is redundant or there is
possible null pointer dereference: rn.

Signed-off-by: Ilya Shipitsin <chipitsine@gmail.com>
Paul Jakma
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.
Paul Jakma
lib: Fix assert in thread_add_unuse if thread_execute was used
* thread.c: (thread_call) thread_execute passes in a dummy thread, on its
  stack, with a NULL thread master. Those shouldn't be added to the unuse
  list or thread_add_unuse rightly asserts.

  Fix this very dumb bug.

  See https://bugzilla.quagga.net/show_bug.cgi?id=975

  With thanks to Sergey Popov, admin@pinkbyte.ru, and Andreas Nilsson,
  andrnils@resilans.se, for help with diagnosis and testing.
Scott Leggett
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.
Gerrie Roos
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

  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.
Paul Jakma
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*-<ver> 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.
Paul Jakma
doc: Bring documentation on Zserv header up to date.
Paul Jakma
release: Quagga 1.2.3
Paul Jakma
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
Paul Jakma
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
Paul Jakma
buildbot/master: use a helper generator for make cmd string list
Paul Jakma
doc: Add commit message template,  suitable for commit.template
* doc/commit-template.txt: Add git commit template, that can be enabled

  git config --add commit.template doc/commit-template.txt
Paul Jakma
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
Paul Jakma
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.
Balaji Gurudoss
Updated the protocol supported list
Paul Jakma
doc: document that changing bgp distance needs a hard clear of routes
Paul Jakma
release/scripts: Add short subject log location to print out
* The short, by subject log is useful too, include its location in summary
Paul Jakma
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.
Debian QA Group
vtysh: print error if PAM auth does not succeed
Scott Leggett
vtysh: Fix spelling errors in strings flagged by lintian.
Paul Jakma
lib/thread: Address other paths from thread_execute to thread_add_unuse
* lib/thread.c: There are further paths from thread_execute to
  thread_add_unuse, beyond that from bugzilla bug#975.

  Make the paths from thread_execute to thread_add_unuse, inc.
  thread_add_unuse itself, tolerant to the "dummy" threads of

  Another option would be to have thread_execute properly obtain a
  thread struct, rather than fake one on its heap.

  (thread_add_unuse) Be tolerant of NULL master threads passed in, that
  /ought/ to be dummy/non-heap threads, and just ignore.  Don't assert.
  (thread_call) no point checking for NULL master here anymore.

See https://bugzilla.quagga.net/show_bug.cgi?id=977

With thanks to John Hay, john@sanren.ac.za, for testing and verifying
the fix.
Mathieu Jadin
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
Pier Carlo Chiodi
doc: 'match aspath' should be 'match as-path'
Scott Leggett
doc: Tweak grammar in zebra manpage to keep lintian happy.
Paul Jakma
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.