pf_get_sport() picks a random port from the port range specified in a

nat rule. It should check to see if it's in-use (i.e. matches an existing
PF state), if it is, it cycles sequentially through other ports until
it finds a free one. However the check was being done with the state
keys the wrong way round so it was never actually finding the state
to be in-use.

- switch the keys to correct this, avoiding random state collisions
with nat. Fixes PR 6300 and problems reported by robert@ and viq.

- check pf_get_sport() return code in pf_test(); if port allocation
fails the packet should be dropped rather than sent out untranslated.

Help/ok claudio@.

1 files changed, 13 insertions, 5 deletions

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 66f6aa08d70..6ef6930d789 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.689 2010/01/18 23:52:46 mcbride Exp $ */
+/*	$OpenBSD: pf.c,v 1.690 2010/02/04 14:10:12 sthen Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -2901,8 +2901,13 @@ pf_test_rule(struct pf_rule **rm, struct pf_state **sm, int direction,
 					/* order is irrelevant */
 					SLIST_INSERT_HEAD(&rules, ri, entry);
 					pf_rule_to_actions(r, &act);
-					pf_get_transaddr(r, pd, &saddr, &sport,
-					    &daddr, &dport, sns);
+					if (pf_get_transaddr(r, pd,
+					    &saddr, &sport, &daddr, &dport,
+					    sns) == -1) {
+						REASON_SET(&reason,
+						    PFRES_MEMORY);
+						goto cleanup;
+					}
 				} else {
 					match = 1;
 					*rm = r;
@@ -2928,8 +2933,11 @@ pf_test_rule(struct pf_rule **rm, struct pf_state **sm, int direction,
 	/* apply actions for last matching rule */
 	if (lastr && lastr->action != PF_MATCH) {
 		pf_rule_to_actions(lastr, &act);
-		pf_get_transaddr(lastr, pd, &saddr, &sport, &daddr, &dport,
-		    sns);
+		if (pf_get_transaddr(lastr, pd, &saddr, &sport, &daddr,
+		    &dport, sns) == -1) {
+			REASON_SET(&reason, PFRES_MEMORY);
+			goto cleanup;
+		}
 	}
 
 	REASON_SET(&reason, PFRES_MATCH);