Page MenuHomeFreeBSD

pf: Improve pf_rule input validation
ClosedPublic

Authored by kp on Jan 26 2021, 7:31 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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; 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
1563–1573

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
1563–1573

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
1563–1573

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.