Add support for IP_RECVERR #1161

mcpherrinm · 2019-12-02T01:49:12Z

This pull request adds support for setting the IP_RECVERR sockopt, and then decoding the resulting messages from recvmsg. My intent is to use this to implement a traceroute program (And so need ICMP TTL exceeded messages), though IP_RECVERR is useful in general when writing UDP network programs. It is, unfortunately, a linux-specific API.

There's no tests here, but I've been trying it out with this bit of code:
mcpherrinm/traceroute@8a92b6d

SockExtendedErr is the struct type returned as a Cmsg from recvmsg. It (like the C linux/errqueue.h which defines it) uses u8 and u32. I think we'll want something more user-friendly here, so I'd say this PR is still a work-in-progress.

I am new to the nix codebase (and am not very experienced with Rust), so would appreciate early feedback.

Setting these options enables receiving errors, such as ICMP errors from the network, via recvmsg with MSG_ERRQUEUE

asomers

It's a good start, but it needs tests, a CHANGELOG entry, and a usage example.

And once we start a review in Nix, please don't force-push until the end when you're ready to squash.

asomers · 2019-12-02T02:26:57Z

src/sys/socket/mod.rs

+#[cfg(any(target_os = "android", target_os = "linux"))]
+#[repr(C)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub struct SockExtendedErr {


Is this struct defined anywhere in the C headers? If so, you should add its definition to the libc crate, and reference that here. Nix shouldn't be in the business of repeating the headers.

It's defined in https://elixir.bootlin.com/linux/v5.4.1/source/include/uapi/linux/errqueue.h

I read the libc crate's RFC https://github.com/rust-lang/rfcs/blob/master/text/1291-promote-libc.md and it wasn't clear to me if this struct belonged there or not.

I am happy to open a PR there first and define it there.

Yes, please submit a PR to libc.

mcpherrinm · 2019-12-02T02:49:32Z

Yes, won't force-push anymore. Just wanted to quickly fix a few cfg errors that caused the freebsd CI to fail.

mcpherrinm · 2019-12-02T03:52:48Z

I have opened a PR against libc @ rust-lang/libc#1613

I will revise this pull request to use the struct definition from there once reviewed and merged.

Thank you for your time so far.

This uses a patched libc and should not be merged until libc merges.

asomers · 2019-12-15T17:24:08Z

Looks like this PR is still waiting on rust-lang/libc#1616

WGH- · 2020-02-10T11:05:32Z

src/sys/socket/mod.rs

+                ControlMessageOwned::Ipv4RecvErr(err, addr)
+            },
+            #[cfg(any(target_os = "android", target_os = "linux"))]
+            (libc::IPPROTO_IPV6, libc::IP_RECVERR) => {


It should be IPV6_RECVERR (tested that on Linux).

WGH-

I think this also needs some proper way to calculate space for cmsg.

I looked through some C examples, and they seem to just allocate excessive buffer space instead of calculating properly. I was unable to find a "proper" example yet.

Should it be CMSG_SPACE(sizeof(struct sock_extended_err) + max(sizeof(struct sockaddr_in), sizeof(struct sockaddr_in6)))?

WGH- · 2020-02-10T17:15:10Z

src/sys/socket/mod.rs

+            (libc::IPPROTO_IPV6, libc::IP_RECVERR) => {
+                let ee = p as *const libc::sock_extended_err;
+                let err = ptr::read_unaligned(ee);
+                let addr = ptr::read_unaligned(libc::SO_EE_OFFENDER(ee) as *const sockaddr_in6);


man 2 recvmsg

For local errors, no address is passed (this can be checked with the cmsg_len member of the cmsghdr).

Given that, I think addr should become an Option, and len should be handled appropriately.

WGH- · 2020-02-13T15:28:38Z

Anyway, I tested the code with my toy TCP SYN scanner, and it works fine (except that IPV6_RECVERR thing that I fixed myself).

WGH- · 2020-03-01T21:00:24Z

libc-0.2.67 has been released recently, which includes the missing bits, so it's no longer necessary to use a patch.

iff --git a/Cargo.toml b/Cargo.toml
index 29f0fca..8fb953e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -16,14 +16,11 @@ exclude     = [
 ]
 
 [dependencies]
-libc = { version = "0.2.60", features = [ "extra_traits" ] }
+libc = { version = "0.2.67", features = [ "extra_traits" ] }
 bitflags = "1.0"
 cfg-if = "0.1.2"
 void = "1.0.2"
 
-[patch.crates-io]
-libc = { git = "https://github.com/mcpherrinm/libc" }
-
 [target.'cfg(target_os = "dragonfly")'.build-dependencies]
 cc = "1"

mcpherrinm · 2020-03-04T05:27:46Z

Thanks @WGH-. I'll update this PR soon.

asomers · 2020-03-08T18:46:28Z

Looks like you need to raise the minimum version of libc

Removes patched version and use upstream released version

asomers

This PR still needs documentation, a usage example, tests (those two can be the same), and a CHANGELOG entry. And it will need to be rebased before merging.

mcpherrinm · 2020-04-22T06:59:27Z

Thanks. Sorry this is taking so long, but I have little free time for side project programming right now.

Make the address field of the RecvErr tuples optional, as local-originating errors do not have an address attached to the control message. Use the associated cmsg_len to determine if the address is present when copying the raw cmsg into a ControlMessageOwned.

1514: Add support for IP_RECVERR r=asomers a=cemeyer This pull request adds support for setting the `IP_RECVERR` (and corresponding `IPV6_RECVERR`) sockopt, and then decoding the resulting control messages from `recvmsg()`. It is a Linux-specific API. This PR extends #1161 by: * Making the address associated with a `IpvXRecvErr` control message optional (`recvmsg` documentation claims it is not provided for some kinds of error), per `@WGH-.` * Adding basic tests for both IPv4 and IPv6 (blat a UDP packet at a hopefully-unoccupied port on localhost and observe ICMP port unreachable). * Adding a Changelog entry. I added some trivial doc comments on the `ControlMessageOwned` variants, but I'm not sure exactly what documentation you had in mind earlier on #1161. Co-authored-by: Conrad Meyer <cem@FreeBSD.org>

cemeyer · 2021-09-07T00:11:10Z

We can probably close this one now. Thanks!

asomers · 2021-09-07T00:31:32Z

Yep.

mcpherrinm added 2 commits December 1, 2019 17:03

Add setsockopt support for IP_RECVERR and IPV6_RECVERR

8fcd8b6

Setting these options enables receiving errors, such as ICMP errors from the network, via recvmsg with MSG_ERRQUEUE

Add Ipv{4,6}RecvErr to ControlMessageOwned

f3d7803

asomers requested changes Dec 2, 2019

View reviewed changes

fixup: use libc defined struct

e599890

This uses a patched libc and should not be merged until libc merges.

WGH- reviewed Feb 10, 2020

View reviewed changes

mcpherrinm added 3 commits April 4, 2020 14:37

Upgrade libc to 0.2.68

46e5c7e

Removes patched version and use upstream released version

Fix mistake: IPPROTO_IPV6 goes with IPV6_RECVERR

f93b2dd

Merge remote-tracking branch 'upstream/master' into add-ip-recverr

18ade43

asomers requested changes Apr 19, 2020

View reviewed changes

cemeyer mentioned this pull request Sep 6, 2021

Add support for TTL and RECV_ERR control messages #1428

Open

cemeyer mentioned this pull request Sep 6, 2021

Add support for IP_RECVERR #1514

Merged

asomers closed this Sep 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for IP_RECVERR #1161

Add support for IP_RECVERR #1161

mcpherrinm commented Dec 2, 2019

asomers left a comment

asomers Dec 2, 2019

mcpherrinm Dec 2, 2019

asomers Dec 2, 2019

mcpherrinm commented Dec 2, 2019

mcpherrinm commented Dec 2, 2019

asomers commented Dec 15, 2019

WGH- Feb 10, 2020

WGH- left a comment

WGH- Feb 10, 2020

WGH- commented Feb 13, 2020

WGH- commented Mar 1, 2020

mcpherrinm commented Mar 4, 2020

asomers commented Mar 8, 2020

asomers left a comment

mcpherrinm commented Apr 22, 2020

cemeyer commented Sep 7, 2021

asomers commented Sep 7, 2021

Add support for IP_RECVERR #1161

Add support for IP_RECVERR #1161

Conversation

mcpherrinm commented Dec 2, 2019

asomers left a comment

Choose a reason for hiding this comment

asomers Dec 2, 2019

Choose a reason for hiding this comment

mcpherrinm Dec 2, 2019

Choose a reason for hiding this comment

asomers Dec 2, 2019

Choose a reason for hiding this comment

mcpherrinm commented Dec 2, 2019

mcpherrinm commented Dec 2, 2019

asomers commented Dec 15, 2019

WGH- Feb 10, 2020

Choose a reason for hiding this comment

WGH- left a comment

Choose a reason for hiding this comment

WGH- Feb 10, 2020

Choose a reason for hiding this comment

WGH- commented Feb 13, 2020

WGH- commented Mar 1, 2020

mcpherrinm commented Mar 4, 2020

asomers commented Mar 8, 2020

asomers left a comment

Choose a reason for hiding this comment

mcpherrinm commented Apr 22, 2020

cemeyer commented Sep 7, 2021

asomers commented Sep 7, 2021