Improve parameters handling in veriexec

[hum_semihalf.com]

Summary

Provide more robust parameter parsing in veriexec. Do a little cleanup as well.

[wma]

Rebased version.

[sjg]

sbin/veriexec/veriexec.c

48

Can you please use the @brief format

94

This would break all our usage of veriexec for past 15+ years.
There is no need to pass more arg than is sufficient to be unambiguous.
So 'a' is sufficient. 'e' is sufficient etc.

199

again the requirement to fully spell out debug vs 'd' is a step backwards.

[stephane.rochoy_stormshield.eu]

sbin/veriexec/veriexec.c

94

Well, veriexec(8) do not document it as the expected usage:

The possible states are:

loaded   set automatically when first manifest has been loaded.
active   mac_veriexec(4) will begin checking files.  This state can only be entered from the loaded state.
enforce  mac_veriexec(4) will fail attempts to exec(2) or open(2) files with O_VERIFY unless verified.
locked   prevent loading of any more manifests.

And, to be honest, this behavior is quite surprising (for example, mtree(8) wants keywords, not keyword-abbreviations) that's why we felt it would need to be adjusted. Could we agree on something in-between like strcmp(arg_text, "a") == 0 || strcmp(arg_text, "active") == 0 (and adjust the man accordingly)?

[sjg]

sbin/veriexec/veriexec.c

94

I'm happy to update the man page to explain that a non-ambiguous prefix match is sufficient.

Note strcmp would never be a suitable method of matching, if more than a single character is needed, then strncmp would be useful eg.

if (strncmp("active", arg_text, strlen(arg_text) == 0)

[sebastien.bini_stormshield.eu]

sbin/veriexec/veriexec.c

94

I believe this parameter parsing should be improved:

• veriexec -i a && veriexec -i active && veriexec -i alphonse all resolve to the same status. "alphonse" is not even a prefix of "activate".
• in today's code, "lock" resolves to "locked", but "loc" (which is an unambiguous prefix of "locked") resolved to "loaded".

Personally I find the unambiguous prefix matching a bit overkill for such a small program. I suggest the following: each status can be matched either by a long string ("activate", "locked") or a shortcut string ("a" for "activate", "lock" for "locked", etc...).

[sjg]

sbin/veriexec/veriexec.c

94

FWIW the 'locked' state is something we have never used, it is a hold over from the original NetBSD implementation which relied on manifests loaded during single user and then state locked - the only way to update was to reboot.
Using signed manifests allowed for much greater flexibility.

The use of strncmp as I described earlier is a simple way to allow better matching without breaking backwards compatability.

163

ie. current state Is?

[hum_semihalf.com]

Match args with shortest prefix. Add help message and prevent from running with invalid path.

[sjg]

One style nit, but otherwise looks ok

sbin/veriexec/veriexec.c

248

space before (

[hum_semihalf.com]

Fix style issues.

[hum_semihalf.com]

Rebase the revision.

[sebastien.bini_stormshield.eu]

sbin/veriexec/veriexec.c

101–104

"l" and "lo" resolve to "loaded" while it should yield an error.

[hum_semihalf.com]

Add a simple check for the argument whether it matches only one option. In case of ambiguity return an error.

[sebastien.bini_stormshield.eu]

Looks good, thanks!