auto.smb fails on Windows administrative shares

Experimenting with this a bit, shares with spaces in their names work fine, but anything with $ or & fails. Ampersands are particularly troublesome, crashing the automounter if not escaped properly.

@mark-fox, thanks for filing this report!

If I am reading your report correctly this appears to be specific to autofs and when you do not escape certain characters? My suggestion was going to be escaping these charachters or using the ASCII codes for example:

DOLLARSIGN = \044
AMPERSAND = \046

You have the bash output when trying to do this via the command line, however I would expect that to fail as $ is a special character.

Thanks Josh. I actually tried using ASCII codes for the characters the day I filed the report. No dice. I've managed to make contact with Ian, an (the?) autofs maintainer. He thinks he's narrowed the problem down to a particular commit. If we make any progress, I'll share it here.

Hi Mark! Thanks for following up and even reaching out to the maintainer. Please do share what you find. I'm going to mark this triaged for now.

Ian produced a patch that addresses the problem, at least as far as I've tested it. Whether the target is properly escaped or quoted, it works as expected.

The patches (to AutoFS 5.1.2, available from git.kernel.org) are attached and will likely be integrated into AutoFS 5.1.3.

And the second part of the patch.

The attachment "Patch 0 of 1" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

The two autofs patches Mark has attached aren't yet available on kernel.org
but I will (eventually) commit them to the autofs repo. and upload them.
The change will be included in autofs-5.1.3.

The only difference between what will be upstream and what has been posted
will be a single line description added to the autofs CHANGELOG (since there
hopefully won't be any further changes needed) and a Signed-off-by line
in the description.

The first patch is not strictly needed but the use of alloca() can be
unreliable in stack overflow conditions as (I believe) it never returns
a failure. So since it was used in the area that needed to be changed
I thought it best to fix it as part of the change.

The patch descriptions hopefully provide enough information for others
to understand what the problem was and why the change fixes the problem.

I have made a small change to the patch of comment #6.
I've added initialization of the malloc()ed pmapent variable.
While that wasn't a problem it can be on some architectures.

In reviewing the first patch I've found another problem with
mapent_len possibly not being initialized.

Note the order of application of the patch is:
autofs-5.1.2 - use malloc for expanded map location
autofs-5.1.2 - fix offset mount location multiple expansion

and, although they are against current upstream they should apply.
If not then ask and I'll fix them.

I believe these are the upstream patches:

https://mirrors.edge.kernel.org/pub/linux/daemons/autofs/v5/patches-5.1.3/autofs-5.1.2-use-malloc-for-expanded-map-location.patch
https://mirrors.edge.kernel.org/pub/linux/daemons/autofs/v5/patches-5.1.3/autofs-5.1.2-increase-worker-thread-per-thread-stack-size.patch

And https://mirrors.edge.kernel.org/pub/linux/daemons/autofs/v5/patches-5.1.3/autofs-5.1.2-fix-offset-mount-location-multiple-expansion.patch, mentioned in comment #10 too

ok, the worker-thread patch I think was just a case of an incorrect patch description in lp.

@mark-fox, were those two patches the only changes you needed, or was the change to the awk script (removing the quoting of $) also needed?

I honestly don't know. The initial patches from April 12th of 2017 were all
I needed and I haven't messed with it since.

On Wed, Jun 6, 2018, 09:20 Andreas Hasenack, <email address hidden> wrote:

> @mark-fox, were those two patches the only changes you needed, or was
> the change to the awk script (removing the quoting of $) also needed?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1680224
>
> Title:
> auto.smb fails on Windows administrative shares
>
> Status in autofs package in Ubuntu:
> Triaged
>
> Bug description:
> This is probably related to #385244.
>
> I've been unable to get a Windows administrative share (ie. C$)
> mounted through Autofs. Non-administrative shares work fine.
> Administrative shares are accessible with the same credentials from
> other Windows 10 workstations and through smbclient on Ubuntu (16.04
> LTS) machines. Administrative shares can even be mounted using the
> same credentials via mount on the command-line. However, autofs will
> not mount the same shares, using the same credentials, on the same
> Ubuntu machine.
>
> Administrative shares are listed under /cifs/WindowsHostName, but a cd
> /cifs/WindowsHostname/C$ results in "-bash: cd: C$: No such file or
> directory" from the shell and the following output from 'automount -f
> -v':
>
> -----
> attempting to mount entry /cifs/WindowsHostname/C$
> >> Retrying with upper case share name
> >> mount error(6): No such device or address
> >> Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
> mount(generic): failed to mount //WindowsHostname/C\$ (type cifs) on
> /cifs/WindowsHostname/C$
> failed to mount /cifs/WindowsHostname/C$
> -----
>
> Notice that mount complains of failing to mount 'C\$' rather than
> 'C$'. If the bit of awk script that escapes the $ is removed, it then
> fails to mount 'C', which is also no good. I think if we could
> actually get mount to try to mount 'C$', it would work.
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/ubuntu/+source/autofs/+bug/1680224/+subscriptions
>

I can't get it to work without also removing the quoting in the awk script in /etc/auto.smb, i.e.:
@@ -76,7 +76,7 @@
     loc = $2
     # Enclose mount dir and location in quotes
     # Double quote "$" in location as it is special
- gsub(/\$$/, "\\$", loc);
+ #gsub(/\$$/, "\\$", loc);
     gsub(/\&/,"\\\\&",loc)
     print " \\\n\t \"/" dir "\"", "\"://" key "/" loc "\""

and I'm a bit afraid of doing that. I wouldn't want to have the shell suddenly expanding variables or even executing commands there $(), depending on how mount.cifs is called of course.

tl;dr to get C$ mounted I need to apply both patches and change auto.smb like above.

I haven't revisited this yet due to lack of time. Whoever gets to this next should check the current version of autofs, and also the list of available patches for the version shipped in older ubuntu releases.

I've retested this with autofs 5.1.5 (eoan), and @ahasenack's suggested change to auto.smb is definitely required. With the $ escaped, mount fails as it's given a literal \$ in the path.

For older versions of autofs, the following workaround works (tested with autofs 5.1.2 on bionic):
/etc/default/autofs:
# Use OPTIONS to add automount(8) command line options that
# will be used when the daemon is started.
#
OPTIONS="-Ddollar='$'"

/etc/auto.smb:
@@ -76,7 +76,7 @@
     loc = $2
     # Enclose mount dir and location in quotes
     # Double quote "$" in location as it is special
- gsub(/\$$/, "\\$", loc);
+ gsub(/\$$/, "$dollar", loc);
     gsub(/\&/,"\\\\&",loc)
     print " \\\n\t \"/" dir "\"", "\"://" key "/" loc "\""
   }

Based on case history, to fully address this to all still supported Ubuntu releases:

 autofs | 5.1.2-1ubuntu3 | bionic |
 autofs | 5.1.2-1ubuntu3.1 | bionic-updates |
 autofs | 5.1.2-4ubuntu2 | disco |
 autofs | 5.1.5-1ubuntu1 | eoan |
 autofs | 5.1.6-2 | focal |

we need (possibly):

Ubuntu Bionic:

ef48867 autofs-5.1.2 - increase worker thread per-thread stack size
95dafa1 autofs-5.1.2 - fix offset mount location multiple expansion
2d8d7d0 autofs-5.1.2 - use malloc for expanded map location

Ubuntu Eoan:

a change to /etc/auto.smb

Ubuntu Focal:

a change to /etc/auto.smb

I have done the Bionic SRU merge attempt:

https://code.launchpad.net/~rafaeldtinoco/ubuntu/+source/autofs/+git/autofs/+merge/382338

and asked @ahasenack to review it.

I commented in the MP, but let me add something here too.

Groovy/focal with autofs 5.1.6-2 (same package as debian) show the same problem. And the packages from the attached MP for bionic still won't work:
Jun 1 20:05:15 bionic-autofs kernel: [ 751.949808] CIFS VFS: BAD_NETWORK_NAME: \\bionic-autofs\C$dollar

I don't think that "dollar" was meant to be literal. Actually, it doesn't look like any escaping is needed:

root@bionic-autofs:~# mkdir /manual-cifs
root@bionic-autofs:~# mount -t cifs //bionic-autofs/C$ /manual-cifs -o user=ubuntu
Password for ubuntu@//bionic-autofs/C$: ******
root@bionic-autofs:~# l /manual-cifs/
this-is-c.txt*

and

root@bionic-autofs:~# umount /manual-cifs
root@bionic-autofs:~# mount -t cifs //bionic-autofs/C\$ /manual-cifs -o user=ubuntu
Password for ubuntu@//bionic-autofs/C$: ******
root@bionic-autofs:~# l /manual-cifs/
this-is-c.txt*

But C\\$ won't work:
root@bionic-autofs:~# mount -t cifs //bionic-autofs/C\\$ /manual-cifs -o user=ubuntu
Password for ubuntu@//bionic-autofs/C$: ******
mount error(2): No such file or directory
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
root@bionic-autofs:~#

By removing the $ gsub() handling in auto.smb, it works:

root@focal-autofs:~# /etc/auto.smb focal-autofs
awk: cmd. line:9: warning: regexp escape sequence `\&' is not a known regexp operator
-fstype=cifs,uid=$UID,gid=$GID,credentials=/etc/creds/focal-autofs \
         "/print$" "://focal-autofs/print$" \
         "/myshare" "://focal-autofs/myshare" \
         "/C$" "://focal-autofs/C$"

root@focal-autofs:~# l /cifs/focal-autofs
'C$'/ myshare/ 'print$'/
root@focal-autofs:~# l /cifs/focal-autofs/C\$/ <--- even used tab complete here
this-is-c.txt*
root@focal-autofs:~#

Also works without tab-completion:
root@focal-autofs:~# systemctl restart autofs
root@focal-autofs:~# l /cifs/focal-autofs
'C$'/ myshare/ 'print$'/
root@focal-autofs:~# l /cifs/focal-autofs/C$
this-is-c.txt*
root@focal-autofs:~#

Maybe it's time to confirm with upstream, since groovy has the same issue still and is the latest version?

To simplify testing next time, these are the configs I used:

smb.conf shares:
[myshare]
        path = /myshare
        read only = no
        guest ok = no

[C$]
        comment = test for windows administrative share
        path = /c
        read only = no
        guest ok = no

/etc/auto.master extra line:
/cifs /etc/auto.smb --timeout=300

Extra packages to install: samba smbclient cifs-utils

Then just navigate to /cifs/<hostname>/.

Ah, and I added an ubuntu user: sudo smbpasswd -a ubuntu

Upstream just removed the \$$ gsub() handling in auto.smb:
commit a557e6261afc87d3849b557257185ff72be15a82 (HEAD -> upstream-master, upstream/master)
Author: Ian Kent <email address hidden>
Date: Sun May 31 08:47:29 2020 +0800

    autofs-5.1.6 - fix double quoting in auto.smb

    The example program mount script installed to /etc/auto.smb incorrectly
    adds a quote for the trailing dollar of special Windows mounts. But they
    are already surrounded by double quotes. This may have been handled by
    mount.cifs at some point but it's failing now.

    Signed-off-by: Ian Kent <email address hidden>

diff --git a/samples/auto.smb b/samples/auto.smb
index 6af5d85..4842b68 100755
--- a/samples/auto.smb
+++ b/samples/auto.smb
@@ -75,8 +75,6 @@ $SMBCLIENT $smbopts -gL "$key" 2>/dev/null| awk -v "key=$key" -v "opts=$opts" -F
                  dir = $2
                  loc = $2
                  # Enclose mount dir and location in quotes
- # Double quote "$" in location as it is special
- gsub(/\$$/, "\\$", loc);
                  gsub(/\&/,"\\\\&",loc)
                  print " \\\n\t \"/" dir "\"", "\"://" key "/" loc "\""
                }

Here is a script for the test case:

#!/bin/bash

set -e
set -x

sudo apt update && sudo apt dist-upgrade -y
sudo apt install autofs samba smbclient cifs-utils -y
sudo mkdir /myshare /c
echo "This is myshare." | sudo tee /myshare/myshare.txt
echo "This is C." | sudo tee /c/c.txt
echo "
[myshare]
    path = /myshare
    read only = yes
    guest ok = yes

[C$]
    path = /c
    read only = yes
    guest ok = yes
" | sudo tee -a /etc/samba/smb.conf

echo "/cifs /etc/auto.smb --timeout=300" | sudo tee -a /etc/auto.master

sudo systemctl restart smbd nmbd autofs

echo "
Run these commands now:

cat /cifs/localhost/myshare/myshare.txt
cat /cifs/localhost/C$/c.txt
cat /cifs/localhost/C\\$/c.txt
"

The "C$" case is working with the patch, but "&" is still problematic. I tried with groovy, which has autofs 5.1.6, and a share defined like the below doesn't get mounted:

[a&b]
        path = /ab
        read only = yes
        guest ok = yes

mount(generic): calling mount -t cifs -o guest //localhost/a\&b /cifs/localhost/a&b
>> mount error(2): No such file or directory
>> Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)
mount(generic): failed to mount //localhost/a\&b (type cifs) on /cifs/localhost/a&b

If I remove the gsub for & in /etc/auto.smb:
--- auto.smb 2020-06-03 14:35:10.280687540 +0000
+++ /etc/auto.smb 2020-06-03 14:35:20.391377469 +0000
@@ -75,7 +75,6 @@
                  dir = $2
                  loc = $2
                  # Enclose mount dir and location in quotes
- gsub(/\&/,"\\\\&",loc)
                  print " \\\n\t \"/" dir "\"", "\"://" key "/" loc "\""
                }
        END { if (!first) print "\n"; else exit 1 }

Then it works:
ubuntu@groovy-autofs:~$ l /cifs/localhost/a\&b/
total 4.0K
drwxr-xr-x 2 root root 0 Jun 3 14:27 .
drwxr-xr-x 6 root root 0 Jun 3 14:35 ..
-rwxr-xr-x 1 root root 23 Jun 3 14:27 ab.txt

attempting to mount entry /cifs/localhost/a&b
lookup_mount: lookup(program): /cifs/localhost/a&b -> -fstype=cifs,guest ://localhost/a&b
parse_mount: parse(sun): expanded entry: -fstype=cifs,guest ://localhost/a&b
parse_mount: parse(sun): gathered options: fstype=cifs,guest
sun_mount: parse(sun): mounting root /cifs/localhost/a&b, mountpoint /cifs/localhost/a&b, what //localhost/a&b, fstype cifs, options guest
do_mount: //localhost/a&b /cifs/localhost/a&b type cifs options guest using module generic
mount_mount: mount(generic): calling mkdir_path /cifs/localhost/a&b
mount(generic): calling mount -t cifs -o guest //localhost/a&b /cifs/localhost/a&b
mount_mount: mount(generic): mounted //localhost/a&b type cifs on /cifs/localhost/a&b

Same fix works for bionic's autofs 5.1.2.

/etc/auto.smb is already quoting the paths, so escaping these characters before that quoting just makes the escape character "\" a literal, and then things fail.

Here is the new test script, with a case for "&":
#!/bin/bash

set -e
set -x

sudo apt update
sudo apt dist-upgrade -y
sudo apt install autofs samba smbclient cifs-utils -y
sudo mkdir /myshare /c /a\&b
echo "This is myshare." | sudo tee /myshare/myshare.txt
echo "This is C." | sudo tee /c/c.txt
echo "This is a&b." | sudo tee /a\&b/a\&b.txt
echo "
[myshare]
    path = /myshare
    read only = yes
    guest ok = yes

[C$]
    path = /c
    read only = yes
    guest ok = yes

[a&b]
    path = /a&b
    read only = yes
    guest ok = yes
" | sudo tee -a /etc/samba/smb.conf

echo "/cifs /etc/auto.smb --timeout=300" | sudo tee -a /etc/auto.master

sudo systemctl restart smbd nmbd autofs

echo "
Run these commands now:

cat /cifs/localhost/myshare/myshare.txt
cat /cifs/localhost/C$/c.txt
cat /cifs/localhost/C\\$/c.txt
cat /cifs/localhost/a\&b/a\&b.txt
"

Let's also attach it

I have uploaded fixes for:

Groovy
Focal
Eoan

and I'm uploading for Bionic.

Groovy contains the fix for "&" and "$"
Focal contains the fix for "$"
Eoan contains the fix for "$"
Bionic contains the fix for "$" (without the mem alloc changes since they're not needed and SRU would likely not accept those without a specific test case for them).

More information can be found in the merges linked to this bug.

This bug was fixed in the package autofs - 5.1.6-2ubuntu1

---------------
autofs (5.1.6-2ubuntu1) groovy; urgency=medium

  [ Andreas Hasenack ]
  * Allow administrative shares to work (LP: #1680224):
    - d/p/lp1680224-fix-double-quoting-in-auto_smb.patch
    - d/p/lp1680224-fix-and-double-quoting-in-auto_smb.patch

 -- Rafael David Tinoco <email address hidden> Wed, 03 Jun 2020 00:30:58 +0000

Talking to @bdmurray there was a concern about SRU'ing the:

  - d/p/lp1680224-fix-and-double-quoting-in-auto_smb.patch

to previous versions as this new SRU will require a new bug to be opened.

I'm documenting it here so this is not forgotten.

Hello Mark, or anyone else affected,

Accepted autofs into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/autofs/5.1.6-2ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Hello Mark, or anyone else affected,

Accepted autofs into eoan-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/autofs/5.1.5-1ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-eoan to verification-done-eoan. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-eoan. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Hello Mark, or anyone else affected,

Accepted autofs into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/autofs/5.1.2-1ubuntu3.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

VERIFICATION

----
BIONIC

(k)rafaeldtinoco@autofslabb:~$ dpkg-query -f '${Package} ${Version}\n' -W 'autofs'
autofs 5.1.2-1ubuntu3.2

(k)rafaeldtinoco@autofslabb:~$ cat /cifs/localhost/myshare/myshare.txt
This is myshare.
(k)rafaeldtinoco@autofslabb:~$ cat /cifs/localhost/C$/c.txt
This is C.
(k)rafaeldtinoco@autofslabb:~$ cat /cifs/localhost/C\$/c.txt
This is C.

----

EOAN

(k)rafaeldtinoco@autofslabe:~$ dpkg-query -f '${Package} ${Version}\n' -W 'autofs'
autofs 5.1.5-1ubuntu2

(k)rafaeldtinoco@autofslabe:~$ cat /cifs/localhost/myshare/myshare.txt
This is myshare.
(k)rafaeldtinoco@autofslabe:~$ cat /cifs/localhost/C$/c.txt
This is C.
(k)rafaeldtinoco@autofslabe:~$ cat /cifs/localhost/C\$/c.txt
This is C.

----

FOCAL

(k)rafaeldtinoco@autofslabf:~$ dpkg-query -f '${Package} ${Version}\n' -W 'autofs'
autofs 5.1.6-2ubuntu0.1

(k)rafaeldtinoco@autofslabf:~$ cat /cifs/localhost/myshare/myshare.txt
This is myshare.
(k)rafaeldtinoco@autofslabf:~$ cat /cifs/localhost/C$/c.txt
This is C.
(k)rafaeldtinoco@autofslabf:~$ cat /cifs/localhost/C\$/c.txt
This is C.

This bug was fixed in the package autofs - 5.1.6-2ubuntu0.1

---------------
autofs (5.1.6-2ubuntu0.1) focal; urgency=medium

  * Allow administrative shares to work (LP: #1680224):
    - d/p/lp1680224-fix-double-quoting-in-auto_smb.patch

 -- Rafael David Tinoco <email address hidden> Wed, 03 Jun 2020 00:30:58 +0000

The verification of the Stable Release Update for autofs has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package autofs - 5.1.5-1ubuntu2

---------------
autofs (5.1.5-1ubuntu2) eoan; urgency=medium

  * Allow administrative shares to work (LP: #1680224):
    - d/p/lp1680224-fix-double-quoting-in-auto_smb.patch

 -- Rafael David Tinoco <email address hidden> Wed, 03 Jun 2020 01:01:46 +0000

This bug was fixed in the package autofs - 5.1.2-1ubuntu3.2

---------------
autofs (5.1.2-1ubuntu3.2) bionic; urgency=medium

  * Allow administrative shares to work (LP: #1680224):
    - d/p/lp1680224-fix-double-quoting-in-auto_smb.patch
    - d/p/lp1680224-fix-offset-mount-location-multiple-expa.patch

 -- Rafael David Tinoco <email address hidden> Wed, 15 Apr 2020 22:13:27 +0000