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

Previous Next

Package: guix-patches;

Reported by: vicvbcun <guix <at> ikherbers.com>

Date: Sun, 16 Jun 2024 16:00:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


Message #8 received at 71594 <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: vicvbcun <guix <at> ikherbers.com>
Cc: 71594 <at> debbugs.gnu.org
Subject: Re: [bug#71594] [PATCH] file-systems: Allow specifying CIFS
 credentials in a file.
Date: Tue, 18 Jun 2024 09:55:42 -0400
> +  (define (read-credential-file file)
> +    ;; Read password, user and domain options from file
> +    (with-input-from-file file
> +      (lambda ()
> +        (let loop
> +            ((next-line (read-line))
> +             (lines '()))
> +          (if (not (eof-object? next-line))
> +              (loop (read-line)
> +                    (cond
> +                     ((string-match "^[[:space:]]*pass" next-line)
> +                      ;; mount.cifs escapes commas in the password by doubling
> +                      ;; them
> +                      (cons (string-replace-substring (string-trim next-line) "," ",,")
> +                            lines))
> +                     ((string-match "^[[:space:]]*(user|dom)" next-line)
> +                      (cons (string-trim next-line) lines))
> +                     ;; Ignore all other lines.
> +                     (else
> +                      lines)))
> +              lines)))))

I'd personally rename this to read-cifs-credential-file or
cifs-read-credential-file if it's only used with cifs.

You may be able to make this more compact by following a structure
similar to authorized-shell-directory? in (guix scripts shell).

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.

> + (credential-file (and=> (string-match "(^|,)(credentials|cred)=([^,]+)(,|$)" options)

Line's a bit long, can we add a newline before options?

> + (string-join (read-credential-file credential-file) "," 'prefix)

Ditto with  ",".

Otherwise looks good to me. Thanks, with this I think we handle every
mount option the same way as mount.cifs. 😄

[1]: https://sambaxp.org/fileadmin/user_upload/sambaxp2024-Slides/sxp24-French-accessing_remote.pdf,
slide 25

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




This bug report was last modified 1 year and 17 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.