buglwIP - A Lightweight TCP/IP stack - Bugs: bug #38803, Source address in broadcast ping...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #38803: Source address in broadcast ping reply

Submitter:  Jens Nielsen <deft>
Submitted:  Mon 22 Apr 2013 01:04:25 PM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.4.1

Jump to the original submission

Sun 22 Feb 2015 09:07:29 PM UTC, comment #6: 

Re comment #1:

> The patch seems wrong, unfortunately, because in the presense
> of multiple interfaces it will use incorrect specific-
> destination address for packets received on one interface with
> destination being a broadcast address of another.


Isn't that rather esotherical given that the reply is sent on 'inp' as well? If not, this would be worth a 2nd bug to me.

(BTW: This would be a reason for passing up the accepting netif, not inp.)

Fixed-for-now as suggested in the original patch.

Simon Goldschmidt <goldsimon>
Group administrator
Sun 27 Oct 2013 12:12:57 PM UTC, comment #5: 

On the other hand, I did some more reading in RFC 1122. This is from 3.2.1.3, which defines the "specific-destination address" mentioned in the section I quoted in my original post.

"The specific-destination address is defined to be the destination address in the IP header unless the header contains a broadcast or multicast address, in which case the specific-destination is an IP address assigned to the physical interface on which the datagram arrived."

The behaviour seems odd, but the requirements are clear as far as I can tell.. In the case of broadcast ping we should use the address from "inp" regardless of which interface the destination address matched. Direct ping should still use the destination address from the request.

The ip_addr_isbroadcast() check is still done on the wrong interface though.

Jens Nielsen <deft>
Fri 25 Oct 2013 05:49:21 PM UTC, comment #4: 

Drifting off-topic and out of my comfort zone now, but isn't this a problem with pretty much every use of ip_addr_isbroadcast (and a couple of other operations as well) then? They all compare with "inp" rather than with the matched netif, so both interfaces should go to most input functions in that case.

Jens Nielsen <deft>
Fri 25 Oct 2013 12:01:38 PM UTC, comment #3: 

Yes, looks like it.  Perhaps both incoming interface and "accepting" interface should be passed as arguments.

Valery Ushakov <uwe>
Fri 25 Oct 2013 11:38:50 AM UTC, comment #2: 

Sounds reasonable, but in that case there is also a problem with this piece of code in icmp_input()

#if !LWIP_BROADCAST_PING
      /* broadcast destination address? */
      if (ip_addr_isbroadcast(&current_iphdr_dest, inp)) {
        accepted = 0;
      }
#endif /* LWIP_BROADCAST_PING */

Meaning we will accept a broadcast ping even if LWIP_BROADCAST_PING is 0, if the ping is received on another netif.

Jens Nielsen <deft>
Thu 24 Oct 2013 05:02:15 PM UTC, comment #1: 

The patch seems wrong, unfortunately, because in the presense of multiple interfaces it will use incorrect specific-destination address for packets received on one interface with destination being a broadcast address of another.

ip_input() has "inp" variable for the interface the packet was received on and "netif" variable for the destination interface.  icmp_input() is passed "inp", but the packet might have been recognized as "for us" using broacast address of another interface ("netif").  Echo reply should have "netif" address as its source, not that of "inp", but "netif" is not passed.

Valery Ushakov <uwe>
Mon 22 Apr 2013 01:04:25 PM UTC, original submission:  

I enabled broadcast ping (#define LWIP_BROADCAST_PING 1) and noticed lwip replies to a broadcast ping with the broadcast address as source address. In icmp.c the source and destination addresses are simply switched when building the echo response.

I suppose the attached diff would do the trick (using the netif address as source address instead)


(ref RFC 1122 section 3.2.2.6 and RFC 1812 section 4.3.3.6, "the source address in an ICMP Echo Reply MUST be the same as the specific-destination address of the corresponding ICMP Echo Request message". I.e. the Echo Request destination address if unicast, or the network interface address in case of multicast or broadcast)

Jens Nielsen <deft>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #27924:  broadcast_ping.diff added by deft (577B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by uwe (Posted a comment)
  • -email is unavailable- added by deft (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-22 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2013-04-22 deft Attached File- Added broadcast_ping.diff, #27924

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code