From: Ted X Toth <txtoth_at_gmail.com>
Date: Fri, 04 May 2007 15:15:04 -0500

Stephen Smalley wrote:

> On Fri, 2007-05-04 at 13:56 -0500, Ted X Toth wrote:
>   
>> Stephen Smalley wrote:
>>     

>>> How about the revised patch below (only including the newrole.c and
>>> Makefile diffs since the hashtab code is unchanged)? The changes from
>>> your patch are:
>>> - Make sure everything is properly enabled/disabled by USE_PAM and move
>>> the code into the existing USE_PAM block where appropriate.
>>> - Call the config file newrole_pam.conf since there could be other
>>> newrole config files in the future.
>>> - Distinguish missing config file (ok) from errors during parsing of the
>>> config file (should abort).
>>> - Remove the Authenticating <username> message since it could be
>>> confusing in the case where you are using a pam config that doesn't
>>> require it and it doesn't really provide any benefit.
>>> - Improve error checking and handling.
>>> - Coding style cleanups (indentation, comment style, etc).
>>>
>>> To test, I created a /etc/pam.d/newrole-noauth config that had
>>> pam_permit.so for its auth module and created
>>> a /etc/selinux/newrole_pam.conf that mapped one program to
>>> newrole-noauth.
>>>
>>> The alternative model would be to eliminate /etc/selnux/newrole_pam.conf
>>> entirely from the equation, and just have newrole look for (test via
>>> access()) a /etc/pam.d/newrole_<appname> config and use
>>> newrole_<appname> as the service name if present.
>>>
>>>

>> I like it but one thing I had thought about was that if the current 
>> patch were to check for the existence of the pam configuration file 
>> (service name) and the file was missing the user could be warned of 
>> that. Of course there is no help in the event that newrole_pam.conf is 
>> misconfigured.
>>     
>
> Sorry, are you suggesting eliminating the need for newrole_pam.conf
> altogether (as I described above as an alternative, implicitly mapping
> each app to a newrole_<app> service name if present) or retaining it but
> just adding a test for the presence of the pam config for error
> reporting purposes?
>   

I don't have a strong feeling one way or the other. Eliminating newrole_pam.conf would certainly simplify the implementation. I was just thinking about whether there were any good arguments for keeping it and usability was the only thing I could come up with.

> I'll incorporate Darrel's suggestion about using the basename() of the
> command in the next version of the patch.
>
>
>   

>>> Index: policycoreutils/newrole/newrole.c
>>> ===================================================================
>>> --- policycoreutils/newrole/newrole.c (revision 2422)
>>> +++ policycoreutils/newrole/newrole.c (working copy)
>>> @@ -58,6 +58,7 @@
>>> #include <stdio.h>
>>> #include <stdlib.h> /* for malloc(), realloc(), free() */
>>> #include <pwd.h> /* for getpwuid() */
>>> +#include <ctype.h>
>>> #include <sys/types.h> /* to make getuid() and getpwuid() happy */
>>> #include <sys/wait.h> /* for wait() */
>>> #include <getopt.h> /* for getopt_long() form of getopt() */
>>> @@ -92,6 +93,10 @@
>>> /* USAGE_STRING describes the command-line args of this program. */
>>> #define USAGE_STRING "USAGE: newrole [ -r role ] [ -t type ] [ -l level ] [ -p ] [ -V ] [ -- args ]"
>>>
>>> +#ifdef USE_PAM
>>> +#define PAM_SERVICE_CONFIG "/etc/selinux/newrole_pam.conf";
>>> +#endif
>>> +
>>> #define DEFAULT_PATH "/usr/bin:/bin"
>>> #define DEFAULT_CONTEXT_SIZE 255 /* first guess at context size */
>>>
>>> @@ -159,7 +164,7 @@
>>> #include <security/pam_appl.h> /* for PAM functions */
>>> #include <security/pam_misc.h> /* for misc_conv PAM utility function */
>>>
>>> -#define SERVICE_NAME "newrole" /* the name of this program for PAM */
>>> +char *service_name = "newrole";
>>>
>>> /* authenticate_via_pam()
>>> *
>>> @@ -209,6 +214,113 @@
>>> return result;
>>> } /* authenticate_via_pam() */
>>>
>>> +#include "hashtab.h"
>>> +
>>> +static int free_hashtab_entry(hashtab_key_t key, hashtab_datum_t d,
>>> + void *args __attribute__ ((unused)) )
>>> +{
>>> + free(key);
>>> + free(d);
>>> + return 0;
>>> +}
>>> +
>>> +static unsigned int reqsymhash(hashtab_t h, hashtab_key_t key)
>>> +{
>>> + char *p, *keyp;
>>> + size_t size;
>>> + unsigned int val;
>>> +
>>> + val = 0;
>>> + keyp = (char *)key;
>>> + size = strlen(keyp);
>>> + for (p = keyp; ((size_t) (p - keyp)) < size; p++)
>>> + val =
>>> + (val << 4 | (val >> (8 * sizeof(unsigned int) - 4))) ^ (*p);
>>> + return val & (h->size - 1);
>>> +}
>>> +
>>> +static int reqsymcmp(hashtab_t h
>>> + __attribute__ ((unused)), hashtab_key_t key1,
>>> + hashtab_key_t key2)
>>> +{
>>> + char *keyp1, *keyp2;
>>> +
>>> + keyp1 = (char *)key1;
>>> + keyp2 = (char *)key2;
>>> + return strcmp(keyp1, keyp2);
>>> +}
>>> +
>>> +static hashtab_t app_service_names = NULL;
>>> +#define PAM_SERVICE_SLOTS 64
>>> +
>>> +static int process_pam_config(FILE * cfg)
>>> +{
>>> + const char *config_file_path = PAM_SERVICE_CONFIG;
>>> + char *line_buf = NULL;
>>> + unsigned long lineno = 0;
>>> + size_t len = 0;
>>> + char *app = NULL;
>>> + char *service = NULL;
>>> + int ret;
>>> +
>>> + while (getline(&line_buf, &len, cfg) > 0) {
>>> + char *buffer = line_buf;
>>> + lineno++;
>>> + while (isspace(*buffer))
>>> + buffer++;
>>> + if (buffer[0] == '#')
>>> + continue;
>>> + if (buffer[0] == '\n' || buffer[0] == '\0')
>>> + continue;
>>> +
>>> + app = service = NULL;
>>> + ret = sscanf(buffer, "%as %as\n", &app, &service);
>>> + if (ret < 2 || !app || !service)
>>> + goto err;
>>> +
>>> + ret = hashtab_insert(app_service_names, app, service);
>>> + if (ret == HASHTAB_OVERFLOW) {
>>> + fprintf(stderr,
>>> + _("newrole: service name configuration hashtable overflow\n"));
>>> + goto err;
>>> + }
>>> + }
>>> +
>>> + free(line_buf);
>>> + return 0;
>>> + err:
>>> + free(app);
>>> + free(service);
>>> + fprintf(stderr, _("newrole: %s: error on line %lu.\n"), config_file_path, lineno);
>>> + free(line_buf);
>>> + return -1;
>>> +}
>>> +
>>> +/*
>>> + * Read config file ignoring comment lines.
>>> + * Files specified one per line executable with a corresponding
>>> + * pam service name.
>>> + */
>>> +static int read_pam_config()
>>> +{
>>> + const char *config_file_path = PAM_SERVICE_CONFIG;
>>> + FILE *cfg = NULL;
>>> + cfg = fopen(config_file_path, "r");
>>> + if (!cfg)
>>> + return 0; /* This configuration is optional. */
>>> + app_service_names =
>>> + hashtab_create(reqsymhash, reqsymcmp, PAM_SERVICE_SLOTS);
>>> + if (!app_service_names)
>>> + goto err;
>>> + if (process_pam_config(cfg))
>>> + goto err;
>>> + fclose(cfg);
>>> + return 0;
>>> + err:
>>> + fclose(cfg);
>>> + return -1;
>>> +}
>>> +
>>> #else /* else !USE_PAM */
>>>
>>> /************************************************************************
>>> @@ -1027,9 +1139,33 @@
>>> if (extract_pw_data(&pw))
>>> goto err_free;
>>>
>>> - printf(_("Authenticating %s.\n"), pw.pw_name);
>>> #ifdef USE_PAM
>>> - pam_status = pam_start(SERVICE_NAME, pw.pw_name, &pam_conversation,
>>> + if (read_pam_config()) {
>>> + fprintf(stderr, _("error on reading PAM service configuration.\n"));
>>> + goto err_free;
>>> + }
>>> +
>>> + if (app_service_names != NULL && optind < argc) {
>>> + if (strcmp(argv[optind], "-c") == 0 && optind < (argc - 1)) {
>>> + /*
>>> + * Check for a separate pam service name for the command when
>>> + * invoked by newrole.
>>> + */
>>> + char *cmd = NULL;
>>> + rc = sscanf(argv[optind + 1], "%as", &cmd);
>>> + if (rc != EOF && cmd != NULL) {
>>> + char *app_service_name =
>>> + (char *)hashtab_search(app_service_names,
>>> + cmd);
>>> + free(cmd);
>>> + if (app_service_name != NULL) {
>>> + service_name = app_service_name;
>>> + }
>>> + }
>>> + }
>>> + }
>>> +
>>> + pam_status = pam_start(service_name, pw.pw_name, &pam_conversation,
>>> &pam_handle);
>>> if (pam_status != PAM_SUCCESS) {
>>> fprintf(stderr, _("failed to initialize PAM\n"));
>>> @@ -1118,6 +1254,8 @@
>>> pam_strerror(pam_handle, rc));
>>> exit_code = -1;
>>> }
>>> + hashtab_map(app_service_names, free_hashtab_entry, NULL);
>>> + hashtab_destroy(app_service_names);
>>> #endif
>>> free(pw.pw_name);
>>> free(pw.pw_dir);
>>> @@ -1223,5 +1361,11 @@
>>> free(pw.pw_dir);
>>> free(pw.pw_shell);
>>> free(shell_argv0);
>>> +#ifdef USE_PAM
>>> + if (app_service_names) {
>>> + hashtab_map(app_service_names, free_hashtab_entry, NULL);
>>> + hashtab_destroy(app_service_names);
>>> + }
>>> +#endif
>>> return -1;
>>> } /* main() */
>>> Index: policycoreutils/newrole/Makefile
>>> ===================================================================
>>> --- policycoreutils/newrole/Makefile (revision 2422)
>>> +++ policycoreutils/newrole/Makefile (working copy)
>>> @@ -21,10 +21,12 @@
>>> VERSION = $(shell cat ../VERSION)
>>>
>>> CFLAGS ?= -Werror -Wall -W
>>> +EXTRA_OBJS =
>>> override CFLAGS += -DVERSION=\"$(VERSION)\" $(LDFLAGS) -I$(PREFIX)/include -DUSE_NLS -DLOCALEDIR="\"$(LOCALEDIR)\"" -DPACKAGE="\"policycoreutils\""
>>> LDLIBS += -lselinux -lsepol -L$(PREFIX)/lib
>>> ifeq (${PAMH}, /usr/include/security/pam_appl.h)
>>> override CFLAGS += -DUSE_PAM
>>> + EXTRA_OBJS += hashtab.o
>>> LDLIBS += -lpam -lpam_misc
>>> else
>>> override CFLAGS += -D_XOPEN_SOURCE=500
>>> @@ -53,9 +55,10 @@
>>> MODE := 0555
>>> endif
>>>
>>> -TARGETS=$(patsubst %.c,%,$(wildcard *.c))
>>> +all: newrole
>>>
>>> -all: $(TARGETS)
>>> +newrole: newrole.o $(EXTRA_OBJS)
>>> + $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS)
>>>
>>> install: all
>>> test -d $(BINDIR) || install -m 755 -d $(BINDIR)
>>>
>>>
>>>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Received on Fri 4 May 2007 - 16:15:16 EDT