patchGNU nano - Patches: patch #10277, files: take in account CAP_DAC_*...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #10277: files: take in account CAP_DAC_* and immutable flag on permission check

Submitter:  Nekun <nekun>
Submitted:  Sat 10 Sep 2022 10:27:12 AM UTC
   
 
Priority:  5 - Normal Status:  Need Info
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Release:  None

Sun 12 Mar 2023 09:53:38 AM UTC, comment #3: 

Closing due to lack of response.

Benno Schulenberg <bens>
Group administrator
Sun 11 Sep 2022 02:50:45 PM UTC, comment #2: 

Thanks for the patch.  However, I know nothing about capabilities, so... I will need a review from someone who does.  The best chance to get this is when you send the patch to the nano-devel mailing list.

But before you do that, I have some comments about some details of the patch that you may want to fix:

  • Instead of is_accessible(...) == FALSE, I would write !is_accessible(...).
  • In accessible = (access(path, mode)) == 0; the last closing parenthesis is in the wrong place -- it should be before the semicolon.  Same thing three lines down.
  • The statement capable &= statable; makes 'capable' FALSE whenever 'statable' is FALSE, making the code before it pointless work.  It would be better to change the governing if (!accessible) to if (!accessible && statable).
  • The fragment with the two 'goto' statements is horrible.  It needs to be rewritten without any goto or label -- which is easy, and will be shorter too.
  • The comments are too verbose.  Detailed explanations should be in the commit message, not in the comments.  Also, the grammar needs some attention -- here and there the word "is" is missing, for example.
Benno Schulenberg <bens>
Group administrator
Sat 10 Sep 2022 10:37:52 AM UTC, comment #1: 

Just for testing:

~$ alias sufs='bash -c "sudo capsh --user=nobody --caps=\"cap_dac_override=eip cap_setpcap=ep\" --secbits=48 --addamb=cap_dac_override -- -c \"\${*:0}\""'

~$ alias sufsr='bash -c "sudo capsh --user=nobody --caps=\"cap_dac_read_search=eip cap_setpcap=ep\" --secbits=48 --addamb=cap_dac_read_search -- -c \"\${*:0}\""'

~$ sufsr cat /root/.bashrc

~$ sufs tee /root/new <<< lol

~$ sufs nano /etc/localtime
# "File /etc/localtime is unwritable"

~$ sufsr nano /root/.bashrc
# "Path /root is unaccessible"

~$ mkdir test; cp /etc/nanorc test/

~$ sudo chown root:root test; sudo chmod 700 test

~$ sufsr nano -f test/nanorc
# "Specified rcfile does not exist"

Nekun <nekun>
Sat 10 Sep 2022 10:27:12 AM UTC, original submission:  

Introduce more accurate file permissions checking on Linux: request
CAP_DAC_OVERRIDE | CAP_DAC_READ_SEARCH process capabilities and
linux-specific 'immutable' inode attribute to determine actual perms
when running in non-zero uid environments with DAC-override capabilities.
In such environments access() lies, pretends that user is still obeying
DAC restrictions, only overriding by uid 0 handled correctly.

Wrong permissions check produces errors "File is unwritable / Path
is not accessible", prevents from creating lockfiles and reading nanorc
from paths which is actually accessible.

To avoid adding libcap dependency, corresponding syscall invoked
directly, it's API/ABI stable (should work even on 2.2.x kernels) and
not too complex when accessing only first 32-bits caps from effective set.

Nekun <nekun>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by nekun (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-03-12 bens Open/ClosedOpen Closed
    2022-10-28 bens StatusNone Need Info
    2022-09-10 nekun Attached File- Added 0001-files-take-in-account-CAP_DAC_-and-immutable-flag-on.patch, #53675

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code