| Age | Commit message (Collapse) | Author |
|---|
|
OVERVIEW
The checksum used in OSPF (rfc2328) is specified in rc905 annex B. There is an
sample implementation in rfc1008 which forms the basis of the quagga
implementation. This algorithm works perfectly when generating a checksum;
however, validation is a bit problematic.
The following LSA (generated by a non-quagga implementation) is received by
quagga and marked with an invalid checksum; however, it passes both the rfc905
and rfc1008 validation checks.
static uint8_t lsa_10_121_233_29[] = {
0x0e, 0x10, 0x02, 0x03,
0x09, 0x00, 0x35, 0x40,
0x0a, 0x79, 0xe9, 0x1d,
0x80, 0x00, 0x00, 0x03,
0x00, 0x8a, 0x00, 0x1c,
0xff, 0xff, 0xff, 0xe0,
0x00, 0x00, 0x36, 0xb0
};
LS Type: Summary-LSA (IP network)
LS Age: 3600 seconds
Do Not Age: False
Options: 0x02 (E)
Link-State Advertisement Type: Summary-LSA (IP network) (3)
Link State ID: 9.0.53.64
Advertising Router: 10.121.233.29 (10.121.233.29)
LS Sequence Number: 0x80000003
LS Checksum: 0x008a
Length: 28
Netmask: 255.255.255.224
Metric: 14000
You'll note that one byte of the checksum is 0x00; quagga would calculate the
checksum as 0xff8a.
It can be argued that the sourcing implementation generates an incorrect
checksum; however, rfc905 indicates that, for 1's complement arithmetic, the
value 255 shall be regarded as 0, thus either values are valid.
EXPLANATION
The quagga ospfd and ospf6d implementations operate by copying the PDU's
existing checksum in a holding variable, calculating the checksum, and comparing
the resulting checksum to the original. As a note, this implementation has the
side effect of modifying the contents of the PDU.
Evaluation of both rfc905 and rfc1008 shows that checksum validation should
involve calculating the sum over the PDU and checking that both resulting C0 and
C1 values are zero. This behavior is enacted in the rfc1008 implementation by
calling encodecc with k = 0 (checksum offset); however, this functionality had
been omitted from the quagga implementation.
PATCH
This patch adds the ability to call the quagga's fletcher_checksum() with a
checksum offset value of 0xffff (aka FLETCHER_CHECKSUM_VALIDATE) which returns
the sum over the buffer (a value of 0 indicates a valid checksum). This is
similar to the mechanism in rfc1008 when called with k = 0. The patch also
introduces ospf_lsa_checksum_valid().
ospf6d had it's own implementation of the fletcher checksum in
ospf6_lsa_checksum(); it's the same algorithm as in fletcher_checksum(). This
patch removes the local implementation in favor of the library's as well as creates
and uses ospf6_lsa_checksum_valid().
quagga's ISIS implementation suffers from the same problem; however, I do not
have the facilities to validate a fix to ISIS, thus this change has been left to
the ISIS maintainers. The function iso_csum_verify() should be reduced to
running the fletcher checksum over the buffer using an offset of 0.
Signed-off-by: JR Rivers <jrrivers@cumulusnetworks.com>
Reviewed-by: Scott Feldman <sfeldma@cumulusnetworks.com>
Reviewed-by: Nolan Leake <nolan@cumulusnetworks.com>
Reviewed-by: Ayan Banerjee <ayan@cumulusnetworks.com>
Reviewed-by: Shrijeet Mukherjee <shm@cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
|
|
This vulnerability (CERT-FI #514839) was reported by CROSS project.
When Database Description LSA header list contains trailing zero octets,
ospf6d tries to process this data as an LSA header. This triggers an
assertion in the code and ospf6d shuts down.
* ospf6_lsa.c
* ospf6_lsa_is_changed(): handle header-only argument(s)
appropriately, do not treat LSA length underrun as a fatal error.
|
|
A clean exit makes it easier to use memory debuggers.
* ospf6_asbr.c: (ospf6_asbr_terminate) Add a function to do route map
cleanup.
* ospf6_lsa.c: (ospf6_lsa_terminate) Add a function to cleanup the lsa
handler vector.
* ospf6_main.c: (ospf6_exit) Add an function that causes ospf6d to
gracefully exit.
* ospf6_message.c: (ospf6_message_terminate) Add a function that frees
the send and receive buffers.
* ospf6_top.c: (ospf6_delete) Enable the ospf6_delete() function.
Disable ospf6 before freeing everything.
|
|
It seems that there is a bug in ospf6d in ospf6_lsa_compare(): If LSA A
has sequence number smaller than 0x80000000 and LSA B has sequence
number larger than 0x80000000, ospf6_lsa_compare() returns that B is
more recent than A, although RFC says that sequence numbers should be
compared as signed numbers (0x8000001 smallest and 0x7FFFFFFF largest).
In ospfd, the function ospf_lsa_more_recent() has it right.
The problem appears when Quagga is used together with OSPFv3 in
development version of BIRD daemon ( http://bird.network.cz/ ),
which creates LSAs with maximum sequence number (0x7FFFFFFF)
as a part of flushing/premature aging LSA from OSPF area.
Because both daemons has different idea of which LSA instance
is more recent, it would lead to LSA storm.
|
|
* ospf6_lsa.c: (ospf6_lsa_age_current) arithmetical compares make no sense
in non-host order..
|
|
* ospf6_lsa.c: (ospf6_lsa_premature_aging) set age to MAX_AGE - don't
rely on 0 magically meaning same.
(ospf6_lsa_age_current) handle MAXAGE.
|
|
|
|
Simple conversion of XMALLOC/memset to XCALLOC
|
|
2008-08-15 Paul Jakma <paul.jakma@sun.com>
* {ospf6d,ripngd}/*: Finish job of marking functions as static, or
exporting declarations for them, to quell warning noise with
Quagga's GCC default high-level of warning flags. Thus allowing
remaining, more useful warnings to be more easily seen.
|
|
2007-06-07 Pavol Rusnak <prusnak@suse.cz>
* ospf6_lsa.c: (no_debug_ospf6_lsa_hex_cmd) Fix bug: must use strcmp
to compare strings.
|
|
2007-02-27 Pavol Rusnak <prusnak@suse.cz>
* ospf6_lsa.c: (ospf6_lsa_handler_name) Fix bug: must use strcmp
to compare strings.
|
|
* (global) update all c files to match the lib/vector.h rename of
(struct vector).active to max, and vector_max macro to
vector_active.
* lib/vector.h: Rename to (struct vector).max to slightly less
confusing active, for the number of active slots, distinct from
allocated or active-and-not-empty. Rename vector_max to
vector_active for same reason.
|
|
|
|
|
|
* global: Replace strerror with safe_strerror. And vtysh/vtysh.c
needs to include "log.h" to pick up the declaration.
|
|
* ospf6_route.c: Add const qualifier to various char arrays of
constants. signed/unsigned fixes.
(ospf6_linkstate_table_show) argv is const
* ospf6_snmp.c: listnode typedef is dead.
(ospf6_snmp_init) Take struct thread_master arg, needed for
smux_init.
* ospf6_snmp.h: update ospf6_snmp_init declaration.
* ospf6d.c: (ospf6_init) add const qualifier to sargv, pass master
to ospf_snmp6_init.
* ospf6_asbr.c: const char update.
* ospf6_interface.c: ditto, plus signed/unsigned fixes.
(ipv6_ospf6_cost) Check whether cost fits in u_int32_t and use
strtoul.
* ospf6_intra.c: const char update. Parenthesise expression.
* ospf6_lsa.c: signed/unsigned and const char updates.
* ospf6_proto.c: ditto.
* ospf6_message.c: ditto.
* ospf6_lsdb.c: signed/unsigned update.
* ospf6_main.c: const char update.
* ospf6_neighbor.c: ditto.
* ospf6_spf.c: ditto.
* ospf6_top.c: ditto.
|
|
to ospf6_network.h.
|
|
|
|
|
|
ospf6_lsdb prototype declaration from ospf6_lsa.h. Report sent to Yasuhiro
Ohara as well.
|
|
|
|
|
|
fixes.
|
|
zclient changes.
|
|
* (many) reference <lib/version.h> rather than "version.h",
because version.h is a generated file and not present in the
source tree when using objdir builds.
(committed by gdt)
works fine with normal builds; didn't try objdir
|
|
|
