> On 12 May 2021, at 18:42, YASUOKA Masahiko <yasu...@openbsd.org> wrote:
>
> On Wed, 12 May 2021 17:26:51 +0300
> Vitaliy Makkoveev <m...@openbsd.org> wrote:
>> On Wed, May 12, 2021 at 07:11:09PM +0900, YASUOKA Masahiko wrote:
>>> Radek reported a problem to misc@ that multiple Windows clients behind a NAT
>>> cannot use a L2TP/IPsec server simultaneously.
>>>
>>> https://marc.info/?t=160996816100001&r=1&w=2
>>>
>>> There is two problems. First is pipex(4) doesn't pass the proper
>>> ipsecflowinfo to ip_output(). Second is the IPsec policy check which is
>>> done by ipsp_spd_lookup() returns -1 (EINVAL) if the given tdb is not
>>> cached. This happens when its flow is shared by another tdb (for another
>>> client of the same NAT).
>>>
>>> The following 2 diffs fix these problem.
>>>
>>> comment?
>>> ok?
>>>
>>
>> Hi.
>>
>> I have two comments for the diff 1:
>>
>> 1. You should add PACKET_TAG_IPSEC_FLOWINFO description to
>> m_tag_get(9).
>> 2. You introduced mbuf(9) leak in pipex_l2tp_output() error path. I
>> pointed the place in your diff.
>
> Good catch. Thanks.
>
m_freem(9) accepts NULL so this check before is redundant.
It seems to me that "Used by the IPv4 stack to specify the IPsec flow
of an output IP packet. The tag contains a u_int32_t identifying the
IPsec flow.” is enough. Anyway it’s better to ask jmc@.
Also I like to remove PACKET_TAG_PIPEX with separate diff.
The rest of this diff looks ok by me.
>
> Let me update the diff.
>
> Index: sys/net/pipex.c
> ===================================================================
> RCS file: /disk/cvs/openbsd/src/sys/net/pipex.c,v
> retrieving revision 1.132
> diff -u -p -r1.132 pipex.c
> --- sys/net/pipex.c 10 Mar 2021 10:21:48 -0000 1.132
> +++ sys/net/pipex.c 12 May 2021 15:33:33 -0000
> @@ -1628,6 +1628,7 @@ pipex_l2tp_output(struct mbuf *m0, struc
> #ifdef INET6
> struct ip6_hdr *ip6;
> #endif
> + struct m_tag *mtag;
>
> hlen = sizeof(struct pipex_l2tp_header) +
> ((pipex_session_is_l2tp_data_sequencing_on(session))
> @@ -1704,6 +1705,15 @@ pipex_l2tp_output(struct mbuf *m0, struc
> ip->ip_tos = 0;
> ip->ip_off = 0;
>
> + if (session->proto.l2tp.ipsecflowinfo > 0) {
> + if ((mtag = m_tag_get(PACKET_TAG_IPSEC_FLOWINFO,
> + sizeof(u_int32_t), M_NOWAIT)) == NULL)
> + goto drop;
> + *(u_int32_t *)(mtag + 1) =
> + session->proto.l2tp.ipsecflowinfo;
> + m_tag_prepend(m0, mtag);
> + }
> +
> ip_send(m0);
> break;
> #ifdef INET6
> @@ -1733,6 +1743,8 @@ pipex_l2tp_output(struct mbuf *m0, struc
>
> return;
> drop:
> + if (m0 != NULL)
> + m_freem(m0);
> session->stat.oerrors++;
> }
>
> Index: sys/netinet/ip_input.c
> ===================================================================
> RCS file: /disk/cvs/openbsd/src/sys/netinet/ip_input.c,v
> retrieving revision 1.359
> diff -u -p -r1.359 ip_input.c
> --- sys/netinet/ip_input.c 30 Apr 2021 13:52:48 -0000 1.359
> +++ sys/netinet/ip_input.c 12 May 2021 15:31:52 -0000
> @@ -1790,6 +1790,8 @@ ip_send_do_dispatch(void *xmq, int flags
> struct mbuf_queue *mq = xmq;
> struct mbuf *m;
> struct mbuf_list ml;
> + struct m_tag *mtag;
> + u_int32_t ipsecflowinfo = 0;
>
> mq_delist(mq, &ml);
> if (ml_empty(&ml))
> @@ -1797,7 +1799,12 @@ ip_send_do_dispatch(void *xmq, int flags
>
> NET_LOCK();
> while ((m = ml_dequeue(&ml)) != NULL) {
> - ip_output(m, NULL, NULL, flags, NULL, NULL, 0);
> + if ((mtag = m_tag_find(m, PACKET_TAG_IPSEC_FLOWINFO, NULL))
> + != NULL) {
> + ipsecflowinfo = *(u_int32_t *)(mtag + 1);
> + m_tag_delete(m, mtag);
> + }
> + ip_output(m, NULL, NULL, flags, NULL, NULL, ipsecflowinfo);
> }
> NET_UNLOCK();
> }
> Index: sys/sys/mbuf.h
> ===================================================================
> RCS file: /disk/cvs/openbsd/src/sys/sys/mbuf.h,v
> retrieving revision 1.252
> diff -u -p -r1.252 mbuf.h
> --- sys/sys/mbuf.h 25 Feb 2021 02:43:31 -0000 1.252
> +++ sys/sys/mbuf.h 12 May 2021 15:31:52 -0000
> @@ -469,6 +469,7 @@ struct m_tag *m_tag_next(struct mbuf *,
> /* Packet tag types */
> #define PACKET_TAG_IPSEC_IN_DONE 0x0001 /* IPsec applied, in */
> #define PACKET_TAG_IPSEC_OUT_DONE 0x0002 /* IPsec applied, out */
> +#define PACKET_TAG_IPSEC_FLOWINFO 0x0004 /* IPsec flowinfo */
> #define PACKET_TAG_WIREGUARD 0x0040 /* WireGuard data */
> #define PACKET_TAG_GRE 0x0080 /* GRE processing done
> */
> #define PACKET_TAG_DLT 0x0100 /* data link layer type
> */
> @@ -479,7 +480,7 @@ struct m_tag *m_tag_next(struct mbuf *,
> #define PACKET_TAG_CARP_BAL_IP 0x4000 /* carp(4) ip balanced
> marker */
>
> #define MTAG_BITS \
> - ("\20\1IPSEC_IN_DONE\2IPSEC_OUT_DONE\3IPSEC_IN_CRYPTO_DONE" \
> + ("\20\1IPSEC_IN_DONE\2IPSEC_OUT_DONE\3IPSEC_FLOWINFO" \
> "\4IPSEC_OUT_CRYPTO_NEEDED\5IPSEC_PENDING_TDB\6BRIDGE\7WG\10GRE\11DLT" \
> "\12PF_DIVERT\14PF_REASSEMBLED\15SRCROUTE\16TUNNEL\17CARP_BAL_IP")
>
> Index: share/man/man9/mbuf_tags.9
> ===================================================================
> RCS file: /disk/cvs/openbsd/src/share/man/man9/mbuf_tags.9,v
> retrieving revision 1.41
> diff -u -p -r1.41 mbuf_tags.9
> --- share/man/man9/mbuf_tags.9 21 Jun 2020 15:25:30 -0000 1.41
> +++ share/man/man9/mbuf_tags.9 12 May 2021 15:33:33 -0000
> @@ -122,6 +122,13 @@ The tag contains a
> identifying the security association applied to the packet.
> This tag is primarily used to detect and avoid loops in IPsec
> processing on output.
> +.It PACKET_TAG_IPSEC_FLOWINFO
> +Used by the IPv4 stack to specify the IPsec flow of an output IP packet.
> +This is useful to specify the desired IPsec flow when there are multiple
> +flows for multiple peers behind a same NAT with IPsec NAT-T.
> +The tag contains a
> +.Va u_int32_t
> +identifying the IPsec flow.
> .It PACKET_TAG_WIREGUARD
> Used by the
> .Xr wg 4
> @@ -152,12 +159,6 @@ The tag contains a
> .Va struct pf_divert
> identifying the port, address and routing domain the packet should be
> diverted to.
> -.It PACKET_TAG_PIPEX
> -Used by
> -.Xr pipex 4
> -to cache its session information.
> -The tag contains a
> -.Va struct pipex_tag .
> .It PACKET_TAG_PF_REASSEMBLED
> Used by
> .Xr pf 4
> @@ -258,7 +259,6 @@ The tag-manipulating code is contained i
> .Xr gre 4 ,
> .Xr ipsec 4 ,
> .Xr pf 4 ,
> -.Xr pipex 4 ,
> .Xr mbuf 9
> .Sh HISTORY
> The packet tags first appeared in
>
