Bug 103459

Summary: gdm-2.4.2.102-2.src.rpm missing files
Product: [Retired] Red Hat Raw Hide Reporter: Valdis Kletnieks <valdis.kletnieks>
Component: gdmAssignee: Havoc Pennington <hp>
Status: CLOSED RAWHIDE QA Contact: Mike McLean <mikem>
Severity: medium Docs Contact:
Priority: medium    
Version: 1.0CC: alexl, jirka
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2003-10-06 14:20:36 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Valdis Kletnieks 2003-08-31 03:54:00 UTC
Description of problem:
gdm-2.4.2.102-2.src.rpm is missing (at least) 2 files referenced by
gdm-selinux.patch - the patch adds references to gdm-selinux.c and gdm-selinux.h
neither of which are in the .tar.bz2 or created by the patch.


Version-Release number of selected component (if applicable):
2.4.2.102-2

How reproducible:


Steps to Reproduce:
1. Add "%WITH_SELINUX 1" to your .rpmmacros
2. rpm -i gdm-2.4.2.102-2.src.rpm
3. rpmbuild -bb /usr/src/redhat/SPECS/gdm.spec
    
Actual results:
Compile failure because gdm-selinux.h missing 


Expected results:


Additional info:
I suspect this is a packaging problem when creating gdm-selinux.patch, where a
tool was trying to do a 'diff oldfile newfile' and failed to deal with the fact
that for a *new* file, there's no oldfile to diff against...

Comment 1 Alexander Larsson 2003-09-01 10:32:04 UTC
george, this was your selinux patch. Can you regenerate it with the missing files?


Comment 2 George Lebl 2003-09-02 16:15:33 UTC
Sure, though I'm just going to make a much simpler patch as we discussed on
rhselinux-list, that is, just set the default context and leave the role
changing to a helper program that can be run by the user (or from Xsession)

Comment 3 Valdis Kletnieks 2003-09-02 17:47:02 UTC
Hmm.. I'll have to go read the mailing list, but the first thought I have is
that it can't easily work from within .Xsession, because a helper/wrapper
program would be spawning off a *SUB*shell (similar to what 'newrole' or 'su'
do.  I'm failing to see how using a 'newrole/su' style helper will get the
security context of anything the session has already launched. The only way to
make this work is to make the .xsession be only one line, of the form:

/usr/bin/newXsession --select-context < .rest-of-xsession-for-selinux

or some similar creeping horror....

Note that there's a very subtle difference between this case, and what many
ssh users do in .xession:

eval `ssh-agent`
ssh-add < /dev/null

The difference being that the shell can assign an environment variable through
the 'eval', but the *last* thing you want the .xesssion shell doing is assigning
a security context.

Comment 4 George Lebl 2003-09-02 18:28:29 UTC
Well, the way this would work is following:  There would be a helper program
ran before ssh-agent in the following way:

selinux-helper -- ssh-agent -- gnome-session

The selinux-helper may or may not present a gui to change whatever is changable
and work like newrole (but would take stuff from the GUI instead of from command
line)

Comment 5 Valdis Kletnieks 2003-09-02 19:00:11 UTC
You're missing the point - the problem that needs to be solved for the .xsession
case is the *SAME* sort as the infamous "why can't a subshell set an environment
variable or do a 'cd' command".

Let's take part of my .xsession and your comment above for an example:

# The following is the "obvious"
# xmodmap -e "pointer = 1 2 3 6 7 4 5"
# but we want the 2 big buttons to be left and middle, and small right to
# be right - leaves small left as 6 and click-scrollwheel as 7.
xmodmap -e "pointer = 1 7 2 6 3 4 5"

xrdb -merge .Xdefaults
xset fp+ /home/valdis/fonts
xset fp rehash
#xset dpms 10 0 0
xscreensaver -no-splash &
selinux-helper -- ssh-agent -- gnome-session

Hey look at that - the xrdb and xset and xmodmap and xscreensaver all run in the
wrong context.  In addition, only the ssh-agent and gnome-session (hey, I don't
even *use* that ;) would be in the *right* context.  As I said - the only way to
make it "work" is

selinux-helper -- .xsession-real

as the *only* line in .xsession

which is a maintenance/support headache.

I'll also point out that having this as a hook in .xsession means that if the
system administrator wants to force my X session into a specific context, I can
*TOTALLY BYPASS* that by the simple expedient of renaming my .xsession - so you
REALLY want to do this *outside* user control...

Comment 6 George Lebl 2003-09-02 20:19:29 UTC
It depends on the Sysadmin/system integrator of how it is set up.  Obviously I
think this should be done in the system Xsession script.  The obviously simple
way for a sysadmin/distributor to implement this is to have two Xsession
scripts, say

/etc/X11/gdm/Xsession.selinux and /etc/X11/gdm/Xsession

then set the Xsession.selinux as your BaseXsession in gdm.conf and have it be

exec selinux-helper -- /etc/X11/gdm/Xsession

(or whatever naming you like).  No support nightmare, just a very clean
solution.  selinux-helper can be maintained on selinux schedule rather then GDM
schedule, so no messy selinux patches to GDM.  No subtle bugs from conflicts of
GDM code changes and such patches.  The above change doesn't require any action
on the part of the users, existing startup scripts work exactly the wame way,
etc...  There are other ways to do it in the script too some more messy then
others.  Something on the order of

if [ -z "$foobar" ]; then
   foobar=1 exec selinux-helper -- "$0" "$@"
fi
unset foobar

at the top of the Xsession script (system one, I'm not talking about user
session setup scripts) will do the trick.  When I said "can be run by the user"
I mean on a system where the sysadmin doesn't set this up for all users.

If you don't want users to have the ability to change this, then what's
preventing them from overriding things in their .xsession ANYWAY with newrole.

The above Xsession solution is functionally equivalent to C code in GDM to do
it.  Except it's more flexible and doesn't require extensive GDM changes.  The
more code that goes directly into GDM the more likely it is to break and have to
be patched on further SELinux releases.  As much code as possible should be
maintained separately by some helper app on selinux release schedule.

Comment 7 Valdis Kletnieks 2003-09-02 21:50:04 UTC
OK.. if it's going into the sysadmin-maintained /etc/X11/gdm/* files, that
closes the issue I had with the .xsession.

My comment about users changing it was that if the sysadmin was using
selinux-helper to set a 'user_r' context from within .xsession, then if the user
nuked .xsession, he'd get a session with whatever context gdm happened to have.
 So it's not so much "the user can use newrole" (because newrole will
re-authenticate etc etc) as "/bin/login didn't *set* a context and user got a
too-powerful default one" type of issue...  Of course, this isn't a problem if
it's done in scripts not under the user control.

I'll wait for a new code drop to show up on rawhide, see how it looks with the
hooks in /etc/X11/gdm/* - that does sound like a workable solution...

Comment 8 George Lebl 2003-09-03 00:31:17 UTC
OK, I see the misunderstanding.  No the user Xsession will never run (not even
the system Xsession script) with the gdm context.  The new patch I sent to alex
sets the context to the default context of the user that is logging in before it
even
runs the Xsession script.

The selinux-helper program doesn't exist, it was an imaginary program that
"should" exist for a nice GUI solution.  So no change to the Xsession stuff will
need to be done, GDM does set the security context for the user just before
execing the Xsession now, it just doesn't let him pick if there are multiple ones.

Comment 9 Alexander Larsson 2003-10-06 14:20:36 UTC
Ok. The last patch from George is now in, so i hope this is fixed. I think
selinux  support is in cvs upstream too, so we'll soon get it that way.