| Message ID | 20201103023217.27685-1-ajderossi@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [ipsec] xfrm: Pass template address family to xfrm_state_look_at | expand |
On Mon, Nov 02, 2020 at 06:32:19PM -0800, Anthony DeRossi wrote: > This fixes a regression where valid selectors are incorrectly skipped > when xfrm_state_find is called with a non-matching address family (e.g. > when using IPv6-in-IPv4 ESP in transport mode). > > The state's address family is matched against the template's family > (encap_family) in xfrm_state_find before checking the selector in > xfrm_state_look_at. The template's family should also be used for > selector matching, otherwise valid selectors may be skipped. > > Fixes: e94ee171349d ("xfrm: Use correct address family in xfrm_state_find") > Signed-off-by: Anthony DeRossi <ajderossi@gmail.com> > --- > net/xfrm/xfrm_state.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Your patch reintroduces the same bug that my patch was trying to fix, namely that when you do the comparison on flow you must use the original family and not some other value. Cheers,
On Mon, Nov 02, 2020 at 06:32:19PM -0800, Anthony DeRossi wrote: > This fixes a regression where valid selectors are incorrectly skipped > when xfrm_state_find is called with a non-matching address family (e.g. > when using IPv6-in-IPv4 ESP in transport mode). Why are we even allowing v6-over-v4 in transport mode? Isn't that the whole point of BEET mode? Cheers,
On Tue, Nov 3, 2020 at 4:05 AM Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Mon, Nov 02, 2020 at 06:32:19PM -0800, Anthony DeRossi wrote: > > This fixes a regression where valid selectors are incorrectly skipped > > when xfrm_state_find is called with a non-matching address family (e.g. > > when using IPv6-in-IPv4 ESP in transport mode). > > > > The state's address family is matched against the template's family > > (encap_family) in xfrm_state_find before checking the selector in > > xfrm_state_look_at. The template's family should also be used for > > selector matching, otherwise valid selectors may be skipped. > > > > Fixes: e94ee171349d ("xfrm: Use correct address family in xfrm_state_find") > > Signed-off-by: Anthony DeRossi <ajderossi@gmail.com> > > --- > > net/xfrm/xfrm_state.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Your patch reintroduces the same bug that my patch was trying to > fix, namely that when you do the comparison on flow you must use > the original family and not some other value. My mistake, I misunderstood the original bug. Anthony
On Tue, Nov 3, 2020 at 4:08 AM Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Mon, Nov 02, 2020 at 06:32:19PM -0800, Anthony DeRossi wrote: > > This fixes a regression where valid selectors are incorrectly skipped > > when xfrm_state_find is called with a non-matching address family (e.g. > > when using IPv6-in-IPv4 ESP in transport mode). > > Why are we even allowing v6-over-v4 in transport mode? Isn't that > the whole point of BEET mode? I'm not sure. This is the outgoing policy that strongSwan creates for an IPv6-in-IPv4 tunnel when compression is enabled: src fd02::/16 dst fd02::2/128 dir out priority 326271 ptype main tmpl src 10.0.0.8 dst 192.168.1.231 proto comp spi 0x0000d00e reqid 1 mode tunnel tmpl src 0.0.0.0 dst 0.0.0.0 proto esp spi 0xc543e950 reqid 1 mode transport After your patch, outgoing IPv6 packets fail to match the associated state: src 10.0.0.8 dst 192.168.1.231 proto esp spi 0xc543e950 reqid 1 mode transport replay-window 0 auth-trunc hmac(sha256) 0x143b570f59b23eaa560905f19a922451c6dfa5694ba2e45e1b065bb1863421aa 128 enc cbc(aes) 0x526ed144ca087125ce30e36c8f20d972 encap type espinudp sport 4501 dport 4500 addr 0.0.0.0 anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000 sel src 0.0.0.0/0 dst 0.0.0.0/0 Is this an invalid configuration? Anthony
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index ee6ac32bb06d..065f1bd8479a 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -1075,7 +1075,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr, tmpl->mode == x->props.mode && tmpl->id.proto == x->id.proto && (tmpl->id.spi == x->id.spi || !tmpl->id.spi)) - xfrm_state_look_at(pol, x, fl, family, + xfrm_state_look_at(pol, x, fl, encap_family, &best, &acquire_in_progress, &error); } if (best || acquire_in_progress) @@ -1092,7 +1092,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr, tmpl->mode == x->props.mode && tmpl->id.proto == x->id.proto && (tmpl->id.spi == x->id.spi || !tmpl->id.spi)) - xfrm_state_look_at(pol, x, fl, family, + xfrm_state_look_at(pol, x, fl, encap_family, &best, &acquire_in_progress, &error); }
This fixes a regression where valid selectors are incorrectly skipped when xfrm_state_find is called with a non-matching address family (e.g. when using IPv6-in-IPv4 ESP in transport mode). The state's address family is matched against the template's family (encap_family) in xfrm_state_find before checking the selector in xfrm_state_look_at. The template's family should also be used for selector matching, otherwise valid selectors may be skipped. Fixes: e94ee171349d ("xfrm: Use correct address family in xfrm_state_find") Signed-off-by: Anthony DeRossi <ajderossi@gmail.com> --- net/xfrm/xfrm_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)