public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] superseded: [PATCH pve-storage] cifs: fix connection check for kerberos authenticated shares
Date: Thu, 18 Dec 2025 10:34:05 +0100	[thread overview]
Message-ID: <5f989873-7364-4948-b17b-3be3cf745baf@proxmox.com> (raw)
In-Reply-To: <20251216130005.36544-1-h.laimer@proxmox.com>

superseded-by: 
https://lore.proxmox.com/pve-devel/20251218093243.1267-1-h.laimer@proxmox.com/T/#u

On 12/16/25 14:00, Hannes Laimer wrote:
> With smbclient 4.22 (shipped with trixie) `-U Guest -N` does not fall
> back to not using a username at all, but just failed for shares where
> kerberos was used. smbclient 4.17 (shipped with bookworm) did fall back
> to anonymouse, which then succeeded if kbr was used.
> 
> Since -U is never correct with kerberos and just worked because
> smbclient fell back to no user if `-U Guest` didn't work, we drop
> `-U` here.
> 
> This fixes the connection check for cifs shares with kerberos. The most
> recent changes to when and how a fallback to no user is done in the
> smbclient was from 2016 (samba, 35051a860c75bc119e0ac7755bd69a9ea06695a1[1]).
> The handling of `-U` also didn't change between the two versions, also a
> change in default smb verion didn't seem to have happened in that
> timeframe either cause the last I could find was in 2019 (samba,
> 3264b1f317d6c603cc72eb2a150fe244c47aa3ac[2]). I did not find a
> conclusive answer for why exactly this stopped working, but given that
> we shouldn't use `-U Guest` with kerberos at all, this change would also
> make sense even if it would have continued working.
> 
> We use the presence of `sec=krb5...` in the options to determine if
> kerberos is used.
> 
> [1] https://gitlab.com/samba-team/samba/-/commit/35051a860c75bc119e0ac7755bd69a9ea06695a1
> [2] https://gitlab.com/samba-team/samba/-/commit/3264b1f317d6c603cc72eb2a150fe244c47aa3ac
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> this came up in support.
> 
> I could also reproduce this locally, this patch fixes it.
> 
> As to the concrete change between 4.17 and 4.22 that could have caused
> this I could not find something, so maybe someone who is more familiar with
> cifs, sepcifically in combination with kerberos, could take a look.
> 
>   src/PVE/Storage/CIFSPlugin.pm | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm
> index 5b35daf..5b61e67 100644
> --- a/src/PVE/Storage/CIFSPlugin.pm
> +++ b/src/PVE/Storage/CIFSPlugin.pm
> @@ -66,6 +66,17 @@ sub get_cred_file {
>       return undef;
>   }
>   
> +sub cifs_uses_kerberos : prototype($) {
> +    my ($scfg) = @_;
> +
> +    my $options = $scfg->{options};
> +    return 0 if !defined($options) || $options eq '';
> +
> +    $options =~ s/\s+//g;
> +
> +    return $options =~ m/(?:^|,)sec=krb5(?:i|p)?(?:,|$)/i;
> +}
> +
>   sub cifs_mount : prototype($$$$$) {
>       my ($scfg, $storeid, $smbver, $user, $domain) = @_;
>   
> @@ -77,7 +88,10 @@ sub cifs_mount : prototype($$$$$) {
>   
>       my $cmd = ['/bin/mount', '-t', 'cifs', $source, $mountpoint, '-o', 'soft', '-o'];
>   
> -    if (my $cred_file = get_cred_file($storeid)) {
> +    if (cifs_uses_kerberos($scfg)) {
> +        # no options needed for kerberos, adding username= or domain= would only be informal
> +        # adding the if-branch here to have it explicit, and not just by not adding guest
> +    } elsif (my $cred_file = get_cred_file($storeid)) {
>           push @$cmd, "username=$user", '-o', "credentials=$cred_file";
>           push @$cmd, '-o', "domain=$domain" if defined($domain);
>       } else {
> @@ -280,7 +294,9 @@ sub check_connection {
>           push @$cmd, '-m', "smb" . int($scfg->{smbversion});
>       }
>   
> -    if (my $cred_file = get_cred_file($storeid)) {
> +    if (cifs_uses_kerberos($scfg)) {
> +        push @$cmd, '--use-kerberos=required';
> +    } elsif (my $cred_file = get_cred_file($storeid)) {
>           push @$cmd, '-U', $scfg->{username}, '-A', $cred_file;
>           push @$cmd, '-W', $scfg->{domain} if $scfg->{domain};
>       } else {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


      reply	other threads:[~2025-12-18  9:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-16 13:00 [pve-devel] " Hannes Laimer
2025-12-18  9:34 ` Hannes Laimer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5f989873-7364-4948-b17b-3be3cf745baf@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal