genhomedircon replacement in libsemanage

plain text document attachment (genhomedircon-replacement-initial) Remove python script genhomedircon from libsemanage and replace with C functionality.

Index: ghdc/libsemanage/src/genhomedircon.c
===================================================================

libsemanage/src/genhomedircon.c new file +19 -0

@@ -0,0 +1,590 @@
+/* Author: Mark Goldman   <[email protected]>
+ *
+ * Copyright (C) 2007 Tresys Technology, LLC
+ *
+ *  This library is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU Lesser General Public License as
+ *  published by the Free Software Foundation; either version 2.1 of the
+ *  License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ *  02110-1301  USA
+ */

libsemanage/src/genhomedircon.c new file +10 -0

+int semanage_genhomedircon(semanage_handle_t * sh, int usepasswd)
+{
+	state_t s;
+
+	char outfile[PATH_MAX];
+	char hdt[PATH_MAX];
+	char fcf[PATH_MAX];
+
+	FILE *out = NULL;
+	int retval = 0;

libsemanage/src/genhomedircon.c new file +12 -0

+	semanage_list_push(&list, "/bin/csh");
+	semanage_list_push(&list, "/bin/tcsh");
+	semanage_list_push(&list, "/bin/ksh");
+	semanage_list_push(&list, "/bin/bsh");
+	semanage_list_push(&list, "/bin/ash");
+	semanage_list_push(&list, "/usr/bin/ksh");
+	semanage_list_push(&list, "/usr/bin/pdksh");
+	semanage_list_push(&list, "/bin/zsh");
+	semanage_list_push(&list, "/bin/sh");
+	semanage_list_push(&list, "/bin/bash");
+	return list;
+}

libsemanage/src/genhomedircon.c new file +52 -0

+		if (!strcmp(temp, "/sbin/nologin")) {
+			free(temp);
+			continue;
+		}
+		semanage_list_push(&list, temp);
+		free(temp);
+	}
+	return list;
+}
+
+static list_t *get_home_dirs(state_t * s)
+{
+	list_t *l = NULL;
+	list_t *shells = NULL;
+	char *path;
+	size_t minuid = 2000000;	/* sentinal value, UIDs can't be
+					 * above ~64k */
+	size_t temp;
+	struct passwd *pwbuf;
+	struct stat buf;
+
+	path = semanage_findval("/etc/default/useradd", "HOME", "=");
+	if (path && *path) {
+		semanage_list_push(&l, path);
+	}
+	free(path);
+
+	path = semanage_findval("/etc/libuser.conf", "LU_HOMEDIRECTORY", "=");
+	if (path && *path) {
+		semanage_list_push(&l, path);
+	}
+	free(path);
+
+	if (!l)
+		semanage_list_push(&l, strdup("/home"));
+
+	if (!stat("/export/home", &buf)) {
+		if (buf.st_mode | S_IFDIR)
+			semanage_list_push(&l, strdup("/export/home"));
+	}
+
+	if (!(s->usepasswd))
+		return l;
+
+	path = semanage_findval("/etc/login.defs", "UID_MIN", NULL);
+	if (path && *path) {
+		temp = atoi(path);
+		if (temp < minuid)
+			minuid = temp;
+	}
+	free(path);
+

libsemanage/src/genhomedircon.c new file +24 -0

+		if (pwbuf->pw_uid < minuid)
+			continue;
+		if (!semanage_list_find(shells, pwbuf->pw_shell))
+			continue;
+		if (strcmp(pwbuf->pw_dir, "/") == 0)
+			continue;
+		if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
+			continue;
+		path = strdup(pwbuf->pw_dir);
+		semanage_rtrim(path, '/');
+		if (!semanage_list_find(l, path)) {
+			semanage_list_push(&l, path);	
+		}
+		free(path);
+	}
+
+	/* clean up: I'm returning l so I don't need to do anything with it.
+	 *           I free the memory pointed to by path as soon as I'm done.
+	 *           That leaves the shells list which I destoy here.
+	 */
+	endpwent();
+	semanage_list_destroy(&shells);
+
+	semanage_list_sort(&l);

libsemanage/src/genhomedircon.c new file +80 -0

+
+	fprintf(out,
+		"#\n#\n# User-specific file contexts, generated via libsemanage\n");
+	fprintf(out,
+		"# use semanage command to manage system users to change the"
+		" file_context\n#\n#\n");
+	return 0;
+}
+
+/* Predicates for use with semanage_slurp_file_filter() the homedir_template file
+ * currently contains lines that serve as the template for a user's
+ * homedir.
+ *
+ * It also contains lines that are the template for the parent of a
+ * user's home directory.
+ *
+ * Currently, the only lines that apply to the the root of a user's home
+ * directory are all prefixed with the string "HOME_ROOT".  All other
+ * lines apply to a user's home directory.  If this changes the
+ * following predicates need to change to reflect that.
+ */
+static int HOME_ROOT_PRED(char *string)
+{
+	return semanage_prefix_of(string, "HOME_ROOT");
+}
+
+static int HOME_DIR_PRED(char *string)
+{
+	return semanage_prefix_of(string, "HOME_DIR");
+}
+
+static int USER_CONTEXT_PRED(char *string)
+{
+	return (int)(strstr(string, "USER") != NULL);
+}
+
+/* make_home_dir_tempate
+ * make_home_root_template
+ * @param	s	  the state holding the paths to various files
+ * @return   a list of lines from the template file with inappropriate
+ *	    lines filtered out.
+ */
+static list_t *make_home_dir_template(state_t * s)
+{
+	FILE *template_file = NULL;
+	list_t *template_data = NULL;
+
+	template_file = fopen(s->homedir_template_path, "r");
+	if (!template_file)
+		return NULL;
+	template_data = semanage_slurp_file_filter(template_file, &HOME_DIR_PRED);
+	fclose(template_file);
+	return template_data;
+}
+
+static list_t *make_home_root_template(state_t * s)
+{
+	FILE *template_file = NULL;
+	list_t *template_data = NULL;
+
+	template_file = fopen(s->homedir_template_path, "r");
+	if (!template_file)
+		return NULL;
+	template_data = semanage_slurp_file_filter(template_file, &HOME_ROOT_PRED);
+	fclose(template_file);
+	return template_data;
+}
+
+static list_t *make_user_context_template(state_t * s)
+{
+	FILE *template_file = NULL;
+	list_t *template_data = NULL;
+
+	template_file = fopen(s->homedir_template_path, "r");
+	if (!template_file)
+		return NULL;
+	template_data = semanage_slurp_file_filter(template_file, &USER_CONTEXT_PRED);
+	fclose(template_file);
+	return template_data;
+}

libsemanage/src/genhomedircon.c new file +231 -0

+		if (!temp)
+			return -1;
+		line = temp;
+		temp = semanage_sed(line, "ROLE", prefix);
+		free(line);
+		if (!temp)
+			return -1;
+		line = temp;
+		temp = semanage_sed(line, "system_u", seuser);
+		free(line);
+		if (!temp)
+			return -1;
+		line = temp;
+		fputs(line, out);
+		fputs("\n", out);
+		free(line);
+	}
+	return 0;
+}
+
+static int write_home_root_context(FILE * out, list_t * tpl, char *homedir)
+{
+	char *temp;
+
+	for (; tpl; tpl = tpl->next) {
+		temp = semanage_sed(tpl->data, "HOME_ROOT", homedir);
+		if (!temp)
+			return -1;
+		fputs(temp, out);
+		fputs("\n", out);
+		free(temp);
+	}
+	return 0;
+}
+
+static int write_user_context(FILE * out, list_t * tpl, char *user,
+			      char *seuser, char *prefix)
+{
+	char *temp;
+	char *line;
+
+	for (; tpl; tpl = tpl->next) {
+		temp = semanage_sed(tpl->data, "USER", user);
+		if (!temp)
+			return -1;
+		line = temp;
+		temp = semanage_sed(line, "ROLE", prefix);
+		free(line);
+		if (!temp)
+			return -1;
+		line = temp;
+		temp = semanage_sed(line, "system_u", seuser);
+		free(line);
+		if (!temp)
+			return -1;
+		line = temp;
+		fputs(line, out);
+		fputs("\n", out);
+		free(line);
+	}
+	return 0;
+}
+
+static int user_sort_func(semanage_user_t ** arg1, semanage_user_t ** arg2)
+{
+	return strcmp(semanage_user_get_name(*arg1),
+		      semanage_user_get_name(*arg2));
+}
+
+static int name_user_cmp(char *key, semanage_user_t ** val)
+{
+	int ret = strcmp(key, semanage_user_get_name(*val));
+	return ret;
+}
+
+static int push_user_entry(user_entry_t ** list, const char *n, const char *sen,
+			   const char *pre, const char *h)
+{
+	user_entry_t *temp = NULL;
+	char *name = NULL;
+	char *sename = NULL;
+	char *prefix = NULL;
+	char *home = NULL;
+
+	temp = malloc(sizeof(user_entry_t));
+	if (!temp)
+		goto cleanup;
+	name = strdup(n);
+	if (!name)
+		goto cleanup;
+	sename = strdup(sen);
+	if (!sename)
+		goto cleanup;
+	prefix = strdup(pre);
+	if (!prefix)
+		goto cleanup;
+	home = strdup(h);
+	if (!home)
+		goto cleanup;
+
+	temp->name = name;
+	temp->sename = sename;
+	temp->prefix = prefix;
+	temp->home = home;
+	temp->next = (*list);
+	(*list) = temp;
+
+	return 0;
+
+      cleanup:
+	free(name);
+	free(sename);
+	free(prefix);
+	free(home);
+	free(temp);
+	return -1;
+}
+
+static void pop_user_entry(user_entry_t ** list)
+{
+	user_entry_t *temp;
+
+	if (!list || !(*list))
+		return;
+	temp = *list;
+	*list = temp->next;
+	free(temp->name);
+	free(temp->sename);
+	free(temp->prefix);
+	free(temp->home);
+	free(temp);
+	return;
+}
+
+static user_entry_t *get_users(state_t * s)
+{
+	user_entry_t *head = NULL;
+
+	semanage_seuser_t **seuser_list = NULL;
+	unsigned int nseusers = 0;
+
+	semanage_user_t **user_list = NULL;
+	unsigned int nusers = 0;
+
+	semanage_user_t **u = NULL;
+	const char *name = NULL;
+	const char *seuname = NULL;
+	const char *prefix = NULL;
+
+	struct passwd *pwent = NULL;
+
+	unsigned int i;
+	int retval;
+
+	if ((retval =
+	     semanage_seuser_list(s->h_semanage, &seuser_list, &nseusers) < 0)
+	    || (nseusers < 1)) {
+		/* if there are no users, this function can't do any other work */
+		return NULL;
+	}
+
+	if (semanage_user_list(s->h_semanage, &user_list, &nusers) < 0) {
+		nusers = 0;
+	}
+
+	qsort(user_list, nusers, sizeof(semanage_user_t *),
+	      (int (*)(const void *, const void *))&user_sort_func);
+
+	for (i = 0; i < nseusers; i++) {
+		name = semanage_seuser_get_name(seuser_list[i]);
+		seuname = semanage_seuser_get_sename(seuser_list[i]);
+
+		if (strcmp(seuname, "user_u") == 0)
+			continue;
+		if (strcmp(seuname, "__default__") == 0)
+			continue;
+		if (strcmp(seuname, "system_u") == 0)
+			continue;
+
+		/* find the user structure givent the name */
+		u = bsearch(name, user_list, nusers, sizeof(semanage_user_t *),
+			    (int (*)(const void *, const void *))
+			    &name_user_cmp);
+		if (u) {
+			prefix = semanage_user_get_prefix(*u);
+		} else {
+			prefix = name;
+		}
+
+		pwent = getpwnam(name);
+		if (!pwent) {
+			/* user not in passwd file, genhomedircon
+			 * printed a message here */
+			continue;
+		}
+
+		if (strcmp(pwent->pw_dir, "/") == 0) {
+			/* don't relabel / genhomdircon checked to see if root
+			 * was the user and if so, set his home directory to
+			 * /root */
+			continue;
+		}
+
+		push_user_entry(&head, name, seuname, prefix, pwent->pw_dir);
+	}
+	for (i = 0; i < nseusers; i++) {
+		semanage_seuser_free(seuser_list[i]);
+	}
+	free(seuser_list);
+
+	for (i = 0; i < nusers; i++) {
+		semanage_user_free(user_list[i]);
+	}
+	free(user_list);
+	return head;
+}
+
+static int write_gen_home_dir_context(FILE * out, state_t * s,
+				      list_t * user_context_tpl,
+				      list_t * homedir_context_tpl)
+{
+	user_entry_t *users;
+
+	for (users = get_users(s); users; pop_user_entry(&users)) {
+		write_home_dir_context(out, homedir_context_tpl,
+				       users->name,
+				       users->sename, users->home,
+				       users->prefix);
+		write_user_context(out, user_context_tpl, users->name,
+				   users->sename, users->prefix);
+	}

libsemanage/src/genhomedircon.c new file +17 -0

+	list_t *homedirs = NULL;
+	list_t *h = NULL;
+	list_t *user_context_tpl = NULL;
+	list_t *homedir_context_tpl = NULL;
+	list_t *homeroot_context_tpl = NULL;
+
+	char *temp;
+	int s1len;
+
+	homedirs = get_home_dirs(s);
+	if (!homedirs) {
+		WARN(s->h_semanage,
+		     "no home directories were available, exiting without writing");
+		return -1;	/* No homedirs so no output */
+	}
+
+	fwriteheader(s, out);

libsemanage/src/genhomedircon.c new file +20 -0

+
+		write_home_dir_context(out,
+				       homedir_context_tpl, "user_u", "user_u",
+				       temp, "user");
+		write_home_root_context(out, homeroot_context_tpl, h->data);
+
+		free(temp);
+	}
+	write_user_context(out, user_context_tpl, ".*", "user_u", "user");
+	write_gen_home_dir_context(out, s, user_context_tpl,
+				   homedir_context_tpl);
+
+	/* Cleanup */
+	semanage_list_destroy(&homedirs);
+	semanage_list_destroy(&user_context_tpl);
+	semanage_list_destroy(&homedir_context_tpl);
+	semanage_list_destroy(&homeroot_context_tpl);
+
+	return 0;
+}

Index: ghdc/libsemanage/src/genhomedircon.h
===================================================================

libsemanage/src/genhomedircon.h new file +12 -0

@@ -0,0 +1,42 @@
+/* Author: Mark Goldman   <[email protected]>
+ *
+ * Copyright (C) 2007 Tresys Technology, LLC
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

libsemanage/src/genhomedircon.h new file +3 -0

+int semanage_genhomedircon(semanage_handle_t * sh, int usepasswd);
+
+#endif

Index: ghdc/libsemanage/src/semanage_store.c
===================================================================

libsemanage/src/semanage_store.c +23 -13

@@ -34,6 +34,7 @@ typedef struct dbase_policydb dbase_t;
 #include "semanage_store.h"
 #include "database_policydb.h"
 #include "handle.h"
+#include "genhomedircon.h"
 
 #include <selinux/selinux.h>
 #include <sepol/policydb.h>

@@ -110,6 +111,7 @@ static const char *semanage_sandbox_path
 	"/seusers.final",
 	"/users_extra",
 	"/netfilter_contexts",
+	"/file_contexts.homedirs",
 };
 
 /* A node used in a linked list of file contexts; used for sorting.

@@ -1008,14 +1010,15 @@ static int semanage_install_active(seman
 	const char *active_fc = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_FC);
 	const char *active_fc_loc =
 	    semanage_path(SEMANAGE_ACTIVE, SEMANAGE_FC_LOCAL);
-	const char *active_hd =
-	    semanage_path(SEMANAGE_ACTIVE, SEMANAGE_HOMEDIR_TMPL);
 	const char *active_seusers =
 	    semanage_path(SEMANAGE_ACTIVE, SEMANAGE_SEUSERS);
 	const char *active_nc = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_NC);
+	const char *active_fc_hd =
+	    semanage_path(SEMANAGE_ACTIVE, SEMANAGE_FC_HOMEDIRS);
 
 	const char *running_fc = selinux_file_context_path();
 	const char *running_fc_loc = selinux_file_context_local_path();
+	const char *running_fc_hd = selinux_file_context_homedir_path();
 	const char *running_hd = selinux_homedir_context_path();
 	const char *running_policy = selinux_binary_policy_path();
 	const char *running_seusers = selinux_usersconf_path();

@@ -1027,14 +1030,15 @@ static int semanage_install_active(seman
 	 * POLICYTYPE and should probably be done in the future. */
 	char store_fc[PATH_MAX];
 	char store_fc_loc[PATH_MAX];
-	char store_hd[PATH_MAX];
 	char store_pol[PATH_MAX];
 	char store_seusers[PATH_MAX];
 	char store_nc[PATH_MAX];
+	char store_fc_hd[PATH_MAX];
 
 	len = strlen(really_active_store);
 	running_fc += len;
 	running_fc_loc += len;
+	running_fc_hd += len;
 	running_hd += len;
 	running_policy += len;
 	running_seusers += len;

@@ -1055,9 +1059,10 @@ static int semanage_install_active(seman
 		goto cleanup;
 	}
 
-	snprintf(store_hd, PATH_MAX, "%s%s", storepath, running_hd);
-	if (semanage_copy_file(active_hd, store_hd, sh->conf->file_mode) == -1) {
-		ERR(sh, "Could not copy %s to %s.", active_hd, store_hd);
+	snprintf(store_fc_hd, PATH_MAX, "%s%s", storepath, running_fc_hd);
+	if (semanage_copy_file(active_fc_hd, store_fc_hd, sh->conf->file_mode)
+	    == -1) {
+		ERR(sh, "Could not copy %s to %s.", active_fc_hd, store_fc_hd);
 		goto cleanup;
 	}
 

@@ -1197,6 +1202,10 @@ static int semanage_commit_sandbox(seman
 		retval = -1;
 		goto cleanup;
 	}
+
+	/* clean up some files from the sandbox before install */
+	/* remove homedir_template from sandbox */
+
 	if (rename(sandbox, active) == -1) {
 		ERR(sh, "Error while renaming %s to %s.", sandbox, active);
 		/* note that if an error occurs during the next

@@ -1227,7 +1236,8 @@ static int semanage_commit_sandbox(seman
 	if (!sh->conf->save_previous) {
 		retval = semanage_remove_directory(backup);
 		if (retval < 0) {
-			ERR(sh, "Could not delete previous directory %s.", backup);
+			ERR(sh, "Could not delete previous directory %s.",
+			    backup);
 			goto cleanup;
 		}
 	}

@@ -1257,15 +1267,15 @@ int semanage_install_sandbox(semanage_ha
 		goto cleanup;
 	}
 
-	if ((commit_num = semanage_commit_sandbox(sh)) < 0) {
-		retval = commit_num;
+	if ((retval =
+	     semanage_genhomedircon(sh, 1)) != 0) {
+		ERR(sh, "semanage_genhomedircon returned error code %d.",
+		    retval);
 		goto cleanup;
 	}
 
-	if ((retval =
-	     semanage_exec_prog(sh, sh->conf->genhomedircon,
-				sh->conf->store_path, "")) != 0) {
-		ERR(sh, "genhomedircon returned error code %d.", retval);
+	if ((commit_num = semanage_commit_sandbox(sh)) < 0) {
+		retval = commit_num;
 		goto cleanup;
 	}
 

Index: ghdc/libsemanage/src/utilities.c
===================================================================

libsemanage/src/utilities.c new file +27 -0

@@ -0,0 +1,359 @@
+/* Author: Mark Goldman   <[email protected]>
+ *
+ * Copyright (C) 2007 Tresys Technology, LLC
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <string.h>
+#include <sys/types.h>
+#include <regex.h>
+
+#include "utilities.h"

libsemanage/src/utilities.c new file +20 -0

+
+	if ((fd = fopen(file, "r")) == NULL)
+		return NULL;
+
+	while (!feof(fd)) {
+		if (!(buff = semanage_fgetline(fd))){
+			goto err;
+		}
+		if (semanage_prefix_of(buff, var)) {
+			/* Found it */
+			temp = semanage_split(buff, delim);
+			/* if semanage_split succedes dup the token */
+			if (temp != NULL)
+				temp = strdup(temp);
+			free(buff);	/* free the buffer */
+			goto out;
+		}
+		free(buff);	/* free the buffer for next time through the loop */
+	}
+

libsemanage/src/utilities.c new file +24 -0

+	if (!str)
+		return 0;
+	if (!val)
+		return 1;
+
+	while ((*str) && (*val) && (*str == *val)) {
+		++str;
+		++val;
+	}
+
+	return !(*val);
+}
+
+char *semanage_split_on_space(char *str)
+{
+	if (!str)
+		return NULL;
+
+	while ((*str != '\0') && !(isspace(*str)))
+		str++;
+	while ((*str != '\0') && (isspace(*str)))
+		str++;
+
+	return str;

Quoted 14 lines

libsemanage/src/genhomedircon.c new file +14 -0

+int semanage_genhomedircon(semanage_handle_t * sh, int usepasswd)
+{
+	state_t s;
+
+	char outfile[PATH_MAX];
+	char hdt[PATH_MAX];
+	char fcf[PATH_MAX];
+
+	FILE *out = NULL;
+	int retval = 0;
+
+	assert(sh);
+
+	s.homedir_template_path = hdt;

libsemanage/src/genhomedircon.c line 63

Quoted 6 lines

libsemanage/src/genhomedircon.c new file +6 -0

+	snprintf(s.homedir_template_path, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_HOMEDIR_TMPL));
+
+	snprintf(outfile, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_FC_HOMEDIRS));
+

strdup?

Review

Quoted 2 lines

(quoted diff) +2 -0

+static list_t *default_shell_list(void)
+{

What is this function for? Why not error out when /etc/shells is not available?

Review

Quoted 1 line

(quoted diff) +1 -0

+	list_t *list = NULL;

Why are you adding another list type when we just merged one into libsepol? If you want this merged into trunk before policyrep let's move the list type over early.

libsemanage/src/genhomedircon.c line 110

Quoted 3 lines

libsemanage/src/genhomedircon.c new file +3 -0

+	shells = fopen("/etc/shells", "r");
+	if (!shells)
+		return default_shell_list();

See above - when is this error possible and why not make it fatal.

libsemanage/src/genhomedircon.c line 178

Quoted 2 lines

libsemanage/src/genhomedircon.c new file +2 -0

+	setpwent();
+	while ((pwbuf = getpwent())) {

You need to check errno to see if there was an error.

It is not clear from the python docs, but getpwent will definitely pull user entries from LDAP, NIS, etc. Is that what we want or should we limit to /etc/passwd only?
From what I've been able to gather, python's getpwall is a wrapper

Review

Quoted 2 lines

(quoted diff) +2 -0

+static int fwriteheader(state_t * s, FILE * out)
+{

Needs a better function name.

Quoted 6 lines

libsemanage/src/genhomedircon.c new file +6 -0

+static int write_home_dir_context(FILE * out, list_t * tpl, char *user,
+				  char *seuser, char *home, char *prefix)
+{
+	char *temp;
+	char *line;
+

libsemanage/src/genhomedircon.c line 302

Quoted 3 lines

libsemanage/src/genhomedircon.c new file +3 -0

+	fprintf(out, "\n\n#\n# Home Context for user %s\n#\n\n", user);
+	for (; tpl; tpl = tpl->next) {
+		temp = semanage_sed(tpl->data, "HOME_DIR", home);

Wow - I understand that this string handling in C is painful, but is this really the best alternative? Perhaps parsing the input to make the substitution easier would be better? Perhaps we should look for some library code to do this?

libsemanage/src/genhomedircon.c line 543

Quoted 2 lines

libsemanage/src/genhomedircon.c new file +2 -0

+static int fwriteoutput(state_t * s, FILE * out)
+{

This function name should be more descriptive.

libsemanage/src/genhomedircon.c line 570

Quoted 5 lines

libsemanage/src/genhomedircon.c new file +5 -0

+	for (h = homedirs; h; h = h->next) {
+		s1len = strlen(h->data);
+		temp = malloc(s1len + sizeof("/[^/]*"));
+		strcpy(temp, h->data);
+		strcpy(temp + s1len, "/[^/]*");

strdup

libsemanage/src/utilities.c line 37

Quoted 9 lines

libsemanage/src/utilities.c new file +9 -0

+char *semanage_findval(char *file, char *var, char *delim)
+{
+	FILE *fd;
+
+	char *buff;
+	char *temp = NULL;
+
+	if (!file || !var || !delim)
+		return NULL;

assert?

libsemanage/src/utilities.c line 101

Quoted 8 lines

libsemanage/src/utilities.c new file +8 -0

+char *semanage_split(char *str, char *delim)
+{
+	char *temp = str;
+
+	if (!str)
+		return NULL;
+	if (!delim || (*delim == '\0'))
+		return semanage_split_on_space(str);

Having NULL mean space seems odd.

On Tue, 2007-05-22 at 17:08 -0400, Karl MacMillan wrote:
<snip>

Quoted 14 lines

Quoted 14 lines

libsemanage/src/genhomedircon.c new file +14 -0

+int semanage_genhomedircon(semanage_handle_t * sh, int usepasswd)
+{
+	state_t s;
+
+	char outfile[PATH_MAX];
+	char hdt[PATH_MAX];
+	char fcf[PATH_MAX];
+
+	FILE *out = NULL;
+	int retval = 0;
+
+	assert(sh);
+
+	s.homedir_template_path = hdt;

Quoted 8 lines

libsemanage/src/genhomedircon.c line 63

Quoted 6 lines

libsemanage/src/genhomedircon.c new file +6 -0

+	snprintf(s.homedir_template_path, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_HOMEDIR_TMPL));
+
+	snprintf(outfile, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_FC_HOMEDIRS));
+

strdup?

The paths are allocated on the stack. Can't strdup them. All things being equal, I would prefer to keep them on the stack since that is three fewer items I could possibly leak.

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+static list_t *default_shell_list(void)
+{

What is this function for? Why not error out when /etc/shells is not available?

Well, the justification here as with several other places is "genhomedircon didn't".

Quoted 5 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	list_t *list = NULL;

Why are you adding another list type when we just merged one into libsepol? If you want this merged into trunk before policyrep let's move the list type over early.

I'm working off of trunk. If it isn't in trunk it I can't use it.

<snip>

Quoted 5 lines

libsemanage/src/genhomedircon.c line 110

Quoted 3 lines

libsemanage/src/genhomedircon.c new file +3 -0

+	shells = fopen("/etc/shells", "r");
+	if (!shells)
+		return default_shell_list();

See above - when is this error possible and why not make it fatal.

Again, "it wasn't fatal in genhomedircon"

<snip>

Quoted 5 lines

libsemanage/src/genhomedircon.c line 178

Quoted 2 lines

libsemanage/src/genhomedircon.c new file +2 -0

+	setpwent();
+	while ((pwbuf = getpwent())) {

You need to check errno to see if there was an error.

Quoted 4 lines

It is not clear from the python docs, but getpwent will definitely pull user entries from LDAP, NIS, etc. Is that what we want or should we limit to /etc/passwd only?
From what I've been able to gather, python's getpwall is a wrapper

for getpwent. As for whether we want this behavior or not, I'm open to suggestions.

<snip>

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+static int fwriteheader(state_t * s, FILE * out)
+{

Needs a better function name.

Suggestions?

<snip>

Quoted 6 lines

Quoted 6 lines

libsemanage/src/genhomedircon.c new file +6 -0

+static int write_home_dir_context(FILE * out, list_t * tpl, char *user,
+				  char *seuser, char *home, char *prefix)
+{
+	char *temp;
+	char *line;
+

Quoted 8 lines

libsemanage/src/genhomedircon.c line 302

Quoted 3 lines

libsemanage/src/genhomedircon.c new file +3 -0

+	fprintf(out, "\n\n#\n# Home Context for user %s\n#\n\n", user);
+	for (; tpl; tpl = tpl->next) {
+		temp = semanage_sed(tpl->data, "HOME_DIR", home);

Wow - I understand that this string handling in C is painful, but is this really the best alternative? Perhaps parsing the input to make the substitution easier would be better? Perhaps we should look for some library code to do this?

I am not really sure what the problem with this is. We need to replace arbitrary substrings. I searched in the C libraries for something that might do what I want. I couldn't find anything, so I wrote a function to do it.

As for parsing the input to make the substitution easier, I think it would be a lot more work to write or use a full fledged parser. What we want in this situation is what I wrote in my opinion, although I am open to suggestions.

<snip>

Quoted 4 lines

libsemanage/src/genhomedircon.c line 543

Quoted 2 lines

libsemanage/src/genhomedircon.c new file +2 -0

+static int fwriteoutput(state_t * s, FILE * out)
+{

This function name should be more descriptive.

Suggestions?

<snip>

Quoted 7 lines

libsemanage/src/genhomedircon.c line 570

Quoted 5 lines

libsemanage/src/genhomedircon.c new file +5 -0

+	for (h = homedirs; h; h = h->next) {
+		s1len = strlen(h->data);
+		temp = malloc(s1len + sizeof("/[^/]*"));
+		strcpy(temp, h->data);
+		strcpy(temp + s1len, "/[^/]*");

strdup

I needed extra space at the end of the string for the suffix, so I malloc'd it all in one go.

<snip>

Quoted 11 lines

libsemanage/src/utilities.c line 37

Quoted 9 lines

libsemanage/src/utilities.c new file +9 -0

+char *semanage_findval(char *file, char *var, char *delim)
+{
+	FILE *fd;
+
+	char *buff;
+	char *temp = NULL;
+
+	if (!file || !var || !delim)
+		return NULL;

assert?

As with other things, genhomedircon would not have aborted on this condition. It returned "" on all exceptional paths.

<snip>

Quoted 10 lines

libsemanage/src/utilities.c line 101

Quoted 8 lines

libsemanage/src/utilities.c new file +8 -0

+char *semanage_split(char *str, char *delim)
+{
+	char *temp = str;
+
+	if (!str)
+		return NULL;
+	if (!delim || (*delim == '\0'))
+		return semanage_split_on_space(str);

Having NULL mean space seems odd.

This function was a python emulation function. It behaves in the manner that python's split would if you don't pass in a delimiter.

Consider all unmentioned comments Noted and fixed.

Quoted 26 lines · Mark Goldman

On Tue, 2007-05-22 at 17:08 -0400, Karl MacMillan wrote:
<snip>

Quoted 14 lines

Quoted 14 lines

libsemanage/src/genhomedircon.c new file +14 -0

+int semanage_genhomedircon(semanage_handle_t * sh, int usepasswd)
+{
+	state_t s;
+
+	char outfile[PATH_MAX];
+	char hdt[PATH_MAX];
+	char fcf[PATH_MAX];
+
+	FILE *out = NULL;
+	int retval = 0;
+
+	assert(sh);
+
+	s.homedir_template_path = hdt;

Quoted 9 lines

libsemanage/src/genhomedircon.c line 63

Quoted 7 lines

libsemanage/src/genhomedircon.c new file +6 -0

+	snprintf(s.homedir_template_path, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_HOMEDIR_TMPL));
+
+	snprintf(outfile, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_FC_HOMEDIRS));
+

strdup?

Quoted 5 lines

The paths are allocated on the stack. Can't strdup them. All things being equal, I would prefer to keep them on the stack since that is three fewer items I could possibly leak.

I think malloc'd would be better because otherwise you have to do this fun with PATH_MAX and snprintfs (which is hard to read) and end up with a lot of data on the stack. Question - do you even need to copy these? Why not just store a pointer to what is returned by semanage_path?

Quoted 10 lines

Quoted 6 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

+static list_t *default_shell_list(void)
+{

What is this function for? Why not error out when /etc/shells is not available?

Well, the justification here as with several other places is "genhomedircon didn't".

I think the behavior should be similar, but there is no reason that we can't make changes to corner case behavior if we think it would be better. So I don't think an _exact_ port is necessary.

Quoted 9 lines

Quoted 6 lines

Review

Quoted 2 lines

(quoted diff) +1 -0

+	list_t *list = NULL;

Why are you adding another list type when we just merged one into libsepol? If you want this merged into trunk before policyrep let's move the list type over early.

I'm working off of trunk. If it isn't in trunk it I can't use it.

I think another list type is unacceptable so you can either work off policyrep or propose early merging of the list types. Either one is fine with me.

Quoted 8 lines

<snip>

Quoted 6 lines

libsemanage/src/genhomedircon.c line 110

Quoted 4 lines

libsemanage/src/genhomedircon.c new file +3 -0

+	shells = fopen("/etc/shells", "r");
+	if (!shells)
+		return default_shell_list();

See above - when is this error possible and why not make it fatal.

Quoted 3 lines

Again, "it wasn't fatal in genhomedircon"

Dan / anyone - why have the default shell list?

Quoted 11 lines

<snip>

Quoted 9 lines

libsemanage/src/genhomedircon.c line 178

Quoted 3 lines

libsemanage/src/genhomedircon.c new file +2 -0

+	setpwent();
+	while ((pwbuf = getpwent())) {

You need to check errno to see if there was an error.

It is not clear from the python docs, but getpwent will definitely pull user entries from LDAP, NIS, etc. Is that what we want or should we limit to /etc/passwd only?

Quoted 4 lines

Quoted 1 line

From what I've been able to gather, python's getpwall is a wrapper

for getpwent. As for whether we want this behavior or not, I'm open to suggestions.

Ok - I'm really just concerned that if we try this against a large LDAP database things will not go well. Anyone else have an opinion?

Quoted 10 lines

<snip>

Quoted 5 lines

Review

Quoted 3 lines

(quoted diff) +2 -0

+static int fwriteheader(state_t * s, FILE * out)
+{

Needs a better function name.

Suggestions?

write_file_context_header? Or don't make it a function at all.

Quoted 17 lines

<snip>

Quoted 6 lines

Quoted 6 lines

libsemanage/src/genhomedircon.c new file +6 -0

+static int write_home_dir_context(FILE * out, list_t * tpl, char *user,
+				  char *seuser, char *home, char *prefix)
+{
+	char *temp;
+	char *line;
+

Quoted 9 lines

libsemanage/src/genhomedircon.c line 302

Quoted 4 lines

libsemanage/src/genhomedircon.c new file +3 -0

+	fprintf(out, "\n\n#\n# Home Context for user %s\n#\n\n", user);
+	for (; tpl; tpl = tpl->next) {
+		temp = semanage_sed(tpl->data, "HOME_DIR", home);

Wow - I understand that this string handling in C is painful, but is this really the best alternative? Perhaps parsing the input to make the substitution easier would be better? Perhaps we should look for some library code to do this?

Quoted 6 lines

I am not really sure what the problem with this is. We need to replace arbitrary substrings. I searched in the C libraries for something that might do what I want. I couldn't find anything, so I wrote a function to do it.

This substitution is inefficient and this type of string manipulation in general is error prone.

Quoted 5 lines

As for parsing the input to make the substitution easier, I think it would be a lot more work to write or use a full fledged parser. What we want in this situation is what I wrote in my opinion, although I am open to suggestions.

Quoted 10 lines

<snip>

Quoted 5 lines

libsemanage/src/genhomedircon.c line 543

Quoted 3 lines

libsemanage/src/genhomedircon.c new file +2 -0

+static int fwriteoutput(state_t * s, FILE * out)
+{

This function name should be more descriptive.

Suggestions?

Not at the moment - but this is the main entrypoint as far as I can tell and the name doesn't imply that at all.

Quoted 10 lines

<snip>

Quoted 8 lines

libsemanage/src/genhomedircon.c line 570

Quoted 6 lines

libsemanage/src/genhomedircon.c new file +5 -0

+	for (h = homedirs; h; h = h->next) {
+		s1len = strlen(h->data);
+		temp = malloc(s1len + sizeof("/[^/]*"));
+		strcpy(temp, h->data);
+		strcpy(temp + s1len, "/[^/]*");

strdup

Quoted 4 lines

I needed extra space at the end of the string for the suffix, so I malloc'd it all in one go.

Ok.

Quoted 14 lines

<snip>

Quoted 12 lines

libsemanage/src/utilities.c line 37

Quoted 10 lines

libsemanage/src/utilities.c new file +9 -0

+char *semanage_findval(char *file, char *var, char *delim)
+{
+	FILE *fd;
+
+	char *buff;
+	char *temp = NULL;
+
+	if (!file || !var || !delim)
+		return NULL;

assert?

Quoted 4 lines

As with other things, genhomedircon would not have aborted on this condition. It returned "" on all exceptional paths.

Is it possible for these to be NULL or a sign of an earlier error?

Quoted 13 lines

<snip>

Quoted 11 lines

libsemanage/src/utilities.c line 101

Quoted 9 lines

libsemanage/src/utilities.c new file +8 -0

+char *semanage_split(char *str, char *delim)
+{
+	char *temp = str;
+
+	if (!str)
+		return NULL;
+	if (!delim || (*delim == '\0'))
+		return semanage_split_on_space(str);

Having NULL mean space seems odd.

Quoted 4 lines

This function was a python emulation function. It behaves in the manner that python's split would if you don't pass in a delimiter.

I'd prefer things to be more idiomatic to C rather than a straight python port.

Quoted 3 lines

Consider all unmentioned comments Noted and fixed.

Great.

Karl

Quoted 24 lines · Mark Goldman

On Tue, 2007-05-22 at 17:08 -0400, Karl MacMillan wrote:
<snip>

Quoted 14 lines

Quoted 14 lines

libsemanage/src/genhomedircon.c new file +14 -0

+int semanage_genhomedircon(semanage_handle_t * sh, int usepasswd)
+{
+	state_t s;
+
+	char outfile[PATH_MAX];
+	char hdt[PATH_MAX];
+	char fcf[PATH_MAX];
+
+	FILE *out = NULL;
+	int retval = 0;
+
+	assert(sh);
+
+	s.homedir_template_path = hdt;

Quoted 8 lines

libsemanage/src/genhomedircon.c line 63

Quoted 6 lines

libsemanage/src/genhomedircon.c new file +6 -0

+	snprintf(s.homedir_template_path, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_HOMEDIR_TMPL));
+
+	snprintf(outfile, PATH_MAX - 1, "%s",
+		 semanage_path(SEMANAGE_TMP, SEMANAGE_FC_HOMEDIRS));
+

strdup?

Quoted 4 lines

The paths are allocated on the stack. Can't strdup them. All things being equal, I would prefer to keep them on the stack since that is three fewer items I could possibly leak.

I think malloc'd would be better because otherwise you have to do this fun with PATH_MAX and snprintfs (which is hard to read) and end up with a lot of data on the stack. Question - do you even need to copy these? Why not just store a pointer to what is returned by semanage_path?

Quoted 5 lines

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+static list_t *default_shell_list(void)
+{

What is this function for? Why not error out when /etc/shells is not available?

Quoted 3 lines

Well, the justification here as with several other places is "genhomedircon didn't".

I think the behavior should be similar, but there is no reason that we can't make changes to corner case behavior if we think it would be better. So I don't think an _exact_ port is necessary.

Quoted 7 lines

Quoted 5 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	list_t *list = NULL;

Why are you adding another list type when we just merged one into libsepol? If you want this merged into trunk before policyrep let's move the list type over early.

I'm working off of trunk. If it isn't in trunk it I can't use it.

I think another list type is unacceptable so you can either work off policyrep or propose early merging of the list types. Either one is fine with me.

Quoted 11 lines

Quoted 11 lines

libsemanage/src/utilities.c line 37

Quoted 9 lines

libsemanage/src/utilities.c new file +9 -0

+char *semanage_findval(char *file, char *var, char *delim)
+{
+	FILE *fd;
+
+	char *buff;
+	char *temp = NULL;
+
+	if (!file || !var || !delim)
+		return NULL;

assert?

Quoted 3 lines

As with other things, genhomedircon would not have aborted on this condition. It returned "" on all exceptional paths.

Is it possible for these to be NULL or a sign of an earlier error?

Quoted 11 lines

<snip>

Quoted 10 lines

libsemanage/src/utilities.c line 101

Quoted 8 lines

libsemanage/src/utilities.c new file +8 -0

+char *semanage_split(char *str, char *delim)
+{
+	char *temp = str;
+
+	if (!str)
+		return NULL;
+	if (!delim || (*delim == '\0'))
+		return semanage_split_on_space(str);

Having NULL mean space seems odd.

Quoted 3 lines

This function was a python emulation function. It behaves in the manner that python's split would if you don't pass in a delimiter.

I'd prefer things to be more idiomatic to C rather than a straight python port.

Quoted 2 lines · Karl MacMillan

Quoted 1 line · Mark Goldman

On Tue, 2007-05-22 at 17:08 -0400, Karl MacMillan wrote:

Quoted 3 lines

I think the behavior should be similar, but there is no reason that we can't make changes to corner case behavior if we think it would be better. So I don't think an _exact_ port is necessary.

I didn't know if this particular instance was, in fact, a corner case. I'd be just as happy for it to error if it is truly a rare condition. Comments?

Quoted 8 lines

Quoted 7 lines

Quoted 5 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	list_t *list = NULL;

Why are you adding another list type when we just merged one into libsepol? If you want this merged into trunk before policyrep let's move the list type over early.

I'm working off of trunk. If it isn't in trunk it I can't use it.

Quoted 3 lines

I think another list type is unacceptable so you can either work off policyrep or propose early merging of the list types. Either one is fine with me.

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Quoted 3 lines · Mark Goldman

Quoted 2 lines · Karl MacMillan

Quoted 1 line · Mark Goldman

On Tue, 2007-05-22 at 17:08 -0400, Karl MacMillan wrote:

<snip>

Quoted 7 lines

Quoted 3 lines

I think the behavior should be similar, but there is no reason that we can't make changes to corner case behavior if we think it would be better. So I don't think an _exact_ port is necessary.

I didn't know if this particular instance was, in fact, a corner case. I'd be just as happy for it to error if it is truly a rare condition. Comments?

I say make it an error but will defer to others.

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

Quoted 5 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	list_t *list = NULL;

Why are you adding another list type when we just merged one into libsepol? If you want this merged into trunk before policyrep let's move the list type over early.

I'm working off of trunk. If it isn't in trunk it I can't use it.

Quoted 7 lines

Quoted 3 lines

I think another list type is unacceptable so you can either work off policyrep or propose early merging of the list types. Either one is fine with me.

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the additional list types.

Quoted 9 lines · Karl MacMillan

Quoted 4 lines

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the additional list types.

There aren't additional list types because there aren't any in trunk. This has nothing to do with policyrep and doesn't break ABI compatibility and should be in trunk. I hate this idea of pushing everything into policyrep because its going to make it that much harder to get good testing done and the source of bugs will be harder to track down.

If you are so insistent on using the list types in policyrep why not port them to trunk so we can use them?

I am trying to use audit2allow to generate module out of audit messages.

So, I installed refpolicy headers first. Then tried sepolgen-ifgen

%sepolgen-ifgen
% ... some output ...
%parsing file /usr/share/selinux/devel/include/services/snmp.if %error parsing file /usr/share/selinux/devel/include/services/snmap.if:
%could not parse text: "Syntax error on line 1 ( [type=OPAREN]"

Parser had problem parsing almost all if files and generated above error.

It would been helpful if error included correct line number. Is it parser bug?
Am I doing anything wrong?
Is there any documentation available on how to use this policy generation tools?
What are the dependencies these tools have?

Thanks,

Anand Patel
Tresys Technology
[email protected]
410-290-1411xt153

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 1 line · Anand Patel

I am trying to use audit2allow to generate module out of audit

Quoted 7 lines

So, I installed refpolicy headers first. Then tried sepolgen-ifgen

%sepolgen-ifgen
% ... some output ...
%parsing file /usr/share/selinux/devel/include/services/snmp.if %error parsing file

Quoted 4 lines

%could not parse text: "Syntax error on line 1 ( [type=OPAREN]"

Parser had problem parsing almost all if files and generated above error.

The problem is that the refpolicy "language" is far from fixed and parsing it cleanly is almost impossible because of M4. So the parser sometimes gets out of sync with refpolicy (hence the policyrep work).

I just need some more specific information about the refpolicy version you are using and I should be able to fix-up the parser.

Thanks - Karl

Quoted 16 lines

It would been helpful if error included correct line number. Is it parser bug?
Am I doing anything wrong?
Is there any documentation available on how to use this policy generation tools?
What are the dependencies these tools have?

Thanks,

Anand Patel
Tresys Technology
[email protected]
410-290-1411xt153

Quoted 9 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 1 line · Anand Patel

I am trying to use audit2allow to generate module out of audit

messages.

Quoted 7 lines

Quoted 7 lines

So, I installed refpolicy headers first. Then tried sepolgen-ifgen

%sepolgen-ifgen
% ... some output ...
%parsing file /usr/share/selinux/devel/include/services/snmp.if %error parsing file

/usr/share/selinux/devel/include/services/snmap.if:

Quoted 5 lines

Quoted 4 lines

%could not parse text: "Syntax error on line 1 ( [type=OPAREN]"

Parser had problem parsing almost all if files and generated above error.

Quoted 6 lines

The problem is that the refpolicy "language" is far from fixed and parsing it cleanly is almost impossible because of M4. So the parser sometimes gets out of sync with refpolicy (hence the policyrep work).

I just need some more specific information about the refpolicy version you are using and I should be able to fix-up the parser.

I was using current release from oss.tresys.com/projects/refpolicy.

So, every new version of refpolicy will require tweaks to audit2allow parser.

What tweaks does it require? Where does it fail? Can you give me
some insight of how parser works?

Thanks,

Quoted 19 lines

Thanks - Karl

Quoted 16 lines

It would been helpful if error included correct line number. Is it parser bug?
Am I doing anything wrong?
Is there any documentation available on how to use this policy generation tools?
What are the dependencies these tools have?

Thanks,

Anand Patel
Tresys Technology
[email protected]
410-290-1411xt153

Anand Patel
Tresys Technology
[email protected]
410-290-1411xt153

Quoted 9 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 1 line · Anand Patel

I am trying to use audit2allow to generate module out of audit

messages.

Quoted 7 lines

Quoted 7 lines

So, I installed refpolicy headers first. Then tried sepolgen-ifgen

%sepolgen-ifgen
% ... some output ...
%parsing file /usr/share/selinux/devel/include/services/snmp.if %error parsing file

/usr/share/selinux/devel/include/services/snmap.if:

Quoted 5 lines

Quoted 4 lines

%could not parse text: "Syntax error on line 1 ( [type=OPAREN]"

Parser had problem parsing almost all if files and generated above error.

Quoted 6 lines

The problem is that the refpolicy "language" is far from fixed and parsing it cleanly is almost impossible because of M4. So the parser sometimes gets out of sync with refpolicy (hence the policyrep work).

I just need some more specific information about the refpolicy version you are using and I should be able to fix-up the parser.

I was using current release from oss.tresys.com/projects/refpolicy.

Quoted 27 lines · Anand Patel

Quoted 9 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 1 line · Anand Patel

I am trying to use audit2allow to generate module out of audit

messages.

Quoted 7 lines

Quoted 7 lines

So, I installed refpolicy headers first. Then tried sepolgen-ifgen

%sepolgen-ifgen
% ... some output ...
%parsing file /usr/share/selinux/devel/include/services/snmp.if %error parsing file

/usr/share/selinux/devel/include/services/snmap.if:

Quoted 9 lines

Quoted 4 lines

%could not parse text: "Syntax error on line 1 ( [type=OPAREN]"

Parser had problem parsing almost all if files and generated above error.

The problem is that the refpolicy "language" is far from fixed and parsing it cleanly is almost impossible because of M4. So the parser sometimes gets out of sync with refpolicy (hence the policyrep work).

Quoted 5 lines

Quoted 2 lines

I just need some more specific information about the refpolicy version you are using and I should be able to fix-up the parser.

I was using current release from oss.tresys.com/projects/refpolicy.

What version of sepolgen were you using? The latest (in rawhide) seems to work fine on the latest available refpolicy.

Karl

Quoted 7 lines · Anand Patel

Quoted 7 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 5 lines

Quoted 2 lines · Anand Patel

Quoted 1 line

I am trying to use audit2allow to generate module out of audit

messages.

Quoted 2 lines

Quoted 2 lines

So, I installed refpolicy headers first. Then tried

Quoted 9 lines

Quoted 5 lines

Quoted 5 lines

%sepolgen-ifgen
% ... some output ...
%parsing file /usr/share/selinux/devel/include/services/snmp.if %error parsing file

/usr/share/selinux/devel/include/services/snmap.if:

Quoted 3 lines

Quoted 3 lines

%could not parse text: "Syntax error on line 1 ( [type=OPAREN]"

Parser had problem parsing almost all if files and generated

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

error.

The problem is that the refpolicy "language" is far from fixed and parsing it cleanly is almost impossible because of M4. So the

Quoted 1 line

Quoted 1 line

sometimes gets out of sync with refpolicy (hence the policyrep

Quoted 2 lines

Quoted 2 lines

I just need some more specific information about the refpolicy

Quoted 4 lines

Quoted 1 line

you are using and I should be able to fix-up the parser.

I was using current release from oss.tresys.com/projects/refpolicy.

What version of sepolgen were you using? The latest (in rawhide) seems to work fine on the latest available refpolicy.

Karl

Karl,

I found that sepol-ifgen parser fails when it sees multiple types in type declaration statement.
e.g.

	type foo_t, hello_t;

Then I went back and change to following

	type foo_t;
	type hello_t;

and it parsed successfully.

-Anand Patel

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Anand Patel
Sent: Monday, June 11, 2007 9:48 AM
To: Karl MacMillan
Cc: SE Linux; Daniel J Walsh
Subject: RE: audit2allow module generation

Quoted 8 lines · Karl MacMillan

Quoted 7 lines · Anand Patel

Quoted 7 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines · Anand Patel

Quoted 1 line

I am trying to use audit2allow to generate module out of audit

messages.

Quoted 2 lines

Quoted 2 lines

So, I installed refpolicy headers first. Then tried

sepolgen-ifgen

Quoted 9 lines

Quoted 9 lines

Quoted 5 lines

Quoted 5 lines

%sepolgen-ifgen
% ... some output ...
%parsing file /usr/share/selinux/devel/include/services/snmp.if %error parsing file

/usr/share/selinux/devel/include/services/snmap.if:

Quoted 3 lines

Quoted 3 lines

%could not parse text: "Syntax error on line 1 ( [type=OPAREN]"

Parser had problem parsing almost all if files and generated

above

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 1 line

error.

The problem is that the refpolicy "language" is far from fixed and parsing it cleanly is almost impossible because of M4. So the

parser

Quoted 1 line

Quoted 1 line

Quoted 1 line

sometimes gets out of sync with refpolicy (hence the policyrep

work).

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I just need some more specific information about the refpolicy

version

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

you are using and I should be able to fix-up the parser.

I was using current release from oss.tresys.com/projects/refpolicy.

Quoted 2 lines

What version of sepolgen were you using? The latest (in rawhide) seems to work fine on the latest available refpolicy.

Version .1
It seems to work with pristine refpolicy. However, upon modifications to policy, new macros and moduels, it generates error during interface index parse.

I am currently investigating where it fails.

Quoted 5 lines

Karl

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Monday, September 03, 2007 12:14 PM
To: Anand Patel
Cc: SE Linux
Subject: RE: audit2allow module generation

Quoted 2 lines · Anand Patel

Karl,

Quoted 4 lines

I found that sepol-ifgen parser fails when it sees multiple types in type declaration statement.
e.g.

	type foo_t, hello_t;

This isn't a valid way to define two types - you are instead defining the type foo_t with the attribute hello_t. It's an easy mistake to

because of the similarity to the require syntax for requiring multiple types.

I'm sending a large update to the parser soon that will fix this

though.

Karl

Quoted 9 lines

Then I went back and change to following

	type foo_t;
	type hello_t;

and it parsed successfully.

-Anand Patel

-----Original Message-----
From: [email protected]

Quoted 15 lines

On Behalf Of Anand Patel
Sent: Monday, June 11, 2007 9:48 AM
To: Karl MacMillan
Cc: SE Linux; Daniel J Walsh
Subject: RE: audit2allow module generation

Quoted 8 lines · Karl MacMillan

Quoted 7 lines · Anand Patel

Quoted 7 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines · Anand Patel

Quoted 1 line

I am trying to use audit2allow to generate module out of

Quoted 8 lines

Quoted 3 lines

Quoted 3 lines

messages.

Quoted 2 lines

Quoted 2 lines

So, I installed refpolicy headers first. Then tried

sepolgen-ifgen

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

%sepolgen-ifgen
% ... some output ...
%parsing file

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

%error parsing file

/usr/share/selinux/devel/include/services/snmap.if:

Quoted 1 line

Quoted 1 line

%could not parse text: "Syntax error on line 1 (

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Parser had problem parsing almost all if files and generated

above

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

error.

The problem is that the refpolicy "language" is far from fixed

Quoted 10 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

parsing it cleanly is almost impossible because of M4. So the

parser

Quoted 1 line

Quoted 1 line

Quoted 1 line

sometimes gets out of sync with refpolicy (hence the policyrep

work).

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I just need some more specific information about the refpolicy

version

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

you are using and I should be able to fix-up the parser.

I was using current release from

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

What version of sepolgen were you using? The latest (in rawhide)

Quoted 3 lines

Quoted 1 line

to work fine on the latest available refpolicy.

Version .1
It seems to work with pristine refpolicy. However, upon

Quoted 7 lines

policy, new macros and moduels, it generates error during interface index parse.

I am currently investigating where it fails.

Quoted 1 line

Quoted 6 lines

Quoted 4 lines

Karl

How do I generate perm_map file for audit2allow?

Thanks
-Anand Patel

Quoted 10 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Monday, September 03, 2007 12:14 PM
To: Anand Patel
Cc: SE Linux
Subject: RE: audit2allow module generation

Quoted 2 lines · Anand Patel

Karl,

Quoted 7 lines

Quoted 4 lines

I found that sepol-ifgen parser fails when it sees multiple types in type declaration statement.
e.g.

	type foo_t, hello_t;

This isn't a valid way to define two types - you are instead defining the type foo_t with the attribute hello_t. It's an easy mistake to

make

Quoted 4 lines

because of the similarity to the require syntax for requiring multiple types.

I'm sending a large update to the parser soon that will fix this

problem

Quoted 13 lines

though.

Karl

Quoted 9 lines

Then I went back and change to following

	type foo_t;
	type hello_t;

and it parsed successfully.

-Anand Patel

-----Original Message-----
From: [email protected]

[mailto:[email protected]]

Quoted 15 lines

Quoted 15 lines

On Behalf Of Anand Patel
Sent: Monday, June 11, 2007 9:48 AM
To: Karl MacMillan
Cc: SE Linux; Daniel J Walsh
Subject: RE: audit2allow module generation

Quoted 8 lines · Karl MacMillan

Quoted 7 lines · Anand Patel

Quoted 7 lines

-----Original Message-----
From: Karl MacMillan [mailto:[email protected]]
Sent: Tuesday, May 29, 2007 5:12 PM
To: Anand Patel
Cc: SE Linux; Daniel J Walsh
Subject: Re: audit2allow module generation

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines · Anand Patel

Quoted 1 line

I am trying to use audit2allow to generate module out of

audit

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

Quoted 3 lines

messages.

Quoted 2 lines

Quoted 2 lines

So, I installed refpolicy headers first. Then tried

sepolgen-ifgen

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

%sepolgen-ifgen
% ... some output ...
%parsing file

/usr/share/selinux/devel/include/services/snmp.if

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

%error parsing file

/usr/share/selinux/devel/include/services/snmap.if:

Quoted 1 line

Quoted 1 line

%could not parse text: "Syntax error on line 1 (

[type=OPAREN]"

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Parser had problem parsing almost all if files and generated

above

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

error.

The problem is that the refpolicy "language" is far from fixed

and

Quoted 10 lines

Quoted 10 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

parsing it cleanly is almost impossible because of M4. So the

parser

Quoted 1 line

Quoted 1 line

Quoted 1 line

sometimes gets out of sync with refpolicy (hence the policyrep

work).

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I just need some more specific information about the refpolicy

version

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

you are using and I should be able to fix-up the parser.

I was using current release from

oss.tresys.com/projects/refpolicy.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

What version of sepolgen were you using? The latest (in rawhide)

seems

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

to work fine on the latest available refpolicy.

Version .1
It seems to work with pristine refpolicy. However, upon

modifications to

Quoted 13 lines

Quoted 13 lines

policy, new macros and moduels, it generates error during interface index parse.

I am currently investigating where it fails.

Quoted 5 lines

Karl

Quoted 9 lines · Joshua Brindle

Quoted 8 lines · Karl MacMillan

Quoted 4 lines

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the additional list types.

Quoted 7 lines

There aren't additional list types because there aren't any in trunk. This has nothing to do with policyrep and doesn't break ABI compatibility and should be in trunk. I hate this idea of pushing everything into policyrep because its going to make it that much harder to get good testing done and the source of bugs will be harder to track down.

Ok.

Quoted 2 lines

If you are so insistent on using the list types in policyrep why not port them to trunk so we can use them?

That option is fine with me (and was one of my suggestions), though I don't have time to do the port right now. Can you or Mark send a patch pulling over the iterators and list?

Quoted 23 lines · Karl MacMillan

Quoted 9 lines · Joshua Brindle

Quoted 8 lines · Karl MacMillan

Quoted 4 lines

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the additional list types.

Quoted 7 lines

There aren't additional list types because there aren't any in trunk. This has nothing to do with policyrep and doesn't break ABI compatibility and should be in trunk. I hate this idea of pushing everything into policyrep because its going to make it that much harder to get good testing done and the source of bugs will be harder to track down.

Ok.

Quoted 2 lines

If you are so insistent on using the list types in policyrep why not port them to trunk so we can use them?

Quoted 3 lines

That option is fine with me (and was one of my suggestions), though I don't have time to do the port right now. Can you or Mark send a patch pulling over the iterators and list?

I'm going to bring this back up because I think this patch is important to get into upstream as a first step toward generic label generation.

Since we aren't using the policyrep branch anymore and libpolicyrep is going to be in c++ do you still have reservations against this patch?

Quoted 23 lines · Joshua Brindle

Quoted 22 lines · Karl MacMillan

Quoted 8 lines · Joshua Brindle

Quoted 7 lines · Karl MacMillan

Quoted 4 lines

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the additional list types.

Quoted 7 lines

There aren't additional list types because there aren't any in trunk. This has nothing to do with policyrep and doesn't break ABI compatibility and should be in trunk. I hate this idea of pushing everything into policyrep because its going to make it that much harder to get good testing done and the source of bugs will be harder to track down.

Ok.

Quoted 2 lines

If you are so insistent on using the list types in policyrep why not port them to trunk so we can use them?

Quoted 8 lines

Quoted 3 lines

That option is fine with me (and was one of my suggestions), though I don't have time to do the port right now. Can you or Mark send a patch pulling over the iterators and list?

I'm going to bring this back up because I think this patch is important to get into upstream as a first step toward generic label generation.

Can you tell me a little more about what you are thinking with this?

Quoted 4 lines

Since we aren't using the policyrep branch anymore and libpolicyrep is going to be in c++ do you still have reservations against this patch?

I'm still convinced about the maintenance long term and potential for errors. Have all of the implementation issues that James brought up been resolved? Remind me why this is better in C than in an external Python helper.

Quoted 28 lines · Karl MacMillan

Quoted 27 lines · Joshua Brindle

Quoted 25 lines · Karl MacMillan

Quoted 8 lines · Joshua Brindle

Quoted 7 lines · Karl MacMillan

Quoted 4 lines

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the additional list types.

Quoted 7 lines

There aren't additional list types because there aren't any in trunk. This has nothing to do with policyrep and doesn't break ABI compatibility and should be in trunk. I hate this idea of pushing everything into policyrep because its going to make it that much harder to get good testing done and the source of bugs will be harder to track down.

Ok.

Quoted 2 lines

If you are so insistent on using the list types in policyrep why not port them to trunk so we can use them?

That option is fine with me (and was one of my suggestions), though I don't have time to do the port right now. Can you or Mark send a patch pulling over the iterators and list?

Quoted 7 lines

Quoted 4 lines

I'm going to bring this back up because I think this patch is important to get into upstream as a first step toward generic label generation.

Can you tell me a little more about what you are thinking with this?

Quoted 10 lines

Quoted 4 lines

Since we aren't using the policyrep branch anymore and libpolicyrep is going to be in c++ do you still have reservations against this patch?

I'm still convinced about the maintenance long term and potential for errors. Have all of the implementation issues that James brought up been resolved? Remind me why this is better in C than in an external Python helper.

Its very hacky, we have to close the current transaction (from within the library) so that genhomedircon can connect and get a read lock on the newly created active store and then it modifies the files outside of the module store, non-ideal in almost every way.

Quoted 4 lines · Joshua Brindle

Quoted 3 lines · Karl MacMillan

Quoted 2 lines · Joshua Brindle

Quoted 1 line · Karl MacMillan

On Fri, 2007-05-25 at 13:06 -0400, Joshua Brindle wrote:

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines · Karl MacMillan

Quoted 4 lines

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the

additional list types.

Quoted 22 lines

Quoted 22 lines

Quoted 21 lines

Quoted 8 lines

Quoted 7 lines

There aren't additional list types because there aren't any in trunk. This has nothing to do with policyrep and doesn't break ABI compatibility and should be in trunk. I hate this idea of pushing everything into policyrep because its going to make it that much harder to get good testing done and the source of bugs will be harder to track down.

Quoted 8 lines

Ok.

Quoted 2 lines

If you are so insistent on using the list types in policyrep why not port them to trunk so we can use them?

That option is fine with me (and was one of my suggestions), though I don't have time to do the port right now. Can you or Mark send a patch pulling over the iterators and list?

I'm going to bring this back up because I think this patch is important to get into upstream as a first step toward generic label generation.

Quoted 2 lines

Quoted 2 lines

Can you tell me a little more about what you are thinking with this?

??

Quoted 11 lines

Quoted 10 lines

Quoted 4 lines

Since we aren't using the policyrep branch anymore and libpolicyrep is going to be in c++ do you still have reservations against this patch?

I'm still convinced about the maintenance long term and potential for errors. Have all of the implementation issues that James brought up been resolved? Remind me why this is better in C than in an external Python helper.

Quoted 5 lines

Its very hacky, we have to close the current transaction (from within the library) so that genhomedircon can connect and get a read lock on the newly created active store and then it modifies the files outside of the module store, non-ideal in almost every way.

Why does it need to get the read lock - why can't it be changed to take the sandbox location and work on that directly? Fixing that might be better long term as it seems advantageous to allow helper programs in general (for separation and least-privilege).

Quoted 36 lines · Karl MacMillan

Quoted 4 lines · Joshua Brindle

Quoted 3 lines · Karl MacMillan

Quoted 2 lines · Joshua Brindle

Quoted 1 line · Karl MacMillan

On Fri, 2007-05-25 at 13:06 -0400, Joshua Brindle wrote:

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines · Karl MacMillan

Quoted 4 lines

If you would like to move the list types out of policyrep and into trunk, I'll be happy to use it. Otherwise it is out of the scope of this patch.

Please rebase against the policyrep branch - we can then consider merging the list types and this patch to trunk if desired. I will NACK this patch if it contains the

additional list types.

Quoted 22 lines

Quoted 22 lines

Quoted 21 lines

Quoted 8 lines

Quoted 7 lines

There aren't additional list types because there aren't any in trunk. This has nothing to do with policyrep and doesn't break ABI compatibility and should be in trunk. I hate this idea of pushing everything into policyrep because its going to make it that much harder to get good testing done and the source of bugs will be harder to track down.

Quoted 8 lines

Ok.

Quoted 2 lines

If you are so insistent on using the list types in policyrep why not port them to trunk so we can use them?

That option is fine with me (and was one of my suggestions), though I don't have time to do the port right now. Can you or Mark send a patch pulling over the iterators and list?

I'm going to bring this back up because I think this patch is important to get into upstream as a first step toward generic label generation.

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

Can you tell me a little more about what you are thinking with this?

??

We need labels for various object managers (like policy server) generated per user. For example, if you look back at the PoC apache metapolicy you'll see some hardcoded labels like user_apache_types_t and so on, these need to be generated for each user.

Quoted 17 lines

Quoted 11 lines

Quoted 10 lines

Quoted 4 lines

Since we aren't using the policyrep branch anymore and libpolicyrep is going to be in c++ do you still have reservations against this patch?

I'm still convinced about the maintenance long term and potential for errors. Have all of the implementation issues that James brought up been resolved? Remind me why this is better in C than in an external Python helper.

Quoted 5 lines

Its very hacky, we have to close the current transaction (from within the library) so that genhomedircon can connect and get a read lock on the newly created active store and then it modifies the files outside of the module store, non-ideal in almost every way.

Quoted 6 lines

Why does it need to get the read lock - why can't it be changed to take the sandbox location and work on that directly? Fixing that might be better long term as it seems advantageous to allow helper programs in general (for separation and least-privilege).