#714455 molly-guard: Add debconf support

#714455#5
Date:
2013-06-29 14:51:43 UTC
From:
To:
Dear Maintainer,

In [1] (molly-guard: should divert rather than relying on $PATH ordering),
you have suggested to use debconf to divert the sysvint's shutdown
commands, and in a private email (in which you asked me to report
it as a bug - see [2] (molly-guard: SSH_CONNECTION can be kept with
specific sudoers config)), you talked to me about debconf to modify
sudo configuration: (Defaults env_keep+=SSH_CONNECTION).

So, as I had enough time this week to work on it, I have written a
debconf stuff that could be used as a base to include in the next
molly-guard version. I provide it as a patch (attachment 1) and as a
tarball (attachment 2) containing a git clone of molly-guard with an
additional 'debconf' branch.

Note that the molly-guard code is not modified: maintainer scripts,
a sudoers file, and another shell script have been added, the Makefile
has been modified to include the new files (and other similar and
debian related things in debian/control, debian/rules).

Debconf asks three questions, with the following template names:
- molly-guard/always_query_hostname
- molly-guard/keep_SSH_CONNECTION
- molly-guard/divert_shutdown_commands

They are booleans, and all default to 'false' (i.e do nothing).
Can you give it a try, please (or at least a review) ?

For the case you plan to include it in the next release of molly-guard,
(this should close the bugs #660064 and #705183, I can provide a
french translation for the po-debconf related files.

Cheers,
quidame

[1]: <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=660064#10>
[2]: <http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705183>

#714455#10
Date:
2013-07-23 10:41:44 UTC
From:
To:
also sprach bilibop project <quidame@poivron.org> [2013.06.29.1651 +0200]:

Hi!

Thanks, this is great. I am sorry I took so long to reply. I am
still very busy, but this is on my agenda for DebCamp on 6 August.

#714455#15
Date:
2013-07-23 10:41:44 UTC
From:
To:
also sprach bilibop project <quidame@poivron.org> [2013.06.29.1651 +0200]:

Hi!

Thanks, this is great. I am sorry I took so long to reply. I am
still very busy, but this is on my agenda for DebCamp on 6 August.

#714455#20
Date:
2013-07-24 10:38:52 UTC
From:
To:
Hi,
preseedable). You should review this one instead (see attachments).

Cheers,
quidame

#714455#25
Date:
2013-08-08 10:35:45 UTC
From:
To:
I looked at the second patch you sent, on 24 July 2013. Here are
some thoughts:

also sprach bilibop project <quidame@poivron.org> [2013.06.29.1651 +0200]:

[1] I am really not comfortable with diverting these integral
commands. The reason is that there might well be other tools that
rely on these and might trip over the requirement for a tty, or
unexpected output to stdout.

Don't you think this is too intrusive?

[2] Instead of setting env_keep globally, I suggest to create
a Cmnd_Alias for the commands molly-guard protects, and then to use
this syntax:

  Defaults!MOLLY_GUARD_COMMANDS env_keep+=SSH_CONNECTION

In general, this looks well done. I am really just hesitant to make
such changes at the moment, and this is mainly because I know I will
not be able to deal with bug reports if they come in over the next
months.

How would you feel taking over molly-guard maintenance? Then I would
suggest that we upload a new package to experimental and get some
testers.

Cheers,

#714455#30
Date:
2013-08-08 22:28:15 UTC
From:
To:
Hi,
I read "test -t 0 || exit 0" (in 30-query-hostname): so tty is required
to protect a command, not to execute it. The only one side effect I have
found comes from molly-guard itself (needs to export MOLLYGUARD_CMD
earlier in the shutdown script to avoid infinite loops with 'shutdown
-c'). But as you suggest below, this should be heavily tested.
default, and the question is asked with debconf priority "low". So the
diversion should be rare and only done by experienced users. Side
effects (if any) should be clearly exposed by debconf, with the question.

Additionally, when a command is diverted, this is generally to replace
it by another one; for example, if you install gnu-fdisk, this will
divert fdisk and cfdisk; then if you invoke 'cfdisk', the diverted
command will not be executed; here, the diversion is used to add a
check-point before the execution of the diverted command, and then call
it or exit. Finally, this is always the sysvinit's commands that are
used to halt/reboot/poweroff/shutdown the system. Maybe this usage of
dpkg-divert is a little bit hackish or not very canonical, but I don't
feel it so intrusive.

The global scheme is the following:

root@debian:~# halt (is /usr/sbin/halt)
then:
-> /usr/sbin/halt: *CHECK*
-> exit, or exec /sbin/halt
-> exec /sbin/halt.real

user@debian:~$ halt (is alias halt='sudo /sbin/halt')
then:
-> /sbin/halt (test the value of "$MOLLYGUARD_CMD")
-> exec /usr/sbin/halt: *CHECK* (and export MOLLYGUARD_CMD)
-> exit, or exec /sbin/halt (test the value of "$MOLLYGUARD_CMD")
-> exec /sbin/halt.real

If /usr/sbin/halt is not executable (/usr not mounted...), then:
-> /sbin/halt
-> exec /sbin/halt.real

Oh, yes (even if I think there is no need to be so precautionous with a
such variable)

I hear. But as I would be the author of such bugs, I should be able to
deal with reports about them. And I have enough time for that.

Alright. Sounds good to me. I'm not very experienced as a package
maintainer, so my question is: how to proceed ? (I suppose I have to
bump to compat 9 and update the Standards-Version to the current one,
add my name and email address here and there, create the missing
debian/source/format as "quilt", provide fr.po for debconf as proposed
in a previous email, fix lintian warnings and so on, and finally add an
entry in the changelog to report that and close the related bugs; but
after ?)

And for the tests... maybe the author of #660064 could be interested ?

Cheers,
quidame

#714455#35
Date:
2013-08-10 12:20:33 UTC
From:
To:
Hi,

user@debian:~$ sudo cat /etc/sudoers.d/molly-guard
# /etc/sudoers.d/molly-guard
# [...]
Cmnd_Alias MOLLY_GUARD_COMMANDS = /usr/sbin/halt, /sbin/halt, [...]
Defaults!MOLLY_GUARD_COMMANDS env_keep += SSH_CONNECTION

user@debian:~$ echo $SSH_CONNECTION
(source_ip source_port dest_ip dest_port)
user@debian:~$ sudo halt
(molly-guard prompts for the hostname)
user@debian:~$ sudo -s
root@debian:/home/user# echo $SSH_CONNECTION

root@debian:/home/user# halt
(unguarded; end of story)

In this example, I invoke directly a root shell; but the same happens if
I invoke another command that provides a shell, either directly (screen
or tmux), or by escaping to a shell or running a shell command (vim,
emacs, mc...)

So I would prefer: Defaults env_keep+=SSH_CONNECTION
Otherwise, the usecases for which shutdown commands are protected are
too specific and then these restricted sudoers settings provide no real
benefit.

There are some other pros:
- I don't think the export of SSH_CONNEXION into sudo environment is
  able to introduce a security issue; it is even not a pathname.
- Knowing that a lot of users use 'sudo -E' when they need to use such
  or such variable into sudo environment, provide some useful variable
  in the env_keep whitelist can refrain them to use the (unsecure) -E
  option.
- SSH_CONNECTION is exported in 'su', why not to export it in 'sudo' ?

Cheers,
quidame