Re: [PATCH v2 2/6] export/fuse: Add allow-other option

[Kevin Wolf <kwolf@redhat.com>]

Am 25.06.2021 um 16:23 hat Max Reitz geschrieben:
> Without the allow_other mount option, no user (not even root) but the
> one who started qemu/the storage daemon can access the export.  Allow
> users to configure the export such that such accesses are possible.
> 
> While allow_other is probably what users want, we cannot make it an
> unconditional default, because passing it is only possible (for non-root
> users) if the global fuse.conf configuration file allows it.  Thus, the
> default is an 'auto' mode, in which we first try with allow_other, and
> then fall back to without.
> 
> FuseExport.allow_other reports whether allow_other was actually used as
> a mount option or not.  Currently, this information is not used, but a
> future patch will let this field decide whether e.g. an export's UID and
> GID can be changed through chmod.
> 
> One notable thing about 'auto' mode is that libfuse may print error
> messages directly to stderr, and so may fusermount (which it executes).
> Our export code cannot really filter or hide them.  Therefore, if 'auto'
> fails its first attempt and has to fall back, fusermount will print an
> error message that mounting with allow_other failed.
> 
> This behavior necessitates a change to iotest 308, namely we need to
> filter out this error message (because if the first attempt at mounting
> with allow_other succeeds, there will be no such message).
> 
> Furthermore, common.rc's _make_test_img should use allow-other=off for
> FUSE exports, because iotests generally do not need to access images
> from other users, so allow-other=on or allow-other=auto have no
> advantage.  OTOH, allow-other=on will not work on systems where
> user_allow_other is disabled, and with allow-other=auto, we get said
> error message that we would need to filter out again.  Just disabling
> allow-other is simplest.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qapi/block-export.json       | 33 ++++++++++++++++++++++++++++++++-
>  block/export/fuse.c          | 28 +++++++++++++++++++++++-----
>  tests/qemu-iotests/308       |  6 +++++-
>  tests/qemu-iotests/common.rc |  6 +++++-
>  4 files changed, 65 insertions(+), 8 deletions(-)
> 
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index e819e70cac..0ed63442a8 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -120,6 +120,23 @@
>  	    '*logical-block-size': 'size',
>              '*num-queues': 'uint16'} }
>  
> +##
> +# @FuseExportAllowOther:
> +#
> +# Possible allow_other modes for FUSE exports.
> +#
> +# @off: Do not pass allow_other as a mount option.
> +#
> +# @on: Pass allow_other as a mount option.
> +#
> +# @auto: Try mounting with allow_other first, and if that fails, retry
> +#        without allow_other.
> +#
> +# Since: 6.1
> +##
> +{ 'enum': 'FuseExportAllowOther',
> +  'data': ['off', 'on', 'auto'] }

Why not use the generic OnOffAuto type from common.json?

But since the external interface is unaffected so we can later change
this as a code cleanup and soft freeze is approaching, I won't consider
this a blocker.

Kevin