Zfs: security issue: sharenfs always gives read access for world

[ggzengel]

Since 0.6.4 sharenfs always gives read access to everyone.

root@xenserver1:~# zfs set [email protected],async,no_subtree_check,no_root_squash,no_all_squash,nohide,insecure zpool1/media/test_nfs 

root@xenserver1:~# grep test_nfs /var/lib/nfs/etab 
/srv/nfs/media/test_nfs 1.2.3.4(ro,async,wdelay,nohide,nocrossmnt,insecure,no_root_squash,no_all_squash,no_subtree_check,secure_locks,acl,anonuid=65534,anongid=65534,sec=sys,ro,no_root_squash,no_all_squash)
/srv/nfs/media/test_nfs *(ro,sync,wdelay,hide,nocrossmnt,secure,root_squash,no_all_squash,no_subtree_check,secure_locks,acl,anonuid=65534,anongid=65534,sec=sys,ro,root_squash,no_all_squash)

root@xenserver1:~# zfs set [email protected],[email protected],async,no_subtree_check,no_root_squash,no_all_squash,nohide,insecure zpool1/media/test_nfs 

root@xenserver1:~# grep test_nfs /var/lib/nfs/etab 
/srv/nfs/media/test_nfs 1.2.3.4(ro,async,wdelay,nohide,nocrossmnt,insecure,no_root_squash,no_all_squash,no_subtree_check,secure_locks,acl,anonuid=65534,anongid=65534,sec=sys,ro,no_root_squash,no_all_squash)
/srv/nfs/media/test_nfs 1.1.1.1(rw,sync,wdelay,hide,nocrossmnt,secure,root_squash,no_all_squash,no_subtree_check,secure_locks,acl,anonuid=65534,anongid=65534,sec=sys,rw,root_squash,no_all_squash)
/srv/nfs/media/test_nfs *(ro,sync,wdelay,hide,nocrossmnt,secure,root_squash,no_all_squash,no_subtree_check,secure_locks,acl,anonuid=65534,anongid=65534,sec=sys,ro,root_squash,no_all_squash)

[FransUrbo]

Where did you get your software? Exactly…

Also see #2790

[ggzengel]

root@xenserver1:~# uname -a
Linux xenserver1 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt9-2 (2015-04-13) x86_64 GNU/Linux
root@xenserver1:~# cat /etc/apt/sources.list.d/zfsonlinux.list

## This file is installed by the zfsonlinux package.

#deb http://archive.zfsonlinux.org/debian wheezy main
deb http://archive.zfsonlinux.org/debian jessie main
#deb-src http://archive.zfsonlinux.org/debian wheezy main contrib

[FransUrbo]

Ok, then it's my patch. I'll look into it. Could you, in the meantime modify your initial comment and use the "triple back ticks option" to make it easier to read?

[ggzengel]

If you look at this code perhaps you can fix this segfault?

root@xenserver1:~# zfs set sharenfs=( zpool1/media/test_nfs
Segmentation fault

[FransUrbo]

root@xenserver1:~# zfs set sharenfs=( zpool1/media/test_nfs
Segmentation fault

I'll have a look at that to. Thanx.

[ggzengel]

I didn't find your changes in zfsonlinux repository.
Does debian have an other repo?

[FransUrbo]

Yes: https://github.com/zfsonlinux/pkg-zfs (and https://github.com/zfsonlinux/pkg-spl).

Tags:

spl => master/debian/wheezy/0.6.4-1-wheezy
zfs => master/debian/wheezy/0.6.4-1-2-wheezy

[ggzengel]

The upstream branch in this repository is an unmodified copy of the http://github.com/zfsonlinux/zfs.git mainline.

But I didn't find changes. The last change was in 0.6.3. Where are the changes for 0.6.4?
I'm searching in https://github.com/zfsonlinux/zfs/tree/zfs-0.6.4-release/lib/libshare.

[FransUrbo]

I just told you where you could find the packages repository.

[ggzengel]

Does that means the debian packages are made from unseen/untrusted source?

[FransUrbo]

Does that means the debian packages are made from unseen/untrusted source?

What do you mean by that? They're made by me, in the ZoL name and signed
with my personal key. As it say on the ZoL website...

[ggzengel]

I'm searching the changes you made and didn't find them.
I suggest the changes must be made in libshare but it doesn't change since 0.6.3.
I can not trust you if I don't know that the package you made is from official code base.

It's like the first sentence in this discussion:
http://security.stackexchange.com/questions/41734/trust-issues-relative-to-open-source

[FransUrbo]

If you look at the repos I gave you, you'll find it.

[ggzengel]

Do you have a link for me?

[FransUrbo]

Look further up.

[ggzengel]

Ok, then it's my patch.

Please give me a link to this patch.

[FransUrbo]

https://github.com/zfsonlinux/zfs/issues/3319#issuecomment-94291692

[ggzengel]

But this change is not official in 0.6.4.
Why do you put code which nobody expects?

[FransUrbo]

Because I've tested it dailies for several months, and no one complained. So I assumed it was stable.

[ggzengel]

I think I can speak for others and nobody wants hidden code in official packages.
That's what I mean with trust.
I honor your work you do and use zfsonlinux intensive. Please stay on official code base.

[FransUrbo]

Why?

[ggzengel]

1. quality management
2. reproduction

[ggzengel]

1. documentation

[FransUrbo]

How about trying to fix a huge problem with the existing code?

But we can debate this all year. The code stays. And I'm going to fix it.

[behlendorf]

@FransUrbo I disagree. Issue #2790 is not a huge issue with the existing code. It's true there is a problem but it impacts a limited number of users who are setting up more complicated share options. In those case they may be better off setting up an /etc/exports file anyway. I'd strongly encourage you to drop this patch from the Debian packages.

[FransUrbo]

It should be fixed in 0.6.4-1-2.1-wheezy (and 0.6.4-1-6-wheezy for the dailies).

If I get more reports that it doesn't work, I'll reconsider backing it out. Unless someone wants to take over the repo building that is...

[ggzengel]

I hope you will still build a lot of packages, but ...

... it's a breach of etiquette to build undocumented packages and surprise the community.

The most time I look for closed issues to make a rough estimate what happens on next update.
Please make a fork for 0.6.4-1-2.1-wheezy to have a look for the code. Have you made more changes?

Please correct this:
Upstream Repositories
The upstream branch in this repository is an unmodified copy of the http://github.com/zfsonlinux/zfs.git mainline.

[FransUrbo]

@ggzengel I don't know what you mean by "build undocumented packages". But since @kpande is now waging war against me, let's dispel something:

http://list.zfsonlinux.org/pipermail/zfs-announce/2015-April/000002.html

It is _very clearly_ said at the bottom that I'm including my three libshare patches [that's been waiting in the 'in queue' for years]. These have been running not only on my own primaries (WITHOUT this or the previous issue - http://list.zfsonlinux.org/pipermail/zfs-announce/2015-April/000004.html even been indicated) since 0.6.3 dailies. Neither have ANYONE that uses/used daily had anything to say about this. I don't know (and can't find out easily) how many is actually using dailies, but "there's a few"…

I therefor MUST conclude that the patch(es) was working just fine. They do not touch the 'inner workings of ZoL/ZFS', so the data is safe (which is another reason I felt comfortable with including it).

And I'm also not alone in including 'extra' patches.

But if you really want, require or even DEMAND 'pure breed' packages, I recommend building your own either from tar ball or GIT. It's only a matter of reading http://zfsonlinux.org/generic-deb.html.

Now, @ggzengel do my updated packages fix the issue, yes or no? Or didn't you try, and prefer to bitch and whine about this some more?

[aqw]

@FransUrbo First of all, I'd like to thank you for putting the time and effort into building and maintaining these packages.

While I can see that you carefully selected the PRs that you included, those PRs are not in master for a reason (and Brian is a very reasonable maintainer). It is common practice (/especially/ for an official upstream repo vs some-random-PPA) to package as vanilla as possible. It's convention, helps auditability, and is also a matter of respect for the sysadmins who are depending on the packages you kindly provide.

If you decide to go vanilla, please let us know. Otherwise, I (and I assume many others) will do our own builds. Also, the ZoL website will have to be updated to no longer include the following phrase:

These packages track the latest official upstream tag and are refreshed as new releases are made available."

---Alex

[FransUrbo]

This issue is against pkg-zfs, not 'zfs. So it should be closed and moved.

Secondly, I consider the issue sharenfs gives read access for world fixed. If someone wants to push the issue against me supplying the Debian GNU/Linux community with added functionality, without sacrificing or risking the users data, I suggest opening a separate issue about this in the pkg-zfs issue tracker.

In either case, this issue should be closed as "invalid".

[FransUrbo]

You can disagree how much you like - there is no security issue in the "zfs" (and this issue therefor does not apply).

[ggzengel]

@FransUrbo thx, the security issue is gone.
If you come from http://zfsonlinux.org/debian.html or http://zfsonlinux.org there is nothing written of PRs or ZFS-PKG. It's only written every thing is good and code is from http://github.com/zfsonlinux/zfs.
If you are not an insider and didn't know exactly about mailing lists, spl, zfs, zfs-pkg, PRs it's not transparent.
@All please fix docu about these dependencies

[FransUrbo]

The relationship between all these and where to report issues is documented in https://github.com/zfsonlinux/zfsonlinux.github.com/pull/27 which is currently waiting for acceptance.

As to where to document the additional patches added, other than in the changelog of the package, the git commit log AND in the release document, I don't think the debian.html page is suitable for this. As far as I see, it is quite irrelevant what extra patches is applied, as long as any issues is dealt with swiftly and accurately.

WHEN/If an issue have been detected in these, I think it's important that this is identified and acknowledged and if no fix can be found in a reasonable time frame, then a backing out of this is (and will be) ONE course of action.

@ggzengel Since the security issue is now fixed, as you have acknowledged AND this is not an issue against ZFS, would you be so kind to close the issue?

If you still feel that the fact that I'm adding extra patches, without specifying this on the 'front door' (so to speak) - but in SEVERAL other places - then please open a special issue about this on the pkg-zfs issue tracker.

I CAN say though, that it's going to be much, much convincing for me to do this reverting these patches, but I _will_ consider it IF there's a proper issue with a proper rational for doing so. And enough people 'sign of' on it.

[behlendorf]

@FransUrbo thank you for promptly addressing the issue, and @ggzengel thank you for verifying the fix. I'm glad it was resolved quickly, but this has clearly exposed a problem and multiple people have expressed what I consider legitimate concerns. The heart of which is that end users should be able to know exactly what they are running. This is important for obvious reasons that I hope we can all agree on.

• Documentation - It needs to be clear what a tag means and it needs to be communicated clearly to end users. This is definitely an area where we could improve and the updated releases page is a small step in that direction. If distribution maintainers add additional features or functional changes, and I strongly discourage this, that needs to be clearly documented somewhere. The distribution download page would be a good location or we could look at enabling the project wiki.
• Quality control - Despite everyone's best efforts and good intentions it's surprising easy for flaws to accidentally be introduced. This is going to happen from time to time but it's less likely the more review and testing a patch has seen. This is in part why I like the testing repositories. It allows for more active and knowledgeable users to help us catch issues before end up in a tag. It also makes it clear what those users are signing up for.
• Reproducible - We have to be able to determine where and when an issue was introduced and how widespread the potential impact is. Nobody likes chasing a bug in the issue tracker which doesn't exist in the code base they're working with.

Definitely part of the problem here is that as the project lead I haven't released official tags very frequently. This can force the distribution maintainers to apply patches from master to address issues their users are encountering. I'd like to try and attack this problem on a few fronts.

• x.y.z-release branch - Maintain an official release branch for distribution maintainers with more frequent point releases that address _only_:
  
  
  
  • Critical stability issues
  
  
  • Critical security issues
  
  
  • Build issue due to upstream kernel.org changes
  
  
• Shorter development cycles on master - The last two development cycles took way to long. I'd like to try and shorten to development cycle to ~3 months. All new major features and functionality must be finalized and merged during the first two months. Then master would be frozen for a month with only bugs fixes made as we verify no regressions have been introduced. Once I'm happy the tree is in a good place I'll tag a new release. This is something I've been thinking about for a while and now that the 0.6.4 release is out I'll write up my thoughts and post some long overdue developer documentation.
• Distribution repositories - Distribution maintainers should have their default ZFS repository track the official upstream point releases. If there's a critical issue they need addressed they can make their case to the maintainer of the release branch so all distributions can easily pick it up. But I also think it's also entirely appropriate for distribution maintainers to provide _additional repositories_ at their discretion. For example, a testing repository which tracks the master branch is very convenient. Alternately a non-default repository which include additional functionality / features not yet included upstream.

Anyway, that's where I stand on things. I'll attempt to distill some of this down to some proper documentation for contributors and get it posted in a section of the website.

[ggzengel]

nothing changes?
FYI: #3803