LinuxLists.cc - problems with latest smbfs changes on 2.4.34 and security backports

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Hello Santiago,

On Wed, Jan 17, 2007 at 11:00:30AM +0100, Santiago Garcia Mantinan wrote:
> Hi!
>
> I have discovered a problem with the changes applied to smbfs in 2.4.34 and
> in the security backports like last Debian's 2.4 kernel update these changes
> seem to be made to solve CVE-2006-5871 and they have broken symbolic links
> and changed the way that special files (like devices) are seen.
>
> For example:
> Before: symbolic links were seen as that, symbolic links an thus if you tried
> to open the file the link was followed and you ended up reading the
> destination file
> Now: symbolic links are seen as normal files (at least with a ls) but their
> length (N) is the length of the symbolic link, if you read it, you'll get the
> first N characters of the destination file. For example, on my filesystem
> bin/sh is a symlink to bash, thus it is 4 bytes long, if I to a cat bin/sh I
> get ELF (this is, the first 4 characters of the bash program, first one
> being a DEL).
>
> Another example:
> Before: if you did a ls of a device file, like dev/zero you could see it as
> a device, if you tried opening it, it wouldn't work, but if you did a cp -a
> of that file to a local filesystem the result was a working zero device.
> Now: the devices are seen as normal files with a length of 0 bytes.
>
> Seems to me like a mask is erasing some mode bits that shouldn't be erased.
>
> I have carried my tests on a Debian Sarge machine always mounting the share
> using: smbmount //server/share /mnt without any other options. The tests
> were carried on a unpatched 2.4.34 comparing it to 2.4.33 and also on
> Debian's 2.4.27 comparing 2.4.27-10sarge4 vs -10sarge5. The server is a
> samba 3.0.23d and I have experienced the same behaviour with samba's
> unix extensions = yes and unix extensions = no.
>
> I don't know what else to add, if you need any more info or want me to do
> any tests just ask for it.

Well, there is not much to add there. Thanks very much for all your tests.
This problem was not easy to fix, and Dann Frazier did a careful job at
backporting it and testing it. Unfortunately, corner cases like this may
sometimes pass through the tests.

Dann, do you still have your samba server ready to try to reproduce this
problem ? Also, there are very suspect lines right there in the patch :

@@ -505,8 +510,13 @@
mnt->file_mode = (oldmnt->file_mode & S_IRWXUGO) | S_IFREG;
mnt->dir_mode = (oldmnt->dir_mode & S_IRWXUGO) | S_IFDIR;

- mnt->flags = (oldmnt->file_mode >> 9);
+ mnt->flags = (oldmnt->file_mode >> 9) | SMB_MOUNT_UID |
+ SMB_MOUNT_GID | SMB_MOUNT_FMODE | SMB_MOUNT_DMODE;
} else {
+ mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
+ S_IROTH | S_IXOTH | S_IFREG;
+ mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
+ S_IROTH | S_IXOTH | S_IFDIR;
if (parse_options(mnt, raw_data))
goto out_bad_option;
}

See above ? mnt->dir_mode being assigned 3 times. It still *seems* to do the
expected thing like this but I wonder if the initial intent was exactly this.
Also, would not it be necessary to add "|S_IFLNK" to the file_mode ? Maybe
what I say is stupid, but it's just a guess.

Santiago, if you feel brave enough to try completely untested code, I
would suggest to try this change :

} else {
- mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
- S_IROTH | S_IXOTH | S_IFREG;
- mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
- S_IROTH | S_IXOTH | S_IFDIR;
+ mnt->file_mode = S_IRWXU | S_IRGRP | S_IXGRP |
+ S_IROTH | S_IXOTH | S_IFREG | S_IFLNK;
+ mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
+ S_IROTH | S_IXOTH | S_IFDIR;
if (parse_options(mnt, raw_data))
goto out_bad_option;

Also, please try making symlinks to directories to see how they behave.

Thanks in advance,
Willy

2007-01-18 04:21:32

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Hi Grant !

On Thu, Jan 18, 2007 at 11:09:57AM +1100, Grant Coady wrote:
(...)
> > } else {
> >- mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> >- S_IROTH | S_IXOTH | S_IFREG;
> >- mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> >- S_IROTH | S_IXOTH | S_IFDIR;
> >+ mnt->file_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> >+ S_IROTH | S_IXOTH | S_IFREG | S_IFLNK;
> >+ mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> >+ S_IROTH | S_IXOTH | S_IFDIR;
> > if (parse_options(mnt, raw_data))
> > goto out_bad_option;
>
> I'm comparing 2.4.33.3 with 2.4.34, with 2.4.34 and above patch symlinks
> to directories seen as target, nor can they be created (Operation not
> permitted...)

Thanks very much Grant for the test. Could you try a symlink to a file ?
And while we're at it, would you like to try to add "|S_IFLNK" to
mnt->dir_mode ? If my suggestion was stupid, at least let's test it to
full extent ;-)

I had another idea looking at the code but since I really don't know it,
I would not like to propose random changes till this magically works. I'd
wait for Dann who understood the code.

Best regards,
Willy

2007-01-18 22:51:13

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Wed, Jan 17, 2007 at 10:55:19PM +0100, Willy Tarreau wrote:
> Dann, do you still have your samba server ready to try to reproduce this
> problem ? Also, there are very suspect lines right there in the patch :

I can set it up again, hopefully have some feedback by tomorrow.

--
dann frazier

2007-01-19 01:00:49

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Wed, Jan 17, 2007 at 10:55:19PM +0100, Willy Tarreau wrote:
> @@ -505,8 +510,13 @@
> mnt->file_mode = (oldmnt->file_mode & S_IRWXUGO) | S_IFREG;
> mnt->dir_mode = (oldmnt->dir_mode & S_IRWXUGO) | S_IFDIR;
>
> - mnt->flags = (oldmnt->file_mode >> 9);
> + mnt->flags = (oldmnt->file_mode >> 9) | SMB_MOUNT_UID |
> + SMB_MOUNT_GID | SMB_MOUNT_FMODE | SMB_MOUNT_DMODE;
> } else {
> + mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> + S_IROTH | S_IXOTH | S_IFREG;
> + mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> + S_IROTH | S_IXOTH | S_IFDIR;
> if (parse_options(mnt, raw_data))
> goto out_bad_option;
> }
>
>
> See above ? mnt->dir_mode being assigned 3 times. It still *seems* to do the
> expected thing like this but I wonder if the initial intent was
> exactly this.

Wow - sorry about that, that's certainly a cut & paste error. But the
end result appears to match current 2.6, which was the intent.

> Also, would not it be necessary to add "|S_IFLNK" to the file_mode ? Maybe
> what I say is stupid, but it's just a guess.

I really don't know the correct answer to that, I was merely copying
the 2.6 flags.

[Still working on getting a 2.4 smbfs test system up...]

--
dann frazier

2007-01-19 05:15:15

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Hi Dann !

On Thu, Jan 18, 2007 at 06:00:40PM -0700, dann frazier wrote:
> On Wed, Jan 17, 2007 at 10:55:19PM +0100, Willy Tarreau wrote:
> > @@ -505,8 +510,13 @@
> > mnt->file_mode = (oldmnt->file_mode & S_IRWXUGO) | S_IFREG;
> > mnt->dir_mode = (oldmnt->dir_mode & S_IRWXUGO) | S_IFDIR;
> >
> > - mnt->flags = (oldmnt->file_mode >> 9);
> > + mnt->flags = (oldmnt->file_mode >> 9) | SMB_MOUNT_UID |
> > + SMB_MOUNT_GID | SMB_MOUNT_FMODE | SMB_MOUNT_DMODE;
> > } else {
> > + mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> > + S_IROTH | S_IXOTH | S_IFREG;
> > + mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> > + S_IROTH | S_IXOTH | S_IFDIR;
> > if (parse_options(mnt, raw_data))
> > goto out_bad_option;
> > }
> >
> >
> > See above ? mnt->dir_mode being assigned 3 times. It still *seems* to do the
> > expected thing like this but I wonder if the initial intent was
> > exactly this.
>
> Wow - sorry about that, that's certainly a cut & paste error. But the
> end result appears to match current 2.6, which was the intent.

OK.

> > Also, would not it be necessary to add "|S_IFLNK" to the file_mode ? Maybe
> > what I say is stupid, but it's just a guess.
>
> I really don't know the correct answer to that, I was merely copying
> the 2.6 flags.

Don't waste your time on this one, it did not work.

> [Still working on getting a 2.4 smbfs test system up...]

Thanks !

Best regards,
Willy

2007-01-20 01:05:53

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Thu, Jan 18, 2007 at 06:00:40PM -0700, dann frazier wrote:
> On Wed, Jan 17, 2007 at 10:55:19PM +0100, Willy Tarreau wrote:
> > @@ -505,8 +510,13 @@
> > mnt->file_mode = (oldmnt->file_mode & S_IRWXUGO) | S_IFREG;
> > mnt->dir_mode = (oldmnt->dir_mode & S_IRWXUGO) | S_IFDIR;
> >
> > - mnt->flags = (oldmnt->file_mode >> 9);
> > + mnt->flags = (oldmnt->file_mode >> 9) | SMB_MOUNT_UID |
> > + SMB_MOUNT_GID | SMB_MOUNT_FMODE | SMB_MOUNT_DMODE;
> > } else {
> > + mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> > + S_IROTH | S_IXOTH | S_IFREG;
> > + mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
> > + S_IROTH | S_IXOTH | S_IFDIR;
> > if (parse_options(mnt, raw_data))
> > goto out_bad_option;
> > }
> >
> >
> > See above ? mnt->dir_mode being assigned 3 times. It still *seems* to do the
> > expected thing like this but I wonder if the initial intent was
> > exactly this.
>
> Wow - sorry about that, that's certainly a cut & paste error. But the
> end result appears to match current 2.6, which was the intent.
>
> > Also, would not it be necessary to add "|S_IFLNK" to the file_mode ? Maybe
> > what I say is stupid, but it's just a guess.
>
> I really don't know the correct answer to that, I was merely copying
> the 2.6 flags.
>
> [Still working on getting a 2.4 smbfs test system up...]

Ah, think I see the problem now:

--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
@@ -1997,7 +1997,7 @@
fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
!(S_ISDIR(fattr->f_mode)) )
- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);

}

Santiago: Thanks for reporting this - can you test this patch out on
your system and let me know if there are still any problems?

Willy: I'll do some more testing and get you a patch that fixes this
and the double assignment nonsense. Would you prefer a single patch or
two?

--
dann frazier

2007-01-20 06:18:33

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Fri, Jan 19, 2007 at 06:05:44PM -0700, dann frazier wrote:
(...)
> Ah, think I see the problem now:
>
> --- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
> +++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
> @@ -1997,7 +1997,7 @@
> fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
> else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
> !(S_ISDIR(fattr->f_mode)) )
> - fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
> + fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
>
> }
>
> Santiago: Thanks for reporting this - can you test this patch out on
> your system and let me know if there are still any problems?
>
> Willy: I'll do some more testing and get you a patch that fixes this
> and the double assignment nonsense. Would you prefer a single patch or
> two?

Since the double assignment is not a bug per se, don't bother making a
separate patch, put everything in the same one.

Thanks a lot for your very fast response !

Cheers,
Willy

2007-01-21 23:03:39

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Hi Grant !

On Mon, Jan 22, 2007 at 09:52:44AM +1100, Grant Coady wrote:
> On Fri, 19 Jan 2007 18:05:44 -0700, dann frazier <[email protected]> wrote:
>
> >On Thu, Jan 18, 2007 at 06:00:40PM -0700, dann frazier wrote:
> >Ah, think I see the problem now:
> >
> >--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
> >+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
> >@@ -1997,7 +1997,7 @@
> > fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
> > else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
> > !(S_ISDIR(fattr->f_mode)) )
> >- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
> >+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
> >
> > }
> >
> client running 2.4.34 with above patch, server is running 2.6.19.2 to
> eliminate it from the problem space (hopefully ;) :
> grant@sempro:/home/other$ uname -r
> 2.4.34b
> grant@sempro:/home/other$ ls -l
> total 9
> drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dir/
> drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dirlink/
> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 filelink*

It seems to me that there is a difference, because filelink now appears the
same size as file. It's just as if we had hard links instead of symlinks.

> grant@sempro:/home/other$ ls -l dirlink/
> total 1
> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:44 file*
> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:44 filelink*
> grant@sempro:/home/other$
>
> problem is still there :(

At least, directories appear as hard links too.

> With client 2.4.33.3 (slackware-11 distro kernel):
> grant@sempro:/home/other$ uname -r
> 2.4.33.3
> grant@sempro:/home/other$ ls -l
> total 2048
> drwxr-xr-x 1 root root 0 2007-01-21 11:44 dir/
> lrwxrwxrwx 1 root root 3 2007-01-21 11:43 dirlink -> dir/
> -rw-r--r-- 1 root root 15 2007-01-21 11:43 file
> lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
> grant@sempro:/home/other$ ls -l dirlink/
> total 2048
> -rw-r--r-- 1 root root 15 2007-01-21 11:44 file
> lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
> grant@sempro:/home/other$ cat filelink
> this is a test
>
> No problem with symlinks, execute flag.
>
> Grant.

Thanks for your tests !
Willy

2007-01-21 23:50:52

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Thu, 18 Jan 2007 05:21:16 +0100, Willy Tarreau <[email protected]> wrote:

>Hi Grant !
>
>On Thu, Jan 18, 2007 at 11:09:57AM +1100, Grant Coady wrote:
>(...)
>> > } else {
>> >- mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>> >- S_IROTH | S_IXOTH | S_IFREG;
>> >- mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>> >- S_IROTH | S_IXOTH | S_IFDIR;
>> >+ mnt->file_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>> >+ S_IROTH | S_IXOTH | S_IFREG | S_IFLNK;
>> >+ mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>> >+ S_IROTH | S_IXOTH | S_IFDIR;
>> > if (parse_options(mnt, raw_data))
>> > goto out_bad_option;
>>
>> I'm comparing 2.4.33.3 with 2.4.34, with 2.4.34 and above patch symlinks
>> to directories seen as target, nor can they be created (Operation not
>> permitted...)
>
>Thanks very much Grant for the test. Could you try a symlink to a file ?

"Operation not permitted"

>And while we're at it, would you like to try to add "|S_IFLNK" to
>mnt->dir_mode ? If my suggestion was stupid, at least let's test it to
>full extent ;-)

Yep, already tried the obvious ;) no difference :(

2.4.33.5 onwards also have a problem with symlinks, but it is slightly
different presentation, the directory symlinks appear as normal files.

With 2.4.33.7 one is able to create file and directory symlinks, though
the links appear as files. Content problem as noted by OP:

grant@sempro:/home/other$ uname -r
2.4.33.7
grant@sempro:/home/other$ cat file
this is a test
grant@sempro:/home/other$ cat filelink
thisgrant@sempro:/home/other$

grant@sempro:/home/other$ mkdir directory
grant@sempro:/home/other$ ln -s directory directorylink
grant@sempro:/home/other$ cp file* directory
grant@sempro:/home/other$ ls directory
file* filelink*
grant@sempro:/home/other$ ls directorylink
directorylink*

Now, WinXP sees the contents of directorylink:

X:\>cd directorylink

X:\directorylink>dir
Volume in drive X is other
Volume Serial Number is 107E-052F

Directory of X:\directorylink

2007-01-18 16:36 <DIR> .
2007-01-18 16:33 <DIR> ..
2007-01-18 16:36 15 file
2007-01-18 16:36 4 filelink
2 File(s) 19 bytes
2 Dir(s) 2,558,922,752 bytes free

X:\directorylink>type file
this is a test

X:\directorylink>type filelink
this
X:\directorylink>
>
>I had another idea looking at the code but since I really don't know it,
>I would not like to propose random changes till this magically works. I'd
>wait for Dann who understood the code.

Grant.

2007-01-21 23:50:51

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Mon, 22 Jan 2007 00:03:21 +0100, Willy Tarreau <[email protected]> wrote:

>Hi Grant !
>
>On Mon, Jan 22, 2007 at 09:52:44AM +1100, Grant Coady wrote:
>> On Fri, 19 Jan 2007 18:05:44 -0700, dann frazier <[email protected]> wrote:
>>
>> >On Thu, Jan 18, 2007 at 06:00:40PM -0700, dann frazier wrote:
>> >Ah, think I see the problem now:
>> >
>> >--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
>> >+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
>> >@@ -1997,7 +1997,7 @@
>> > fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
>> > else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
>> > !(S_ISDIR(fattr->f_mode)) )
>> >- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
>> >+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
>> >
>> > }
>> >
>> client running 2.4.34 with above patch, server is running 2.6.19.2 to
>> eliminate it from the problem space (hopefully ;) :
>> grant@sempro:/home/other$ uname -r
>> 2.4.34b
>> grant@sempro:/home/other$ ls -l
>> total 9
>> drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dir/
>> drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dirlink/
>> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
>> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 filelink*
>
>It seems to me that there is a difference, because filelink now appears the
>same size as file. It's just as if we had hard links instead of symlinks.

Hi Willy,

No, those dir and files were created server-side, sorry I gave wrong
impression, I still get on client side:

grant@sempro:/home/other$ uname -r
2.4.34b
grant@sempro:/home/other$ mkdir test
grant@sempro:/home/other$ ln -s test testlink
ln: creating symbolic link `testlink' to `test': Operation not permitted
grant@sempro:/home/other$ echo "this is also a test" > test/file
grant@sempro:/home/other$ ln -s test/file test2
ln: creating symbolic link `test2' to `test/file': Operation not permitted

trying to create symlinks.

No problems creating symlinks with 2.4.33.3.

Grant.

2007-01-22 07:21:57

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Fri, 19 Jan 2007 18:05:44 -0700, dann frazier <[email protected]> wrote:

>On Thu, Jan 18, 2007 at 06:00:40PM -0700, dann frazier wrote:
>Ah, think I see the problem now:
>
>--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
>+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
>@@ -1997,7 +1997,7 @@
> fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
> else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
> !(S_ISDIR(fattr->f_mode)) )
>- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
>+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
>
> }
>
client running 2.4.34 with above patch, server is running 2.6.19.2 to
eliminate it from the problem space (hopefully ;) :
grant@sempro:/home/other$ uname -r
2.4.34b
grant@sempro:/home/other$ ls -l
total 9
drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dir/
drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dirlink/
-rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
-rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 filelink*
grant@sempro:/home/other$ ls -l dirlink/
total 1
-rwxr-xr-x 1 grant wheel 15 2007-01-21 11:44 file*
-rwxr-xr-x 1 grant wheel 15 2007-01-21 11:44 filelink*
grant@sempro:/home/other$

problem is still there :(

With client 2.4.33.3 (slackware-11 distro kernel):
grant@sempro:/home/other$ uname -r
2.4.33.3
grant@sempro:/home/other$ ls -l
total 2048
drwxr-xr-x 1 root root 0 2007-01-21 11:44 dir/
lrwxrwxrwx 1 root root 3 2007-01-21 11:43 dirlink -> dir/
-rw-r--r-- 1 root root 15 2007-01-21 11:43 file
lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
grant@sempro:/home/other$ ls -l dirlink/
total 2048
-rw-r--r-- 1 root root 15 2007-01-21 11:44 file
lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
grant@sempro:/home/other$ cat filelink
this is a test

No problem with symlinks, execute flag.

Grant.

2007-01-22 08:54:20

by Santiago Garcia Mantinan

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Hi again!

I tried to replicate the problem at home during the weekend with my laptop,
but I couldn't get it to show links with previous kernels, so I guess I had
something different on my samba server or similar, I'm at the real machines
now so I have done the real tests and they look promising. I'm getting
completely different results than those of Grant, which seems really weird.

I applied just this patch:

> > >--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
> > >+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
> > >@@ -1997,7 +1997,7 @@
> > > fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
> > > else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
> > > !(S_ISDIR(fattr->f_mode)) )
> > >- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
> > >+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
> > >
> > > }

To an unpatched 2.4.34, the client is an IBM NetworkStation 1000 (a PowerPC
based thin client), and the server is a normal amd64 based PC running
2.6.19.1, both running Debian, the client runs Sarge and the Server Etch.
I'm descriving this to see if differences on the architectures could be
causing the differences on behaviour between my tests and Grant's.

> > client running 2.4.34 with above patch, server is running 2.6.19.2 to
> > eliminate it from the problem space (hopefully ;) :
> > grant@sempro:/home/other$ uname -r
> > 2.4.34b
> > grant@sempro:/home/other$ ls -l
> > total 9
> > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dir/
> > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dirlink/
> > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
> > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 filelink*
>
> It seems to me that there is a difference, because filelink now appears the
> same size as file. It's just as if we had hard links instead of symlinks.

Here is what I did, I mounted the remote filesystem on /mnt on my client,
the share on the server has a normal Debian Sarge PowerPC filesystem on it.

$ pwd
/mnt/usr
$ ls -l
total 0
drwxr-xr-x 1 root root 0 Feb 15 2005 X11R6
drwxr-xr-x 1 root root 0 Jan 16 2007 bin
drwxr-xr-x 1 root root 0 Jan 16 2007 doc
drwxr-xr-x 1 root root 0 Feb 10 2005 games
drwxr-xr-x 1 root root 0 Jan 16 2007 include
lrwxr-xr-x 1 root root 10 Jan 16 2007 info -> share/info
drwxr-xr-x 1 root root 0 Jan 16 2007 lib
drwxr-xr-x 1 root root 0 Feb 10 2005 local
drwxr-xr-x 1 root root 0 Jan 16 2007 sbin
drwxr-xr-x 1 root root 0 Jan 5 2006 share
drwxr-xr-x 1 root root 0 Dec 15 2004 src
$ ls -l info/
total 249856
-rwxr-xr-x 1 root root 150109 Jul 16 2004 coreutils.info.gz
-rwxr-xr-x 1 root root 1299 Jan 16 2007 dir
-rwxr-xr-x 1 root root 1299 Jan 16 2007 dir.old
-rwxr-xr-x 1 root root 28019 Mar 20 2005 find.info.gz
-rwxr-xr-x 1 root root 26136 Nov 22 2004 grep.info.gz
-rwxr-xr-x 1 root root 12914 Sep 16 2006 gzip.info.gz
-rwxr-xr-x 1 root root 12316 Sep 18 2005 ipc.info.gz
-rwxr-xr-x 1 root root 21432 Jan 23 2005 rl5userman.info.gz
-rwxr-xr-x 1 root root 26647 Dec 1 2004 sed.info.gz
-rwxr-xr-x 1 root root 123382 Dec 1 2006 tar.info.gz
-rwxr-xr-x 1 root root 54876 May 23 2005 wget.info.gz
$ cd ../bin
$ ls -l sh
lrwxr-xr-x 1 root root 4 Jan 16 2007 sh -> bash
$ dd if=sh bs=1 count=6
ELF6+0 records in
6+0 records out
6 bytes transferred in 0.001432 seconds (4190 bytes/sec)

As you can see I now can see the symbolic links perfectly and they work as
expected.

In fact, this patch is working so well that it poses a security risk, as now
the devices on my /mnt/dev directory are not only seen as devices (like they
were seen on 2.4.33) but they also work (which didn't happen on 2.4.33).

So... for me now the remote filesystem works as if it was a local
filesystem, without any difference of behaviour, not even on special files
like devices or whatever.

As I said before... this behaviour of having the remote device files work...
seems a security problem and I don't think is desirable, other than that it
seems to work well on my PowerPC, I'll try to run the tests on a normal x86
client and report back.

Regards...
--
Santiago Garc?a Manti??n

2007-01-22 09:18:31

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Hi Santiago !

On Mon, Jan 22, 2007 at 09:54:00AM +0100, Santiago Garcia Mantinan wrote:
> Hi again!
>
> I tried to replicate the problem at home during the weekend with my laptop,
> but I couldn't get it to show links with previous kernels, so I guess I had
> something different on my samba server or similar, I'm at the real machines
> now so I have done the real tests and they look promising. I'm getting
> completely different results than those of Grant, which seems really weird.
>
> I applied just this patch:
>
> > > >--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
> > > >+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
> > > >@@ -1997,7 +1997,7 @@
> > > > fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
> > > > else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
> > > > !(S_ISDIR(fattr->f_mode)) )
> > > >- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
> > > >+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
> > > >
> > > > }
>
> To an unpatched 2.4.34, the client is an IBM NetworkStation 1000 (a PowerPC
> based thin client), and the server is a normal amd64 based PC running
> 2.6.19.1, both running Debian, the client runs Sarge and the Server Etch.
> I'm descriving this to see if differences on the architectures could be
> causing the differences on behaviour between my tests and Grant's.
>
> > > client running 2.4.34 with above patch, server is running 2.6.19.2 to
> > > eliminate it from the problem space (hopefully ;) :
> > > grant@sempro:/home/other$ uname -r
> > > 2.4.34b
> > > grant@sempro:/home/other$ ls -l
> > > total 9
> > > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dir/
> > > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dirlink/
> > > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
> > > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 filelink*
> >
> > It seems to me that there is a difference, because filelink now appears the
> > same size as file. It's just as if we had hard links instead of symlinks.
>
> Here is what I did, I mounted the remote filesystem on /mnt on my client,
> the share on the server has a normal Debian Sarge PowerPC filesystem on it.
>
> $ pwd
> /mnt/usr
> $ ls -l
> total 0
> drwxr-xr-x 1 root root 0 Feb 15 2005 X11R6
> drwxr-xr-x 1 root root 0 Jan 16 2007 bin
> drwxr-xr-x 1 root root 0 Jan 16 2007 doc
> drwxr-xr-x 1 root root 0 Feb 10 2005 games
> drwxr-xr-x 1 root root 0 Jan 16 2007 include
> lrwxr-xr-x 1 root root 10 Jan 16 2007 info -> share/info
> drwxr-xr-x 1 root root 0 Jan 16 2007 lib
> drwxr-xr-x 1 root root 0 Feb 10 2005 local
> drwxr-xr-x 1 root root 0 Jan 16 2007 sbin
> drwxr-xr-x 1 root root 0 Jan 5 2006 share
> drwxr-xr-x 1 root root 0 Dec 15 2004 src
> $ ls -l info/
> total 249856
> -rwxr-xr-x 1 root root 150109 Jul 16 2004 coreutils.info.gz
> -rwxr-xr-x 1 root root 1299 Jan 16 2007 dir
> -rwxr-xr-x 1 root root 1299 Jan 16 2007 dir.old
> -rwxr-xr-x 1 root root 28019 Mar 20 2005 find.info.gz
> -rwxr-xr-x 1 root root 26136 Nov 22 2004 grep.info.gz
> -rwxr-xr-x 1 root root 12914 Sep 16 2006 gzip.info.gz
> -rwxr-xr-x 1 root root 12316 Sep 18 2005 ipc.info.gz
> -rwxr-xr-x 1 root root 21432 Jan 23 2005 rl5userman.info.gz
> -rwxr-xr-x 1 root root 26647 Dec 1 2004 sed.info.gz
> -rwxr-xr-x 1 root root 123382 Dec 1 2006 tar.info.gz
> -rwxr-xr-x 1 root root 54876 May 23 2005 wget.info.gz
> $ cd ../bin
> $ ls -l sh
> lrwxr-xr-x 1 root root 4 Jan 16 2007 sh -> bash
> $ dd if=sh bs=1 count=6
> ELF6+0 records in
> 6+0 records out
> 6 bytes transferred in 0.001432 seconds (4190 bytes/sec)
>
> As you can see I now can see the symbolic links perfectly and they work as
> expected.
>
> In fact, this patch is working so well that it poses a security risk, as now
> the devices on my /mnt/dev directory are not only seen as devices (like they
> were seen on 2.4.33) but they also work (which didn't happen on 2.4.33).

Why do you consider this a security problem ? Is any user able to create a
device entry with enough permissions ? As a general rule of thumb, networked
file systems should be mounted with the "nodev" option.

> So... for me now the remote filesystem works as if it was a local
> filesystem, without any difference of behaviour, not even on special files
> like devices or whatever.
>
> As I said before... this behaviour of having the remote device files work...
> seems a security problem and I don't think is desirable, other than that it
> seems to work well on my PowerPC, I'll try to run the tests on a normal x86
> client and report back.

Thanks very much for your tests.

Grant, just to be sure, are you really certain that you tried the fixed kernel ?
It is possible that you booted a wrong kernel during one of your tests. I'm
intrigued by the fact that it changed nothing for you and that it fixed the
problem for Santiago.

Best regards,
Willy

2007-01-22 09:36:44

by Santiago Garcia Mantinan

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

> > As you can see I now can see the symbolic links perfectly and they work as
> > expected.
> >
> > In fact, this patch is working so well that it poses a security risk, as now
> > the devices on my /mnt/dev directory are not only seen as devices (like they
> > were seen on 2.4.33) but they also work (which didn't happen on 2.4.33).
>
> Why do you consider this a security problem ? Is any user able to create a
> device entry with enough permissions ? As a general rule of thumb, networked
> file systems should be mounted with the "nodev" option.

You are completely right on that, it is just that I thought those devices
didn't work on 2.4.33, but I just retested again and they work ok, only that
they were not working to me on the PC I tested the other day and it was
because of a nodev option :-) just that.

So... I have finised with my tests, I have tested an x86 client on which it
worked ok, just like on the PowerPC client, both working perfectly just like
they used to do on 2.4.33.

> Grant, just to be sure, are you really certain that you tried the fixed kernel ?
> It is possible that you booted a wrong kernel during one of your tests. I'm
> intrigued by the fact that it changed nothing for you and that it fixed the
> problem for Santiago.

Maybe he had also applied some of the earlier patches you had sent and that
I did not apply to mine?

Just to clear things up a bit, I'm sure I'm with the 2.4.34 kernel and...
I'm running a pristine kernel with just this latest patch applied, the one
that changes S_IFREG for (fattr->f_mode & S_IFMT).

Regards...
--
Santiago Garc?a Manti??n

2007-01-22 10:46:20

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Mon, 22 Jan 2007 10:18:16 +0100, Willy Tarreau <[email protected]> wrote:

>Grant, just to be sure, are you really certain that you tried the fixed kernel ?
>It is possible that you booted a wrong kernel during one of your tests. I'm
>intrigued by the fact that it changed nothing for you and that it fixed the
>problem for Santiago.

Closest I get to Santiago's results are with the 2.4.33.7 plus the patch,
with 'use default NLS' option turned on, as well as the unix extensions.

2.4.34 was a no go for me. Changing the default NLS made no difference,
now trying with unix extensions turned on. . . Yeah, that works, apart
from the test file gaining execute bits, compared to operation under
2.4.33.3, this is 2.4.34 + patch + default NLS and unix extensions:

grant@sempro:/home/other$ cat dirlink/filelink
this is a test
grant@sempro:/home/other$ echo "this is a test" > testfile
grant@sempro:/home/other$ ls -l
total 4096
drwxr-xr-x 1 grant wheel 0 2007-01-21 11:44 dir/
lrwxr-xr-x 1 grant wheel 3 2007-01-21 11:43 dirlink -> dir/
-rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
lrwxr-xr-x 1 grant wheel 4 2007-01-21 11:44 filelink -> file*
drwxr-xr-x 1 grant wheel 0 2007-01-22 10:45 test/
-rwxr-xr-x 1 grant wheel 15 2007-01-22 21:31 testfile*
lrwxr-xr-x 1 grant wheel 4 2007-01-22 21:29 testlink -> test/
grant@sempro:/home/other$ ln -s testfile testfilelink
grant@sempro:/home/other$ cat testfilelink
this is a test

The fix depends on the smbfs configuration? Is this the requirement?
I ask as the other mode of operation (unix extensions turned off): do
not display symlinks, prevent creation of symlinks, seems to be logically
self-consistent, as well as matching what I see from a 'doze box.

Grant.

2007-01-22 10:49:53

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Mon, 22 Jan 2007 10:36:30 +0100, Santiago Garcia Mantinan <[email protected]> wrote:

>> > As you can see I now can see the symbolic links perfectly and they work as
>> > expected.
>> >
>> > In fact, this patch is working so well that it poses a security risk, as now
>> > the devices on my /mnt/dev directory are not only seen as devices (like they
>> > were seen on 2.4.33) but they also work (which didn't happen on 2.4.33).
>>
>> Why do you consider this a security problem ? Is any user able to create a
>> device entry with enough permissions ? As a general rule of thumb, networked
>> file systems should be mounted with the "nodev" option.
>
>You are completely right on that, it is just that I thought those devices
>didn't work on 2.4.33, but I just retested again and they work ok, only that
>they were not working to me on the PC I tested the other day and it was
>because of a nodev option :-) just that.
>
>So... I have finised with my tests, I have tested an x86 client on which it
>worked ok, just like on the PowerPC client, both working perfectly just like
>they used to do on 2.4.33.
>
>> Grant, just to be sure, are you really certain that you tried the fixed kernel ?
>> It is possible that you booted a wrong kernel during one of your tests. I'm
>> intrigued by the fact that it changed nothing for you and that it fixed the
>> problem for Santiago.
>
>Maybe he had also applied some of the earlier patches you had sent and that
>I did not apply to mine?
>
>Just to clear things up a bit, I'm sure I'm with the 2.4.34 kernel and...
>I'm running a pristine kernel with just this latest patch applied, the one
>that changes S_IFREG for (fattr->f_mode & S_IFMT).

Same kernel + patch here for latest results posting :) We seem to get
similar results now -- though I query the file execute bits coming up.

Grant.
>
>Regards...

2007-01-22 18:19:56

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Mon, Jan 22, 2007 at 10:50:47AM +1100, Grant Coady wrote:
> On Mon, 22 Jan 2007 00:03:21 +0100, Willy Tarreau <[email protected]> wrote:
> grant@sempro:/home/other$ uname -r
> 2.4.34b
> grant@sempro:/home/other$ mkdir test
> grant@sempro:/home/other$ ln -s test testlink
> ln: creating symbolic link `testlink' to `test': Operation not permitted
> grant@sempro:/home/other$ echo "this is also a test" > test/file
> grant@sempro:/home/other$ ln -s test/file test2
> ln: creating symbolic link `test2' to `test/file': Operation not permitted
>
> trying to create symlinks.
>
> No problems creating symlinks with 2.4.33.3.

Yes, I've found that this varies depending upon the options passed. If
uid=0, I can create symlinks, otherwise I always get permission
denied. This behavior appears to be consistent with 2.6.

I also need to do some testing with the proposed patch to smbmount
that will let you omit options (current versions will always pass an
option to the kernel, even if you the user did not provide one).
If you do not pass options, the behavior should fallback to
server-provided values.

Note that this bug has been my only interaction with smbfs, so I'm
certainly no expert on how it *should* behave. My plan is to
take all of the use cases we're coming up with and try to maintain
the "historic" 2.4 behavior as much as possible, but still not
silently dropping user-provided mount options. When the behavior needs
to change to honor them, I'll try to match what current 2.6
does. Make sense?

--
dann frazier

2007-01-23 05:43:21

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Hi Dann,

On Mon, Jan 22, 2007 at 11:19:43AM -0700, dann frazier wrote:
> On Mon, Jan 22, 2007 at 10:50:47AM +1100, Grant Coady wrote:
> > On Mon, 22 Jan 2007 00:03:21 +0100, Willy Tarreau <[email protected]> wrote:
> > grant@sempro:/home/other$ uname -r
> > 2.4.34b
> > grant@sempro:/home/other$ mkdir test
> > grant@sempro:/home/other$ ln -s test testlink
> > ln: creating symbolic link `testlink' to `test': Operation not permitted
> > grant@sempro:/home/other$ echo "this is also a test" > test/file
> > grant@sempro:/home/other$ ln -s test/file test2
> > ln: creating symbolic link `test2' to `test/file': Operation not permitted
> >
> > trying to create symlinks.
> >
> > No problems creating symlinks with 2.4.33.3.
>
> Yes, I've found that this varies depending upon the options passed. If
> uid=0, I can create symlinks, otherwise I always get permission
> denied. This behavior appears to be consistent with 2.6.
>
> I also need to do some testing with the proposed patch to smbmount
> that will let you omit options (current versions will always pass an
> option to the kernel, even if you the user did not provide one).
> If you do not pass options, the behavior should fallback to
> server-provided values.
>
> Note that this bug has been my only interaction with smbfs, so I'm
> certainly no expert on how it *should* behave. My plan is to
> take all of the use cases we're coming up with and try to maintain
> the "historic" 2.4 behavior as much as possible, but still not
> silently dropping user-provided mount options. When the behavior needs
> to change to honor them, I'll try to match what current 2.6
> does. Make sense?

Yes, it does for me. So to sum up, I apply your patch to 2.4.34.1
and it restores the same behaviour for Santiago and Grant as they get
in 2.6. Whether it's the expected behaviour or not is not the point,
as it will be easier for us to later mimic 2.6 if a change is needed
since we're not experts at all in this area.

If we're all OK for this, I'll go with that.

Thanks guys,
Willy

2007-01-23 20:19:49

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Mon, Jan 22, 2007 at 12:03:21AM +0100, Willy Tarreau wrote:
> Hi Grant !
>
> On Mon, Jan 22, 2007 at 09:52:44AM +1100, Grant Coady wrote:
> > On Fri, 19 Jan 2007 18:05:44 -0700, dann frazier <[email protected]> wrote:
> >
> > >On Thu, Jan 18, 2007 at 06:00:40PM -0700, dann frazier wrote:
> > >Ah, think I see the problem now:
> > >
> > >--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
> > >+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
> > >@@ -1997,7 +1997,7 @@
> > > fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
> > > else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
> > > !(S_ISDIR(fattr->f_mode)) )
> > >- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
> > >+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
> > >
> > > }
> > >
> > client running 2.4.34 with above patch, server is running 2.6.19.2 to
> > eliminate it from the problem space (hopefully ;) :
> > grant@sempro:/home/other$ uname -r
> > 2.4.34b
> > grant@sempro:/home/other$ ls -l
> > total 9
> > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dir/
> > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dirlink/
> > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
> > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 filelink*
>
> It seems to me that there is a difference, because filelink now appears the
> same size as file. It's just as if we had hard links instead of symlinks.

I was running into this yesterday - turns out that Debian's current
smbfs package contains a patch that checks for user passed options,
and disables unix capabilities in that case. It was added in
3.0.14a-4. I've filed a bug requesting the removal of this patch:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=408033

Grant: Do you know if are you running a version of smbfs w/ this
patch?

--
dann frazier

2007-01-23 21:04:46

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Tue, 23 Jan 2007 13:19:37 -0700, dann frazier <[email protected]> wrote:

>On Mon, Jan 22, 2007 at 12:03:21AM +0100, Willy Tarreau wrote:
>> Hi Grant !
>>
>> On Mon, Jan 22, 2007 at 09:52:44AM +1100, Grant Coady wrote:
>> > On Fri, 19 Jan 2007 18:05:44 -0700, dann frazier <[email protected]> wrote:
>> >
>> > >On Thu, Jan 18, 2007 at 06:00:40PM -0700, dann frazier wrote:
>> > >Ah, think I see the problem now:
>> > >
>> > >--- kernel-source-2.4.27.orig/fs/smbfs/proc.c 2007-01-19 17:53:57.247695476 -0700
>> > >+++ kernel-source-2.4.27/fs/smbfs/proc.c 2007-01-19 17:49:07.480161733 -0700
>> > >@@ -1997,7 +1997,7 @@
>> > > fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
>> > > else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
>> > > !(S_ISDIR(fattr->f_mode)) )
>> > >- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
>> > >+ fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | (fattr->f_mode & S_IFMT);
>> > >
>> > > }
>> > >
>> > client running 2.4.34 with above patch, server is running 2.6.19.2 to
>> > eliminate it from the problem space (hopefully ;) :
>> > grant@sempro:/home/other$ uname -r
>> > 2.4.34b
>> > grant@sempro:/home/other$ ls -l
>> > total 9
>> > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dir/
>> > drwxr-xr-x 1 grant wheel 4096 2007-01-21 11:44 dirlink/
>> > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
>> > -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 filelink*
>>
>> It seems to me that there is a difference, because filelink now appears the
>> same size as file. It's just as if we had hard links instead of symlinks.
>
>I was running into this yesterday - turns out that Debian's current
>smbfs package contains a patch that checks for user passed options,
>and disables unix capabilities in that case. It was added in
>3.0.14a-4. I've filed a bug requesting the removal of this patch:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=408033
>
>Grant: Do you know if are you running a version of smbfs w/ this
> patch?

Hi Dann,
I'm running slackware-11, no smbfs package, 'smbmnt' is from samba-3.0.23c
package with two tiny unrelated (?) patches:

grant@deltree:/home/mirror/slackware-11.0/source$ find . -name samba*
./n/samba
./n/samba/samba.cups.diff.gz
./n/samba/samba.ssl.diff.gz
./n/samba/samba.SlackBuild
./n/samba/samba-3.0.23c.tar.asc
./n/samba/samba-3.0.23c.tar.bz2
grant@deltree:/home/mirror/slackware-11.0/source$ zless n/samba/samba.cups.diff.gz
--- ./source/configure.orig 2002-10-23 18:06:43.000000000 -0700
+++ ./source/configure 2002-10-23 18:07:12.000000000 -0700
@@ -3789,7 +3789,7 @@

CFLAGS="$CFLAGS `$CUPS_CONFIG --cflags`"
LDFLAGS="$LDFLAGS `$CUPS_CONFIG --ldflags`"
- LIBS="$LIBS `$CUPS_CONFIG --libs`"
+ LIBS="-lcrypt $LIBS `$CUPS_CONFIG --libs`"
fi
fi

grant@deltree:/home/mirror/slackware-11.0/source$ zcat n/samba/samba.ssl.diff.gz
--- ./source/configure.orig 2002-10-09 13:27:21.000000000 -0700
+++ ./source/configure 2002-10-23 18:06:43.000000000 -0700
@@ -12479,7 +12479,7 @@

fi

- LIBS="-lssl -lcrypto $LIBS"
+ LIBS="$LIBS -lssl -lcrypto"

# if test ! -d ${withval}; then
# echo "configure: error: called with --with-ssl, but ssl base directory ${withval} does not exist or is not a directory. Aborting config" 1>&2

Grant.

2007-01-23 21:13:07

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

Users have reported a symlink issue with my recent smbfs backport.
Turns out my backport overlooked a second 2.6 patch w/ the fix:
http://linux.bkbits.net:8080/linux-2.6/?PAGE=cset&REV=419e7b76CdrmRG_NZ8LKj9DUUBGu1w

This is a backport of Haroldo Gamal's 2.6 patch that fixes the symlink
issue, and also cleans up an unnecessary double assignment. As his
commit message notes, you will need the userspace patches from Samba
Bug #999 in order to use the permission/ownership assigned by the
server.

Signed-off-by: dann frazier <[email protected]>

diff --git a/fs/smbfs/inode.c b/fs/smbfs/inode.c
index be975fe..7fd9b51 100644
--- a/fs/smbfs/inode.c
+++ b/fs/smbfs/inode.c
@@ -513,10 +513,10 @@ smb_read_super(struct super_block *sb, void *raw_data, int silent)
mnt->flags = (oldmnt->file_mode >> 9) | SMB_MOUNT_UID |
SMB_MOUNT_GID | SMB_MOUNT_FMODE | SMB_MOUNT_DMODE;
} else {
- mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
- S_IROTH | S_IXOTH | S_IFREG;
- mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
- S_IROTH | S_IXOTH | S_IFDIR;
+ mnt->file_mode = S_IRWXU | S_IRGRP | S_IXGRP |
+ S_IROTH | S_IXOTH | S_IFREG;
+ mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
+ S_IROTH | S_IXOTH | S_IFDIR;
if (parse_options(mnt, raw_data))
goto out_bad_option;
}
diff --git a/fs/smbfs/proc.c b/fs/smbfs/proc.c
index 7f0794c..5570007 100644
--- a/fs/smbfs/proc.c
+++ b/fs/smbfs/proc.c
@@ -1994,10 +1994,11 @@ void smb_decode_unix_basic(struct smb_fattr *fattr, struct smb_sb_info *server,

if ( (server->mnt->flags & SMB_MOUNT_DMODE) &&
(S_ISDIR(fattr->f_mode)) )
- fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
+ fattr->f_mode = (server->mnt->dir_mode & S_IRWXUGO) | S_IFDIR;
else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
!(S_ISDIR(fattr->f_mode)) )
- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
+ fattr->f_mode = (server->mnt->file_mode & S_IRWXUGO) |
+ (fattr->f_mode & S_IFMT);

}

--
dann frazier

2007-01-23 21:35:15

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Wed, Jan 24, 2007 at 08:04:36AM +1100, Grant Coady wrote:
> Hi Dann,
> I'm running slackware-11, no smbfs package, 'smbmnt' is from samba-3.0.23c
> package with two tiny unrelated (?) patches:

Thanks again Grant. You might check out the patch I just submitted -
turns out this was an issue that was already fixed in 2.6, and that
fix is more complete than my previous one. You'll need the additional
userspace patches to use the server-provided perms (i.e., get rid of
the +x bits).

--
dann frazier

2007-01-23 22:11:49

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Tue, Jan 23, 2007 at 02:12:57PM -0700, dann frazier wrote:
> Users have reported a symlink issue with my recent smbfs backport.
> Turns out my backport overlooked a second 2.6 patch w/ the fix:
> http://linux.bkbits.net:8080/linux-2.6/?PAGE=cset&REV=419e7b76CdrmRG_NZ8LKj9DUUBGu1w

Perfect. Patch queued.
Thanks for your investigatinos, Dann.

Cheers,
Willy

2007-01-23 23:46:28

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Tue, 23 Jan 2007 14:12:57 -0700, dann frazier <[email protected]> wrote:

>Users have reported a symlink issue with my recent smbfs backport.
>Turns out my backport overlooked a second 2.6 patch w/ the fix:
> http://linux.bkbits.net:8080/linux-2.6/?PAGE=cset&REV=419e7b76CdrmRG_NZ8LKj9DUUBGu1w
>
>This is a backport of Haroldo Gamal's 2.6 patch that fixes the symlink
>issue, and also cleans up an unnecessary double assignment. As his
>commit message notes, you will need the userspace patches from Samba
>Bug #999 in order to use the permission/ownership assigned by the
>server.

Server-side:
grant@deltree:/home/other$ uname -r
2.6.19.2a
grant@deltree:/home/other$ ls -l
total 8
drwxr-xr-x 2 root root 96 2007-01-21 11:44 dir/
lrwxrwxrwx 1 root root 3 2007-01-21 11:43 dirlink -> dir/
-rw-r--r-- 1 root root 15 2007-01-21 11:43 file
lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
-rw-r--r-- 1 grant wheel 20 2007-01-24 10:24 test
lrwxrwxrwx 1 grant wheel 4 2007-01-24 10:23 testlink -> test

Client-side, 2.4.34c is with this new patch, 2.4.33.3 and 2.6.19.2
for comparison:

grant@sempro:/home/other$ uname -r
2.4.33.3
grant@sempro:/home/other$ ls -l
total 4096
drwxr-xr-x 1 root root 0 2007-01-21 11:44 dir/
lrwxrwxrwx 1 root root 3 2007-01-21 11:43 dirlink -> dir/
-rw-r--r-- 1 root root 15 2007-01-21 11:43 file
lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
-rw-r--r-- 1 grant wheel 20 2007-01-24 10:24 test
lrwxrwxrwx 1 grant wheel 4 2007-01-24 10:23 testlink -> test

grant@sempro:~$ uname -r
2.6.19.2a
grant@sempro:~$ ls -l /home/other/
total 10
drwxr-xr-x 1 grant wheel 0 2007-01-21 11:44 dir/
lrwxr-xr-x 1 grant wheel 3 2007-01-21 11:43 dirlink -> dir/
-rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
lrwxr-xr-x 1 grant wheel 4 2007-01-21 11:44 filelink -> file*
-rwxr-xr-x 1 grant wheel 20 2007-01-24 10:24 test*
lrwxr-xr-x 1 grant wheel 4 2007-01-24 10:23 testlink -> test*

grant@sempro:~$ uname -r
2.4.34c
grant@sempro:~$ ls -l /home/other/
total 4096
drwxr-xr-x 1 grant wheel 0 2007-01-21 11:44 dir/
lrwxr-xr-x 1 grant wheel 3 2007-01-21 11:43 dirlink -> dir/
-rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
lrwxr-xr-x 1 grant wheel 4 2007-01-21 11:44 filelink -> file*
-rwxr-xr-x 1 grant wheel 20 2007-01-24 10:24 test*
lrwxr-xr-x 1 grant wheel 4 2007-01-24 10:23 testlink -> test*

Grant.

>
>Signed-off-by: dann frazier <[email protected]>
>
>diff --git a/fs/smbfs/inode.c b/fs/smbfs/inode.c
>index be975fe..7fd9b51 100644
>--- a/fs/smbfs/inode.c
>+++ b/fs/smbfs/inode.c
>@@ -513,10 +513,10 @@ smb_read_super(struct super_block *sb, void *raw_data, int silent)
> mnt->flags = (oldmnt->file_mode >> 9) | SMB_MOUNT_UID |
> SMB_MOUNT_GID | SMB_MOUNT_FMODE | SMB_MOUNT_DMODE;
> } else {
>- mnt->file_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>- S_IROTH | S_IXOTH | S_IFREG;
>- mnt->dir_mode = mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>- S_IROTH | S_IXOTH | S_IFDIR;
>+ mnt->file_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>+ S_IROTH | S_IXOTH | S_IFREG;
>+ mnt->dir_mode = S_IRWXU | S_IRGRP | S_IXGRP |
>+ S_IROTH | S_IXOTH | S_IFDIR;
> if (parse_options(mnt, raw_data))
> goto out_bad_option;
> }
>diff --git a/fs/smbfs/proc.c b/fs/smbfs/proc.c
>index 7f0794c..5570007 100644
>--- a/fs/smbfs/proc.c
>+++ b/fs/smbfs/proc.c
>@@ -1994,10 +1994,11 @@ void smb_decode_unix_basic(struct smb_fattr *fattr, struct smb_sb_info *server,
>
> if ( (server->mnt->flags & SMB_MOUNT_DMODE) &&
> (S_ISDIR(fattr->f_mode)) )
>- fattr->f_mode = (server->mnt->dir_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFDIR;
>+ fattr->f_mode = (server->mnt->dir_mode & S_IRWXUGO) | S_IFDIR;
> else if ( (server->mnt->flags & SMB_MOUNT_FMODE) &&
> !(S_ISDIR(fattr->f_mode)) )
>- fattr->f_mode = (server->mnt->file_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) | S_IFREG;
>+ fattr->f_mode = (server->mnt->file_mode & S_IRWXUGO) |
>+ (fattr->f_mode & S_IFMT);
>
> }
>

2007-01-24 00:11:33

[permalink] [raw]

Subject: Re: problems with latest smbfs changes on 2.4.34 and security backports

On Wed, Jan 24, 2007 at 10:46:24AM +1100, Grant Coady wrote:
> On Tue, 23 Jan 2007 14:12:57 -0700, dann frazier <[email protected]> wrote:
>
> >Users have reported a symlink issue with my recent smbfs backport.
> >Turns out my backport overlooked a second 2.6 patch w/ the fix:
> > http://linux.bkbits.net:8080/linux-2.6/?PAGE=cset&REV=419e7b76CdrmRG_NZ8LKj9DUUBGu1w
> >
> >This is a backport of Haroldo Gamal's 2.6 patch that fixes the symlink
> >issue, and also cleans up an unnecessary double assignment. As his
> >commit message notes, you will need the userspace patches from Samba
> >Bug #999 in order to use the permission/ownership assigned by the
> >server.
>
> Server-side:
> grant@deltree:/home/other$ uname -r
> 2.6.19.2a
> grant@deltree:/home/other$ ls -l
> total 8
> drwxr-xr-x 2 root root 96 2007-01-21 11:44 dir/
> lrwxrwxrwx 1 root root 3 2007-01-21 11:43 dirlink -> dir/
> -rw-r--r-- 1 root root 15 2007-01-21 11:43 file
> lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
> -rw-r--r-- 1 grant wheel 20 2007-01-24 10:24 test
> lrwxrwxrwx 1 grant wheel 4 2007-01-24 10:23 testlink -> test
>
> Client-side, 2.4.34c is with this new patch, 2.4.33.3 and 2.6.19.2
> for comparison:
>
> grant@sempro:/home/other$ uname -r
> 2.4.33.3
> grant@sempro:/home/other$ ls -l
> total 4096
> drwxr-xr-x 1 root root 0 2007-01-21 11:44 dir/
> lrwxrwxrwx 1 root root 3 2007-01-21 11:43 dirlink -> dir/
> -rw-r--r-- 1 root root 15 2007-01-21 11:43 file
> lrwxrwxrwx 1 root root 4 2007-01-21 11:44 filelink -> file
> -rw-r--r-- 1 grant wheel 20 2007-01-24 10:24 test
> lrwxrwxrwx 1 grant wheel 4 2007-01-24 10:23 testlink -> test
>
> grant@sempro:~$ uname -r
> 2.6.19.2a
> grant@sempro:~$ ls -l /home/other/
> total 10
> drwxr-xr-x 1 grant wheel 0 2007-01-21 11:44 dir/
> lrwxr-xr-x 1 grant wheel 3 2007-01-21 11:43 dirlink -> dir/
> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
> lrwxr-xr-x 1 grant wheel 4 2007-01-21 11:44 filelink -> file*
> -rwxr-xr-x 1 grant wheel 20 2007-01-24 10:24 test*
> lrwxr-xr-x 1 grant wheel 4 2007-01-24 10:23 testlink -> test*
>
> grant@sempro:~$ uname -r
> 2.4.34c
> grant@sempro:~$ ls -l /home/other/
> total 4096
> drwxr-xr-x 1 grant wheel 0 2007-01-21 11:44 dir/
> lrwxr-xr-x 1 grant wheel 3 2007-01-21 11:43 dirlink -> dir/
> -rwxr-xr-x 1 grant wheel 15 2007-01-21 11:43 file*
> lrwxr-xr-x 1 grant wheel 4 2007-01-21 11:44 filelink -> file*
> -rwxr-xr-x 1 grant wheel 20 2007-01-24 10:24 test*
> lrwxr-xr-x 1 grant wheel 4 2007-01-24 10:23 testlink -> test*

Great, that's what I'd expect. If you patch your userspace, you can
avoid the executable bits.

--
dann frazier

2007-01-27 08:25:42