ospfd: check dst addr for hello packets

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

ospfd: check dst addr for hello packets

Remi Locherer
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 -0000 1.32
+++ packet.c 11 Aug 2019 09:17:51 -0000
@@ -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, &addr);
- if (ip_hdr.ip_dst.s_addr == addr.s_addr) {
- log_debug("recv_packet: invalid destination IP "
-     "address");
- break;
- }
+ inet_aton(AllSPFRouters, &addr);
+ 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;

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: check dst addr for hello packets

Sebastian Benoit-3

reads ok

Remi Locherer([hidden email]) 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 -0000 1.32
> +++ packet.c 11 Aug 2019 09:17:51 -0000
> @@ -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, &addr);
> - if (ip_hdr.ip_dst.s_addr == addr.s_addr) {
> - log_debug("recv_packet: invalid destination IP "
> -     "address");
> - break;
> - }
> + inet_aton(AllSPFRouters, &addr);
> + 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;
>