[bug#71594] [PATCH] file-systems: Allow specifying CIFS credentials in a file.

[guix]

Hi!

On 2024-06-20T11:22:15-0400, Richard Sent wrote:
> Hi vicvbcun!
>
> vicvbcun <guix@ikherbers.com> writes:
>
> > Hi,
> >
> > thanks for the review!
>
> > > I believe CIFS will add a password2 mount option in 6.9.4 [1]. We should
> > > check if mount.cifs supports putting that option in the credentials file
> > > and match their behavior. If that's too much an ask (Guix's mount.cifs
> > > may not be new enough), I think a comment or proactive bug report is
> > > appropriate.
> >
> > Looking at the latest version of mount.cifs[0], it doesn't seem to
> > handle `password2' intentionally: Passing `password2' on the command
> > line should work, but only because the return value of `parse_opt_token'
> > is not checked for `OPT_ERROR'; in a credentials file it is accepted (as
> > `parse_cred_line' only checks for a "pass" prefix) but passed as
> > `password' instead.
> >
> > I think that being able to specify `password2' in a credentials file
> > makes sense and my patch doesn't forbid it.
> >
> > If exposing an interface identical to that of `mount.cifs' and
> > preserving the exact semantics (e.g `mount.cifs' complains when multiple
> > passwords are specified and takes the first one) is the ultimate goal,
> > I'd just shell out to `mount.cifs'. I certainly won't implement all the
> > idiosyncrasies :).
> >
> > 0: 
> > https://git.samba.org/?p=cifs-utils.git;a=blob;f=mount.cifs.c;h=3b7a6b3c22e8c3b563c7ea92ecb9891fdfac01a6;hb=refs/heads/for-next
>
> Agreed, emulating mount.cifs in totality is too much. My concern with
> divergences in functionality is most users will read mount.cifs
> documentation for CIFS mount-options and whatnot, then potentially get
> bit when Guix does something different.
> In this case the divergence is small and shouldn't cause issues.
As long as what Guix offers is a superset, feature requests won't be 
sent our way :). With the patch, I think there is should be a reasonable 
match between the behaviour as documented in mount.cifs(8) and that of 
Guix.

> I think a XXX: style comment is appropriate.
>
> --8<---------------cut here---------------start------------->8---
> ;; Read password, user and domain options from file
> ;;
> ;; XXX: Unlike mount.cifs this function reads password2 in the
> ;; credential file and returns it separately from password.
> --8<---------------cut here---------------end--------------->8---
done, I have elaborated a bit more though

> I wouldn't be surprised if mount.cifs eventually adopts the same
> behavior. I can't think of a reason why putting password2 in the
> credentials file shouldn't be supported.
The `password2' options is seems mainly useful for remounting when a new 
password is available. Creating a temporary credential file might be 
considered overcomplicated. I suspect that when developing the feature 
it just worked when specified on the command line and was forgotten 
about subsequently.

> --
> Take it easy,
> Richard Sent
> Making my computer weirder one commit at a time.

vicvbcun