views:

103

answers:

6

I'm trying to build a simple echo server/client that works on ethernet level(using raw sockets). The server side by itself works and shows all incoming packets on eth0. The client works and sends ethernet packets on eth0 (I checked this with wireshark and can see the packets going out.) I now want to make a filter to only look at the packets I'm interested. (This is based on the destination/source addresses.)

In the code below, could someone please explain to me why strncmp returns zero(meaning the strings match) but yet the "if(ethernet_header->h_dest == mac)" fails to execute (don't match). Both the variables "mac" and "ethernet_header->h_dest" is the same type and length.

Some more background: - This is done on linux 64bit (ubuntu) - I'm using eth0 on the same machine for sending/receiving....I don't think this should be a problem?

I just don't understand why strcmp returns a match and if doesn't. What am I missing??

void ParseEthernetHeader(unsigned char *packet, int len) {
    struct ethhdr *ethernet_header;
 unsigned char mac[ETH_ALEN] = {0x01, 0x55, 0x56, 0x88, 0x32, 0x7c}; 

 if (len > sizeof(struct ethhdr)) {
  ethernet_header = (struct ethhdr *) packet;

  int result = strncmp(ethernet_header->h_dest, mac, ETH_ALEN);
  printf("Result: %d\n", result);

  if(ethernet_header->h_dest == mac) {
   /* First set of 6 bytes are Destination MAC */
   PrintInHex("Destination MAC: ", ethernet_header->h_dest, 6);
   printf("\n");

   /* Second set of 6 bytes are Source MAC */
   PrintInHex("Source MAC: ", ethernet_header->h_source, 6);
   printf("\n");

   /* Last 2 bytes in the Ethernet header are the protocol it carries */
   PrintInHex("Protocol: ", (void *) &ethernet_header->h_proto, 2);
   printf("\n\n");
   printf("Length: %d\n",len);
  }

 } else {
  printf("Packet size too small (length: %d)!\n",len);
 }

}
A: 

What is this code?

if(ethernet_header->h_dest == mac)

Does not do a string compare in C, just a pointer comparision which always be false in your case.

leppie
A: 

The if(ethernet_header->h_dest == mac) just compares the raw pointer values. That means it checks, if both strings start at the same memory address. Usually, that's not what you want.

To compare the content of two c-strings, always use strncmp().

cypheon
+1  A: 

You can't test for string equality using the == operator. That's why the strcmp() functions exist in the first place.

anon
+8  A: 

Neither strncmp nor a naked if should be used for comparing MAC addresses.

The first won't work properly in cases where they may have an embedded zero byte which would cause strncmp to state they're equal when they're actually not. That's because a strncmp of the following two values:

ff ff 00 ff ff ff
ff ff 00 aa aa aa

would be true (it only checks up to the first zero byte).

The second won't work because you're comparing pointers rather than the contents that the pointers point to. If you have the following memory layout:

0x12345678 (mac) | 0x11111111 |
0x1234567c (eth) | 0x11111111 |

then comparing mac with eth with if (mac == eth) will give you false since they are distinct pointers, one ending in 78, the other in 7c.

You should use memcmp instead since it will compare the raw memory bytes without stopping at an early zero byte:

int result = memcmp (ethernet_header->h_dest, mac, ETH_ALEN);
paxdiablo
Note that this kind of bug (using `strcmp()` where `memcmp()` was warranted) has been a security issue in the past, eg. when comparing hash values.
caf
+1  A: 

strncmp takes pointers to char as its first two arguments.

strncmp returns zero because the strings at those two locations are same for ETH_ALEN characters - that doesn't mean that ethernet_header->h_dest and mac are equal. They are two different pointers.

int main()
{
        char a1[] = "asdf";
        char a2[] = "asdf";
        char *p1 = "asdf";
        char *p2 = "asdf";
        char *s1 = malloc(5);
        char *s2 = malloc(5);
        strcpy(s1, "asdf");
        strcpy(s2, "asdf");
        printf("a1 and a2: strcmp gives %d and they are %s\n", strcmp(a1, a2), a1 == a2 ? "equal" : "different");
        printf("p1 and p2: strcmp gives %d and they are %s\n", strcmp(p1, p2), p1 == p2 ? "equal" : "different");
        printf("s1 and s2: strcmp gives %d and they are %s\n", strcmp(s1, s2), s1 == s2 ? "equal" : "different");
        return 0;
}

The output:

a1 and a2: strcmp gives 0 and they are different  
p1 and p2: strcmp gives 0 and they are equal  
s1 and s2: strcmp gives 0 and they are different
  • p1 and p2 are equal because they both are pointing to the same const string in the memory.
  • In case of arrays, a continuous block of 5 bytes is allocated to each array variables (in the stack) and the string asdf\0 is copied into those locations.
  • s1 and s2 are two different pointers that point to two different blocks of 5 byte sequences in the heap that happens to contain same value.
Amarghosh
A: 

In C, == does not work on strings as you think. You must use strncmp() instead.

Just change

 if(ethernet_header->h_dest == mac) {

to

if(result == 0) {
vtorhonen