clalancette / pycdlib

Python library to read and write ISOs
GNU Lesser General Public License v2.1
147 stars 38 forks source link

Unable to get POSIX permissions from files on a RockRidge ISO #33

Closed F5-jansen closed 5 years ago

F5-jansen commented 5 years ago

I'm attempting to write a modified file back to an opened ISO while preserving its permissions. It seems that this would work fine for modify_file_in_place, but since I can't guarantee that the modified file will be <= the block count of the original file I'm following a remove-and-add approach instead.

Unfortunately, when I call get_file_from_iso(dest, rr_path=src) the file which is written to my local disk is created using the default umask instead of the permissions from the file on the ISO. Perhaps I'm missing something, but I don't see any kind of flag for controlling this behavior. Something equivalent to the file_mode parameter for add_file and add_directory would be nice here.

Similarly, I don't see this information anywhere in the DirectoryRecord objects that we can get by walking the ISO or calling get_record. I tried diving into the code where it's traversing inodes and such, but I wasn't able to pinpoint exactly where the POSIX bits might be available.

Is this a supported feature, planned, unplanned, or infeasible?

clalancette commented 5 years ago

I'm attempting to write a modified file back to an opened ISO while preserving its permissions. It seems that this would work fine for modify_file_in_place, but since I can't guarantee that the modified file will be <= the block count of the original file I'm following a remove-and-add approach instead.

Yep, that's the correct way to go about it.

Unfortunately, when I call get_file_from_iso(dest, rr_path=src) the file which is written to my local disk is created using the default umask instead of the permissions from the file on the ISO. Perhaps I'm missing something, but I don't see any kind of flag for controlling this behavior. Something equivalent to the file_mode parameter for add_file and add_directory would be nice here.

No, you are not missing anything. It's just something that I haven't really cared about up until now, so it does indeed just write out the file with the default umask.

Similarly, I don't see this information anywhere in the DirectoryRecord objects that we can get by walking the ISO or calling get_record. I tried diving into the code where it's traversing inodes and such, but I wasn't able to pinpoint exactly where the POSIX bits might be available.

It is true that the ISO filesystem does not have this information, but the Rock Ridge extensions do store this. See https://github.com/clalancette/pycdlib/blob/master/pycdlib/rockridge.py#L402 (which pretty much all Rock Ridge ISOs have).

Is this a supported feature, planned, unplanned, or infeasible?

It's totally feasible, though only when manipulating the file via Rock Ridge or UDF (we don't know the relevant via ISO or Joliet). The question is how to do it (which is kind of why I haven't done it up until now).

There are two main ways we could go about this:

  1. Try to set the permission bits as we extract the file. The problem with this is that it is unclear to me how this interacts with the users umask, and also whether we should do the same for UID and GID. That won't always work, however, since if the UID/GID is 0/0 on the ISO, you'd need to be sudo for this to work.
  2. Provide a nice way to get the permission bits so you can pass it back in when calling add_to_iso. I'll note that this is actually feasible right now with something like:
import pycdlib
iso = pycdlib.PyCdlib()
iso.open('/path/to/iso')
rec = iso.get_record(rr_path='/path/on/iso/to/modify')
iso.get_file_from_iso('local_path', rr_path='/path/on/iso/to/modify')
iso.rm_file(rr_path='/path/on/iso/to/modify')
modify('local_path')  # whatever you want to do to modify the file here

file_mode = None
if rec.rock_ridge is not None:
    if rec.rock_ridge.dr_entries.px_record is not None:
        file_mode = rec.rock_ridge.dr_entries.px_record.posix_file_mode
    elif rec.rock_ridge.ce_entries.px_record is not None:
        file_mode = rec.rock_ridge.ce_entries.px_record.posix_file_mode

iso.add_file('local_file', rr_path='/path/on/iso/to/modify', file_mode=file_mode)
iso.close()

But that bit around the file_mode is pretty wordy, depends on a lot of deep knowledge of the code, and touches APIs that I don't guarantee to be stable. I think the nicer way to do it would be to add a new method like file_mode() that returns None if it doesn't apply, or does the above logic and returns the result if it does.

After working it out above, I highly prefer the 2nd approach. What do you think about that? If you think that will work for you, I can whip up a patch in the next few days.

Thanks for the bug report!

clalancette commented 5 years ago

Update: look at https://github.com/clalancette/pycdlib/commit/0ef1faac63fb13e1d85024ce895944efb4e112f4 for what I'm thinking about adding to support this.

F5-jansen commented 5 years ago

This makes sense. The first approach is too ambiguous and would require pycdlib to either:

  1. Do things on behalf of the user without telling them (IE: writing files as the local user if the current user is not root but the file in the ISO belongs to root).
  2. Complicate the API with lots of confusing options for permissions management in an attempt to cover all possible use cases.

The second approach doesn't prevent the user from being able to do anything. It simply provides them with all the information which they'd need to make those decisions on their own.

I'd be quite pleased to have that file_mode() method available!

clalancette commented 5 years ago

I'd be quite pleased to have that file_mode() method available!

Great, thanks for the feedback. I've merged this into master in commit c7bfce9a130c72f5ae983c2349d6bc620b7feef6 , so I'll close this out for now.