Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel_xfrm: record extended ack from netlink response #1716

Closed
wants to merge 1 commit into from

Conversation

ueno
Copy link
Contributor

@ueno ueno commented May 22, 2024

This enables pluto to log any error message reported through extended ACK attributes[1] in a netlink response, to make diagnostic easier when an error occurs. Suggested by Sabrina Dubroca.

  1. https://docs.kernel.org/userspace-api/netlink/intro.html#ext-ack

@ueno
Copy link
Contributor Author

ueno commented May 22, 2024

I'm currently testing this using the KVM tests (e.g., pluto/ikev2-algo-sha2-03), though haven't managed to receive ACK TLVs, even if NETLINK_{CAP,EXT}_ACKS is set with setsockopt. Perhaps the support might not be part of the kernel 6.8.9?

@letoams
Copy link
Member

letoams commented May 22, 2024 via email

@qsn
Copy link

qsn commented May 22, 2024

6.8.9 should work for your tests, but you will only get the extack response if your netlink request is invalid and the state/policy can't be installed. This may not be easy to trigger since I expect libreswan to generate correct requests in almost all cases. I'm assuming you validate strings taken from the user's config before converting them into netlink requests, so generating a request with a bogus ICV length or non-existent algorithm ("foobar" instead of "rfc4106(gcm(aes))", or some algorithm that's not supported on the current kernel), or a bogus IP mask length, or 42 instead of XFRM_MODE_TUNNEL, etc, is likely not easy.

The kernel's IPsec implementation has been providing extacks since 6.2 (the bulk of the changes went into 6.1), but extacks existed for a long time before that (4.12, see merge commit).

In case the kernel just doesn't support extack, the setsockopt enabling extack on the socket should fail and do nothing (and the return value is ignored in this patch). No extack information will be returned in the kernel's answer.

In case the kernel supports extack but xfrm doesn't fill that information, the userspace application will just not get any extra details on why the request failed. This can also happen on 6.2+ kernels if the kernel does not explicitly set the error (which I think is the case for all ENOMEM type of situations, because I didn't see any value in adding a string that would basically repeat ENOMEM's meaning - and maybe a few other cases).

@ueno
Copy link
Contributor Author

ueno commented May 24, 2024

I've confirmed that, if I add req.p.mode = 42 in netlink_add_sa, I get:

ERROR: "westnet-eastnet-ipv4-psk-ikev2" #2: netlink response for Add SA esp.b455
0ed8@192.1.2.23: Invalid argument (errno 22)
"westnet-eastnet-ipv4-psk-ikev2" #2: netlink ext_ack: Unsupported mode

from ikev2-algo-sha2-01 test. Not sure how this can be exercised properly in the test suite though.

@qsn
Copy link

qsn commented May 24, 2024

I've confirmed that, if I add req.p.mode = 42 in netlink_add_sa, I get:

ERROR: "westnet-eastnet-ipv4-psk-ikev2" #2: netlink response for Add SA esp.b455
0ed8@192.1.2.23: Invalid argument (errno 22)
"westnet-eastnet-ipv4-psk-ikev2" #2: netlink ext_ack: Unsupported mode

Nice :)

from ikev2-algo-sha2-01 test. Not sure how this can be exercised properly in the test suite though.

Does libreswan prevent the combinations of options that the kernel doesn't support? AH and IPcomp don't work with UDP encap or TFCPAD, IPcomp only works with transport/tunnel mode (not BEET), TFCPAD only works in tunnel mode, NIC offload only works with ESP but not AH (and for crypto offload, not with UDP/TCP encap or TFCPAD). If some of those are considered valid by libreswan, the kernel should reject them and return an extack message.

@letoams
Copy link
Member

letoams commented May 25, 2024 via email

This enables pluto to log any error message reported through extended
ACK attributes[1] in a netlink response, to make diagnostic easier
when an error occurs.  Suggested by Sabrina Dubroca.

1. https://docs.kernel.org/userspace-api/netlink/intro.html#ext-ack

Signed-off-by: Daiki Ueno <dueno@redhat.com>
@ueno ueno marked this pull request as ready for review May 25, 2024 22:40
@cagney
Copy link
Collaborator

cagney commented May 27, 2024

testing

@cagney
Copy link
Collaborator

cagney commented May 27, 2024

I'll change:

@@ -515,6 +555,7 @@ static bool sendrecv_xfrm_msg(struct nlmsghdr *hdr,
                 */
                ldbg(logger, "%s() netlink response for %s %s included non-error error",
                     __func__, description, story);
+               llog_ext_ack(DEBUG_STREAM, logger, &rsp.n);
                /* ignore */
        }

to

  if (DBGP(DBG_LOG)) {
    LDBG_log(logger, ...)
    llog_ext_ack(DEBUG_STREAM, ...)
}

so it doesn't always debug log

@cagney
Copy link
Collaborator

cagney commented May 28, 2024

pushed

@cagney cagney closed this May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants