Bug 1208309 - shadow: newuidmap, newgidmap: consider using capabilities instead of setuid-root
Summary: shadow: newuidmap, newgidmap: consider using capabilities instead of setuid-root
Status: RESOLVED FIXED
Alias: None
Product: SUSE Security Incidents
Classification: Novell Products
Component: Audits (show other bugs)
Version: unspecified
Hardware: Other Other
: P5 - None : Normal
Target Milestone: ---
Assignee: Matthias Gerstner
QA Contact: Security Team bot
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 1207754
  Show dependency treegraph
 
Reported: 2023-02-15 12:47 UTC by Matthias Gerstner
Modified: 2023-04-25 08:59 UTC (History)
3 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Gerstner 2023-02-15 12:47:02 UTC
+++ This bug was initially created as a clone of Bug #1207754

The security team is currently investigating the setuid and setgid binaries
present in SUSE default installations.

We have been made aware that RHEL/Fedora based systems don't apply setuid root
permissions to newuidmap and newgidmap. Instead they set capabilities:

/usr/bin/newuidmap cap_setuid=ep
/usr/bin/newgidmap cap_setgid=ep

In the upstream code the comment is a bit difficult to understand (for me) but
it sounds like using capabilities is considered superior not even from a
security point of view but also from a functionality point of view:

https://github.com/shadow-maint/shadow/blob/master/libmisc/idmapping.c#L124

I made a simple test on a current Tumbleweed installation and applying the
capabilities as shown above worked out for me.

Apart from this, in our permissions profiles we are currently giving ownership
of these binaries to the shadow group:

    profiles/permissions.secure:# newgidmap / newuidmap (bsc#979282, bsc#1048645)
    profiles/permissions.secure:/usr/bin/newgidmap root:shadow     4755
    
    profiles/permissions.easy:# newgidmap / newuidmap (bsc#979282, bsc#1048645)
    profiles/permissions.easy:/usr/bin/newgidmap root:shadow     4755

This doesn't really make sense and should be changed to root:root.

I would like to get a statement of the current shadow maintainers if they deem
this change (using cap_setuid, cap_setgid) appropriate. If so then I will
change the permissions settings accordingly - for Factory as a start.
Comment 1 Michael Vetter 2023-02-16 08:09:43 UTC
I think your change makes sense.

> it sounds like using capabilities is considered superior not even from a
security point of view but also from a functionality point of view

Indeed sounds like. I was not aware of that.

I see that Fedora is doing:
	
> %attr(0755,root,root) %caps(cap_setgid=ep) %{_bindir}/newgidmap
> %attr(0755,root,root) %caps(cap_setuid=ep) %{_bindir}/newuidmap

Will you send a SR?
Comment 2 Michael Vetter 2023-02-16 11:13:23 UTC
Since the capabilities are implemented using xattr I read: 

https://manpages.opensuse.org/Tumbleweed/man-pages/xattr.7.en.html#Filesystem_differences

And I guess all our filesystems are there.

Is the following relevant to us?

> Some filesystems, such as Reiserfs (and, historically, ext2 and ext3), require he filesystem to be mounted with the user_xattr mount option in order for user extended attributes to be used.
Comment 3 Matthias Gerstner 2023-02-16 13:54:37 UTC
(In reply to mvetter@suse.com from comment #1)
> I see that Fedora is doing:
> 	
> > %attr(0755,root,root) %caps(cap_setgid=ep) %{_bindir}/newgidmap
> > %attr(0755,root,root) %caps(cap_setuid=ep) %{_bindir}/newuidmap
> 
> Will you send a SR?

That won't work, because packaging capabilities in RPM doesn't work (or at
least we don't support it at SUSE yet?). Anyway we change it in the
permissions package. The %set_permissions invocation already present in the
shadow spec file will suffice.
Comment 4 Matthias Gerstner 2023-02-16 13:58:41 UTC
(In reply to mvetter@suse.com from comment #2)
> Since the capabilities are implemented using xattr I read: 
> 
> https://manpages.opensuse.org/Tumbleweed/man-pages/xattr.7.en.html#Filesystem_differences
> 
> And I guess all our filesystems are there.
> 
> Is the following relevant to us?
> 
> > Some filesystems, such as Reiserfs (and, historically, ext2 and ext3), require he filesystem to be mounted with the user_xattr mount option in order for user extended attributes to be used.

We're using capabilities for a long time already for quite some other stuff
without issues so there should be no worries.

I believe the "user_xattr" options are automatically added to /etc/fstab if
necessary but shouldn't be needed any more. Probably YaST has some logic for
that still somwhere when setting up file systems.
Comment 5 Michael Vetter 2023-02-16 14:02:48 UTC
I see that this might have been confusing :-)
What I meant to say was:

(In reply to Michael Vetter from comment #1)
> I think your change makes sense.
> 
> > it sounds like using capabilities is considered superior not even from a
> security point of view but also from a functionality point of view
> 
> Indeed sounds like. I was not aware of that.
> 
> I see that Fedora is doing:
> 	
> > %attr(0755,root,root) %caps(cap_setgid=ep) %{_bindir}/newgidmap
> > %attr(0755,root,root) %caps(cap_setuid=ep) %{_bindir}/newuidmap
(As a sign of agreement)

Then independently I meant

> Will you send a SR?

To the related security packages and shadow package if needed.

So basically I wanted to ask if there was anything for me to do or only checking if I agree with the change.
Comment 6 Michael Vetter 2023-02-16 14:03:14 UTC
> So basically I wanted to ask if there was anything for me to do or only
> checking if I agree with the change.

Since the bug is assigned to me.
Comment 7 Matthias Gerstner 2023-02-16 14:43:04 UTC
Ah I see. No I don't need you to do anything else, I just wanted your
agreement and to document the change.

I will perform the necessary changes, thank you for the input!
Comment 8 Matthias Gerstner 2023-02-17 10:31:45 UTC
Permissions PR# created for this:

https://github.com/openSUSE/permissions/pull/172
Comment 9 OBSbugzilla Bot 2023-02-17 11:55:04 UTC
This is an autogenerated message for OBS integration:
This bug (1208309) was mentioned in
https://build.opensuse.org/request/show/1066365 Factory / permissions
Comment 11 Matthias Gerstner 2023-04-25 08:59:52 UTC
The change is by now in Tumbleweed, I just tested the current setup and it all
looks fine.

Thanks for helping out.

I'm closing this bug as fixed.