Bug 24636 - When using GDM+Xorg, adding urpmi media in MCC may set wrong file permissions, causing mgaapplet and urpm commands run as normal user to fail
Summary: When using GDM+Xorg, adding urpmi media in MCC may set wrong file permissions...
Status: RESOLVED FIXED
Alias: None
Product: Mageia
Classification: Unclassified
Component: RPM Packages (show other bugs)
Version: Cauldron
Hardware: All Linux
Priority: release_blocker normal
Target Milestone: ---
Assignee: Mageia tools maintainers
QA Contact:
URL:
Whiteboard:
Keywords:
: 24933 24960 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-04-07 13:55 CEST by Martin Whitaker
Modified: 2019-09-28 17:11 CEST (History)
9 users (show)

See Also:
Source RPM: rpmdrake
CVE:
Status comment:


Attachments

Description Martin Whitaker 2019-04-07 13:55:57 CEST
To reproduce, do a fresh install of GNOME. Do not add on-line media during installation. After booting into the installed system, start a GNOME on Xorg session, start MCC from the applications menu, select "Configure media sources...", and use the Add button to add a full set of media sources. Then check the directory/file permissions in /var/lib/urpmi - none of added directories/files will be world readable.

If you remove and re-add the on-line media sources without exiting drakrpm-editmedia, the directory/file permissions will be set correctly. If you exit and restart drakrpm-editmedia between removing and re-adding the on-line media sources, the directory/file permissions will again be incorrect.

This bug also occurs when using any other DE with GDM. It does not occur when running a GNOME on Wayland session. It does not occur when using another DM (e.g. LightDM).

Tested in VirtualBox using the Mageia-7-beta3-x86_64 ISO, although I've no reason to believe it can't be reproduced using a net install.
Ben McMonagle 2019-04-07 21:50:59 CEST

CC: (none) => westel

Comment 1 Marja Van Waes 2019-04-09 10:33:29 CEST
CC'ing tv and the gnome and mageia tools maintainers, I don't know whom to assign to (nor to which package).

CC: (none) => gnome, mageiatools, marja11, thierry.vignaud

Comment 2 Olav Vitters 2019-04-09 19:37:38 CEST
What is the bug about? I understand the default umask differs between GNOME+X vs GNOME+Wayland session. However, if MCC requires specific permissions then MCC needs to set those permissions instead of relying on a specific umask. So is a fix wanted from MCC or is the intention that umask is same across GNOME sessions?

CC: (none) => olav

Comment 3 Martin Whitaker 2019-04-09 20:59:27 CEST
The bug is about making the Mageia tools work for our users, if they choose (or are forced) to use GNOME on Xorg, or choose a multi-DE install that includes GNOME. I don't really care how it's fixed, but I would tend to agree that it's rpmdrake's responsibility to set the correct permissions.

What I don't understand is why removing and then immediately re-adding the media sets the permissions correctly. If the umask is wrong, I'd expect the permissions to be wrong every time.
Comment 4 Marja Van Waes 2019-04-12 21:36:59 CEST
(In reply to Martin Whitaker from comment #3)
> The bug is about making the Mageia tools work for our users, if they choose
> (or are forced) to use GNOME on Xorg, or choose a multi-DE install that
> includes GNOME. I don't really care how it's fixed, but I would tend to
> agree that it's rpmdrake's responsibility to set the correct permissions.

Assinging to the Mageia tools maintainers, then :-)
> 
> What I don't understand is why removing and then immediately re-adding the
> media sets the permissions correctly. If the umask is wrong, I'd expect the
> permissions to be wrong every time.

Assignee: bugsquad => mageiatools
Source RPM: (none) => rpmdrake

Comment 5 Martin Whitaker 2019-06-09 22:03:52 CEST
*** Bug 24933 has been marked as a duplicate of this bug. ***
Comment 6 Ben McMonagle 2019-06-09 22:15:30 CEST
24933 is different, and it seems to be hardware related, in that 2 of my test laptops (hp 8510w and compaq c700) exhibit the issue, my toshiba and chromebooks do not.
the issue only seems to occur when adding on-line media from Mageia Welcome.
right clicking the mgaapplet to bring up the on-line media list and then closing it again is enough to fix the issue.
Comment 7 Ben McMonagle 2019-06-10 09:03:59 CEST
i am incorrect, 
adding from MCC as well also exhibits issue
Comment 8 Ben McMonagle 2019-06-11 08:07:16 CEST
what is the likelihood of creating the "final" .isos without GDM?

would still be available from repos as normal if required by usres
Comment 9 Martin Whitaker 2019-06-11 10:03:28 CEST
(In reply to ben mcmonagle from comment #8)
> what is the likelihood of creating the "final" .isos without GDM?

Can't do that - it is required by GNOME. Anyway, we would need it on the ISOs to support upgrades.
Comment 10 Martin Whitaker 2019-06-14 11:04:17 CEST
@Thierry, would you consider changing the umask when adding media an acceptable fix for this bug? I see that is already done when writing the urpmi.cfg file (which is why removing the media and re-adding them works (answering my comment #3).
Comment 11 Thomas Backlund 2019-06-21 20:56:47 CEST
(In reply to Martin Whitaker from comment #10)
> @Thierry, would you consider changing the umask when adding media an
> acceptable fix for this bug? I see that is already done when writing the
> urpmi.cfg file (which is why removing the media and re-adding them works
> (answering my comment #3).

I think we should fix this one before release...

@Martin, can you fix it up with umask... 

if tv wants it another way, we can do a follow-up fix later

CC: (none) => tmb
Priority: Normal => release_blocker

Comment 12 Martin Whitaker 2019-06-21 21:10:19 CEST
(In reply to Thomas Backlund from comment #11)
> I think we should fix this one before release...
> 
> @Martin, can you fix it up with umask... 

Will see what I can do.
Comment 13 Martin Whitaker 2019-06-22 00:34:24 CEST
OK, fixed in urpmi-8.117-1. I don't think this is a perfect fix, but it's good enough to allow mgaapplet to work.
Comment 14 Martin Whitaker 2019-06-22 00:44:34 CEST
*** Bug 24960 has been marked as a duplicate of this bug. ***

CC: (none) => tablackwell

Comment 15 Thomas Backlund 2019-06-22 01:30:48 CEST
(In reply to Martin Whitaker from comment #13)
> OK, fixed in urpmi-8.117-1. I don't think this is a perfect fix, but it's
> good enough to allow mgaapplet to work.

Isnt there a thinko here:
http://gitweb.mageia.org/software/rpm/urpmi/commit/?id=f8c0d00fa23a094173dbe17ef4644f860f871f20

first you do: my $old_umask = umask 0022;

then you call: umask $old_umask;

which if I read it correctly will translate to: "umask umask 0022;"
Comment 16 Thomas Backlund 2019-06-22 04:00:42 CEST
Was the intention to do:
diff --git a/urpm/media.pm b/urpm/media.pm
index 52575ff0..75267ac8 100644
--- a/urpm/media.pm
+++ b/urpm/media.pm
@@ -555,7 +555,8 @@ sub write_urpmi_cfg {
     #- a normal user won't be able to read it. We enforce umask here in the case
     #- where the msec security level is set to 'secure' (which means umask 077)
     #- or where we are run from a gdm-x-session (mga#24636)
-    my $old_umask = umask 0022;
+    my $old_umask = umask;
+    umask 0022;
     urpm::cfg::dump_config($urpm->{config}, $config)
        or $urpm->{fatal}(6, N("unable to write config file [%s]", $urpm->{config}));
     umask $old_umask;
@@ -978,7 +979,8 @@ sub add_medium {
     #- as a normal user won't be able to read them. We enforce umask here in the case
     #- where the msec security level is set to 'secure' (which means umask 077) or
     #- where we are run from a gdm-x-session (mga#24636)
-    my $old_umask = umask 0022;
+    my $old_umask = umask;
+    umask 0022;
     #- those files must not be there (cf mdvbz#36267)
     _clean_statedir_medium_files($urpm, $medium);
     if (!($options{virtual} && _local_file($medium))
Comment 17 Martin Whitaker 2019-06-22 07:49:28 CEST
(In reply to Thomas Backlund from comment #15)
> Isnt there a thinko here:
> http://gitweb.mageia.org/software/rpm/urpmi/commit/
> ?id=f8c0d00fa23a094173dbe17ef4644f860f871f20
> 
> first you do: my $old_umask = umask 0022;
> 
> then you call: umask $old_umask;
> 
> which if I read it correctly will translate to: "umask umask 0022;"

No, "umask" is a perl function that sets the new umask and returns the previous value.

If I'd done this from scratch, I would have written it as

  my $old_umask = umask(0022);

but I preserved the style of what was already there.
Comment 18 Martin Whitaker 2019-06-22 10:10:16 CEST
But unfortunately this still isn't quite enough to fix it. It works the first time you add media, but if you remove and re-add them, the downloaded synthesis files get the wrong permissions. That probably means that it would break when updating the media as well.

The simplest fix would be to set the umask when starting the various rpmdrake tools. Otherwise it could take me a while to fix this. WDYT?
Comment 19 Martin Whitaker 2019-06-22 10:56:41 CEST
Found a better way. Should be fixed in urpmi-8.118-1.
Comment 20 Thomas Backlund 2019-06-22 12:47:33 CEST
(In reply to Martin Whitaker from comment #17)
> (In reply to Thomas Backlund from comment #15)
> > Isnt there a thinko here:
> > http://gitweb.mageia.org/software/rpm/urpmi/commit/
> > ?id=f8c0d00fa23a094173dbe17ef4644f860f871f20
> > 
> > first you do: my $old_umask = umask 0022;
> > 
> > then you call: umask $old_umask;
> > 
> > which if I read it correctly will translate to: "umask umask 0022;"
> 
> No, "umask" is a perl function that sets the new umask and returns the
> previous value.
> 
> If I'd done this from scratch, I would have written it as
> 
>   my $old_umask = umask(0022);
> 
> but I preserved the style of what was already there.

Ah, ok.
Thanks for the explanation
Castro B 2019-07-10 16:29:16 CEST

CC: (none) => castro8583bennett

Comment 22 Martin Whitaker 2019-09-28 17:11:32 CEST
Was fixed.

Resolution: (none) => FIXED
Status: NEW => RESOLVED


Note You need to log in before you can comment on or make changes to this bug.