Discussion:
[PATCH v2] saned: reorganize flags, remove run_mode SANED_RUN_DEBUG
(too old to reply)
l***@gmail.com
2017-09-18 07:25:37 UTC
Permalink
Raw Message
From: Luiz Angelo Daros de Luca <***@gmail.com>

Flags like -a, -d and -s have many overlap effects. This patch restricts
the effect of flags to a simple action.

New -u (user) flag replaces -a optional argument for running saned as a different user.
The code that retrieve the user info and drop privileges migrated to runas_user().
As a side effect, PID file can be created even if getting user info fails.

New -l (listen) flag sets run_mode to standalone.
New -D (daemonize) flag daemonizes saned after bind.
New -o (once) make saned exit after the first client disconnects.
Flag -s (syslog) is gone. Previous behavior can be reproduced with '-a -d level -o -f'.
New -e (stderr) flag for redirecting output to stderr, instead of syslog.

Flag -d (debug) now only sets the debug level and argument is required. Previous behavior
can be reproduced with '-a -d level -o -f -e'.

The run_mode SANED_RUN_DEBUG and SANED_RUN_ALONE shared most of its code
path. With the new flags dealing with their difference, SANED_RUN_DEBUG is gone.

Flag '-a' still works as before but it can be replaced by '-l -D -u user'.

Current uses of -d (debug) or -s (syslog) will break.

Signed-off-by: Luiz Angelo Daros de Luca <***@gmail.com>
---
NEWS | 8 ++
doc/saned.man | 72 +++++++++------
frontend/saned.c | 272 +++++++++++++++++++++++++++++--------------------------
3 files changed, 197 insertions(+), 155 deletions(-)

diff --git a/NEWS b/NEWS
index 9ff64bf2..7d186ad5 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,12 @@
-*-Mode: outline-*-
+New with the development version, not yet released:
+
+* Saned options where reorganized (See "man 8 saned" for details):
+ o New: -l (listen), -D (daemonize), -o (once), -e (stderr), -u (user).
+ o Removed: -s (syslog). Use '-a -d level -o -f' for the old behavior.
+ o Changed: -d (debug). Use '-a -d level -o -f -e' for the old behavior.
+
+
New with 1.0.27 (see Note 1), released 2017-05-22:

* Significant enhancements to canon_dr, epjitsu, epsonds, fujitsu,
diff --git a/doc/saned.man b/doc/saned.man
index b716122d..98a5d5e6 100644
--- a/doc/saned.man
+++ b/doc/saned.man
@@ -6,15 +6,21 @@ saned \- SANE network daemon
.B saned
.B [ \-a
.I [ username ]
+.B ]
+.B [ \-u
+.I username
+.B ]
.B [ \-b
.I address
.B ]
-.B | \-d
-.I [ n ]
-.B | \-s
-.I [ n ]
-.B | \-h
+.B [ \-l ]
+.B [ \-D ]
+.B [ \-o ]
+.B [ \-d
+.I n
.B ]
+.B [ \-e ]
+.B [ \-h ]
.SH DESCRIPTION
.B saned
is the SANE (Scanner Access Now Easy) daemon that allows remote clients
@@ -22,51 +28,59 @@ to access image acquisition devices available on the local host.
.SH OPTIONS
.PP
The
-.B \-a
+.B \-l
flag requests that
.B saned
run in standalone daemon mode. In this mode,
.B saned
-will detach from the console and run in the background, listening for incoming
-client connections;
+will listening for incoming client connections;
.B inetd
is not required for
.B saned
-operations in this mode. If the optional
-.B username
-is given after
+operations in this mode. The
+.B \-b
+flag can control which address
+.B saned
+will bind. The
+.B \-u
+.I username
+flag requests that
+.B saned
+drop root privileges and run as this user (and group) after bind.
+The
+.B \-D
+flag will request
+.B saned
+to detach from the console and run in the background.
+The flag
.B \-a
-,
+is equals to
+.B \-l \-B \-u
+.I username
+.
+.PP
+The
+.B \-e
+flag will request that
.B saned
-will drop root privileges and run as this user (and group).
+output to stderr instead of syslog.
.PP
The
.B \-d
-and
-.B \-s
-flags request that
-.B saned
-run in debug mode (as opposed to
-.BR inetd (8)
-daemon mode). In this mode,
+flag sets the debug level of
.B saned
-explicitly waits for a connection request. When compiled with
-debugging enabled, these flags may be followed by a number to request
+. When compiled with debugging enabled, these flags may be followed by a number to request
debug info. The larger the number, the more verbose the debug output.
E.g.,
.B \-d128
will request printing of all debug info. Debug level 0 means no debug output
-at all. The default value is 2. If flag
-.B \-d
-is used, the debug messages will be printed to stderr while
-.B \-s
-requests using syslog.
+at all. The default value is 2.
.PP
The
-.B \-b
+.B \-o
flag requests that
.B saned
-bind to a specific address.
+exits after the first client disconnects. Useful for debugging.
.PP
If
.B saned
diff --git a/frontend/saned.c b/frontend/saned.c
index da965421..ed9fec7c 100644
--- a/frontend/saned.c
+++ b/frontend/saned.c
@@ -251,6 +251,8 @@ static Wire wire;
static int num_handles;
static int debug;
static int run_mode;
+static int run_foreground;
+static int run_once;
static int data_connect_timeout = 4000;
static Handle *handle;
static char *bind_addr;
@@ -299,9 +301,7 @@ static SANE_Bool log_to_syslog = SANE_TRUE;
static int process_request (Wire * w);

#define SANED_RUN_INETD 0
-#define SANED_RUN_DEBUG 1
-#define SANED_RUN_ALONE 2
-
+#define SANED_RUN_ALONE 1

#define DBG_ERR 1
#define DBG_WARN 2
@@ -3046,93 +3046,129 @@ do_bindings (int *nfds, struct pollfd **fds)


static void
-run_standalone (char *user)
+runas_user (char *user)
{
- struct pollfd *fds = NULL;
- struct pollfd *fdp = NULL;
- int nfds;
- int fd = -1;
- int i;
- int ret;
-
uid_t runas_uid = 0;
gid_t runas_gid = 0;
struct passwd *pwent;
gid_t *grplist = NULL;
struct group *grp;
int ngroups = 0;
- FILE *pidfile;
+ int ret;

- do_bindings (&nfds, &fds);
+ pwent = getpwnam(user);

- if (run_mode != SANED_RUN_DEBUG)
+ if (pwent == NULL)
{
- if (user)
- {
- pwent = getpwnam(user);
+ DBG (DBG_ERR, "FATAL ERROR: user %s not found on system\n", user);
+ bail_out (1);
+ }

- if (pwent == NULL)
- {
- DBG (DBG_ERR, "FATAL ERROR: user %s not found on system\n", user);
- bail_out (1);
- }
+ runas_uid = pwent->pw_uid;
+ runas_gid = pwent->pw_gid;

- runas_uid = pwent->pw_uid;
- runas_gid = pwent->pw_gid;
+ /* Get group list for runas_uid */
+ grplist = (gid_t *)malloc(sizeof(gid_t));

- /* Get group list for runas_uid */
- grplist = (gid_t *)malloc(sizeof(gid_t));
+ if (grplist == NULL)
+ {
+ DBG (DBG_ERR, "FATAL ERROR: cannot allocate memory for group list\n");

- if (grplist == NULL)
- {
- DBG (DBG_ERR, "FATAL ERROR: cannot allocate memory for group list\n");
+ exit (1);
+ }

- exit (1);
- }
+ ngroups = 1;
+ grplist[0] = runas_gid;

- ngroups = 1;
- grplist[0] = runas_gid;
+ setgrent();
+ while ((grp = getgrent()) != NULL)
+ {
+ int i = 0;

- setgrent();
- while ((grp = getgrent()) != NULL)
- {
- int i = 0;
+ /* Already added current group */
+ if (grp->gr_gid == runas_gid)
+ continue;

- /* Already added current group */
- if (grp->gr_gid == runas_gid)
- continue;
+ while (grp->gr_mem[i])
+ {
+ if (strcmp(grp->gr_mem[i], user) == 0)
+ {
+ int need_to_add = 1, j;

- while (grp->gr_mem[i])
+ /* Make sure its not already in list */
+ for (j = 0; j < ngroups; j++)
{
- if (strcmp(grp->gr_mem[i], user) == 0)
- {
- int need_to_add = 1, j;
-
- /* Make sure its not already in list */
- for (j = 0; j < ngroups; j++)
- {
- if (grp->gr_gid == grplist[i])
- need_to_add = 0;
- }
- if (need_to_add)
- {
- grplist = (gid_t *)realloc(grplist,
- sizeof(gid_t)*ngroups+1);
- if (grplist == NULL)
- {
- DBG (DBG_ERR, "FATAL ERROR: cannot reallocate memory for group list\n");
-
- exit (1);
- }
- grplist[ngroups++] = grp->gr_gid;
- }
- }
- i++;
- }
+ if (grp->gr_gid == grplist[i])
+ need_to_add = 0;
+ }
+ if (need_to_add)
+ {
+ grplist = (gid_t *)realloc(grplist,
+ sizeof(gid_t)*ngroups+1);
+ if (grplist == NULL)
+ {
+ DBG (DBG_ERR, "FATAL ERROR: cannot reallocate memory for group list\n");
+
+ exit (1);
+ }
+ grplist[ngroups++] = grp->gr_gid;
+ }
}
- endgrent();
+ i++;
+ }
+ }
+ endgrent();
+
+ /* Drop privileges if requested */
+ if (runas_uid > 0)
+ {
+ ret = setgroups(ngroups, grplist);
+ if (ret < 0)
+ {
+ DBG (DBG_ERR, "FATAL ERROR: could not set group list: %s\n", strerror(errno));
+
+ exit (1);
+ }
+
+ free(grplist);
+
+ ret = setegid (runas_gid);
+ if (ret < 0)
+ {
+ DBG (DBG_ERR, "FATAL ERROR: setegid to gid %d failed: %s\n", runas_gid, strerror (errno));
+
+ exit (1);
+ }
+
+ ret = seteuid (runas_uid);
+ if (ret < 0)
+ {
+ DBG (DBG_ERR, "FATAL ERROR: seteuid to uid %d failed: %s\n", runas_uid, strerror (errno));
+
+ exit (1);
}

+ DBG (DBG_WARN, "Dropped privileges to uid %d gid %d\n", runas_uid, runas_gid);
+ }
+}
+
+
+static void
+run_standalone (char *user)
+{
+ struct pollfd *fds = NULL;
+ struct pollfd *fdp = NULL;
+ int nfds;
+ int fd = -1;
+ int i;
+ int ret;
+
+ FILE *pidfile;
+
+ do_bindings (&nfds, &fds);
+
+ if (run_foreground == SANE_FALSE)
+ {
DBG (DBG_MSG, "run_standalone: daemonizing now\n");

fd = open ("/dev/null", O_RDWR);
@@ -3175,42 +3211,13 @@ run_standalone (char *user)

setsid ();

- /* Drop privileges if requested */
- if (runas_uid > 0)
- {
- ret = setgroups(ngroups, grplist);
- if (ret < 0)
- {
- DBG (DBG_ERR, "FATAL ERROR: could not set group list: %s\n", strerror(errno));
-
- exit (1);
- }
-
- free(grplist);
-
- ret = setegid (runas_gid);
- if (ret < 0)
- {
- DBG (DBG_ERR, "FATAL ERROR: setegid to gid %d failed: %s\n", runas_gid, strerror (errno));
-
- exit (1);
- }
-
- ret = seteuid (runas_uid);
- if (ret < 0)
- {
- DBG (DBG_ERR, "FATAL ERROR: seteuid to uid %d failed: %s\n", runas_uid, strerror (errno));
-
- exit (1);
- }
-
- DBG (DBG_WARN, "Dropped privileges to uid %d gid %d\n", runas_uid, runas_gid);
- }
-
signal(SIGINT, sig_int_term_handler);
signal(SIGTERM, sig_int_term_handler);
}

+ if (user)
+ runas_user(user);
+
#ifdef WITH_AVAHI
DBG (DBG_INFO, "run_standalone: spawning Avahi process\n");
saned_avahi (fds, nfds);
@@ -3269,13 +3276,13 @@ run_standalone (char *user)
continue;
}

- if (run_mode == SANED_RUN_DEBUG)
- break; /* We have the only connection we're going to handle */
- else
- handle_client (fd);
+ handle_client (fd);
+
+ if (run_once == SANE_TRUE)
+ break; /* We have handled the only connection we're going to handle */
}

- if (run_mode == SANED_RUN_DEBUG)
+ if (run_once == SANE_TRUE)
break;
}

@@ -3283,14 +3290,6 @@ run_standalone (char *user)
close (fdp->fd);

free (fds);
-
- if (run_mode == SANED_RUN_DEBUG)
- {
- if (fd > 0)
- handle_connection (fd);
-
- bail_out(0);
- }
}


@@ -3381,12 +3380,14 @@ static void usage(char *me, int err)
fprintf (stderr,
"Usage: %s [OPTIONS]\n\n"
" Options:\n\n"
- " -a, --alone[=user] run standalone and fork in background as `user'\n"
- " -d, --debug[=level] run foreground with output to stderr\n"
- " and debug level `level' (default is 2)\n"
- " -s, --syslog[=level] run foreground with output to syslog\n"
- " and debug level `level' (default is 2)\n"
- " -b, --bind=addr bind address `addr'\n"
+ " -a, --alone[=user] equals to `-l -D -u user'\n"
+ " -l, --listen run in standalone mode (listen for connection)\n"
+ " -u, --user=user run as `user'\n"
+ " -D, --daemonize run in background\n"
+ " -o, --once exit after first client disconnects\n"
+ " -d, --debug=level set debug level `level' (default is 2)\n"
+ " -e, --stderr output to stderr\n"
+ " -b, --bind=addr bind address `addr' (default all interfaces)\n"
" -h, --help show this help message and exit\n", me);

exit(err);
@@ -3399,8 +3400,12 @@ static struct option long_options[] =
/* These options set a flag. */
{"help", no_argument, 0, 'h'},
{"alone", optional_argument, 0, 'a'},
- {"debug", optional_argument, 0, 'd'},
- {"syslog", optional_argument, 0, 's'},
+ {"listen", no_argument, 0, 'l'},
+ {"user", required_argument, 0, 'u'},
+ {"daemonize", no_argument, 0, 'D'},
+ {"once", no_argument, 0, 'o'},
+ {"debug", required_argument, 0, 'd'},
+ {"stderr", no_argument, 0, 'e'},
{"bind", required_argument, 0, 'b'},
{0, 0, 0, 0 }
};
@@ -3424,20 +3429,35 @@ main (int argc, char *argv[])

numchildren = 0;
run_mode = SANED_RUN_INETD;
+ run_foreground = SANE_TRUE;
+ run_once = SANE_FALSE;

- while((c = getopt_long(argc, argv,"ha::d::s::b:", long_options, &long_index )) != -1)
+ while((c = getopt_long(argc, argv,"ha::lu:Dod:eb:", long_options, &long_index )) != -1)
{
switch(c) {
case 'a':
run_mode = SANED_RUN_ALONE;
+ run_foreground = SANE_FALSE;
+ if (optarg)
+ user = optarg;
+ break;
+ case 'l':
+ run_mode = SANED_RUN_ALONE;
+ break;
+ case 'u':
user = optarg;
break;
+ case 'D':
+ run_foreground = SANE_FALSE;
+ break;
+ case 'o':
+ run_once = SANE_TRUE;
+ break;
case 'd':
+ debug = atoi(optarg);
+ break;
+ case 'e':
log_to_syslog = SANE_FALSE;
- case 's':
- run_mode = SANED_RUN_DEBUG;
- if(optarg)
- debug = atoi(optarg);
break;
case 'b':
bind_addr = optarg;
@@ -3487,7 +3507,7 @@ main (int argc, char *argv[])
DBG (DBG_WARN, "saned from %s ready\n", PACKAGE_STRING);
}

- if ((run_mode == SANED_RUN_ALONE) || (run_mode == SANED_RUN_DEBUG))
+ if (run_mode == SANED_RUN_ALONE)
{
run_standalone(user);
}
--
2.11.0
--
sane-devel mailing list: sane-***@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel
Unsubscribe: Send mail with subject "unsubscribe your_password"
to sane-devel-***@lists.alioth.debian.org
Olaf Meeuwissen
2017-09-29 12:55:10 UTC
Permalink
Raw Message
Hi Luiz,

Thanks for the patch. LGTM with the exception of the manual page and a
typo in the -h output. I've fixed the typo and brushed up the manual
page's OPTIONS section and pushed the lot.

# I've only reviewed your patch and did not actually test anything.

Hope this helps,
--
Olaf Meeuwissen, LPIC-2 FSF Associate Member since 2004-01-27
GnuPG key: F84A2DD9/B3C0 2F47 EA19 64F4 9F13 F43E B8A4 A88A F84A 2DD9
Support Free Software https://my.fsf.org/donate
Join the Free Software Foundation https://my.fsf.org/join
--
sane-devel mailing list: sane-***@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel
Unsubscribe: Send mail with subject "unsubscribe your_password"
to sane-devel-***@lists.alioth.debian.org
Loading...