Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement secpolicy_vnode_setid_retain() #9043

Merged
merged 1 commit into from
Jul 26, 2019
Merged

Conversation

kusumi
Copy link
Member

@kusumi kusumi commented Jul 16, 2019

Motivation and Context

#9035

Description

Don't unconditionally return 0 (i.e. retain SUID/SGID).

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

How Has This Been Tested?

Added a test case functional/suid.
(I originally tried to add this to functional/privilege, but found this doesn't run on Linux.)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf
Copy link
Contributor

With this change applied we're now passing the test cases you referenced, correct?

Most filesystems make this decision within VFS by using a generic file write for fops.

Would you pointing me, and other reviewers, at the relevant VFS code which does this.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 16, 2019
@kusumi
Copy link
Member Author

kusumi commented Jul 16, 2019

With this change applied we're now passing the test cases you referenced, correct?

Correct, also the added test case is same as pjdfstest.

Would you pointing me, and other reviewers, at the relevant VFS code which does this.

via err = file_remove_privs(file); in __generic_file_write_iter().

@kusumi kusumi force-pushed the suid1 branch 3 times, most recently from 38e0c9e to bd4596b Compare July 17, 2019 05:08
@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #9043 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9043      +/-   ##
==========================================
+ Coverage   78.67%   78.69%   +0.02%     
==========================================
  Files         400      402       +2     
  Lines      121009   121004       -5     
==========================================
+ Hits        95199    95226      +27     
+ Misses      25810    25778      -32
Flag Coverage Δ
#kernel 79.56% <100%> (+0.01%) ⬆️
#user 66.15% <ø> (-0.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43a8536...b4acc91. Read the comment docs.

@kusumi
Copy link
Member Author

kusumi commented Jul 18, 2019

build zfs succeeds for BUILD, but failing for TEST.
It builds (and also runs) fine on my local environment.

  1. BUILD
Making all in suid
make[6]: Entering directory '/var/lib/buildbot/slaves/zfs/Ubuntu_18_04_x86_64__BUILD_/build/zfs/tests/zfs-tests/tests/functional/suid'
  CC       suid_write_to_suid.o
  CC       suid_write_to_sgid.o
  CC       suid_write_to_suid_sgid.o
  CCLD     suid_write_to_suid
  CCLD     suid_write_to_sgid
  CCLD     suid_write_to_suid_sgid
  1. TEST - preprocess can't find ./suid_write_to_file.h which exists in the same directory.
Making all in suid
make[8]: Entering directory '/tmp/zfs-build-buildbot-AqgPTMKN/BUILD/zfs-0.8.0/tests/zfs-tests/tests/functional/suid'
  CC       suid_write_to_suid.o
  CC       suid_write_to_sgid.o
suid_write_to_sgid.c:26:10: fatal error: ./suid_write_to_file.h: No such file or directory
 #include "./suid_write_to_file.h"
          ^~~~~~~~~~~~~~~~~~~~~~~~
suid_write_to_suid.c:26:10: fatal error: ./suid_write_to_file.h: No such file or directory
 #include "./suid_write_to_file.h"
          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
compilation terminated.

tests/zfs-tests/tests/functional/suid/Makefile.am Outdated Show resolved Hide resolved
tests/zfs-tests/tests/functional/suid/setup.ksh Outdated Show resolved Hide resolved
tests/zfs-tests/tests/functional/suid/Makefile.am Outdated Show resolved Hide resolved
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Signed-off-by: Tomohiro Kusumi <[email protected]>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 26, 2019
@behlendorf behlendorf merged commit 9fb6abe into openzfs:master Jul 26, 2019
allanjude pushed a commit to allanjude/zfs that referenced this pull request Aug 12, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 13, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 21, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 22, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 23, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 17, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 18, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 23, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
tonyhutter pushed a commit that referenced this pull request Sep 26, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes #9035
Closes #9043
allanjude pushed a commit to allanjude/zfs that referenced this pull request Oct 13, 2019
Don't unconditionally return 0 (i.e. retain SUID/SGID).
Test CAP_FSETID capability.

https://github.com/pjd/pjdfstest/blob/master/tests/chmod/12.t
which expects SUID/SGID to be dropped on write(2) by non-owner fails
without this. Most filesystems make this decision within VFS by using
a generic file write for fops.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tomohiro Kusumi <[email protected]>
Closes openzfs#9035
Closes openzfs#9043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants