[prev in list] [next in list] [prev in thread] [next in thread] 

List:       nmap-dev
Subject:    Re: [PATCH] nping: Fix "Next-Hop MTU" in icmp "Fragmentation required" response
From:       Daniel Miller <bonsaiviking () gmail ! com>
Date:       2016-02-12 5:44:42
Message-ID: CABmvJnMuqYf0Pxcy6Qe_ZdQAY8Tu=w-tTOrcrOQCWnkPvi7qNw () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Sławomir,

Thanks for catching this! I was able to confirm the issue and your fix, and
I committed a simplification in r35623. I chose to directly extract and add
the byte values instead of the existing complication with pointers and
ntohs, but your offset calculation was correct.

Dan

On Tue, Feb 2, 2016 at 10:08 AM, Sławomir Demeszko <
s.demeszko@wireless-instruments.com> wrote:

> Hi.
>
> When I execute a command to check MTU, for example:
>         nping --icmp -c 1 --df --data-length=1400   8.8.8.8
> where one router on the path has set MTU=1300 I get response:
>         RCVD (0.2126s) ICMP [*.*.*.* > *.*.*.* Fragmentation required
> (type=3/code=4) Next-Hop-MTU=1428] IP [ttl=64 id=31483 iplen=576 ]
>
> The "Next-Hop-MTU=1428" is invalid here. It is always 28 bytes greater
> than length of send data and not router MTU.
> I checked in Wireshark and it shows proper value in response packet. MTU
> value is 6 and 7 byte in ICMP header,
> but icmppkt->data is already 4 bytes offset. Following patch resolves
> this. It applies to version 7.01.
>
>
> Signed-off-by: Sławomir Demeszko <s.demeszko@wireless-instruments.com>
> ---
>  libnetutil/netutil.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libnetutil/netutil.cc b/libnetutil/netutil.cc
> index 72d9eb2..b9757c9 100644
> --- a/libnetutil/netutil.cc
> +++ b/libnetutil/netutil.cc
> @@ -2821,7 +2821,7 @@ const char *ippackethdrinfo(const u8 *packet, u32
> len, int detail) {
>
>            case 4:
>              strcpy(icmptype, "Fragmentation required");
> -            nextmtu = (u16 *)(&(icmppkt->data[6]));
> +            nextmtu = (u16 *)(&(icmppkt->data[2]));
>              Snprintf(icmpfields, sizeof(icmpfields), "Next-Hop-MTU=%hu",
> (unsigned short) ntohs(*nextmtu));
>              break;
>
> --
> 2.5.0
>
> _______________________________________________
> Sent through the dev mailing list
> https://nmap.org/mailman/listinfo/dev
> Archived at http://seclists.org/nmap-dev/

[Attachment #5 (text/html)]

<div dir="ltr"><div><div>Sławomir,<br><br></div>Thanks for catching this! I was able \
to confirm the issue and your fix, and I committed a simplification in r35623. I \
chose to directly extract and add the byte values instead of the existing \
complication with pointers and ntohs, but your offset calculation was \
correct.<br><br></div>Dan<br></div><div class="gmail_extra"><br><div \
class="gmail_quote">On Tue, Feb 2, 2016 at 10:08 AM, Sławomir Demeszko <span \
dir="ltr">&lt;<a href="mailto:s.demeszko@wireless-instruments.com" \
target="_blank">s.demeszko@wireless-instruments.com</a>&gt;</span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">Hi.<br> <br>
When I execute a command to check MTU, for example:<br>
            nping --icmp -c 1 --df --data-length=1400     8.8.8.8<br>
where one router on the path has set MTU=1300 I get response:<br>
            RCVD (0.2126s) ICMP [*.*.*.* &gt; *.*.*.* Fragmentation required \
(type=3/code=4) Next-Hop-MTU=1428] IP [ttl=64 id=31483 iplen=576 ]<br> <br>
The &quot;Next-Hop-MTU=1428&quot; is invalid here. It is always 28 bytes greater than \
length of send data and not router MTU.<br> I checked in Wireshark and it shows \
proper value in response packet. MTU value is 6 and 7 byte in ICMP header,<br> but \
icmppkt-&gt;data is already 4 bytes offset. Following patch resolves this. It applies \
to version 7.01.<br> <br>
<br>
Signed-off-by: Sławomir Demeszko &lt;<a \
href="mailto:s.demeszko@wireless-instruments.com">s.demeszko@wireless-instruments.com</a>&gt;<br>
                
---<br>
  libnetutil/netutil.cc | 2 +-<br>
  1 file changed, 1 insertion(+), 1 deletion(-)<br>
<br>
diff --git a/libnetutil/netutil.cc b/libnetutil/netutil.cc<br>
index 72d9eb2..b9757c9 100644<br>
--- a/libnetutil/netutil.cc<br>
+++ b/libnetutil/netutil.cc<br>
@@ -2821,7 +2821,7 @@ const char *ippackethdrinfo(const u8 *packet, u32 len, int \
detail) {<br> <br>
                 case 4:<br>
                    strcpy(icmptype, &quot;Fragmentation required&quot;);<br>
-                  nextmtu = (u16 *)(&amp;(icmppkt-&gt;data[6]));<br>
+                  nextmtu = (u16 *)(&amp;(icmppkt-&gt;data[2]));<br>
                    Snprintf(icmpfields, sizeof(icmpfields), \
&quot;Next-Hop-MTU=%hu&quot;, (unsigned short) ntohs(*nextmtu));<br>  break;<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
2.5.0<br>
<br>
_______________________________________________<br>
Sent through the dev mailing list<br>
<a href="https://nmap.org/mailman/listinfo/dev" rel="noreferrer" \
target="_blank">https://nmap.org/mailman/listinfo/dev</a><br> Archived at <a \
href="http://seclists.org/nmap-dev/" rel="noreferrer" \
target="_blank">http://seclists.org/nmap-dev/</a></font></span></blockquote></div><br></div>




_______________________________________________
Sent through the dev mailing list
https://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic