Created attachment 6575 [details] git patch as text file I'm not adamant about backporting the fix, but it might be a good idea to backport this to the time that the regression was introduced (i.e. client's managing widelinks (sysmlinks on the server) could be a security risk, and was thus disabled). At the very least I'd like to see it early in the 3.6 series if though with 360 in rc2, I understand the "process discomfort" associated with putting it in there -- if I was a project manager, I'd say no as well, but as a user, I can still want it! ;-) The impact on the code is trivial. To minimize the security risk, I'd only announce the feature or new param, in the context of a 'bugfix' to allow previous functionality. That way in considering Andrews comments about some users wanting to try out anything 'new' w/o reading the documentation, such problems might be ameliorated. Tested under 3.5.7 (opensuse) as well as 3.6.0-rc2(tarball).
Jeremy, I think this is mainly up to you to comment on. I haven't tested the patch (it needs some minor reformatting), but you are the strongest proponent of not allowing unix extensions and wide links. Volker
For my future knowledge, what minor reformatting is needed? I thought I followed the existing code and examples, exact, so what'd I miss? (*curious* -- thanks)...
- return lp_widelinks_internal(snum); + if (lp_client_managed_widelinks() + || !lp_unix_extensions()) return lp_widelinks_internal(snum); + + return false; } We always put { } and indentation into the body of if-statements, even if they are just one-liners. Volker
-O- Arigator gazaimus... (thank you :-))
Not for 3.6.0 final. Maybe for 3.6.1. Reducing importance to "normal" from critical. I'll look at this after 3.6.0 ships. Jeremy.
*** Bug 8318 has been marked as a duplicate of this bug. ***
BTW - I'd like to see this backported into the 3.5 series as well. 3.6 introduces some new challenges, that I'd like the possibility to avoid..... I.e. I'd like to see it in a 3.5.10... I'd be willing to do a separate git patch against it if there's any need to do so, (am guessing the current patch will apply against 3.5.x since it was originally developed against 3.5.8, and when originally applied to 3.6.0, applied with some 'fudge' but just fine.
Jeremy, is there a chance to get a review for 3.5.11? Or do you think it should be picked for 3.5.12? Thanks!
Unless there is a reason not to push this out... Just to make sure i's are dotted and t's crossed, I think I should refile this bug against 3.5 so the patch can be sure to be applied there for next release. The issue of wide links breaking came up on the suse list where it was thought that the 'wide-link' disable bug fix from feb 2010 only affected the 3.6 series, thus they thought they were 'covered for wide-link usage using 3.5.7. I did my best to disillusion them, though, as is often the case when does so, I don't know if it was appreciated. Apparently Yet another user, upon some recent upgrade (11.3 had suse 3.5.2, which I think had the 'quick-fix'...11.4 has 3.5.7, 11.2 had 3.4.2), got bit by this as their users use such symlinks -- so they are installing a PC-file access client apart from SMB to overcome this limitation.... (sigh)... I did mention a patch had been submitted...but hadn't been approved or integrated yet...
Could you change this parameter to "global" from "service" context?
There would be no benefit in doing that. The wide links=enabled parameter is already a per-share param, so if you don't want wide links to work on a a share, then you wouldn't enable them. But if asking to have this only on 1 share gives a bogus sense of security. Since a user on that share can create a wide link anywhere on the server. So limiting wide links to 1 share would be no more or less effective than only allowing people in group 'Administrators' have write access to the share where you have enabled wide links. I.e. group controls would be far more flexible. The unix_extensions parameter that disables wide links, does so on a server wide basis, thus the parameter that overrides that needs be server wide as well. Additionally, if you try to do it on a share basis you are asking for problems -- if someone mounts a share that overlaps another and they have different security restrictions, then in some cases the link would be followed, and in other cases, not. That could be a real pain to figure out.... There's no 'half way', If your users have local server access, then this will give them no accessibility increases, but if they don't, and you don't WANT them to have such access, then you can't allow this on ANY share. Enabling this for only 1 share might give the wrong impression (or some false understanding of the security consequences)... At least, this is my understanding given the nature of the problem.
God I really hate this patch :-). How about we call this "allow insecure widelinks" instead. Then I might feel better about this. Jeremy.
If that's what float your boat! I tried to have a name that was descriptive of what it did rather than adding an 'opinion' to it... The links are no more insecure than if the user has access to the server directly. I.e. unless you claim all symlinks are insecure...that's not a fair statement. But hey, whatever works...you wanna call it 'insecure_R_us=yes/no', whatever... just trying to keep it neutral...but descriptive.. as it gives client computers the ability to manage the widelinks...and all the 'security' (or insecurity) that such entails... no?
But I *am* judgemental about this patch, I think it's a horrible idea :-). I want people to know if they set this they're deliberately removing a layer of security that they'd otherwise get. Jeremy.
Jeremy, we have 'force user = root'. If people want to screw up themselves, they can do so today. Make the parameter name look scary, and get it in. My 2ct. Volker
Jeremy, am I incorrect in my assertion that having this ability would be the same as the person logged into the client computer had access to the server, and could create symbolic links that point anywhere? I.e. the problem, as I understood it, is that when used with the unix enhanced calls, it allows remote creation of symlinks directly from a client. Vs. having to ssh into the same server and create them on the server? Does that sum up the effect of this? So if a site has ssh access setup for users to the server in addition to getting files from it, is there a difference in security between them doing remotely vs. locally? This is *presuming* they are running as the same user in both cases, so the same file create permissions would apply in the target directory in both cases (i.e. samba would give them no additional permissions to create a file in that directory than if they were logged in local with the same login. If the above is true, then how is this a horrible security flaw when there site policy is "allow users to create symlinks anywhere they have permission to create them".... In the context of that site policy, how is this a horrible idea/horrible security problem? (I may not understand some of the issues -- this is why I am asking)...
(In reply to comment #14) > But I *am* judgemental about this patch, I think it's a horrible idea :-). I > want people to know if they set this they're deliberately removing a layer of > security that they'd otherwise get. > Jeremy. FWIW, you may be judgemental about it, but I don't think adding personal judgments and feelings about people's site policies would generally be a good thing, I'd prefer to see description and option names to be explicit but without built-in judgment about their use -- if they want 'advice' (someone's judgment) on how to use the option, it seems they'd ask for 'example' or 'suggested use'...etc. But you could just as easily put a spin on any option you didn't like...say, lanman security -- why not rename it to 'bad security option 1'... because lanman security is description of what it is even though it may not be good security compared to other options. Does that rationale make sense? This option was builtin to samba for AGES... and it wasn't considered a problem until someone pointed potential for abuse. It's not introducing a new feature, it's restoring lost functionality that was disabled due to a need to immediately address a potential 'hole'. That doesn't mean the feature, when used with a consistent security policy is 'bad'...(or if it does, I'd like to know why creates a worse consequence than allowing users to have local server access). I *do* understand that many sites don't allow that -- and for them, allowing a client to manage 'wide links' (symlinks that point outside of a share boundary), would be a big potential problem. But if you are not one of those sites, and have a 'permissive site' where you trust users with local access, etc (and SURE, local access users could abusively crash or hack hack into a machine...but giving them local access means you trust them). In the context of trusted users, who already have access to the server, then what layer of security does this patch remove?
I'm in the process of putting it in, but I'm naming it "allow insecure widelinks". The reason for that is I want people to *think* before setting it to "true". Linda Walsh wrote : > "This option was builtin to samba for AGES... and it wasn't considered a problem > until someone pointed potential for abuse. It's not introducing a new feature, > it's restoring lost functionality that was disabled due to a need to > immediately address a potential 'hole'. " Yes that's the point. It was disabled explicitly as it has an effect that most people didn't expect, and caused a youtube video to be posted that had a disasterous effect on our security reputation. Such things are not easy to get back once lost. The combination of following widelinks, with unix extensions on by default is a big problem for most users of Samba who are simply exporting an SMB1/SMB2 share without any other access to the box. I am making a value judgement here that most "normal" users do not want this option turned on. In order to let them know that, I need it to be equated with an insecure name. The reason I'm adding it at all is that I understand that sophisticated sites need this functionality back. These people aren't going to be upset by a scary name, they know what they're doing. But for the people who are naively setting Samba up, I want them to know this isn't something they should be doing. Don't complain too much, it's going in after all. Jeremy.
If you label this one as unsafe, you are opening a can of worms. Does this imply that any options aren't so labeled are secure? I.e. by adding value/judgment to data you are sending out, you are violating the equivalent of a 'safe harbor provision'...as ISP's are protected by ... as long as they don't attempt/claim to filter, they are regarded as neutral parties and not held responsible for content, but if they did, then they would. You'd also, by implication, be saying linux-symlinks are unsafe, as that is all they are. It's only in the context of segmented-exported shares where the users don't have access to the server that this is an issue. In my env, I have root exported, so there's no way it's these likes are any less secure than anything else. (no I don't have that server on the internet!)...
Be happy the feature is going in. Don't complain about the name. Jeremy.
Created attachment 6883 [details] git-am fix for 3.6.1. Fix I've tested in 3.6.1 (and pushed to master). Jeremy.
Pushed to v3-6-test. Closing out bug report. Thanks!