midauth extensions are broken

This might be related: https://bugs.launchpad.net/hipl/+bug/918374

Does the attached patch to hipfw/rewrite.c happen to solve or improve the issue? It seems like the ipq compatibility struct declares "payload" as a pointer, while "payload" was in fact a zero-length array in the original struct. This requires some additional pointer bending.

Has midauth really been tested since merging the nf_queue branch in revision 6420, beyond running the unit tests? Does reverting to 6419 help? I can't test it myself at the moment, but perhaps later this week.

Regarding your temporary solution, I don't recommend leaving the "assume_ipq_buffer_sufficient" flag on permanently at the moment for two reasons:

1. We only have at most HIP_MAX_DATA - x bytes of payload available, where "x" denotes the size of the nf_queue per-packet metadata, but the "splice_param" function currently assumes at least HIP_MAX_DATA bytes. This may be easy to fix by inferring x via pointer arithmetic and passing down the actual remaining space to splice_param via the context structure.

2. It's not clear to me whether or not recv() on the nf_queue socket can in read multiple packets or not. The NFQ docs imply that only a single packet get copied at once, but reading the source code of libnfnl suggests otherwise. Indeed, the callback-driven interface makes it effortless (to the library user) to process multiple packets at once, so why wouldn't they make use of this feature? In any case, multiple packets are a problem because enlarging e.g. the first packet would overwrite the beginning of the second one, if they share the same buffer. So moving memory in one form or another is necessary unless we are manipulating the very last packet in the buffer and the remaining space suffices.

I'd keep the flag around for debug and benchmarking purposes though (e.g. for measuring the impact of copying on effective throughput). Perhaps the purpose of this flag should be more clearly documented?

A compilation issue... scratch_buffer.ipq is not a pointer:

  CC hipfw/rewrite.o
hipfw/rewrite.c: In function ‘hip_fw_context_enable_write’:
hipfw/rewrite.c:150:28: error: incompatible types when assigning to type ‘hip_ipq_packet_msg’ from type ‘struct hip_ipq_packet_msg **’
         scratch_buffer.ipq = &ctx->ipq_packet;

If I fix the compilation problem by making the ipq member a pointer in scratch_buffer, it fails as follows:

debug(hipfw/hipfw.c:1980@hipfw_main): received IPv4 packet from iptables queue
debug(hipfw/hipfw.c:1632@fw_handle_packet): Entering netfilter callback for IPv4
debug(hipfw/hipfw.c:1396@fw_init_context): ip_hdr_len is: 20
debug(hipfw/hipfw.c:1397@fw_init_context): total length: 72
debug(hipfw/hipfw.c:1398@fw_init_context): ttl: 64
debug(hipfw/hipfw.c:1399@fw_init_context): packet length (ipq): 72
debug(hipfw/hipfw.c:1405@fw_init_context): packet src: 172.17.0.2
debug(hipfw/hipfw.c:1406@fw_init_context): packet dst: 172.17.0.1
debug(hipfw/hipfw.c:1408@fw_init_context): IPv4 next header protocol number is 17
debug(hipfw/hipfw.c:1510@fw_init_context): UDP header size is 8 (in header: 52)
debug(hipfw/hipfw.c:1511@fw_init_context): UDP src port: 10500
debug(hipfw/hipfw.c:1512@fw_init_context): UDP dst port: 10500
debug(hipfw/hipfw.c:1525@fw_init_context): zero_bytes: 0x00000000
debug(hipfw/hipfw.c:1531@fw_init_context): Zero SPI found
debug(hipfw/hipfw.c:1554@fw_init_context): UDP encapsulated HIP control packet
debug(hipfw/hipfw.c:1640@fw_handle_packet): packet hook=1, packet type=1
debug(hipfw/hipfw.c:902@filter_hip):
debug(hipfw/hipfw.c:907@filter_hip): The list of rules is empty!!!???
debug(hipfw/hipfw.c:910@filter_hip): HIP type number is 1
info(hipfw/hipfw.c:914@filter_hip): received packet type: I1
info(hipfw/hipfw.c:952@filter_hip): src hit: 2001:0017:03b4:b5cc:bad2:26e7:0eb2:8198
info(hipfw/hipfw.c:953@filter_hip): dst hit: 2001:001b:b6ae:fca7:3d97:0ff1:e489:5f83
info(hipfw/hipfw.c:954@filter_hip): src ip: 172.17.0.2
info(hipfw/hipfw.c:955@filter_hip): dst ip: 172.17.0.1
debug(hipfw/hipfw.c:1060@filter_hip): falling back to default HIP/ESP behavior, target 1
debug(hipfw/conntrack.c:2065@get_tuple_by_hits): get_tuple_by_hits: no connection found
debug(hipfw/conntrack.c:294@get_tuple_by_hip): get_tuple_by_hip: no connection found
debug(hipfw/conntrack.c:1736@check_packet): check packet: type 1
debug(hipfw/dlist.c:137@append_to_list): List is empty inserting first node
debug(hipfw/dlist.c:133@append_to_list): List is not empty. Length 1
debug(hipfw/conntrack.c:2059@get_tuple_by_hits): connection found,
debug(hipfw/conntrack.c:1791@check_packet): udp_encap_hdr=0x7fff63f2bb7c tuple=(nil) err=1
debug(hipfw/hipfw.c:1653@fw_handle_packet): === Verdict: allow packet ===
debug(hipfw/hipfw.c:1600@allow_packet): Packet accepted

debug(hipfw/hipfw.c:1980@hipfw_main): received IPv4 packet from iptables queue
debug(hipfw/hipfw.c:1632@fw_handle_packet): Entering netfilter callback for IPv4
debug(hipfw/hipfw.c:1396@fw_init_context): ip_hdr_len is: 20
debug(hipfw/hipfw.c:1397@fw_init_context): total length: 672
debug(hipfw/hipfw.c:1398@fw_init_context): ttl: 64
debug(hipfw/hipfw.c:1399@fw_init_context): packet length (ipq): 672
debug(hipfw/hipfw.c:1405@fw_init_context): packet src: 172.17.0.1
debug(hipfw/hipfw.c:1406@fw_init_context): packet dst: 172.17.0.2
debug(hipfw/hipfw.c:1408@fw_init_context): IPv4 next header protocol number is 17
debug(hipfw/hipfw.c:1510@fw_init_context): UDP header size is 8 (in header: 652)
debug(hipfw/hipfw.c:1511@fw_in...

This patch compiles but fails as mentioned in the bug report.

You're right; changing this line to the following seems to compile at least (still untested):

scratch_buffer.ipq = *(ctx->ipq_packet);

Copying the ipq struct may not even be necessary at all: perhaps it suffices to reset the payload pointer and overwrite the old struct, since it seems to get reconstructed for every packet anyway.

Christoph, can you commit your code with this the previous modification you suggested? It actually works. Then you can close the bug. Thanks!

Ah, good to hear that. I don't feel comfortable commiting directly since I can't test it yet (have to do the hipd/hipfw tutorial again first...) but I'll create a branch so you can test and merge it yourself.