Hi Pablo,
Thank you for sharing the v2 of this patch taking into account MPTCP!
On 06/04/2023 11:25, Pablo Neira Ayuso wrote:
> IPPROTO_MAX used to be 256, but with the introduction of IPPROTO_MPTCP
> definition, IPPROTO_MAX was bumped to 263.
>
> IPPROTO_MPTCP definition is used for the socket interface from
> userspace (ie. uAPI). It is never used in the layer 4 protocol field of
> IP headers.
(similar to IPPROTO_RAW which is < IPPROTO_MAX)
> IPPROTO_* definitions are used anywhere in the kernel as well as in
> userspace to set the layer 4 protocol field in IP headers as well as
> for uAPI.
>
> At least in Netfilter, there is code in userspace that relies on
> IPPROTO_MAX (not inclusive) to check for the maximum layer 4 protocol.
>
> This patch restores IPPROTO_MAX to 256 for the maximum protocol number
> in the IP headers, and it adds a new IPPROTO_UAPI_MAX for the maximum
> protocol number for uAPI.
>
> Update kernel code to use IPPROTO_UAPI_MAX for inet_diag (mptcp
> registers one for this) and the inet{4,6}_create() IP socket API.
The modification in the kernel looks good to me. But I don't know how to
make sure this will not have any impact on MPTCP on the userspace side,
e.g. somewhere before calling the socket syscall, a check could be done
to restrict the protocol number to IPPROTO_MAX and then breaking MPTCP
support.
Is it not safer to expose something new to userspace, something
dedicated to what can be visible on the wire?
Or recommend userspace programs to limit to lower than IPPROTO_RAW
because this number is marked as "reserved" by the IANA anyway [1]?
Or define something new linked to UINT8_MAX because the layer 4 protocol
field in IP headers is limited to 8 bits?
This limit is not supposed to be directly linked to the one of the enum
you modified. I think we could even say it works "by accident" because
"IPPROTO_RAW" is 255. But OK "IPPROTO_RAW" is there from the "beginning"
[2] :)
WDYT?
Cheers,
Matt
[1] https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
[2]
https://github.com/schwabe/davej-history/blob/9cb9f18b5d26bf176e13edbc0c248d121217c6b3/include/linux/in.h
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
