Discussion:
msdfs on 3.0.25b - hide unreadable on DFS links
(too old to reply)
Jan Martin
2007-08-07 21:56:03 UTC
Permalink
(This is the third - and last - one of our recent problems regarding the
visibility of msdfs contents.)


In Samba 3.0.24, bug 3319 was fixed ... in a way that one of our most
important Samba features vanished.

The situation didn't change by now (SAMBA_3_0_25, this morning)

We are running a central Samba service as a dispatcher for many file
services behind it, relying on its ability to hide all entries that the
user can't see anyway.

When we installed it some time ago, it was tricky to find out how to do
it (we had to understand the source), but we succeeded.

The fix for bug 3319 just excluded all DFS links from hiding. Obviously,
the involved people didn't see a real use (or possibility) of
hiding/unhiding for DFS links.

But for us, it's essential. The hide unreadable feature (even for DFS
links) was the original reason to set up this service on Samba instead
of Windows.

Of course, just reverting the bugfix would result in the old behaviour:
Without further intervention, DFS links are hidden, so most users can't
use them. It would be sufficient for our installation (we are running
the server like that right now as a temporary workaround), but it would
be a setback for the community.

So we need another solution.

The most obvious solution would be to create a share parameter called
"never hide msdfs links" with default "yes" that controls the behaviour.

For other solutions (I think that they are worth a look), I must go a
little bit more into detail.

We are hiding/unhiding DFS links by giving the msdfs:... symlink a valid
target, which must be a directory. After that, we give the proper rights
to the symlink target.

Example:
lrwxrwxrwx 1 root bin 28 Aug 7 17:57 aLink ->
msdfs:someOtherServer\aShare
drwxr-x--- 2 root someGroup 4096 Aug 7 17:54
msdfs:someOtherServer\aShare

In this situation (with hide unreadable enabled and without the bug 3319
fix), only members of the Unix group someGroup can see the msdfslink
aShare.
(In practice, we don't do it manually. We are runnning a script that is
collecting the access rights from all places, converts them to POSIX
access control lists, creates the symlinks with their targets and
applies the access control lists)

Now the second option for our solution becomes clear: Samba may unhide
dangling msdfs:* symlinks only, while respecting the access control of
valid symlink targets, if they exist. This would result in an automatic
switch between the old behaviour (good for users like us) an the current
behaviour (good for new users who wonder why their DFS links don't show
up).

Going a step further (solution no. 3), Samba could automatically try to
create valid targets for dangling links on first access (which should
normally be a findfirst/findnext or similar) using a default uid, gid
and mode (such as Samba's uid/gid with mode 0755).

This would make it easy for unexperienced users to apply access rights.
They can just use their tools (chgrp, chmod, ...) on the symlink, which
would be followed. Of course, security is an issue here. I hope that the
'msdfs:' prefix, together with the removal of all \..\ path elements
(all slash/backslash combinations) and typical dangerous characters
(quotes, semicolon, line feed, ...) is sufficient to prevent misuse.

Just for completeness, we should consider a forth solution: allowing
plain files or directories as msdfs links (instead of symlinks). This
would mean to reinvent all details of the ingenious old msdfs:* symlink
syntax, while getting a chance to make msdfs really easy for beginners
(not everyone can handle these links in a shell without strictly
following a manual). We thought about this for some time and came to the
conclusion that it would be a long discussion and no real fun to write
the code.

Before starting to write a patch (we could try it, at least for solution
no. 2), it's time to ask the Samba community.

What do you think about our problem and the solutions mentioned above?

... or about other solutions?

Many thanks for your patience to everyone who came to this point.



Jan Martin
Jeremy Allison
2007-08-09 04:22:36 UTC
Permalink
Post by Jan Martin
(This is the third - and last - one of our recent problems regarding the
visibility of msdfs contents.)
In Samba 3.0.24, bug 3319 was fixed ... in a way that one of our most
important Samba features vanished.
The situation didn't change by now (SAMBA_3_0_25, this morning)
We are running a central Samba service as a dispatcher for many file
services behind it, relying on its ability to hide all entries that the
user can't see anyway.
When we installed it some time ago, it was tricky to find out how to do
it (we had to understand the source), but we succeeded.
The fix for bug 3319 just excluded all DFS links from hiding. Obviously,
the involved people didn't see a real use (or possibility) of
hiding/unhiding for DFS links.
But for us, it's essential. The hide unreadable feature (even for DFS
links) was the original reason to set up this service on Samba instead
of Windows.
Without further intervention, DFS links are hidden, so most users can't
use them. It would be sufficient for our installation (we are running
the server like that right now as a temporary workaround), but it would
be a setback for the community.
So we need another solution.
Currently this looks to me like a feature request (albeit to return to
a mode that we used to support). The problem is the msdfs links are
designed not to point to a valid target, and on some systems there is
no lchmod call to change permissions on symlinks. The ideal solution
would be to use the permissions on the link itself to determine this,
but this would exclude Linux (which doesn't support permission changes
on symlinks). Would this work for you ?

Jeremy.
Jan Martin
2007-08-09 14:24:37 UTC
Permalink
a feature request (albeit to return to a mode
that we used to support)
Right
on some systems there is no lchmod
call to change permissions on symlinks.
That's our problem
The ideal solution would be to use the permissions
on the link itself to determine this,
but this would exclude Linux (which doesn't
support permission changes on symlinks).
Agreed. That's where we started thinking about alternative ways to
describe msdfs links (proposal no. 4), but after some hours of
brainstorming, we didn't have a solution that was nearly as elegant as
the current syntax.
Would this work for you ?
Unfortunately not, we're using Linux.

What about a small patch as an implementation of my proposal no. 2?
It forces showing of dangling msdfs links only, honoring a target stat
if available.

-----------------------------------------------------
diff -rU 5 samba_30_25/source/smbd/dir.c
samba_30_25_hidemsdfs/source/smbd/dir.c
--- samba_30_25/source/smbd/dir.c 2007-08-07 12:46:40.000000000
+0200
+++ samba_30_25_hidemsdfs/source/smbd/dir.c 2007-08-09
10:28:30.000000000 +0200
@@ -1022,14 +1022,16 @@

if (asprintf(&entry, "%s/%s", dir_path, name) == -1) {
return False;
}

- /* If it's a dfs symlink, ignore _hide xxxx_ options */
+ /* If it's a dangling dfs symlink, ignore _hide xxxx_
options */
if (lp_host_msdfs() &&
lp_msdfs_root(SNUM(conn)) &&
- is_msdfs_link(conn, entry, link_target,
NULL)) {
+ is_msdfs_link(conn, entry, link_target,
NULL) &&
+ !VALID_STAT(*pst) &&
+ (SMB_VFS_STAT(conn, name, pst) != 0)) {
SAFE_FREE(entry);
return True;
}

/* Honour _hide unreadable_ option */
----------------------------------------------------------


Jan Martin
Jeremy Allison
2007-08-10 05:07:20 UTC
Permalink
Post by Jan Martin
What about a small patch as an implementation of my proposal no. 2?
It forces showing of dangling msdfs links only, honoring a target stat
if available.
-----------------------------------------------------
diff -rU 5 samba_30_25/source/smbd/dir.c
samba_30_25_hidemsdfs/source/smbd/dir.c
--- samba_30_25/source/smbd/dir.c 2007-08-07 12:46:40.000000000
+0200
+++ samba_30_25_hidemsdfs/source/smbd/dir.c 2007-08-09
10:28:30.000000000 +0200
@@ -1022,14 +1022,16 @@
if (asprintf(&entry, "%s/%s", dir_path, name) == -1) {
return False;
}
- /* If it's a dfs symlink, ignore _hide xxxx_ options */
+ /* If it's a dangling dfs symlink, ignore _hide xxxx_
options */
if (lp_host_msdfs() &&
lp_msdfs_root(SNUM(conn)) &&
- is_msdfs_link(conn, entry, link_target,
NULL)) {
+ is_msdfs_link(conn, entry, link_target,
NULL) &&
+ !VALID_STAT(*pst) &&
+ (SMB_VFS_STAT(conn, name, pst) != 0)) {
SAFE_FREE(entry);
return True;
}
/* Honour _hide unreadable_ option */
----------------------------------------------------------
This patch doesn't work - you need to be doing a SMB_VFS_STAT
of "entry" here, not "name". In addition, callers of is_visible_file
don't all ensure that the passed in stat struct is zeroed (that's
a bug, I'll fix it :-) plus this may have an effect on servers
operating in POSIX mode, as they must do an LSTAT not a STAT
on paths.

This isn't quite as simple as it looks :-).

Jeremy.
Jan Martin
2007-08-10 15:44:03 UTC
Permalink
Jeremy,

that's funny.
Post by Jeremy Allison
This patch doesn't work
On my installation, I tested it before sending, and it was working:
- Dangling DFS link --> shown
- Valid DFS link, target readable --> shown
- Valid DFS link, target unreadable --> hidden
- link with DFS syntax wrong --> hidden, because of "can't stat"
- Non DFS entries worked as expected.
Post by Jeremy Allison
you need to be doing a SMB_VFS_STAT of "entry" here, not "name".
Ooops, ... it was working, so I didn't look at it.
Name looked just plausible to me.
Maybe it worked because all entries with a VALID_STAT
already had one during my tests, so the code came to SMB_VFS_STAT only
for entries that would be hidden anyway.
Post by Jeremy Allison
callers of is_visible_file don't all ensure
that the passed in stat struct is zeroed (that's
a bug, I'll fix it :-)
plus this may have an effect on servers
operating in POSIX mode, as they must do an
LSTAT not a STAT on paths.
I believe you that both is true ... but I copied the code from
user_can_read_file(), so those three functions (user_can_read_file,
user_can_write_file and file_is_special) must have the both the same
problems anyway. My patch didn't change anything in that
logic, I just copied it.

So as far as I see, nothing would get worse here.
Post by Jeremy Allison
This isn't quite as simple as it looks :-).
Obviously.

Many thanks so far. What can I do now?

Jan Martin
Jan Martin
2007-08-10 15:45:45 UTC
Permalink
Jeremy,

that's funny.
Post by Jeremy Allison
This patch doesn't work
On my installation, I tested it before sending, and it was working:
- Dangling DFS link --> shown
- Valid DFS link, target readable --> shown
- Valid DFS link, target unreadable --> hidden
- link with DFS syntax wrong --> hidden, because of "can't stat"
- Non DFS entries worked as expected.
Post by Jeremy Allison
you need to be doing a SMB_VFS_STAT of "entry" here, not "name".
Ooops, ... it was working, so I didn't look at it.
Name looked just plausible to me.
Maybe it worked because all entries with a VALID_STAT
already had one during my tests, so the code came to SMB_VFS_STAT only
for entries that would be hidden anyway.
Post by Jeremy Allison
callers of is_visible_file don't all ensure
that the passed in stat struct is zeroed (that's
a bug, I'll fix it :-)
plus this may have an effect on servers
operating in POSIX mode, as they must do an
LSTAT not a STAT on paths.
I believe you that both is true ... but I copied the code from
user_can_read_file(), so those three functions (user_can_read_file,
user_can_write_file and file_is_special) must have the both the same
problems anyway. My patch didn't change anything in that
logic, I just copied it.

So as far as I see, nothing would get worse here.
Post by Jeremy Allison
This isn't quite as simple as it looks :-).
Obviously.

Many thanks so far. What can I do now?

Jan Martin

Loading...