Hi David! On 2004-08-02 12:01 +0200, David Zeuthen wrote: > However, if you guys are going to patch hal anyway it would of course be > easier to include the changes upstream if. And, in the long term, if > we're going to run as a unprivileged user we just might start putting it > in. I'm not sure though, there's also the SELinux stuff to consider. I agree. Especially since the privilege dropping is (currently) optional, i. e. even if you adopt all patches, there will still be no incompatible behaviour unless you explicitly start hal with --drop-privileges. I attached four patches which I propose for upstream inclusion. These are based against version 20040711. Let me know if something needs to be adapted to more recent versions. - 01_log_to_syslog actually uses syslog to log instead of logging to stderr. Since stderr is closed and redirected to /dev/null, hal logs into the void. - 02_ioctl_errors adds error log messages for all ioctls which were not checked before. Good for debugging. - 03_min_privileges adds the option --drop-privileges to hald which keeps only necessary capabilities and lets hald run as unprivileged user HAL_USER - 04_fix_hal_conf_in change the hardcoded "root" user to @HAL_USER@ in hal.conf.in. 01, 02 and 04 have nothing to do with security and just fix bugs/enhance functionality. As I already explained, 03 should not break anything, too as long as --drop-privileges is not used. Thanks for considering and have a nice day! Martin -- Martin Pitt Debian GNU/Linux Developer martin piware de mpitt debian org http://www.piware.de http://www.debian.org
--- hal-0.2.93+20040711.orig/hald/logger.c
+++ hal-0.2.93+20040711/hald/logger.c
@@ -31,6 +31,7 @@
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
+#include <syslog.h>
#include "logger.h"
@@ -53,6 +54,8 @@
void
logger_init ()
{
+ openlog( "hald", 0, LOG_DAEMON );
+
}
/** Setup logging entry
@@ -83,6 +86,7 @@
va_list args;
char buf[512];
char *pri;
+ int syslog_prio;
va_start (args, format);
vsnprintf (buf, 512, format, args);
@@ -90,25 +94,30 @@
switch (priority) {
case HAL_LOGPRI_TRACE:
pri = "[T]";
+ syslog_prio = LOG_DEBUG;
break;
case HAL_LOGPRI_DEBUG:
pri = "[D]";
+ syslog_prio = LOG_DEBUG;
break;
case HAL_LOGPRI_INFO:
pri = "[I]";
+ syslog_prio = LOG_INFO;
break;
case HAL_LOGPRI_WARNING:
pri = "[W]";
+ syslog_prio = LOG_WARNING;
break;
default: /* explicit fallthrough */
case HAL_LOGPRI_ERROR:
pri = "[E]";
+ syslog_prio = LOG_ERR;
break;
}
/** @todo Make programmatic interface to logging */
if (priority != HAL_LOGPRI_TRACE)
- fprintf (stderr, "%s %s:%d %s() : %s\n",
+ syslog (syslog_prio, "%s %s:%d %s() : %s\n",
pri, file, line, function, buf);
va_end (args);
--- hal-0.2.93+20040711.orig/hald/linux/input_class_device.c
+++ hal-0.2.93+20040711/hald/linux/input_class_device.c
@@ -38,6 +38,7 @@
#include <limits.h>
#include <fcntl.h>
#include <linux/input.h>
+#include <errno.h>
#include "../logger.h"
#include "../device_store.h"
@@ -135,7 +136,8 @@
if (ioctl(fd, EVIOCGNAME(sizeof name), name) >= 0) {
hal_device_property_set_string (d, "info.product",
name);
- }
+ } else
+ HAL_ERROR(("EVIOCGNAME failed: %s\n", strerror(errno)));
}
/* @todo add more ioctl()'s here */
--- hal-0.2.93+20040711.orig/hald/linux/printer_class_device.c
+++ hal-0.2.93+20040711/hald/linux/printer_class_device.c
@@ -41,6 +41,7 @@
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <fcntl.h>
+#include <errno.h>
#include "../logger.h"
#include "../device_store.h"
@@ -115,7 +116,9 @@
device_id) < 0) {
close (fd);
return;
- }
+ } else
+ HAL_ERROR(("LPIOC_GET_DEVICE_ID failed: %s\n", strerror(errno)));
+
close (fd);
--- hal-0.2.93+20040711.orig/hald/linux/block_class_device.c
+++ hal-0.2.93+20040711/hald/linux/block_class_device.c
@@ -187,7 +187,8 @@
if (fd < 0)
return;
- ioctl (fd, CDROM_SET_OPTIONS, CDO_USE_FFLAGS);
+ if( ioctl (fd, CDROM_SET_OPTIONS, CDO_USE_FFLAGS) < 0 )
+ HAL_ERROR(("CDROM_SET_OPTIONS failed: %s\n", strerror(errno)));
capabilities = ioctl (fd, CDROM_GET_CAPABILITY, 0);
if (capabilities < 0) {
@@ -674,6 +675,10 @@
got_disc = TRUE;
break;
+ case -1:
+ HAL_ERROR(("CDROM_DRIVE_STATUS failed: %s\n", strerror(errno)));
+ break;
+
default:
break;
}
@@ -827,6 +832,10 @@
child, "volume.disc.is_blank", TRUE);
break;
+ case -1:
+ HAL_ERROR(("CDROM_DISC_STATUS failed: %s\n", strerror(errno)));
+ break;
+
default: /* should never see this */
hal_device_property_set_string (child,
"volume.disc_type",
--- hal-0.2.93+20040711.orig/hald/hald.c
+++ hal-0.2.93+20040711/hald/hald.c
@@ -38,6 +38,9 @@
#include <fcntl.h>
#include <errno.h>
#include <signal.h>
+#include <sys/prctl.h>
+#include <sys/capability.h>
+#include <grp.h>
#include <dbus/dbus.h>
#include <dbus/dbus-glib.h>
@@ -161,6 +164,8 @@
fprintf (stderr,
"\n"
" --daemon=yes|no Become a daemon\n"
+ " --drop-privileges Run as normal user instead of root (calling of external\n"
+ " scripts to modify fstab etc. will not work any more)\n"
" --help Show this information and exit\n"
"\n"
"The HAL daemon detects devices present in the system and provides the\n"
@@ -175,6 +180,70 @@
/** If #TRUE, we will daemonize */
static dbus_bool_t opt_become_daemon = TRUE;
+
+/**
+ * Drop all but necessary privileges from hald when it runs as root. Set the
+ * running user id to 'hal' and groups to 'disk', 'floppy', and 'cdrom'. Drops
+ * all unnecessary capabilities. On error, a message is sent to syslog and
+ * the program terminates with exit code -1.
+ */
+void
+drop_privileges()
+{
+ cap_t cap;
+ struct passwd *pw = NULL;
+ struct group *gr = NULL;
+
+ /* determine user id */
+ pw = getpwnam( HAL_USER );
+ if( !pw ) {
+ HAL_ERROR(( "drop_privileges: user " HAL_USER " does not exist\n" ));
+ exit( -1 );
+ }
+
+ /* determine primary group id */
+ gr = getgrnam( HAL_GROUP );
+ if( !gr ) {
+ HAL_ERROR(( "drop_privileges: group " HAL_GROUP " does not exist\n" ));
+ exit( -1 );
+ }
+
+ /* keep capabilities and change uid/gid */
+ if( prctl( PR_SET_KEEPCAPS, 1, 0, 0, 0 ) ) {
+ HAL_ERROR(( "drop_privileges: could not keep capabilities\n" ));
+ exit( -1 );
+ }
+
+ if( initgroups( HAL_USER, gr->gr_gid ) ) {
+ HAL_ERROR(( "drop_privileges: could not initialize groups\n" ));
+ exit( -1 );
+ }
+
+ if( setgid( gr->gr_gid ) ) {
+ HAL_ERROR(( "drop_privileges: could not set group id\n" ));
+ exit( -1 );
+ }
+
+ if( setuid( pw->pw_uid ) ) {
+ HAL_ERROR(( "drop_privileges: could not set user id\n" ));
+ exit( -1 );
+ }
+
+ /* only keep necessary capabilities */
+ cap = cap_from_text( "cap_net_admin=ep" );
+
+ if( cap_set_proc( cap ) ) {
+ HAL_ERROR(( "drop_privileges: could not install capabilities\n" ));
+ exit( -1 );
+ }
+
+ if( cap_free( cap ) ) {
+ HAL_ERROR(( "drop_privileges: cap_free\n" ));
+ exit( -1 );
+ }
+}
+
+
/** Entry point for HAL daemon
*
* @param argc Number of arguments
@@ -194,6 +263,7 @@
static struct option long_options[] = {
{"daemon", 1, NULL, 0},
{"help", 0, NULL, 0},
+ {"drop-privileges", 0, NULL, 0},
{NULL, 0, NULL, 0}
};
@@ -218,7 +288,8 @@
usage ();
return 1;
}
- }
+ } else if (strcmp(opt, "drop-privileges") == 0)
+ drop_privileges ();
break;
default:
--- hal-0.2.93+20040711.orig/hald/Makefile.am
+++ hal-0.2.93+20040711/hald/Makefile.am
@@ -71,7 +71,7 @@
hald_SOURCES += \
linux/volume_id/volume_id.h linux/volume_id/volume_id.c
-hald_LDADD = @PACKAGE_LIBS@
+hald_LDADD = @PACKAGE_LIBS@ -lcap
#### Init scripts fun
SCRIPT_IN_FILES=haldaemon.in
--- hal-0.2.93+20040711.orig/hal.conf.in
+++ hal-0.2.93+20040711/hal.conf.in
@@ -9,7 +9,7 @@
<!-- Only user @HAL_USER@ can own the HAL service and be an agent -->
<!-- policy user="@HAL_USER@" We require root to sniff mii registers -->
- <policy user="root">
+ <policy user="@HAL_USER@">
<allow own="org.freedesktop.Hal"/>
<allow send_interface="org.freedesktop.Hal.AgentManager"
@@ -19,7 +19,7 @@
</policy>
<!-- Only root can use the Linux.Hotplug interface -->
- <policy user="root">
+ <policy user="@HAL_USER@">
<allow send_interface="org.freedesktop.Hal.Linux.Hotplug"
send_destination="org.freedesktop.Hal"/>
<allow receive_interface="org.freedesktop.Hal.Linux.Hotplug"
@@ -28,7 +28,7 @@
<!-- Any user in the @HAL_GROUP@ group can use the AgentManager interface -->
<!-- policy group="@HAL_GROUP@" Doesn't work on dbus 0.20. Works in CVS -->
- <policy user="root">
+ <policy user="@HAL_USER@">
<allow send_interface="org.freedesktop.Hal.AgentManager"
send_destination="org.freedesktop.Hal"/>
<allow receive_interface="org.freedesktop.Hal.AgentManager"
Attachment:
pgppKfIjtXnMd.pgp
Description: PGP signature