reads ok
Remi Locherer(remi.loche...@relo.ch) on 2019.08.11 11:21:36 +0200:
> When ospfd receives a hello packet it takes the src IP address and updates
> the address in its neighbor struct for the given router id unconditionally.
>
> In the case of broadcast interfaces this is not a problem:
> find_iface() checks that the src address is from the same subnet as
> the receiving interface is. It is best practice to only enable ospf in a
> subnet where you control all routers.
>
> But in the case of point-to-point interfaces no sanity checks happen on the
> src or dst IP address.
>
> RFC 2328 says in "9.5. Sending Hello packets":
> On broadcast networks and physical point-to-point networks,
> Hello packets are sent every HelloInterval seconds to the IP
> multicast address AllSPFRouters.
>
>
> I verified that ospfd does it like that. Also FastIron and Junos follow
> this.
>
> I propose that we add a check and only accept hellos on point-to-point and
> broadcast interfaces when the destination is 224.0.0.5 (AllSPFRouters).
>
> The check for AllDRouters is not needed in addition to the proposed check.
>
> OK?
>
> Remi
>
>
> Index: packet.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/packet.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 packet.c
> --- packet.c 15 Jul 2019 18:26:39 - 1.32
> +++ packet.c 11 Aug 2019 09:17:51 -
> @@ -219,12 +219,16 @@ recv_packet(int fd, short event, void *b
> /* switch OSPF packet type */
> switch (ospf_hdr->type) {
> case PACKET_TYPE_HELLO:
> - inet_aton(AllDRouters, );
> - if (ip_hdr.ip_dst.s_addr == addr.s_addr) {
> - log_debug("recv_packet: invalid destination IP "
> - "address");
> - break;
> - }
> + inet_aton(AllSPFRouters, );
> + if (iface->type == IF_TYPE_BROADCAST ||
> + iface->type == IF_TYPE_POINTOPOINT)
> + if (ip_hdr.ip_dst.s_addr != addr.s_addr) {
> + log_warnx("%s: hello ignored on interface %s, "
> + "invalid destination IP address %s",
> + __func__, iface->name,
> + inet_ntoa(ip_hdr.ip_dst));
> + break;
> + }
>
> recv_hello(iface, ip_hdr.ip_src, ospf_hdr->rtr_id, buf, len);
> break;
>
