Page MenuHomeFreeBSD

if_ipsec(4): more carefully validate invalid configuration input
ClosedPublic

Authored by kib on Jan 17 2023, 2:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 9, 7:39 AM
Unknown Object (File)
Thu, May 9, 7:28 AM
Unknown Object (File)
Thu, May 9, 7:16 AM
Unknown Object (File)
Thu, May 9, 5:06 AM
Unknown Object (File)
Thu, May 9, 2:02 AM
Unknown Object (File)
Thu, Apr 25, 9:28 PM
Unknown Object (File)
Apr 1 2024, 1:39 AM
Unknown Object (File)
Feb 11 2024, 11:41 PM

Details

Summary
if_ipsec(4): protect against user supplying unknown address family
if_ipsec(4): handle situations where there are no policy or SADB entry for if

Both issues were found by doing approximately the following sequence of commands on IPv4-only kernel:

ifconfig ipsec0 create reqid 0
ifconfig ipsec0 inet tunnel 1.1.1.2 1.1.1.1
ifconfig ipsec0 up
ifconfig ipsec0 reqid 0

Sponsored by: NVIDIA Networking

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib requested review of this revision.Jan 17 2023, 2:12 AM
kib retitled this revision from if_ipsec(4): more carefully validate invalid input to if_ipsec(4): more carefully validate invalid configuration input.Jan 17 2023, 6:53 AM
This revision is now accepted and ready to land.Jan 17 2023, 8:25 AM
sys/net/if_ipsec.c
670

I'm not sure why we should use EHOSTUNREACH here?

sys/net/if_ipsec.c
670

I selected some error, somewhat inspired by ipsecN_allocsa(). What error code should I use there?

sys/net/if_ipsec.c
816

Would it be possible to use a bit more descriptive name?
As it's a list inside a hash, maybe something like ifl or iflist would be an alternative?

sys/net/if_ipsec.c
816

Would it be possible to use a bit more descriptive name?
As it's a list inside a hash, maybe something like ifl or iflist would be an alternative?

ph
phead

--HPS

kib marked 2 inline comments as done.Jan 17 2023, 11:15 PM
kib added inline comments.
sys/net/if_ipsec.c
816

I changed the name of the local var to iflist. Will upload the new patch after the error value for SIOCGIFPSRCADDR_IN6 is decided.

sys/net/if_ipsec.c
670

If you mean ipsec[46]_allocsa() from netipsec/ipsec_output.c, then it is appropriate to get such error when you are sending packet. But I think it can be confusing when you get "no route to host" trying to get interface configuration.

Now I'm not sure EADDRNOTAVAIL is also good here :)
Maybe ENETDOWN or even ENXIO will be better?

kib marked 2 inline comments as done.Jan 18 2023, 6:07 PM
kib added inline comments.
sys/net/if_ipsec.c
670

ENETDOWN is IMO in the same category as EHOSTUNREACH.
Let it be ENXIO then.

kib marked an inline comment as done.

Change error code to ENXIO.
Rename local var.

This revision now requires review to proceed.Jan 18 2023, 6:07 PM
This revision is now accepted and ready to land.Jan 18 2023, 6:45 PM