Merge lp:~raphink/byobu/acl into lp:byobu

[Raphaël Pinson]

[Raphaël Pinson (raphink)]

Hi :-Dustin,

This branch adds a menu entry in byobu-config to deal with basic ACLs.

Ideally, I wanted to have a menu with existing ACLs to modify them, and another one to add new ACLs. As it turned out, I couldn't find out how to read the current ACLs for the screen. The only command I've found is ^A:displays, but it's a paged display that cannot be parsed, and it doesn't list all ACLs. As a result, you have to specify the full ACLs for every change.

To apply the ACLs, I chose to use a tempfile since there were a few commands to launch. You might prefer to launch the commands one by one instead.

Finally, there's the issue of the setuid root bit, but screen prints a message about that when an invited user tries to join the screen, so it's clear enough this way (with a bit of doc probably).

Cheers,

Raphaël

[lp:~raphink/byobu/acl]

905. By Raphaël Pinson <raphink@rpinson> on 2010-01-28

Main menu has height 10 now

906. By Raphaël Pinson <raphink@rpinson> on 2010-01-29

Dump ACL when made and allow to modify them

907. By Raphaël Pinson <raphink@rpinson> on 2010-01-29

No need to remove acls from the "Add ACL" menu

908. By Raphaël Pinson <raphink@rpinson> on 2010-01-29

Check for setuid root to allow multiuser functions

909. By Raphaël Pinson <raphink@rpinson> on 2010-02-01

Nicer check for setuid root

910. By Raphaël Pinson <raphink@rpinson> on 2010-02-03

Fix execuseracl call

911. By Raphaël Pinson <raphink@rpinson> on 2010-02-03

Reimplement "Remove all rights" in updateacl

[Dustin Kirkland  (kirkland)]

Hi Rafael-

First of all, thanks a bunch for the branch. I think it's a great idea to expose and make screen sharing easier for Byobu users.

As for the implementation, I'm not sure putting all of this into byobu-config is the best way to go about it. Let's discuss it a little. (I'm just now getting Lucid behind me, and opening the byobu tree for Maverick).

What do you think about creating two new utilities, /usr/bin/byobu-share and /usr/bin/byobu-unshare, that toggle the acl's on and off?

Perhaps byobu-share takes a whitespace list of users to share with. The assumption (if unspecified) is read-only. Or the user can add --rw to share in read-write mode.

I suggest that we get byobu-share and byobu-unshare working well first, and once we're satisfied with it (and the surround security concerns), then we add a menu option that just calls these external utilities.

As an Ubuntu developer, I'm a little concerned that the security team might not like such a package in Ubuntu Main (but I'll try to deal with that separately from Byobu as an upstream project).

What do you think? Would you be willing to separate this functionality into two separate scripts (either shell or python is fine) that does this?

review: Needs Fixing

[Raphaël Pinson (raphink)]

Hi Dustin,

2010/4/24 Dustin Kirkland <email address hidden>

> Review: Needs Fixing
>

Yep, I totally agree that it needs fixing.

First of all, thanks a bunch for the branch. I think it's a great idea to
> expose and make screen sharing easier for Byobu users.
>
> As for the implementation, I'm not sure putting all of this into
> byobu-config is the best way to go about it. Let's discuss it a little.
> (I'm just now getting Lucid behind me, and opening the byobu tree for
> Maverick).
>
> What do you think about creating two new utilities, /usr/bin/byobu-share
> and /usr/bin/byobu-unshare, that toggle the acl's on and off?
>
>
Well as you probably noticed, I haven't touched this branch for a while. The
reason is that I couldn't properly access the acls that screen knows about
at a given time. This is a known bug/feature request in screen, that has
been in the whishlist since about 10 years (from reading the changelog on
this subject).

As a result, the changes I've made to byobu allow me to set acls in screen
(provided the setuid root bit is set on the binary of course) but not to
modify them or even display the currently set acls. I came up with a model
that stored the acls that byobu created so that I could find them and modify
them, but there are several issues with this way of doing things:
 * if the users use the builtin acl commands in screen, I have no idea what
they changed and I can't display the new acls ;
 * I currently can't properly support multi-session, which means byobu
thinks the set acls for a session applies to all sessions even though
they've never been applied there ;
 * I don't flush the known acls when the session stops, so when a new
session is started, byobu thinks acls are already applied when they are not.

All this to say that really this feature needs a patch in screen itself, to
allow accessing the current acls somewhere (and not in a non-parseable pager
like screen often displays info). I've begun looking at the code for acls,
but my C skills are very poor, and the code is quite complicated I find.

> As an Ubuntu developer, I'm a little concerned that the security team might
> not like such a package in Ubuntu Main (but I'll try to deal with that
> separately from Byobu as an upstream project).
>
>
I agree entirely with this. Perhaps there could be a debconf question in the
screen package to ask users if they want screen set with a setuid root bit.
After all, this feature is not only useful for byobu, and dealing with it in
debconf would allow to do it cleanly with packaging tools, when most users
would override the setuid bit manually.

I'd be happy to discuss this with you some time soon.

Raphael

Unmerged revisions

911. By Raphaël Pinson <raphink@rpinson> on 2010-02-03

Reimplement "Remove all rights" in updateacl

910. By Raphaël Pinson <raphink@rpinson> on 2010-02-03

Fix execuseracl call

909. By Raphaël Pinson <raphink@rpinson> on 2010-02-01

Nicer check for setuid root

908. By Raphaël Pinson <raphink@rpinson> on 2010-01-29

Check for setuid root to allow multiuser functions

907. By Raphaël Pinson <raphink@rpinson> on 2010-01-29

No need to remove acls from the "Add ACL" menu

906. By Raphaël Pinson <raphink@rpinson> on 2010-01-29

Dump ACL when made and allow to modify them

905. By Raphaël Pinson <raphink@rpinson> on 2010-01-28

Main menu has height 10 now

904. By Raphaël Pinson <raphink@rpinson> on 2010-01-28

Added Remove all ACL button

903. By Raphaël Pinson <raphink@rpinson> on 2010-01-28

Add ACL menu

=== modified file 'usr/bin/byobu-config'
--- usr/bin/byobu-config	2010-01-12 14:46:44 +0000
+++ usr/bin/byobu-config	2010-02-03 16:43:15 +0000
@@ -22,7 +22,7 @@
 #       ./debian/rules get-po
 
 
-import sys, os, os.path, time, string, commands, gettext, glob, snack
+import sys, os, os.path, time, string, commands, gettext, glob, snack, pwd, tempfile, stat
 from ConfigParser import SafeConfigParser
 from snack import *
 
@@ -37,7 +37,9 @@
     DOC = "%s/.%s/%s" % (HOME, PKG, DOC)
 DEF_ESC="A"
 RELOAD = "If you are using the default set of keybindings, press\n<F5> or <ctrl-a-R> to activate these changes.\n\nOtherwise, exit this screen session and start a new one."
-RELOAD_FLAG="/var/run/screen/S-"+USER+"/"+PKG+".reload-required"
+SOCKET_DIR="/var/run/screen/S-"+USER+"/"
+RELOAD_FLAG=SOCKET_DIR+PKG+".reload-required"
+ACL_FILE=SOCKET_DIR+PKG+".acl"
 RELOAD_CMD="screen -X at 0 source $HOME/."+PKG+"/profile"
 ESC = ''
 snack.hotkeys[ESC] = ord(ESC)
@@ -89,7 +91,7 @@
         installtext=_("Byobu currently does not launch at login (toggle on)")
 
 
-    li = Listbox(height = 9, width = 60, returnExit = 1)
+    li = Listbox(height = 10, width = 60, returnExit = 1)
     li.append(_("Help"), 1)
     li.append(_("Change Byobu's background color"), 2)
     li.append(_("Change Byobu's foreground color"), 3)
@@ -98,7 +100,8 @@
     li.append(_("Change escape sequence"), 6)
     li.append(_("Create new windows"), 7)
     li.append(_("Manage default windows"), 8)
-    li.append(installtext, 9)
+    li.append(_("Invite users or modify ACLs"), 9)
+    li.append(installtext, 10)
     bb = ButtonBar(screen, (("Exit", "exit", ESC),), compact=1)
 
     g = GridForm(screen, _(" Byobu Configuration Menu"), 1, 2)
@@ -404,6 +407,215 @@
 
     return 100
 
+def execuseracl(userlist, actions, acl):
+    commandfile=tempfile.mkstemp()
+    f=open(commandfile[1], 'w')
+    try:
+       for user in userlist:
+          f.write("aclumask %s%s\n" % (user, acl))
+          f.write("aclchg %s %s \"#?\"\n" % (user, acl))
+          f.write("aclchg %s +x \"%s\"\n" % (user, " ".join(actions)))
+       f.write("multiuser on\n")
+    except IOError:
+       return None
+    finally:
+       f.close()
+       commands.getoutput("screen -X source %s" % commandfile[1])
+
+    return 100
+
+def getuseracl():
+    users=[]
+    if not os.path.isfile(ACL_FILE):
+       return users
+
+    aclfile=open(ACL_FILE, 'r')
+    try:
+       acls=aclfile.read().splitlines()
+    except IOError:
+       return None
+    finally:
+       aclfile.close()
+
+    for acl in acls:
+       users.append(acl.split(':'))
+
+    return users
+
+def updateacls(acls):
+    aclfile=open(ACL_FILE, 'w')
+    try:
+       for acl in acls:
+          aclfile.write("%s\n" % ":".join(acl))
+    except IOError:
+       return None
+    finally:
+       aclfile.close()
+
+def adduseracl(screen, size, acls):
+    rl=Label("Select users:")
+    count=0
+    userlist=[]
+    loggedusers=[]
+    for line in commands.getoutput('who').splitlines():
+       fields=line.split()
+       if fields[0] not in userlist:
+          loggedusers.append(fields[0])
+
+    allusers=pwd.getpwall()
+
+    # Dump users already with ACLs
+    acledusers=[]
+    for acl in acls:
+       acledusers.append(acl[0])
+
+    # Sort users with logged users first
+    #   then other users >= 1000
+    #   then root, then others
+    loggeduserslist=[]
+    otheruserslist=[]
+    otherslist=[]
+    for user in allusers:
+       if user[0] is not "root" and user[0] not in acledusers:
+          if user[0] in loggedusers:
+             loggeduserslist.append(user[0])
+          elif user[2] >= 1000:
+             otheruserslist.append(user[0])
+          else:
+             otherslist.append(user[0])
+    
+    loggeduserslist.sort()
+    otheruserslist.sort()
+    otherslist.sort()
+
+    userlist.extend(loggeduserslist)
+    userlist.extend(otheruserslist)
+    if "root" not in acledusers:
+       userlist.append("root")
+    userlist.extend(otherslist)
+
+    if len(userlist) > 10:
+        scroll=1
+        size=10
+    else:
+        scroll=0
+        size = len(userlist)
+    r=CheckboxTree(size, scroll=scroll)
+
+    invitedusers=[]
+    for user in userlist:
+       r.append(user,count,selected=0)
+       count=count+1
+    bb = ButtonBar(screen, ((_("Apply"), "apply"), (_("Cancel"), "cancel", ESC)), compact = 1)
+    g = GridForm(screen, _("Add new ACL:"), 2, 6)
+    acl=Entry(7, text="+r-w-x", returnExit=1)
+    acll=Label(_("ACL: "))
+
+
+    g.add(rl, 0, 0, anchorLeft=1, anchorTop=1, padding=(4,0,0,1))
+    g.add(r, 1, 0)
+
+    # TODO: use a list of avail actions instead?
+    actions=Entry(20, text="prev next select detach")
+    actionsl=Label(_("Allowed actions (* for all): "))
+
+    g.add(acll, 0, 2, anchorLeft=1,padding=(4,1,0,1))
+    g.add(acl, 1, 2, anchorLeft=1)
+    g.add(actionsl, 0, 3, anchorLeft=1, anchorTop=1,padding=(4,0,0,1))
+    g.add(actions, 1, 3, anchorLeft=1)
+
+    g.add(bb, 0, 5, padding=(4,1,0,0))
+
+    if bb.buttonPressed(g.runOnce()) != "cancel":
+       count=0
+       for user in userlist:
+           if r.getEntryValue(count)[1]:
+               invitedusers.append(user)
+               useracl=(user, acl.value(), actions.value())
+               acls.append(useracl)
+           count=count+1
+       execuseracl(invitedusers, actions.value().split(), acl.value())
+    return 100
+
+def updateacl(screen, size, acls, id):
+    useracl=acls[id]
+
+    bb = ButtonBar(screen, ((_("Apply"), "apply"), (_("Cancel"), "cancel", ESC)), compact = 1)
+    g = GridForm(screen, _("Update ACL for user %s:" % useracl[0]), 2, 6)
+
+    acl=Entry(7, text=useracl[1], returnExit=1)
+    acll=Label(_("ACL: "))
+
+    actions=Entry(20, text=useracl[2], returnExit=1)
+    actionsl=Label(_("Allowed actions (* for all): "))
+
+    rmall=Checkbox(_("Remove all ACL for this user"))
+
+    g.add(acll, 0, 2, anchorLeft=1,padding=(4,1,0,1))
+    g.add(acl, 1, 2, anchorLeft=1)
+    g.add(actionsl, 0, 3, anchorLeft=1, anchorTop=1,padding=(4,0,0,1))
+    g.add(actions, 1, 3, anchorLeft=1)
+
+    g.add(rmall, 1, 4, anchorLeft=1, padding=(4,1,0,1))
+
+    g.add(bb, 0, 5, padding=(4,1,0,0))
+    
+    if bb.buttonPressed(g.runOnce()) != "cancel":
+       if rmall.value():
+          commands.getoutput("screen -X acldel %s" % useracl[0])
+          acls.pop(id)
+       else:
+          userlist=[]
+          userlist.append(useracl[0])
+          execuseracl(userlist, actions.value().split(), acl.value())
+          acls[id]=(useracl[0], acl.value(), actions.value())
+
+    return 100 
+
+def inviteusers(screen, size):
+    acls=getuseracl()
+    bb = ButtonBar(screen, ((_("Apply"), "apply"), (_("Cancel"), "cancel", ESC)), compact = 1)
+    g = GridForm(screen, _("Invite users of modify ACL:"), 2, 6)
+
+    # Check if screen is setuid root (mode=3633)
+    mode = os.stat('/usr/bin/screen')
+    if not mode.st_mode & stat.S_ISUID:
+       msgtxt=_("""
+/usr/bin/screen must be setuid root to use this option.
+
+Execute "chmod 6755 /usr/bin/screen" as root
+   and relaunch byobu.
+""")
+       msg = Textbox(height = 6, width = 60, text=(msgtxt))
+       ok = Button("OK")
+       g.add(msg, 0, 0)
+       g.add(ok, 0, 1)
+
+       g.runOnce()
+       return 100
+
+    lil=Label("Select a user:")
+    li = Listbox(height = 6, width = 60, returnExit = 1)
+    count=0
+    for acl in acls:
+       li.append(acl[0], count)
+       count=count+1
+    li.append("Add new ACL", count)
+
+    g.add(lil, 0, 0, anchorLeft=1, anchorTop=1, padding=(4,0,0,1))
+    g.add(li, 1, 0)
+
+    g.add(bb, 0, 5, padding=(4,1,0,0))
+
+    if bb.buttonPressed(g.runOnce()) != "cancel":
+       if li.current() == count:
+          adduseracl(screen, size, acls)
+       else:
+          updateacl(screen, size, acls, li.current())
+    updateacls(acls)
+
+    return 100
+
 def install(screen, size, isInstalled):
     if not isInstalled:
         out = commands.getoutput("byobu-launcher-install")
@@ -530,6 +742,8 @@
         elif tag == 8:
             tag = defaultwindows(screen, size)
         elif tag == 9:
+            tag = inviteusers(screen, size)
+        elif tag == 10:
             tag = install(screen, size, isInstalled)
             isInstalled=(tag == 100)