Skip to content

Commit

Permalink
pf: invert direction for inner icmp state lookups
Browse files Browse the repository at this point in the history 
(e.g. traceroute with icmp)
ok henning, jsing

Also extend the test case to cover this scenario.

PR:		280701
Obtained from:	OpenBSD
MFC after:	1 week
Sponsored by:	Rubicon Communications, LLC ("Netgate")
  • Loading branch information
@kprovost @fichtner
kprovost authored and fichtner committed Aug 19, 2024
1 parent 9c53965 commit c61a3c2
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
21 changes: 11 additions & 10 deletions sys/netpfil/pf/pf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ static int pf_test_state_udp(struct pf_kstate **,
int pf_icmp_state_lookup(struct pf_state_key_cmp *,
struct pf_pdesc *, struct pf_kstate **, struct mbuf *,
int, struct pfi_kkif *, u_int16_t, u_int16_t,
int, int *, int);
int, int *, int, int);
static int pf_test_state_icmp(struct pf_kstate **,
struct pfi_kkif *, struct mbuf *, int,
void *, struct pf_pdesc *, u_short *);
Expand Down Expand Up @@ -6611,7 +6611,8 @@ pf_multihome_scan_asconf(struct mbuf *m, int start, int len,
int
pf_icmp_state_lookup(struct pf_state_key_cmp *key, struct pf_pdesc *pd,
struct pf_kstate **state, struct mbuf *m, int direction, struct pfi_kkif *kif,
u_int16_t icmpid, u_int16_t type, int icmp_dir, int *iidx, int multi)
u_int16_t icmpid, u_int16_t type, int icmp_dir, int *iidx, int multi,
int inner)
{
key->af = pd->af;
key->proto = pd->proto;
Expand Down Expand Up @@ -6648,7 +6649,8 @@ pf_icmp_state_lookup(struct pf_state_key_cmp *key, struct pf_pdesc *pd,

/* Is this ICMP message flowing in right direction? */
if ((*state)->rule.ptr->type &&
(((*state)->direction == direction) ?
(((!inner && (*state)->direction == direction) ||
(inner && (*state)->direction != direction)) ?
PF_IN : PF_OUT) != icmp_dir) {
if (V_pf_status.debug >= PF_DEBUG_MISC) {
printf("pf: icmp type %d in wrong direction (%d): ",
Expand Down Expand Up @@ -6706,15 +6708,15 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
*/
ret = pf_icmp_state_lookup(&key, pd, state, m, pd->dir,
kif, virtual_id, virtual_type, icmp_dir, &iidx,
PF_ICMP_MULTI_NONE);
PF_ICMP_MULTI_NONE, 0);
if (ret >= 0) {
if (ret == PF_DROP && pd->af == AF_INET6 &&
icmp_dir == PF_OUT) {
if (*state != NULL)
PF_STATE_UNLOCK((*state));
ret = pf_icmp_state_lookup(&key, pd, state, m,
pd->dir, kif, virtual_id, virtual_type,
icmp_dir, &iidx, multi);
icmp_dir, &iidx, multi, 0);
if (ret >= 0)
return (ret);
} else
Expand Down Expand Up @@ -6798,6 +6800,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
int off2 = 0;

pd2.af = pd->af;
pd2.dir = pd->dir;
/* Payload packet is from the opposite direction. */
pd2.sidx = (pd->dir == PF_IN) ? 1 : 0;
pd2.didx = (pd->dir == PF_IN) ? 0 : 1;
Expand Down Expand Up @@ -7119,10 +7122,9 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
pf_icmp_mapping(&pd2, iih->icmp_type,
&icmp_dir, &multi, &virtual_id, &virtual_type);

pd2.dir = icmp_dir;
ret = pf_icmp_state_lookup(&key, &pd2, state, m,
pd2.dir, kif, virtual_id, virtual_type,
icmp_dir, &iidx, PF_ICMP_MULTI_NONE);
icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1);
if (ret >= 0)
return (ret);

Expand Down Expand Up @@ -7175,10 +7177,9 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
pf_icmp_mapping(&pd2, iih->icmp6_type,
&icmp_dir, &multi, &virtual_id, &virtual_type);

pd2.dir = icmp_dir;
ret = pf_icmp_state_lookup(&key, &pd2, state, m,
pd->dir, kif, virtual_id, virtual_type,
icmp_dir, &iidx, PF_ICMP_MULTI_NONE);
icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1);
if (ret >= 0) {
if (ret == PF_DROP && pd->af == AF_INET6 &&
icmp_dir == PF_OUT) {
Expand All @@ -7187,7 +7188,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
ret = pf_icmp_state_lookup(&key, pd,
state, m, pd->dir, kif,
virtual_id, virtual_type,
icmp_dir, &iidx, multi);
icmp_dir, &iidx, multi, 1);
if (ret >= 0)
return (ret);
} else
Expand Down
4 changes: 3 additions & 1 deletion tests/sys/netpfil/pf/icmp.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ ttl_exceeded_body()
jexec nat pfctl -e
pft_set_rules nat \
"nat on ${epair_int}b from 198.51.100.0/24 -> (${epair_int}b)" \
"pass"
"block" \
"pass inet proto udp" \
"pass inet proto icmp icmp-type { echoreq }"

# Sanity checks
atf_check -s exit:0 -o ignore \
Expand Down
4 changes: 3 additions & 1 deletion tests/sys/netpfil/pf/icmp6.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ ttl_exceeded_body()
jexec nat pfctl -e
pft_set_rules nat \
"nat on ${epair_int}b from 2001:db8:3::/64 -> (${epair_int}b:0)" \
"pass"
"block" \
"pass inet6 proto udp" \
"pass inet6 proto icmp6 icmp6-type { neighbrsol, neighbradv, echoreq }"

# Sanity checks
atf_check -s exit:0 -o ignore \
Expand Down

0 comments on commit c61a3c2

Please sign in to comment.