Bug 7126 - [SMBD] With access denied error smbd return wrong NT_STATUS_OBJECT_PATH_INVALID error
[SMBD] With access denied error smbd return wrong NT_STATUS_OBJECT_PATH_INVA...
Status: RESOLVED FIXED
Product: Samba 3.4
Classification: Unclassified
Component: File services
3.4.5
x64 Linux
: P3 major
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
: 6616 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-10 17:10 UTC by bepi
Modified: 2010-02-14 06:58 UTC (History)
0 users

See Also:


Attachments
Fix for wrong NT_STATUS_OBJECT_PATH_INVALID error (1.66 KB, text/x-patch)
2010-02-10 17:12 UTC, bepi
no flags Details
patch for master. (1.63 KB, patch)
2010-02-12 16:50 UTC, Jeremy Allison
no flags Details
Correct patch for master (1.58 KB, patch)
2010-02-12 17:33 UTC, Jeremy Allison
no flags Details
git-am format patch for 3.5.x. (2.29 KB, patch)
2010-02-12 18:48 UTC, Jeremy Allison
no flags Details
git-am format patch for 3.4.6. (2.10 KB, patch)
2010-02-12 18:53 UTC, Jeremy Allison
no flags Details
git-am format patch for 3.3.11. (2.09 KB, patch)
2010-02-12 18:54 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description bepi 2010-02-10 17:10:53 UTC
Hi.
If the samba login user does not have local access permission to the shared directory, the error returned by smbd must be NT_STATUS_ACCESS_DENIED and not  NT_STATUS_OBJECT_PATH_INVALID.

The wrong errors returned by samba are not a small bug, but can lose a lot of time at many users of samba.

And, for example, the kioslave-smb browsing program not ask to the user the
new account data, and this cause the fail of the share browse.
Comment 1 bepi 2010-02-10 17:12:51 UTC
Created attachment 5323 [details]
Fix for wrong NT_STATUS_OBJECT_PATH_INVALID error
Comment 2 Jeremy Allison 2010-02-12 16:50:03 UTC
Created attachment 5339 [details]
patch for master.

I think this is a more correct fix. We can't return different error codes here such as NT_STATUS_OBJECT_PATH_INVALID - Windows clients might not accept this, we have to be careful about the values we return, so I'm keeping this as NT_STATUS_BAD_NETWORK_NAME, which is the correct error for a bad share.

I think the attached is a more correct patch. Please test and report back.

Thanks,

Jeremy.
Comment 3 Jeremy Allison 2010-02-12 17:03:22 UTC
Actually, I'm not sure my patch is correct either. This is the same as a share definition on Windows with an ACL allowing FULL_ACCESS to everyone, but a path that is owned and restricted to Administrators only. I'm going to look at what Windows does in this case. It might be that the client author needs to fix their code.
Jeremy.
Comment 4 Jeremy Allison 2010-02-12 17:33:00 UTC
Created attachment 5340 [details]
Correct patch for master

This is the correct fix (IMHO). We need to allow connections to a directory with no permissions for the user, but fail all operations with ACCESS_DENIED errors.
The previous code was incorrect in that it errored out with NT_STATUS_BAD_NETWORK_NAME if the SMB_VFS_STAT call failed with errno = EACCESS, whereas it should have silently ignored the error (as does Windows).
Jeremy.
Comment 5 Jeremy Allison 2010-02-12 18:48:57 UTC
Created attachment 5341 [details]
git-am format patch for 3.5.x.

Volker, take a look at this for some version of 3.5.x. (either rc3, or 3.5.1, I don't mind which).
Jeremy.
Comment 6 Jeremy Allison 2010-02-12 18:53:11 UTC
Created attachment 5342 [details]
git-am format patch for 3.4.6.
Comment 7 Jeremy Allison 2010-02-12 18:54:40 UTC
Created attachment 5343 [details]
git-am format patch for 3.3.11.
Comment 8 Jeremy Allison 2010-02-12 20:20:02 UTC
Comment on attachment 5341 [details]
git-am format patch for 3.5.x.

This patch is incorrect - please ignore.
Comment 9 Jeremy Allison 2010-02-12 20:20:28 UTC
Comment on attachment 5342 [details]
git-am format patch for 3.4.6.

Incorrect - ignore.
Comment 10 Jeremy Allison 2010-02-12 20:21:20 UTC
Comment on attachment 5343 [details]
git-am format patch for 3.3.11.

This patch is incorrect. Please ignore.
Comment 11 Jeremy Allison 2010-02-12 20:26:36 UTC
Ok, I'm really not sure about this one, so I've reverted my fix. I must consider this a lot more, this is subtle stuff :-).

Jeremy.
Comment 12 bepi 2010-02-13 10:57:45 UTC
(In reply to comment #11)
> Ok, I'm really not sure about this one, so I've reverted my fix. I must
> consider this a lot more, this is subtle stuff :-).
> 
> Jeremy.
> 

I have tested my patch with Fedora 11 and Windows Vista, and both systems work properly, the two systems just require to the user the access data for shared directory.
Comment 13 Jeremy Allison 2010-02-13 12:00:23 UTC
Yes but your patch is *wrong*, as defined as how Windows behaves in this scenario. I've added a more complete fix to master, which causes smbd to behave like Windows does. The correct behavior when the connecting user has no access permission to the shared directory is to allow the connection, but cause every access to return NT_STATUS_ACCESS_DENIED.

Master now does this and it will be in release 3.6.0. Let me know if you need this fix in any earlier version.

Jeremy.
Comment 14 bepi 2010-02-13 12:31:52 UTC
(In reply to comment #13)
> Yes but your patch is *wrong*, as defined as how Windows behaves in this
]zac[

Thanks for the Windows internal info.

Thinking better, my patch could cause problems to the Windows clients if they use different account simultaneously for connect to shared directorys.

The fix on the 3.6.0 release is good for me.

Gdb
Comment 15 bepi 2010-02-14 06:51:07 UTC
I have tested your patch on my samba 3.4.5 with windows vista and fedora 11,  these work very well.

Thanks for all Jeremy.

Gdb
Comment 16 bepi 2010-02-14 06:58:41 UTC
*** Bug 6616 has been marked as a duplicate of this bug. ***