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

Proposed by Raphaël Pinson on 2010-01-28
Status: Needs review
Proposed branch: lp:~raphink/byobu/acl
Merge into: lp:byobu
Diff against target: 266 lines (+218/-4)
1 file modified
usr/bin/byobu-config (+218/-4)
To merge this branch: bzr merge lp:~raphink/byobu/acl
Reviewer Review Type Date Requested Status
Dustin Kirkland  2010-01-28 Needs Fixing on 2010-04-24
Review via email: mp+18205@code.launchpad.net
To post a comment you must log in.
Raphaël Pinson (raphink) wrote :

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 updated on 2010-02-03
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) wrote :

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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
22# ./debian/rules get-po22# ./debian/rules get-po
2323
2424
25import sys, os, os.path, time, string, commands, gettext, glob, snack25import sys, os, os.path, time, string, commands, gettext, glob, snack, pwd, tempfile, stat
26from ConfigParser import SafeConfigParser26from ConfigParser import SafeConfigParser
27from snack import *27from snack import *
2828
@@ -37,7 +37,9 @@
37 DOC = "%s/.%s/%s" % (HOME, PKG, DOC)37 DOC = "%s/.%s/%s" % (HOME, PKG, DOC)
38DEF_ESC="A"38DEF_ESC="A"
39RELOAD = "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."39RELOAD = "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."
40RELOAD_FLAG="/var/run/screen/S-"+USER+"/"+PKG+".reload-required"40SOCKET_DIR="/var/run/screen/S-"+USER+"/"
41RELOAD_FLAG=SOCKET_DIR+PKG+".reload-required"
42ACL_FILE=SOCKET_DIR+PKG+".acl"
41RELOAD_CMD="screen -X at 0 source $HOME/."+PKG+"/profile"43RELOAD_CMD="screen -X at 0 source $HOME/."+PKG+"/profile"
42ESC = ''44ESC = ''
43snack.hotkeys[ESC] = ord(ESC)45snack.hotkeys[ESC] = ord(ESC)
@@ -89,7 +91,7 @@
89 installtext=_("Byobu currently does not launch at login (toggle on)")91 installtext=_("Byobu currently does not launch at login (toggle on)")
9092
9193
92 li = Listbox(height = 9, width = 60, returnExit = 1)94 li = Listbox(height = 10, width = 60, returnExit = 1)
93 li.append(_("Help"), 1)95 li.append(_("Help"), 1)
94 li.append(_("Change Byobu's background color"), 2)96 li.append(_("Change Byobu's background color"), 2)
95 li.append(_("Change Byobu's foreground color"), 3)97 li.append(_("Change Byobu's foreground color"), 3)
@@ -98,7 +100,8 @@
98 li.append(_("Change escape sequence"), 6)100 li.append(_("Change escape sequence"), 6)
99 li.append(_("Create new windows"), 7)101 li.append(_("Create new windows"), 7)
100 li.append(_("Manage default windows"), 8)102 li.append(_("Manage default windows"), 8)
101 li.append(installtext, 9)103 li.append(_("Invite users or modify ACLs"), 9)
104 li.append(installtext, 10)
102 bb = ButtonBar(screen, (("Exit", "exit", ESC),), compact=1)105 bb = ButtonBar(screen, (("Exit", "exit", ESC),), compact=1)
103106
104 g = GridForm(screen, _(" Byobu Configuration Menu"), 1, 2)107 g = GridForm(screen, _(" Byobu Configuration Menu"), 1, 2)
@@ -404,6 +407,215 @@
404407
405 return 100408 return 100
406409
410def execuseracl(userlist, actions, acl):
411 commandfile=tempfile.mkstemp()
412 f=open(commandfile[1], 'w')
413 try:
414 for user in userlist:
415 f.write("aclumask %s%s\n" % (user, acl))
416 f.write("aclchg %s %s \"#?\"\n" % (user, acl))
417 f.write("aclchg %s +x \"%s\"\n" % (user, " ".join(actions)))
418 f.write("multiuser on\n")
419 except IOError:
420 return None
421 finally:
422 f.close()
423 commands.getoutput("screen -X source %s" % commandfile[1])
424
425 return 100
426
427def getuseracl():
428 users=[]
429 if not os.path.isfile(ACL_FILE):
430 return users
431
432 aclfile=open(ACL_FILE, 'r')
433 try:
434 acls=aclfile.read().splitlines()
435 except IOError:
436 return None
437 finally:
438 aclfile.close()
439
440 for acl in acls:
441 users.append(acl.split(':'))
442
443 return users
444
445def updateacls(acls):
446 aclfile=open(ACL_FILE, 'w')
447 try:
448 for acl in acls:
449 aclfile.write("%s\n" % ":".join(acl))
450 except IOError:
451 return None
452 finally:
453 aclfile.close()
454
455def adduseracl(screen, size, acls):
456 rl=Label("Select users:")
457 count=0
458 userlist=[]
459 loggedusers=[]
460 for line in commands.getoutput('who').splitlines():
461 fields=line.split()
462 if fields[0] not in userlist:
463 loggedusers.append(fields[0])
464
465 allusers=pwd.getpwall()
466
467 # Dump users already with ACLs
468 acledusers=[]
469 for acl in acls:
470 acledusers.append(acl[0])
471
472 # Sort users with logged users first
473 # then other users >= 1000
474 # then root, then others
475 loggeduserslist=[]
476 otheruserslist=[]
477 otherslist=[]
478 for user in allusers:
479 if user[0] is not "root" and user[0] not in acledusers:
480 if user[0] in loggedusers:
481 loggeduserslist.append(user[0])
482 elif user[2] >= 1000:
483 otheruserslist.append(user[0])
484 else:
485 otherslist.append(user[0])
486
487 loggeduserslist.sort()
488 otheruserslist.sort()
489 otherslist.sort()
490
491 userlist.extend(loggeduserslist)
492 userlist.extend(otheruserslist)
493 if "root" not in acledusers:
494 userlist.append("root")
495 userlist.extend(otherslist)
496
497 if len(userlist) > 10:
498 scroll=1
499 size=10
500 else:
501 scroll=0
502 size = len(userlist)
503 r=CheckboxTree(size, scroll=scroll)
504
505 invitedusers=[]
506 for user in userlist:
507 r.append(user,count,selected=0)
508 count=count+1
509 bb = ButtonBar(screen, ((_("Apply"), "apply"), (_("Cancel"), "cancel", ESC)), compact = 1)
510 g = GridForm(screen, _("Add new ACL:"), 2, 6)
511 acl=Entry(7, text="+r-w-x", returnExit=1)
512 acll=Label(_("ACL: "))
513
514
515 g.add(rl, 0, 0, anchorLeft=1, anchorTop=1, padding=(4,0,0,1))
516 g.add(r, 1, 0)
517
518 # TODO: use a list of avail actions instead?
519 actions=Entry(20, text="prev next select detach")
520 actionsl=Label(_("Allowed actions (* for all): "))
521
522 g.add(acll, 0, 2, anchorLeft=1,padding=(4,1,0,1))
523 g.add(acl, 1, 2, anchorLeft=1)
524 g.add(actionsl, 0, 3, anchorLeft=1, anchorTop=1,padding=(4,0,0,1))
525 g.add(actions, 1, 3, anchorLeft=1)
526
527 g.add(bb, 0, 5, padding=(4,1,0,0))
528
529 if bb.buttonPressed(g.runOnce()) != "cancel":
530 count=0
531 for user in userlist:
532 if r.getEntryValue(count)[1]:
533 invitedusers.append(user)
534 useracl=(user, acl.value(), actions.value())
535 acls.append(useracl)
536 count=count+1
537 execuseracl(invitedusers, actions.value().split(), acl.value())
538 return 100
539
540def updateacl(screen, size, acls, id):
541 useracl=acls[id]
542
543 bb = ButtonBar(screen, ((_("Apply"), "apply"), (_("Cancel"), "cancel", ESC)), compact = 1)
544 g = GridForm(screen, _("Update ACL for user %s:" % useracl[0]), 2, 6)
545
546 acl=Entry(7, text=useracl[1], returnExit=1)
547 acll=Label(_("ACL: "))
548
549 actions=Entry(20, text=useracl[2], returnExit=1)
550 actionsl=Label(_("Allowed actions (* for all): "))
551
552 rmall=Checkbox(_("Remove all ACL for this user"))
553
554 g.add(acll, 0, 2, anchorLeft=1,padding=(4,1,0,1))
555 g.add(acl, 1, 2, anchorLeft=1)
556 g.add(actionsl, 0, 3, anchorLeft=1, anchorTop=1,padding=(4,0,0,1))
557 g.add(actions, 1, 3, anchorLeft=1)
558
559 g.add(rmall, 1, 4, anchorLeft=1, padding=(4,1,0,1))
560
561 g.add(bb, 0, 5, padding=(4,1,0,0))
562
563 if bb.buttonPressed(g.runOnce()) != "cancel":
564 if rmall.value():
565 commands.getoutput("screen -X acldel %s" % useracl[0])
566 acls.pop(id)
567 else:
568 userlist=[]
569 userlist.append(useracl[0])
570 execuseracl(userlist, actions.value().split(), acl.value())
571 acls[id]=(useracl[0], acl.value(), actions.value())
572
573 return 100
574
575def inviteusers(screen, size):
576 acls=getuseracl()
577 bb = ButtonBar(screen, ((_("Apply"), "apply"), (_("Cancel"), "cancel", ESC)), compact = 1)
578 g = GridForm(screen, _("Invite users of modify ACL:"), 2, 6)
579
580 # Check if screen is setuid root (mode=3633)
581 mode = os.stat('/usr/bin/screen')
582 if not mode.st_mode & stat.S_ISUID:
583 msgtxt=_("""
584/usr/bin/screen must be setuid root to use this option.
585
586Execute "chmod 6755 /usr/bin/screen" as root
587 and relaunch byobu.
588""")
589 msg = Textbox(height = 6, width = 60, text=(msgtxt))
590 ok = Button("OK")
591 g.add(msg, 0, 0)
592 g.add(ok, 0, 1)
593
594 g.runOnce()
595 return 100
596
597 lil=Label("Select a user:")
598 li = Listbox(height = 6, width = 60, returnExit = 1)
599 count=0
600 for acl in acls:
601 li.append(acl[0], count)
602 count=count+1
603 li.append("Add new ACL", count)
604
605 g.add(lil, 0, 0, anchorLeft=1, anchorTop=1, padding=(4,0,0,1))
606 g.add(li, 1, 0)
607
608 g.add(bb, 0, 5, padding=(4,1,0,0))
609
610 if bb.buttonPressed(g.runOnce()) != "cancel":
611 if li.current() == count:
612 adduseracl(screen, size, acls)
613 else:
614 updateacl(screen, size, acls, li.current())
615 updateacls(acls)
616
617 return 100
618
407def install(screen, size, isInstalled):619def install(screen, size, isInstalled):
408 if not isInstalled:620 if not isInstalled:
409 out = commands.getoutput("byobu-launcher-install")621 out = commands.getoutput("byobu-launcher-install")
@@ -530,6 +742,8 @@
530 elif tag == 8:742 elif tag == 8:
531 tag = defaultwindows(screen, size)743 tag = defaultwindows(screen, size)
532 elif tag == 9:744 elif tag == 9:
745 tag = inviteusers(screen, size)
746 elif tag == 10:
533 tag = install(screen, size, isInstalled)747 tag = install(screen, size, isInstalled)
534 isInstalled=(tag == 100)748 isInstalled=(tag == 100)
535749

Subscribers

People subscribed via source and target branches