Page MenuHomeFreeBSD

ipfilter: Thee commits to verify frentry_t->fr_names and ipnat_t->in_names
Needs ReviewPublic

Authored by cy on Thu, Nov 20, 3:18 PM.
Tags
None
Referenced Files
F137128221: D53843.id166852.diff
Fri, Nov 21, 5:11 AM
F137088268: D53843.id.diff
Fri, Nov 21, 3:03 AM
F137065137: D53843.id.diff
Fri, Nov 21, 2:03 AM
F137026969: D53843.diff
Fri, Nov 21, 12:30 AM
F137011369: D53843.id166852.diff
Thu, Nov 20, 11:45 PM
F137010500: D53843.id.diff
Thu, Nov 20, 11:43 PM
F137009345: D53843.diff
Thu, Nov 20, 11:40 PM

Details

Reviewers
emaste
markj
Summary

This includes a reworked function created by markj@

First commit:

ipfilter: Add ipf_check_names_string()

ipf_check_names_string will verify userland inputs in names strings
(fr.fr_names, in.in_names) for correctness.

Second commit:

ipfilter: Verify frentry on entry into kernel

The frentry struct is built by ipf(8), specifically ipf_y.y when parsing
the ipfilter configuration file (typically ipf.conf). frentry contains
a variable length string field at the end of the struct. This data field,
called fr_names, may contain various text strings such as NIC names,
destination list (dstlist) names, and filter rule comments.  The length
field specifies the length of fr_names within the frentry structure and
fr_size specifies the size of the frentry structure itself.

The upper bound limit to the length of strings field is controlled by the
fr_max_namelen sysctl/kenv or the max_namelen ipfilter tuneable.

The initial concepts were discussed with emaste and jrm.

Third commit:

ipfilter: Verify ipnat on entry into kernel

The ipnat struct is built by ipnat(8), specifically ipnat_y.y when
parsing the ipnat configuration file (typically ipnat.conf). ipnat
contains a variable length string field at the end of the struct. This
data field, called in_names, may contain various text strings such as
NIC names. There is no upper bound limit to the length of strings as
long as the in_namelen length field specifies the length of in_names
within the ipnat structure and in_size specifies the size of the ipnat
structure itself.

Unfortunately, git arc didn't like the squashed commit, telling me "terminated by user." This has been uploaded to phabricator manually.

Test Plan

This has been running here for a day or two.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped