ThomasHabets / arping

ARP Ping
http://www.habets.pp.se/synscan/programs.php
GNU General Public License v2.0
403 stars 63 forks source link

-t not changing Target MAC Address #44

Closed NatasFX closed 2 years ago

NatasFX commented 2 years ago

So, i've tried creating an arp reply to 192.168.0.105 using this command:

arping -P -p -s 12:34:56:78:9a:bc -S 192.168.0.100 -t ba:98:76:54:32:10 192.168.0.105

Basically, i was trying to tell 192.168.0.105 which mac address belongs to 192.168.0.100. -t was supposted to change Target MAC Address of the ARP protocol, but only changed the Ethernet layer destination mac address. It should be the same on both fields.

Because of this, the Target MAC Address was 00:00:00:00:00:00 so 192.168.0.105 refused the arp reply. Is this expected behavior or i've found a bug? Thanks in advance.

ThomasHabets commented 2 years ago

The generated packet shows up as:

11:51:10.506344 12:34:56:78:9a:bc > ba:98:76:54:32:10, ethertype ARP (0x0806), length 58: Reply 192.168.0.100 is-at 12:34:56:78:9a:bc, length 44

It's been a while since Iooked at the spec, but essentially seems to mean "me".

So it's not so much a bug as a missing feature. "Target" here does intend to mean, and does mean, where to send the ethernet frame. A different address in the ARP reply target field needs a different option.

Did you try -U?

But yes, it's not possible to set that ARP field to something specific at the moment. Maybe it should be, with another option.

NatasFX commented 2 years ago

I've read your code and changed this line: (line 1128 of arping.c)

unsolicited ? (uint8_t*)ethxmas : (uint8_t*)ethnull,

to

unsolicited ? (uint8_t*)ethxmas : send_reply ? dstmac : (uint8_t*)ethnull,

So that when replying, set the target address the same as the destination address; but when not replying (requesting) just let that field be nulls.

image

The image shows the altered code working correctly to set the Target MAC Address, inspected with Wireshark. (You can use wireshark using the original version to see the Target MAC Address is not set using -t, only the Ethernet destination mac)

-U will set all bits on Target MAC Address (Broadcast).

May i open a pull request for this?

ThomasHabets commented 2 years ago

Did you try if -U was sufficient? Are you saying it works with your patch, but not with -U?

If that's the case, then yeah a PR sounds good. Place some parenthesis for ease of reading, though.

NatasFX commented 2 years ago

Yes, i works with my patch but not with -U. As i'm replying, it can't be unsolicited because a machine made the request, and i'm replying so i shouldn't be using -U anyways.

Placing -U in the command will set the Target MAC Address as FF:FF:FF:FF:FF:FF, when it should be ba:98...

The code i modified does a simple thing: When -U, Target MAC Address is broadcast. If replying, the Target MAC Address will be the MAC address of the target machine (I've never seen an ARP Reply with Target MAC Address null, so it should be this way). if none then it is a normal request so the field needs to be blank.

If you agree i'll proceed to make a pull request for this. (happy to contribute!)

ThomasHabets commented 2 years ago

Yeah, makes sense. I'm only being careful in case someone, somewhere, is relying on it. But sending replies is rare with arping, so Should Be Fine™.

Just the parenthesis thing, and all good.

NatasFX commented 2 years ago

Sounds right. Prolly they don't exist or if they exist, the machine being replied to doesn't check the if the Target MAC Address matches their machine (which is improbable).

About the parenthesis, is this alright to you? Couldn't really figure out exactly how you wanted. unsolicited ? (uint8_t*)ethxmas : (send_reply ? dstmac : (uint8_t*)ethnull),

ThomasHabets commented 2 years ago

Yeah, like that. No need for reader to have to think about operator precedence and associativity.

(PHP's ternary operator is incorrectly associative. Fun fact)

NatasFX commented 2 years ago

Closed this issue by PR #46. Thanks!