Re: [RFC PATCH 0/6] btrfs-progs: build distinct binaries for specific btrfs subcommands

["Austin S. Hemmelgarn" <ahferroin7@gmail.com>]

On 2018-08-29 13:24, Axel Burri wrote:
> This patch allows to build distinct binaries for specific btrfs
> subcommands, e.g. "btrfs-subvolume-show" which would be identical to
> "btrfs subvolume show".
> 
> 
> Motivation:
> 
> While btrfs-progs offer the all-inclusive "btrfs" command, it gets
> pretty cumbersome to restrict privileges to the subcommands [1].
> Common approaches are to either setuid root for "/sbin/btrfs" (which
> is not recommended at all), or to write sudo rules for each
> subcommand.
> 
> Separating the subcommands into distinct binaries makes it easy to set
> elevated privileges using capabilities(7) or setuid. A typical use
> case where this is needed is when it comes to automated scripts,
> e.g. btrbk [2] [3] creating snapshots and send/receive them via ssh.
Let me start by saying I think this is a great idea to have as an 
option, and that the motivation is a particularly good one.

I've posted my opinions on your two open questions below, but there's 
two other comments I'd like to make:

* Is there some particular reason that this only includes the commands 
it does, and _hard codes_ which ones it works with?  if we just do 
everything instead of only the stuff we think needs certain 
capabilities, then we can auto-generate the list of commands to be 
processed based on function names in the C files, and it will 
automatically pick up any newly added commands.  At the very least, it 
could still parse through the C files and look for tags in the comments 
for the functions to indicate which ones need to be processed this way. 
Either case will make it significantly easier to add new commands, and 
would also better justify the overhead of shipping all the files 
pre-generated (because there would be much more involved in 
pre-generating them).

* While not essential, it would be really neat to have the `btrfs` 
command detect if an associated binary exists for whatever command was 
just invoked, and automatically exec that (possibly with some 
verification) instead of calling the command directly so that desired 
permissions are enforced.  This would mitigate the need for users to 
remember different command names depending on execution context.
> 
> 
> Description:
> 
> Patch 1 adds a template as well as a generator shell script for the
> splitted subcommands.
> 
> Patch 2 adds the generated subcommand source files.
> 
> Patch 3-5 adds a "install-splitcmd-setcap" make target, with different
> approaches (either hardcoded in Makefile, or more generically by
> including "Makefile.install_setcap" generated by "splitcmd-gen.sh").
> 
> 
> Open Questions:
> 
> 1. "make install-splitcmd-setcap" installs the binaries with hardcoded
> group "btrfs". This needs to be configurable (how?). Another approach
> would be to not set the group at all, and leave this to the user or
> distro packaging script.
Leave it to the user or distro.  It's likely to end up standardized on 
the name 'btrfs', but it should be agnostic of that.
> 
> 2. Instead of the "install-splitcmd-setcap" make target, we could
> introduce a "configure --enable-splitted-subcommands" option, which
> would simply add all splitcmd binaries to the "all" and "install"
> targets without special treatment, and leave the setcap stuff to the
> user or distro packaging script (at least in gentoo, this needs to be
> specified using the "fcaps" eclass anyways [5]).
A bit of a nitpick, but 'split' is the proper past tense of the word 
'split', it's one of those exceptions that English has all over the 
place.  Even aside from that though, I think `separate` sounds more 
natural for the configure option, or better yet, just make it 
`--enable-fscaps` like most other packages do.

That aside, I think having a configure option is the best way to do 
this, it makes it very easy for distro build systems to handle it 
because this is what they're used to doing anyway.  It also makes it a 
bit easier on the user, because it just becomes `make` to build 
whichever version you want installed.