Page MenuHomeFreeBSD

pf: Improve pf_rule input validation
ClosedPublic

Authored by kp on Jan 26 2021, 7:31 PM.
Tags
None
Referenced Files
F81549411: D28362.diff
Wed, Apr 17, 9:27 PM
Unknown Object (File)
Dec 23 2023, 12:05 AM
Unknown Object (File)
Dec 14 2023, 10:07 PM
Unknown Object (File)
Sep 25 2023, 9:39 AM
Unknown Object (File)
Sep 25 2023, 9:37 AM
Unknown Object (File)
Sep 25 2023, 9:36 AM
Unknown Object (File)
Sep 6 2023, 8:19 AM
Unknown Object (File)
Aug 14 2023, 11:32 PM

Details

Summary

Move the validation checks to pf_rule_to_krule() to reduce duplication.
This also makes the checks consistent across different ioctls.

MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kp requested review of this revision.Jan 26 2021, 7:31 PM
donner added inline comments.
sys/netpfil/pf/pf_ioctl.c
1564–1574

I'd reverse this logic.

#ifdef INET
     if (rule->af == AF_INET)     ;
     else
#endif
#ifdef INET6
     if (rule->af == AF_INET6)    ;
     else
#endif
      return (EAFNOSUPPORT);

So each family only tests it's own positive case. And all other families fail automatically.
But your approach has the charm, that it does not generate any code, if all families are supported.

sys/netpfil/pf/pf_ioctl.c
1564–1574

Even more clever approach

switch (rule->af) {
#ifdef INET
    case AF_INET: break;
#endif
#ifdef INET6
    case AF_INET6: break;
#endif
    default: return (EAFNOSUPPORT);
}
This revision is now accepted and ready to land.Jan 27 2021, 9:19 AM
kp marked an inline comment as done.Jan 27 2021, 2:17 PM
kp added inline comments.
sys/netpfil/pf/pf_ioctl.c
1564–1574

I like the suggestion, but it turns out not to work. We don't always specify an address family (i.e. rule->af can be 0)

This revision was automatically updated to reflect the committed changes.
kp marked an inline comment as done.