Ticket #1617 (closed defect: fixed)

Opened 7 years ago

Last modified 6 years ago

ownership of files ~/.mc

Reported by: eshkrig Owned by: slavazanko
Priority: critical Milestone: 4.7.0-pre4
Component: mc-core Version: 4.7.0-pre3
Keywords: sudo Cc:
Blocked By: Blocking:
Branch state: Votes for changeset: committed-master

Description

Hi.
Sorry for my English.
There is problem with latest versions of MC:
When working under sudo, for example,
$ sudo bash
# mc
# exit
files in the directory ~/.mc overwritten and hence change of ownership at the root. Later, when starting from a user, MC can not save history (the file ~/.mc/history), because this file is not overwritten.

Change History

comment:1 follow-up: ↓ 3 Changed 7 years ago by slavazanko

  • Status changed from new to closed
  • Resolution set to invalid

In this case any program will write into you home dir as root user, not just mc. Because envirovement variables don't change, just changed user name and rights.

Use:

sudo su -
mc

to avoid this problem.

comment:2 Changed 7 years ago by slavazanko

  • Version 4.7.0-pre2 deleted
  • Milestone 4.7 deleted

comment:3 in reply to: ↑ 1 Changed 7 years ago by andrew_b

Replying to slavazanko:

In this case any program will write into you home dir as root user, not just mc. Because envirovement variables don't change, just changed user name and rights.

comment:ticket:394:13

comment:4 follow-up: ↓ 5 Changed 7 years ago by andrew_b

  • Keywords sudo added

comment:5 in reply to: ↑ 4 Changed 7 years ago by eshkrig

  • Status changed from closed to reopened
  • Resolution invalid deleted

Replying to andrew_b:

Thank you for the hint "sudo su -", but think - how will changed the security context of SELinux?
Maybe better to be more consistent? The other files in the directory ~/.mc have no such problems. Moreover, in previous versions MC do not replace a file - just replace the content, that IMHO, is the most correct action.

comment:6 follow-up: ↓ 7 Changed 7 years ago by ossi

sudo / su should change the security context. if they don't, then it's a mistake in your PAM configuration.

overwriting only the files' content without replacing the actual file is more likely to cause data loss when the power fails or the OS crashes, so it is in fact a less correct approach.

comment:7 in reply to: ↑ 6 Changed 7 years ago by eshkrig

Replying to ossi:
SELinux policy can not allow "sudo su -" for SELinux's user staff_u. Nevertheless, it permits "sudo bash" (Looking ahead, the user after the "sudo bash" can not execute "su -" without a root password).
Several users would like to work with "sudo bash":
used their configs|preferences (including MC)
their history of shell commands
etc.
And how about the fact that the problem with only one file: ~/.mc/history?
Please, just do to the file ~/.mc/history as well as with other files in ~/.mc such as
ini
panels.ini
Tree

Thanks

comment:8 Changed 7 years ago by slavazanko

  • Owner set to slavazanko
  • Status changed from reopened to accepted
  • Version set to master
  • Component changed from mc-core to mc-config-ini
  • Milestone set to 4.7.0-pre4

Trouble in src/mcconfig/common.c (functions mc_config_save_file() and mc_config_save_to_file() )

if (exist_file(mc_config->ini_path))
{
    mc_unlink (mc_config->ini_path);
}

ret = g_file_set_contents(mc_config->ini_path,data,len,NULL);

File unlinked, then created again. Need to overwrite file content without erasing file.

comment:9 Changed 7 years ago by ossi

oh my, that code is an epic fail - it combines the downsides of both approaches.
the canonical way to do it is creating a temp file in the same dir as the old file, writing the new contents to it, closing it and renaming it over the old file. under reasonable file systems this operation is atomic ...

comment:10 Changed 7 years ago by slavazanko

This code "epic fail" in any case, because this ticked raised. :)

Proposal with create temporary file and then renaming don't help too.

Need to strip old content of file (may be, make copy before) and fill again, without change directory content (just change file content).

comment:11 Changed 7 years ago by slavazanko

  • Version changed from master to 4.7.0-pre3

comment:12 Changed 7 years ago by iNode

  • Blocking 1656 added

(In #1656) This ticket related with #1617 and #394.

comment:13 Changed 7 years ago by slavazanko

  • Blocking 1532 added

(In #1532) I will continue work with ticket #1532 after complete #1617

comment:14 Changed 7 years ago by slavazanko

  • severity changed from no branch to on review

Created branch 1617_ownership_of_user_home_files

changesets:

Review, please.

comment:15 Changed 7 years ago by iNode

  • Votes for changeset set to iNode

comment:16 Changed 7 years ago by slavazanko

  • Votes for changeset iNode deleted
  • severity changed from on review to on rework

Sorry, but ~/.mc/Tree and ~/.mc/hotlist files have incorrect ownership too.

Need to rework.

comment:17 Changed 7 years ago by slavazanko

  • severity changed from on rework to on review

Okay, Now I was check and fix ownerships of all files in user home directory. Also, present commit for reorganize work with all files in user home.
Branch rebased.

Initial changeset:17fd3885c8b50f2feeb4523e90b4527de1d375e2

Test case:

$ sudo bash

files in the directory ~/.mc overwritten and hence change of ownership at the root. Later, when starting from a user, MC can not save history (the file ~/.mc/history), because this file is not overwritten.

Fix issue: Now files will overwrite if exists (rather than remove and create new file).

  • Added backup of saved files. If someone wrong in 'write config' stage, backup file still present.
  • Identation of code.

changeset:9a2b1695ca721b0bc65039ba058ff91aa3309cd4

Reorganize work with files. Fixed permissions of files in mc home dir. All file names now accumulated into src/fileloc.h

  • Added common functions for work with backups of main config files.
  • Fixed permissions of ~/.mc/ini;
  • Fixed permissions of ~/.mc/filepos
  • Fixed permissions of ~/.mc/hotlist
  • Fixed permissions of ~/.mc/Tree
  • Fixed ownership for ~/.mc/hotlist file
  • Changed definitions of config files. Now used constants from src/fileloc.h

Also, added ability for change mc user home dir. Just type:

make CFLAGS='-DMC_USERCONF_DIR=\".mc2\"'

And you will have different config files (very usefull for testing or development).

review please :)

comment:18 Changed 7 years ago by andrew_b

  • Votes for changeset set to andrew_b

comment:19 Changed 7 years ago by angel_il

  • Votes for changeset changed from andrew_b to andrew_b angel_il
  • severity changed from on review to approved

comment:20 Changed 7 years ago by slavazanko

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b angel_il to commited-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:21 Changed 7 years ago by slavazanko

  • Status changed from testing to closed
  • Blocking 1532, 1656 removed

comment:22 Changed 7 years ago by slavazanko

  • Status changed from closed to reopened
  • Votes for changeset commited-master deleted
  • severity changed from merged to no branch
  • Component changed from mc-config-ini to mcedit
  • Priority changed from major to critical
  • Resolution fixed deleted

Current editing position of files don't save.

comment:23 Changed 7 years ago by slavazanko

  • Status changed from reopened to accepted

comment:24 Changed 7 years ago by slavazanko

  • Component changed from mcedit to mc-core
  • severity changed from no branch to on review

created branch 1617_editor_filepos

initial changeset:481531fc2f3759a4053e0206fabbc5601891ff40
review, please.

comment:25 Changed 7 years ago by andrew_b

  • Votes for changeset set to andrew_b

comment:26 Changed 7 years ago by iNode

  • Votes for changeset changed from andrew_b to andrew_b iNode
  • severity changed from on review to approved

comment:27 Changed 7 years ago by slavazanko

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b iNode to commited-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:28 Changed 7 years ago by slavazanko

  • Status changed from testing to closed

comment:29 Changed 7 years ago by slavazanko

  • Status changed from closed to reopened
  • Resolution fixed deleted

Reason of reopening: incorrect saving of cursor position of editing/viewing file.

comment:30 Changed 7 years ago by slavazanko

  • Status changed from reopened to accepted

comment:31 Changed 7 years ago by slavazanko

  • Votes for changeset commited-master deleted
  • severity changed from merged to on review

created branch 1617_history_fix

initial changeset:0e83bcf188c87b7e7d8441928cc8a3d9335d2a69

review

comment:32 Changed 7 years ago by andrew_b

  • Votes for changeset set to andrew_b

comment:33 Changed 7 years ago by angel_il

  • Votes for changeset changed from andrew_b to andrew_b angel_il

comment:34 Changed 7 years ago by angel_il

  • severity changed from on review to approved

comment:35 Changed 7 years ago by slavazanko

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b angel_il to commited-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:36 Changed 7 years ago by slavazanko

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.