pf: only allow a subset of netlink calls when securelevel is set

[kp]

Summary

Extend the genl_cmd struct to allow calls to also carry a securelevel.
If that's set compare the current securelevel to only allow the call if
the level is at or lower than that.

If no value is specified continue to allow calls in any securelevel,
as before.

This allows us to easily implement the same securelevel restrictions for
pf as we have for the corresponding ioctls.

MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

[glebius]

sys/netlink/netlink_generic.c

153–154

You aren't using the newly added cmd_securelevel here. Definitely something is wrong.

[kp]

Thanks, I don't know how I messed that up, but mess that up I did.

[glebius]

The previous version (modulo the mistake) looked better. What's the point in the additional bool? All existing declarations rely on sparse initialization, so would have .cmd_securelevel = 0 always. If you add cmd_securelevel_set, it would be .cmd_securelevel_set = false. Thus, checking .cmd_securelevel_set for being true has no difference to checking .cmd_securelevel to be positive. I'd suggest to just do the securelevel_gt() check unconditionally.

P.S. Of course the inverted logic of securelevel_gt() really blows one's mind.

[kp]

In D56390#1291460, @glebius wrote:

The previous version (modulo the mistake) looked better. What's the point in the additional bool? All existing declarations rely on sparse initialization, so would have .cmd_securelevel = 0 always. If you add cmd_securelevel_set, it would be .cmd_securelevel_set = false. Thus, checking .cmd_securelevel_set for being true has no difference to checking .cmd_securelevel to be positive. I'd suggest to just do the securelevel_gt() check unconditionally.

P.S. Of course the inverted logic of securelevel_gt() really blows one's mind.

The issue is that if we always run the check, even on implicitly set cmd_securelevel entries we block all genl netlink calls from securelevel 1 up.
That's basically all of them (starting with CTRL_CMD_GETFAMILY, so .. all genl netlink calls). I don't think that's what we want here.

For pf I could get away with a shortcut (i.e. allow anything that has cmd_securelevel == 0, block only if securelevel > cmd_securelevel) by setting securelevel 1 for to-block calls, because pf is documented as only blocking some calls from level 2, but that's seems like it's less generically useful than this approach. (i.e. how do you use that approach to block a call at securelevel 1?)

[glebius]

Sorry, the unconditional check was a wrong suggestion. The correct one is:

if (cmd->cmd_securelevel > 0 && securelevel_gt(nlp_get_cred(nlp), cmd->cmd_securelevel)) {

[kp]

If we tweak it slightly I guess we can express everything we need.
So here's a version where we deny the call from the indicated securelevel on up,
and don't do anything if the value is 0