On 6/17/19 11:55 PM, Pablo Neira Ayuso wrote:
> On Mon, Jun 17, 2019 at 09:49:43PM +0200, Fernando Fernandez Mancera wrote:
>> Hi Pablo, comments below.
>>
>> On 6/17/19 5:45 PM, Pablo Neira Ayuso wrote:
>>> On Mon, Jun 17, 2019 at 12:32:35PM +0200, Fernando Fernandez Mancera wrote:
>>>> Add SYNPROXY module support in nf_tables. It preserves the behaviour of the
>>>> SYNPROXY target of iptables but structured in a different way to propose
>>>> improvements in the future.
>>>>
>>>> Signed-off-by: Fernando Fernandez Mancera <[email protected]>
>>>> ---
>>>> include/uapi/linux/netfilter/nf_SYNPROXY.h | 4 +
>>>> include/uapi/linux/netfilter/nf_tables.h | 16 +
>>>> net/netfilter/Kconfig | 11 +
>>>> net/netfilter/Makefile | 1 +
>>>> net/netfilter/nft_synproxy.c | 328 +++++++++++++++++++++
>>>> 5 files changed, 360 insertions(+)
>>>> create mode 100644 net/netfilter/nft_synproxy.c
>>>>
>> [...]
>>>> +
>>>> +static void nft_synproxy_eval(const struct nft_expr *expr,
>>>> + struct nft_regs *regs,
>>>> + const struct nft_pktinfo *pkt)
>>>> +{
>>>
>>> You have to check if this is TCP traffic in first place, otherwise UDP
>>> packets may enter this path :-).
>>>
>>>> + switch (nft_pf(pkt)) {
>>>> + case NFPROTO_IPV4:
>>>> + nft_synproxy_eval_v4(expr, regs, pkt);
>>>> + return;
>>>> +#if IS_ENABLED(CONFIG_NF_TABLES_IPV6)
>>>> + case NFPROTO_IPV6:
>>>> + nft_synproxy_eval_v6(expr, regs, pkt);
>>>> + return;
>>>> +#endif
>>>
>>> Please, use skb->protocol instead of nft_pf(), I would like we can use
>>> nft_synproxy from NFPROTO_NETDEV (ingress) and NFPROTO_BRIDGE families
>>> too.
>>>
>>
>> If I use skb->protocol no packet enters in the path. What do you
>> recommend me? Other than that, the rest of the suggestions are done and
>> it has been tested and it worked as expected. Thanks :-)
>
> skb->protocol uses big endian representation, you have to check for:
>
> switch (skb->protocol) {
> case htons(ETH_P_IP):
> ...
> break;
> case htons(ETH_P_IPV6):
> ...
> break;
> }
>
Oh, I didn't know that. A patch series including tests and documentation
it is going to be ready soon if everything seem fine to you. After this,
I think we can implement some improvements. Thanks :-)