Ticket #895 (new defect)

Opened 23 months ago

Last modified 10 months ago

Atlas crashes when it does not have rights to write to a file

Reported by: asmartgoat Owned by: Darth_Malloc
Priority: Nice to Have Milestone: Backlog
Component: Atlas editor Keywords: linux, atlas, root, save
Cc:

Description

When atlas is run without root access in linux it crashes when saving into a folder that requires root access (0ad installation folder, etc.)

Reproduce: LINUX 1) Open a map in atlas 2) modify a bit 3) Try to save in ~/mods/public/scenarios/* 4) Permissions Error and crash.

It should at least ask for permissions or (at worst) ask me to save on a folder that I have permissions to.

Thanks, luke

Attachments

pswdWindow.h (815 bytes) - added by Darth_Malloc 17 months ago.

Change History

comment:1 Changed 23 months ago by asmartgoat

Settings:

Fedora 15

0AD Alpha 6.

comment:2 Changed 22 months ago by asmartgoat

  • Priority changed from Should Have to Must Have
  • Milestone changed from Backlog to Alpha 7

It actually crashes wherever it tries to save without root.

Chaging to alpha 7 as a sort of ping.

comment:3 Changed 22 months ago by feneur

Please don't change milestones. We see the tickets anyway, and are notified if you update the ticket/comment on it, so no need to do something like that. Though for crashes the upcoming release is probably the best milestone so I'll let it remain.

Was a long time since I used Linux, and I'm not a programmer, so I can't really help. Though iirc ~/ is the user folder? Sounds really weird if that gives permissions errors, but perhaps something is weird with the installation. Hope someone with more knowledge will be able to debug it :)

comment:4 follow-up: ↓ 5 Changed 22 months ago by asmartgoat

Note taken...

More information,

By default atlas tries to save maps in /etc/share/0ad/mods/public/maps/scenarios (Which is correct). This folder has read-only access by non-root users and makes atlas crash when trying to write to it without root access. 


I was just using ~/ as placeholder for /usr/share/0ad/ (I didn't recall the exact folder)

comment:5 in reply to: ↑ 4 ; follow-up: ↓ 7 Changed 22 months ago by historic_bruno

Replying to asmartgoat:

By default atlas tries to save maps in /etc/share/0ad/mods/public/maps/scenarios (Which is correct)

Actually it sounds like a silly place to store game data, if it can only be accessed by root. How do they expect mods to be applied? Whoever is in charge of the Fedora package should look into that IMO - it seems the path can be changed during builds. While eventually the whole load/save mechanism in Atlas should be overhauled (#631 has a few details), meanwhile it will need write permissions on the mods directory (the files are accessed through the VFS which has its root there).

We could check permissions in Atlas before writing the file, the VFS seems to just arbitrarily pick the first mod for writing (for most people that is public/ but that's not guaranteed e.g. for developers it may be internal/).

comment:6 Changed 22 months ago by historic_bruno

#889 is also relevant.

comment:7 in reply to: ↑ 5 Changed 22 months ago by Philip

Replying to historic_bruno:

it sounds like a silly place to store game data, if it can only be accessed by root. How do they expect mods to be applied?

Unix is multi-user, and packages are designed to be available to all users once installed, and one user shouldn't be able to disrupt other users (since it'd be annoying and also a security vulnerability), so data should go in root-only-writable directories, so the packages are currently correct.

If the application wants to save data when run by a user, it should use that user's home directory - we should probably mount a directory like ~/.local/share/0ad/mods/user/ or something, and save everything in there by default (except for SVN users who should default to the normal mods/public/ directory).

comment:8 Changed 22 months ago by historic_bruno

Hmm that's true, didn't think of it that way :/ It looks easy to mount another directory in the VFS, but I'm not yet certain how it chooses the default. I think we'd want the user mods directory to take priority for reading as well as writing, so that people could instantly see their changes. That would be a useful change even on Windows.

comment:9 Changed 21 months ago by k776

  • Milestone changed from Alpha 7 to Alpha 8

comment:10 Changed 20 months ago by historic_bruno

So I've tried adding a user mod directory, but it seems to need some VFS changes:

After further testing: if I create a /maps/scenarios subdirectory, the maps get saved there as expected but if I delete it, they end up in internal. Which doesn't seem right, the VFS should be able to create the full path for a new file.

The logic looks slightly broken because the real directory is selected by VfsDirectory::GetSubdirectory inside of vfs_lookup, being called in a loop for each part of the VFS path. So whichever real path matches a part of the VFS path first is the one that gets used, and only after that any missing parts get created. That means with this implementation we have no way of knowing where a file will be written

The tricky bit (this code is kind of complicated because there are multiple different cases, where you do or do not want to add and/or create directories) is deciding at each step whether to use the current realDir, or go back to the one from a previous directory (yet remembering the path from there).

comment:11 follow-up: ↓ 14 Changed 20 months ago by Darth_Malloc

  • Owner set to Darth_Malloc
  • Status changed from new to assigned

comment:12 Changed 18 months ago by k776

  • Priority changed from Must Have to Nice to Have

comment:13 Changed 18 months ago by historic_bruno

  • Milestone changed from Alpha 8 to Alpha 9

comment:14 in reply to: ↑ 11 Changed 18 months ago by historic_bruno

Replying to Darth_Malloc:

Any updates? :)

Changed 17 months ago by Darth_Malloc

comment:15 follow-up: ↓ 16 Changed 17 months ago by Darth_Malloc

My appologies about the delay. Now that I am on holiday things should be picking up speed. Please note that the header file above is just a preview of my approach - there will be a more polished version of it, along with pswdWindow.cpp and the updates to the editor code, posted soon. I am open to any suggestions in the mean time regarding the header file.

Until next time, happy holidays and may the Force be with you.

comment:16 in reply to: ↑ 15 Changed 17 months ago by historic_bruno

Replying to Darth_Malloc:

Please note that the header file above is just a preview of my approach - there will be a more polished version of it, along with pswdWindow.cpp and the updates to the editor code, posted soon.

To be clear on the reason that the map save is failing: the game data is in a location where the user doesn't have permission to write. It's not necessarily an account permissions issue because on Windows, releases are bundled with the game data in a zip archive which can't be modified while the game is running. Maybe some Linux users will be the admin and they'll know the root password and be able to obtain write permission on the shared data directory. But that's not something we can count on, especially on multiuser systems.

Besides being annoying by asking for a password during something as common as saving a map, it also seems like it would introduce security problems that are best avoided. As Philip mentioned above, we'd be allowing and even requiring a single user to modify the game data shared by all users on the system (at least the way Linux packages are installed).

The solution we want is to set up a new, always-writable mod directory in the user's home directory (~/.local/share/0ad/mods/user/ on Linux, and something like My Documents\Path\To\Game\Data on Windows). I think the biggest obstacle to this is the VFS not guaranteeing to write to a specific mod when multiple mods are loaded. We'd also need sensible paths for all OSes but that's that such a big deal.

comment:17 Changed 17 months ago by historic_bruno

Any progress?

comment:18 Changed 16 months ago by historic_bruno

  • Owner Darth_Malloc deleted
  • Status changed from assigned to new

comment:19 Changed 15 months ago by Darth_Malloc

  • Owner set to Darth_Malloc

comment:20 Changed 15 months ago by k776

  • Milestone changed from Alpha 9 to Alpha 10

comment:21 Changed 13 months ago by k776

  • Milestone changed from Alpha 10 to Alpha 11

comment:22 Changed 10 months ago by k776

  • Milestone changed from Alpha 11 to Backlog
Note: See TracTickets for help on using tickets.