genhomedircon uid template

genhomedircon uid template

[Jason Zaman]

Hi all,

XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
that in an fcontext file. It used to be /run/user/USER which is easy but
not UID.

What template keyword should be used for such an entry? UID? USERID?

USERID is perhaps more obvious but has to be replaced before USER but
that should be doable.
https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76

UID does not conflict with USER but this line exists in refpol which
is problematic:
contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache -- gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)

This could also be used for several fcontexts in kerberos. It stores the
tickets in /tmp/krbcc_UID for example.

If we choose a template name I can put together a patch to add it.

-- Jason

Re: genhomedircon uid template

[Stephen Smalley]

On 02/01/2016 04:36 AM, Jason Zaman wrote:
> Hi all,
>
> XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
> that in an fcontext file. It used to be /run/user/USER which is easy but
> not UID.
>
> What template keyword should be used for such an entry? UID? USERID?
>
> USERID is perhaps more obvious but has to be replaced before USER but
> that should be doable.
> https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76
>
> UID does not conflict with USER but this line exists in refpol which
> is problematic:
> contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache -- gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)
>
> This could also be used for several fcontexts in kerberos. It stores the
> tickets in /tmp/krbcc_UID for example.
>
> If we choose a template name I can put together a patch to add it.

No strong preferences from me on the particular name, e.g. USERID is 
fine.  I think it highlights however the problems with the current 
approach; maybe we ought to be using ${USER} and ${UID} in .fc files 
instead?

Re: genhomedircon uid template

[Jason Zaman]

On Mon, Feb 01, 2016 at 02:30:37PM -0500, Stephen Smalley wrote:
> On 02/01/2016 04:36 AM, Jason Zaman wrote:
> > Hi all,
> >
> > XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
> > that in an fcontext file. It used to be /run/user/USER which is easy but
> > not UID.
> >
> > What template keyword should be used for such an entry? UID? USERID?
> >
> > USERID is perhaps more obvious but has to be replaced before USER but
> > that should be doable.
> > https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76
> >
> > UID does not conflict with USER but this line exists in refpol which
> > is problematic:
> > contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache -- gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)
> >
> > This could also be used for several fcontexts in kerberos. It stores the
> > tickets in /tmp/krbcc_UID for example.
> >
> > If we choose a template name I can put together a patch to add it.
>
> No strong preferences from me on the particular name, e.g. USERID is
> fine.  I think it highlights however the problems with the current
> approach; maybe we ought to be using ${USER} and ${UID} in .fc files
> instead?

Yes there are definitely problems but fixing would mean refpol and
probably a lot of other things would need to be updated at the same
time.

HOME_DIR and HOME_ROOT are not really problems since they are only
allowed in the beginning of an fcontext line and other lines start with
a /.

USER, USERID, and possibly other things in future (GROUP, GROUPID?) can
appear at any point in the the line so a more unique token might be
better. %USERID might be better than $USERID since thats a thing in
shells.

If we do go down this path, what are the steps? and what tokens do we
want?

-- Jason

Re: genhomedircon uid template

[Christopher J. PeBenito]

On 2/2/2016 1:26 AM, Jason Zaman wrote:
> On Mon, Feb 01, 2016 at 02:30:37PM -0500, Stephen Smalley wrote:
>> On 02/01/2016 04:36 AM, Jason Zaman wrote:
>>> Hi all,
>>>
>>> XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
>>> that in an fcontext file. It used to be /run/user/USER which is easy but
>>> not UID.
>>>
>>> What template keyword should be used for such an entry? UID? USERID?
>>>
>>> USERID is perhaps more obvious but has to be replaced before USER but
>>> that should be doable.
>>> https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76
>>>
>>> UID does not conflict with USER but this line exists in refpol which
>>> is problematic:
>>> contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache -- gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)
>>>
>>> This could also be used for several fcontexts in kerberos. It stores the
>>> tickets in /tmp/krbcc_UID for example.
>>>
>>> If we choose a template name I can put together a patch to add it.
>>
>> No strong preferences from me on the particular name, e.g. USERID is
>> fine.  I think it highlights however the problems with the current
>> approach; maybe we ought to be using ${USER} and ${UID} in .fc files
>> instead?
> 
> Yes there are definitely problems but fixing would mean refpol and
> probably a lot of other things would need to be updated at the same
> time.
> 
> HOME_DIR and HOME_ROOT are not really problems since they are only
> allowed in the beginning of an fcontext line and other lines start with
> a /.
> 
> USER, USERID, and possibly other things in future (GROUP, GROUPID?) can
> appear at any point in the the line so a more unique token might be
> better. %USERID might be better than $USERID since thats a thing in
> shells.
> 
> If we do go down this path, what are the steps? and what tokens do we
> want?

Neglecting any %, {}, etc. I suggest being explicit: UNAME or USERNAME
rather than USER.  That would make a clearer intent, similar to UID or
USERID.

-- 
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com

Re: genhomedircon uid template

[Stephen Smalley]

On 02/02/2016 01:26 AM, Jason Zaman wrote:
> On Mon, Feb 01, 2016 at 02:30:37PM -0500, Stephen Smalley wrote:
>> On 02/01/2016 04:36 AM, Jason Zaman wrote:
>>> Hi all,
>>>
>>> XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
>>> that in an fcontext file. It used to be /run/user/USER which is easy but
>>> not UID.
>>>
>>> What template keyword should be used for such an entry? UID? USERID?
>>>
>>> USERID is perhaps more obvious but has to be replaced before USER but
>>> that should be doable.
>>> https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76
>>>
>>> UID does not conflict with USER but this line exists in refpol which
>>> is problematic:
>>> contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache -- gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)
>>>
>>> This could also be used for several fcontexts in kerberos. It stores the
>>> tickets in /tmp/krbcc_UID for example.
>>>
>>> If we choose a template name I can put together a patch to add it.
>>
>> No strong preferences from me on the particular name, e.g. USERID is
>> fine.  I think it highlights however the problems with the current
>> approach; maybe we ought to be using ${USER} and ${UID} in .fc files
>> instead?
>
> Yes there are definitely problems but fixing would mean refpol and
> probably a lot of other things would need to be updated at the same
> time.
>
> HOME_DIR and HOME_ROOT are not really problems since they are only
> allowed in the beginning of an fcontext line and other lines start with
> a /.
>
> USER, USERID, and possibly other things in future (GROUP, GROUPID?) can
> appear at any point in the the line so a more unique token might be
> better. %USERID might be better than $USERID since thats a thing in
> shells.
>
> If we do go down this path, what are the steps? and what tokens do we
> want?

I think we would provide backward compatibility for the existing tokens, 
at least for some time.  As far as I know, we only need to modify the 
refpolicy build process and libsemanage to support the new tokens.

You need a way to mark the end of the token, either %USERID% or %{USERID}.

Re: genhomedircon uid template

[Nicolas Iooss]

[-- Attachment #1: Type: text/plain, Size: 3036 bytes --]

On Tue, Feb 2, 2016 at 4:03 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:

> On 02/02/2016 01:26 AM, Jason Zaman wrote:
>
>> On Mon, Feb 01, 2016 at 02:30:37PM -0500, Stephen Smalley wrote:
>>
>>> On 02/01/2016 04:36 AM, Jason Zaman wrote:
>>>
>>>> Hi all,
>>>>
>>>> XDG_RUNTIME_DIR is usually /run/user/$UID but there is no way to label
>>>> that in an fcontext file. It used to be /run/user/USER which is easy but
>>>> not UID.
>>>>
>>>> What template keyword should be used for such an entry? UID? USERID?
>>>>
>>>> USERID is perhaps more obvious but has to be replaced before USER but
>>>> that should be doable.
>>>>
>>>> https://github.com/SELinuxProject/selinux/blob/master/libsemanage/src/genhomedircon.c#L76
>>>>
>>>> UID does not conflict with USER but this line exists in refpol which
>>>> is problematic:
>>>> contrib/fetchmail.fc:13:/var/mail/\.fetchmail-UIDL-cache --
>>>> gen_context(system_u:object_r:fetchmail_uidl_cache_t,s0)
>>>>
>>>> This could also be used for several fcontexts in kerberos. It stores the
>>>> tickets in /tmp/krbcc_UID for example.
>>>>
>>>> If we choose a template name I can put together a patch to add it.
>>>>
>>>
>>> No strong preferences from me on the particular name, e.g. USERID is
>>> fine.  I think it highlights however the problems with the current
>>> approach; maybe we ought to be using ${USER} and ${UID} in .fc files
>>> instead?
>>>
>>
>> Yes there are definitely problems but fixing would mean refpol and
>> probably a lot of other things would need to be updated at the same
>> time.
>>
>> HOME_DIR and HOME_ROOT are not really problems since they are only
>> allowed in the beginning of an fcontext line and other lines start with
>> a /.
>>
>> USER, USERID, and possibly other things in future (GROUP, GROUPID?) can
>> appear at any point in the the line so a more unique token might be
>> better. %USERID might be better than $USERID since thats a thing in
>> shells.
>>
>> If we do go down this path, what are the steps? and what tokens do we
>> want?
>>
>
> I think we would provide backward compatibility for the existing tokens,
> at least for some time.  As far as I know, we only need to modify the
> refpolicy build process and libsemanage to support the new tokens.
>
> You need a way to mark the end of the token, either %USERID% or %{USERID}.


After reading this thread, I was wondering: should a way of escaping
placeholders be documented somewhere? For example, if I ever encounter a
file with "%USERID" in its name, I would use "[%]USERID" in the file
context pattern to give it a label. I find this quite hackish and I may
have missed a better way of handling it. Where are such things usually
documented?

By the way, on systems running systemd, /run/user/$UID is a tmpfs
mountpoint which would be relabeled by systemd-logind right after it is
created (using lsetfiecon with the label defined in the policy), but a
piece of code is missing to enable this on Arch Linux and Debian. I
reported this on https://github.com/systemd/systemd/pull/2508 .

Nicolas

[-- Attachment #2: Type: text/html, Size: 4401 bytes --]

genhomedircon USERID and USERNAME patches

[Jason Zaman]

Hi all,

I finally finished adding more templates to genhomedircon and a lot of general
cleanups.

The first few patches refactor the templating functions so they are smaller and
easier to add new. All the common bits were taken out and they all take the
user_entry_t struct instead of passing args one by one.

The last three patches add the new templating types. I went with %{USERID} and
%{USERNAME}. They now have a clear start and end unlike USER in the past and $
is end of line in regexes so % seems safer. The matcher for USER now
specifically excludes any line that has the new patterns in it too so there can
be no conflict. It appears to work in the testing I have done with adding
strange fcontexts. make test passes in the repo too but i have not run the full
selinux-testsuite.

%{USERNAME} defaults to ".*" in the fallback just like USER originally did
%{USERID} defaults to "[0-9]+" for the fallback.

Another thing I noticed was that HOME_DIR's fallback is "[^/]*", should it be +
instead of *? I dont think it makes a huge difference because then it should
match HOME_ROOT but it still seems wrong.

-- Jason

[PATCH 1/7] genhomedircon: factor out common replacement code

[Jason Zaman]

All the write_*_contexts() methods use exactly the same code.
This splits it off into a common helper function.
---
 libsemanage/src/genhomedircon.c | 69 ++++++++++++++---------------------------
 1 file changed, 23 insertions(+), 46 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 1a9e87e..a95ab16 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -548,23 +548,11 @@ static int check_line(genhomedircon_settings_t * s, Ustr *line)
 	return result;
 }
 
-static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
-				  semanage_list_t * tpl, const char *user,
-				  const char *seuser, const char *home,
-				  const char *role_prefix, const char *level)
+static int write_replacements(genhomedircon_settings_t * s, FILE * out,
+			      semanage_list_t * tpl, replacement_pair_t *repl)
 {
-	replacement_pair_t repl[] = {
-		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
-		{.search_for = TEMPLATE_HOME_DIR,.replace_with = home},
-		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
-		{.search_for = TEMPLATE_LEVEL,.replace_with = level},
-		{NULL, NULL}
-	};
 	Ustr *line = USTR_NULL;
 
-	if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user) < 0)
-		return STATUS_ERR;
-
 	for (; tpl; tpl = tpl->next) {
 		line = replace_all(tpl->data, repl);
 		if (!line)
@@ -582,6 +570,25 @@ static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
 	return STATUS_ERR;
 }
 
+static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
+				  semanage_list_t * tpl, const char *user,
+				  const char *seuser, const char *home,
+				  const char *role_prefix, const char *level)
+{
+	replacement_pair_t repl[] = {
+		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
+		{.search_for = TEMPLATE_HOME_DIR,.replace_with = home},
+		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
+		{.search_for = TEMPLATE_LEVEL,.replace_with = level},
+		{NULL, NULL}
+	};
+
+	if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user) < 0)
+		return STATUS_ERR;
+
+	return write_replacements(s, out, tpl, repl);
+}
+
 static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 				   semanage_list_t * tpl, char *homedir)
 {
@@ -589,23 +596,8 @@ static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 		{.search_for = TEMPLATE_HOME_ROOT,.replace_with = homedir},
 		{NULL, NULL}
 	};
-	Ustr *line = USTR_NULL;
-
-	for (; tpl; tpl = tpl->next) {
-		line = replace_all(tpl->data, repl);
-		if (!line)
-			goto fail;
-		if (check_line(s, line) == STATUS_SUCCESS) {
-			if (!ustr_io_putfileline(&line, out))
-				goto fail;
-		}
-		ustr_sc_free(&line);
-	}
-	return STATUS_SUCCESS;
 
-      fail:
-	ustr_sc_free(&line);
-	return STATUS_ERR;
+	return write_replacements(s, out, tpl, repl);
 }
 
 static int write_user_context(genhomedircon_settings_t * s, FILE * out,
@@ -618,23 +610,8 @@ static int write_user_context(genhomedircon_settings_t * s, FILE * out,
 		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
 		{NULL, NULL}
 	};
-	Ustr *line = USTR_NULL;
-
-	for (; tpl; tpl = tpl->next) {
-		line = replace_all(tpl->data, repl);
-		if (!line)
-			goto fail;
-		if (check_line(s, line) == STATUS_SUCCESS) {
-			if (!ustr_io_putfileline(&line, out))
-				goto fail;
-		}
-		ustr_sc_free(&line);
-	}
-	return STATUS_SUCCESS;
 
-      fail:
-	ustr_sc_free(&line);
-	return STATUS_ERR;
+	return write_replacements(s, out, tpl, repl);
 }
 
 static int user_sort_func(semanage_user_t ** arg1, semanage_user_t ** arg2)
-- 
2.7.3

[PATCH 2/7] genhomedircon: move fallback user to genhomedircon_user_entry_t

[Jason Zaman]

The fallback user is used in all the write functions, making it use a
struct allows us to have everything consistent between normal and
fallback users.
---
 libsemanage/src/genhomedircon.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index a95ab16..3289954 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -83,17 +83,6 @@
 #define FALLBACK_USER_LEVEL "s0"
 #define DEFAULT_LOGIN "__default__"
 
-typedef struct {
-	const char *fcfilepath;
-	int usepasswd;
-	const char *homedir_template_path;
-	char *fallback_user;
-	char *fallback_user_prefix;
-	char *fallback_user_level;
-	semanage_handle_t *h_semanage;
-	sepol_policydb_t *policydb;
-} genhomedircon_settings_t;
-
 typedef struct user_entry {
 	char *name;
 	char *sename;
@@ -104,6 +93,15 @@ typedef struct user_entry {
 } genhomedircon_user_entry_t;
 
 typedef struct {
+	const char *fcfilepath;
+	int usepasswd;
+	const char *homedir_template_path;
+	genhomedircon_user_entry_t *fallback;
+	semanage_handle_t *h_semanage;
+	sepol_policydb_t *policydb;
+} genhomedircon_settings_t;
+
+typedef struct {
 	const char *search_for;
 	const char *replace_with;
 } replacement_pair_t;
@@ -1045,10 +1043,16 @@ int semanage_genhomedircon(semanage_handle_t * sh,
 	s.fcfilepath = semanage_final_path(SEMANAGE_FINAL_TMP,
 					   SEMANAGE_FC_HOMEDIRS);
 
-	s.fallback_user = strdup(FALLBACK_USER);
-	s.fallback_user_prefix = strdup(FALLBACK_USER_PREFIX);
-	s.fallback_user_level = strdup(FALLBACK_USER_LEVEL);
-	if (s.fallback_user == NULL || s.fallback_user_prefix == NULL || s.fallback_user_level == NULL) {
+	s.fallback = calloc(1, sizeof(genhomedircon_user_entry_t));
+	if (s.fallback == NULL) {
+		retval = STATUS_ERR;
+		goto done;
+	}
+
+	s.fallback->sename = strdup(FALLBACK_USER);
+	s.fallback->prefix = strdup(FALLBACK_USER_PREFIX);
+	s.fallback->level = strdup(FALLBACK_USER_LEVEL);
+	if (s.fallback->sename == NULL || s.fallback->prefix == NULL || s.fallback->level == NULL) {
 		retval = STATUS_ERR;
 		goto done;
 	}
@@ -1072,9 +1076,7 @@ done:
 	if (out != NULL)
 		fclose(out);
 
-	free(s.fallback_user);
-	free(s.fallback_user_prefix);
-	free(s.fallback_user_level);
+	pop_user_entry(&(s.fallback));
 	ignore_free();
 
 	return retval;
-- 
2.7.3

[PATCH 3/7] genhomedircon: rename FALLBACK #defines consistent with struct

[Jason Zaman]

---
 libsemanage/src/genhomedircon.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 3289954..61f503f 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -78,9 +78,10 @@
 #define TEMPLATE_SEUSER "system_u"
 #define TEMPLATE_LEVEL "s0"
 
-#define FALLBACK_USER "user_u"
-#define FALLBACK_USER_PREFIX "user"
-#define FALLBACK_USER_LEVEL "s0"
+#define FALLBACK_SENAME "user_u"
+#define FALLBACK_PREFIX "user"
+#define FALLBACK_LEVEL "s0"
+#define FALLBACK_NAME ".*"
 #define DEFAULT_LOGIN "__default__"
 
 typedef struct user_entry {
@@ -750,14 +751,14 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
 			if (semanage_user_query(s->h_semanage, key, &u) < 0)
 			{
 				prefix = name;
-				level = FALLBACK_USER_LEVEL;
+				level = FALLBACK_LEVEL;
 			}
 			else
 			{
 				prefix = semanage_user_get_prefix(u);
 				level = semanage_user_get_mlslevel(u);
 				if (!level)
-					level = FALLBACK_USER_LEVEL;
+					level = FALLBACK_LEVEL;
 			}
 
 			if (set_fallback_user(s, seuname, prefix, level) != 0)
@@ -842,10 +843,10 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 			prefix = semanage_user_get_prefix(*u);
 			level = semanage_user_get_mlslevel(*u);
 			if (!level)
-				level = FALLBACK_USER_LEVEL;
+				level = FALLBACK_LEVEL;
 		} else {
 			prefix = name;
-			level = FALLBACK_USER_LEVEL;
+			level = FALLBACK_LEVEL;
 		}
 
 		retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent);
@@ -1049,10 +1050,14 @@ int semanage_genhomedircon(semanage_handle_t * sh,
 		goto done;
 	}
 
-	s.fallback->sename = strdup(FALLBACK_USER);
-	s.fallback->prefix = strdup(FALLBACK_USER_PREFIX);
-	s.fallback->level = strdup(FALLBACK_USER_LEVEL);
-	if (s.fallback->sename == NULL || s.fallback->prefix == NULL || s.fallback->level == NULL) {
+	s.fallback->name = strdup(FALLBACK_NAME);
+	s.fallback->sename = strdup(FALLBACK_SENAME);
+	s.fallback->prefix = strdup(FALLBACK_PREFIX);
+	s.fallback->level = strdup(FALLBACK_LEVEL);
+	if (s.fallback->name == NULL
+	 || s.fallback->sename == NULL
+	 || s.fallback->prefix == NULL
+	 || s.fallback->level == NULL) {
 		retval = STATUS_ERR;
 		goto done;
 	}
-- 
2.7.3

[PATCH 4/7] genhomedircon: make all write context funcs take user_entry struct

[Jason Zaman]

---
 libsemanage/src/genhomedircon.c | 86 ++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 56 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 61f503f..495f084 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -570,20 +570,23 @@ static int write_replacements(genhomedircon_settings_t * s, FILE * out,
 }
 
 static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
-				  semanage_list_t * tpl, const char *user,
-				  const char *seuser, const char *home,
-				  const char *role_prefix, const char *level)
+				  semanage_list_t * tpl, const genhomedircon_user_entry_t *user)
 {
 	replacement_pair_t repl[] = {
-		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
-		{.search_for = TEMPLATE_HOME_DIR,.replace_with = home},
-		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
-		{.search_for = TEMPLATE_LEVEL,.replace_with = level},
+		{.search_for = TEMPLATE_SEUSER,.replace_with = user->sename},
+		{.search_for = TEMPLATE_HOME_DIR,.replace_with = user->home},
+		{.search_for = TEMPLATE_ROLE,.replace_with = user->prefix},
+		{.search_for = TEMPLATE_LEVEL,.replace_with = user->level},
 		{NULL, NULL}
 	};
 
-	if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user) < 0)
-		return STATUS_ERR;
+	if (strcmp(user->name, FALLBACK_NAME) == 0) {
+		if (fprintf(out, COMMENT_USER_HOME_CONTEXT, FALLBACK_SENAME) < 0)
+			return STATUS_ERR;
+	} else {
+		if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user->name) < 0)
+			return STATUS_ERR;
+	}
 
 	return write_replacements(s, out, tpl, repl);
 }
@@ -600,13 +603,12 @@ static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 }
 
 static int write_user_context(genhomedircon_settings_t * s, FILE * out,
-			      semanage_list_t * tpl, const char *user,
-			      const char *seuser, const char *role_prefix)
+			      semanage_list_t * tpl, const genhomedircon_user_entry_t *user)
 {
 	replacement_pair_t repl[] = {
-		{.search_for = TEMPLATE_USER,.replace_with = user},
-		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
-		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
+		{.search_for = TEMPLATE_USER,.replace_with = user->name},
+		{.search_for = TEMPLATE_ROLE,.replace_with = user->prefix},
+		{.search_for = TEMPLATE_SEUSER,.replace_with = user->sename},
 		{NULL, NULL}
 	};
 
@@ -691,32 +693,6 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
 	free(temp);
 }
 
-static int set_fallback_user(genhomedircon_settings_t *s, const char *user,
-			     const char *prefix, const char *level)
-{
-	char *fallback_user = strdup(user);
-	char *fallback_user_prefix = strdup(prefix);
-	char *fallback_user_level = NULL;
-	if (level) 
-		fallback_user_level = strdup(level);
-
-	if (fallback_user == NULL || fallback_user_prefix == NULL ||
-	    (fallback_user_level == NULL && level != NULL)) {
-		free(fallback_user);
-		free(fallback_user_prefix);
-		free(fallback_user_level);
-		return STATUS_ERR;
-	}
-
-	free(s->fallback_user);
-	free(s->fallback_user_prefix);
-	free(s->fallback_user_level);
-	s->fallback_user = fallback_user;
-	s->fallback_user_prefix = fallback_user_prefix;
-	s->fallback_user_level = fallback_user_level;
-	return STATUS_SUCCESS;
-}
-
 static int setup_fallback_user(genhomedircon_settings_t * s)
 {
 	semanage_seuser_t **seuser_list = NULL;
@@ -761,7 +737,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
 					level = FALLBACK_LEVEL;
 			}
 
-			if (set_fallback_user(s, seuname, prefix, level) != 0)
+			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
+					    seuname, prefix, "", level) != 0)
 				errors = STATUS_ERR;
 			semanage_user_key_free(key);
 			if (u)
@@ -822,7 +799,7 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 		seuname = semanage_seuser_get_sename(seuser_list[i]);
 		name = semanage_seuser_get_name(seuser_list[i]);
 
-		if (strcmp(name,"root") && strcmp(seuname, s->fallback_user) == 0)
+		if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0)
 			continue;
 
 		if (strcmp(name, DEFAULT_LOGIN) == 0)
@@ -915,13 +892,9 @@ static int write_gen_home_dir_context(genhomedircon_settings_t * s, FILE * out,
 	}
 
 	for (; users; pop_user_entry(&users)) {
-		if (write_home_dir_context(s, out, homedir_context_tpl,
-					   users->name,
-					   users->sename, users->home,
-					   users->prefix, users->level))
+		if (write_home_dir_context(s, out, homedir_context_tpl, users))
 			goto err;
-		if (write_user_context(s, out, user_context_tpl, users->name,
-				       users->sename, users->prefix))
+		if (write_user_context(s, out, user_context_tpl, users))
 			goto err;
 	}
 
@@ -983,13 +956,13 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 				goto done;
 			}
 
-			if (write_home_dir_context(s, out,
-						   homedir_context_tpl,
-						   s->fallback_user, s->fallback_user,
-						   ustr_cstr(temp),
-						   s->fallback_user_prefix, s->fallback_user_level) !=
-			    STATUS_SUCCESS) {
+			free(s->fallback->home);
+			s->fallback->home = (char*) ustr_cstr(temp);
+
+			if (write_home_dir_context(s, out, homedir_context_tpl,
+						   s->fallback) != STATUS_SUCCESS) {
 				ustr_sc_free(&temp);
+				s->fallback->home = NULL;
 				retval = STATUS_ERR;
 				goto done;
 			}
@@ -997,17 +970,18 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 						    homeroot_context_tpl,
 						    h->data) != STATUS_SUCCESS) {
 				ustr_sc_free(&temp);
+				s->fallback->home = NULL;
 				retval = STATUS_ERR;
 				goto done;
 			}
 
 			ustr_sc_free(&temp);
+			s->fallback->home = NULL;
 		}
 	}
 	if (user_context_tpl) {
 		if (write_user_context(s, out, user_context_tpl,
-				       ".*", s->fallback_user,
-				       s->fallback_user_prefix) != STATUS_SUCCESS) {
+				       s->fallback) != STATUS_SUCCESS) {
 			retval = STATUS_ERR;
 			goto done;
 		}
-- 
2.7.3

[PATCH 5/7] genhomedircon: Add uid and gid to struct user_entry

[Jason Zaman]

---
 libsemanage/src/genhomedircon.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 495f084..ae4a261 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -82,10 +82,13 @@
 #define FALLBACK_PREFIX "user"
 #define FALLBACK_LEVEL "s0"
 #define FALLBACK_NAME ".*"
+#define FALLBACK_UIDGID "[0-9]+"
 #define DEFAULT_LOGIN "__default__"
 
 typedef struct user_entry {
 	char *name;
+	char *uid;
+	char *gid;
 	char *sename;
 	char *prefix;
 	char *home;
@@ -627,11 +630,13 @@ static int name_user_cmp(char *key, semanage_user_t ** val)
 }
 
 static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
-			   const char *sen, const char *pre, const char *h,
-			   const char *l)
+			   const char *u, const char *g, const char *sen,
+			   const char *pre, const char *h, const char *l)
 {
 	genhomedircon_user_entry_t *temp = NULL;
 	char *name = NULL;
+	char *uid = NULL;
+	char *gid = NULL;
 	char *sename = NULL;
 	char *prefix = NULL;
 	char *home = NULL;
@@ -643,6 +648,12 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 	name = strdup(n);
 	if (!name)
 		goto cleanup;
+	uid = strdup(u);
+	if (!uid)
+		goto cleanup;
+	gid = strdup(g);
+	if (!gid)
+		goto cleanup;
 	sename = strdup(sen);
 	if (!sename)
 		goto cleanup;
@@ -657,6 +668,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 		goto cleanup;
 
 	temp->name = name;
+	temp->uid = uid;
+	temp->gid = gid;
 	temp->sename = sename;
 	temp->prefix = prefix;
 	temp->home = home;
@@ -668,6 +681,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 
       cleanup:
 	free(name);
+	free(uid);
+	free(gid);
 	free(sename);
 	free(prefix);
 	free(home);
@@ -686,6 +701,8 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
 	temp = *list;
 	*list = temp->next;
 	free(temp->name);
+	free(temp->uid);
+	free(temp->gid);
 	free(temp->sename);
 	free(temp->prefix);
 	free(temp->home);
@@ -737,7 +754,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
 					level = FALLBACK_LEVEL;
 			}
 
-			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
+			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
+					    FALLBACK_UIDGID, FALLBACK_UIDGID,
 					    seuname, prefix, "", level) != 0)
 				errors = STATUS_ERR;
 			semanage_user_key_free(key);
@@ -767,6 +785,8 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 	const char *seuname = NULL;
 	const char *prefix = NULL;
 	const char *level = NULL;
+	char uid[10];
+	char gid[10];
 	struct passwd pwstorage, *pwent = NULL;
 	unsigned int i;
 	long rbuflen;
@@ -851,7 +871,13 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 		}
 		if (ignore(pwent->pw_dir))
 			continue;
-		if (push_user_entry(&head, name, seuname,
+
+		if (snprintf(uid, sizeof(uid), "%d", pwent->pw_uid) < 0
+		 || snprintf(gid, sizeof(gid), "%d", pwent->pw_gid) < 0) {
+			*errors = STATUS_ERR;
+			goto cleanup;
+		}
+		if (push_user_entry(&head, name, uid, gid, seuname,
 				    prefix, pwent->pw_dir, level) != STATUS_SUCCESS) {
 			*errors = STATUS_ERR;
 			break;
-- 
2.7.3

[PATCH 6/7] genhomedircon: make USERID, USERNAME context lists

[Jason Zaman]

---
 libsemanage/src/genhomedircon.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index ae4a261..60c9d27 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -73,8 +73,13 @@
    which are searched for and replaced */
 #define TEMPLATE_HOME_ROOT "HOME_ROOT"
 #define TEMPLATE_HOME_DIR "HOME_DIR"
+/* these are legacy */
 #define TEMPLATE_USER "USER"
 #define TEMPLATE_ROLE "ROLE"
+/* new names */
+#define TEMPLATE_USERNAME "%{USERNAME}"
+#define TEMPLATE_USERID "%{USERID}"
+
 #define TEMPLATE_SEUSER "system_u"
 #define TEMPLATE_LEVEL "s0"
 
@@ -463,8 +468,23 @@ static int HOME_DIR_PRED(const char *string)
 	return semanage_is_prefix(string, TEMPLATE_HOME_DIR);
 }
 
+/* new names */
+static int USERNAME_CONTEXT_PRED(const char *string)
+{
+	return (int)(strstr(string, TEMPLATE_USERNAME) != NULL);
+}
+
+static int USERID_CONTEXT_PRED(const char *string)
+{
+	return (int)(strstr(string, TEMPLATE_USERID) != NULL);
+}
+
+/* This will never match USER if USERNAME or USERID are found. */
 static int USER_CONTEXT_PRED(const char *string)
 {
+	if (USERNAME_CONTEXT_PRED(string) || USERID_CONTEXT_PRED(string))
+		return 0;
+
 	return (int)(strstr(string, TEMPLATE_USER) != NULL);
 }
 
@@ -943,16 +963,24 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 {
 	semanage_list_t *homedirs = NULL;
 	semanage_list_t *h = NULL;
-	semanage_list_t *user_context_tpl = NULL;
 	semanage_list_t *homedir_context_tpl = NULL;
 	semanage_list_t *homeroot_context_tpl = NULL;
+	semanage_list_t *username_context_tpl = NULL;
+	semanage_list_t *userid_context_tpl = NULL;
+	semanage_list_t *user_context_tpl = NULL;
 	int retval = STATUS_SUCCESS;
 
 	homedir_context_tpl = make_template(s, &HOME_DIR_PRED);
 	homeroot_context_tpl = make_template(s, &HOME_ROOT_PRED);
+	username_context_tpl = make_template(s, &USERNAME_CONTEXT_PRED);
+	userid_context_tpl = make_template(s, &USERID_CONTEXT_PRED);
 	user_context_tpl = make_template(s, &USER_CONTEXT_PRED);
 
-	if (!homedir_context_tpl && !homeroot_context_tpl && !user_context_tpl)
+	if (!homedir_context_tpl
+	 && !homeroot_context_tpl
+	 && !userid_context_tpl
+	 && !username_context_tpl
+	 && !user_context_tpl)
 		goto done;
 
 	if (write_file_context_header(out) != STATUS_SUCCESS) {
@@ -1021,6 +1049,8 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 done:
 	/* Cleanup */
 	semanage_list_destroy(&homedirs);
+	semanage_list_destroy(&username_context_tpl);
+	semanage_list_destroy(&userid_context_tpl);
 	semanage_list_destroy(&user_context_tpl);
 	semanage_list_destroy(&homedir_context_tpl);
 	semanage_list_destroy(&homeroot_context_tpl);
-- 
2.7.3

[PATCH 7/7] genhomedircon: write contexts for username and userid

[Jason Zaman]

---
 libsemanage/src/genhomedircon.c | 51 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 60c9d27..66ab307 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -625,6 +625,34 @@ static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 	return write_replacements(s, out, tpl, repl);
 }
 
+static int write_username_context(genhomedircon_settings_t * s, FILE * out,
+				  semanage_list_t * tpl,
+				  const genhomedircon_user_entry_t *user)
+{
+	replacement_pair_t repl[] = {
+		{.search_for = TEMPLATE_USERNAME,.replace_with = user->name},
+		{.search_for = TEMPLATE_ROLE,.replace_with = user->prefix},
+		{.search_for = TEMPLATE_SEUSER,.replace_with = user->sename},
+		{NULL, NULL}
+	};
+
+	return write_replacements(s, out, tpl, repl);
+}
+
+static int write_userid_context(genhomedircon_settings_t * s, FILE * out,
+				  semanage_list_t * tpl,
+				  const genhomedircon_user_entry_t *user)
+{
+	replacement_pair_t repl[] = {
+		{.search_for = TEMPLATE_USERID,.replace_with = user->uid},
+		{.search_for = TEMPLATE_ROLE,.replace_with = user->prefix},
+		{.search_for = TEMPLATE_SEUSER,.replace_with = user->sename},
+		{NULL, NULL}
+	};
+
+	return write_replacements(s, out, tpl, repl);
+}
+
 static int write_user_context(genhomedircon_settings_t * s, FILE * out,
 			      semanage_list_t * tpl, const genhomedircon_user_entry_t *user)
 {
@@ -926,6 +954,8 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 }
 
 static int write_gen_home_dir_context(genhomedircon_settings_t * s, FILE * out,
+				      semanage_list_t * username_context_tpl,
+				      semanage_list_t * userid_context_tpl,
 				      semanage_list_t * user_context_tpl,
 				      semanage_list_t * homedir_context_tpl)
 {
@@ -940,6 +970,10 @@ static int write_gen_home_dir_context(genhomedircon_settings_t * s, FILE * out,
 	for (; users; pop_user_entry(&users)) {
 		if (write_home_dir_context(s, out, homedir_context_tpl, users))
 			goto err;
+		if (write_username_context(s, out, username_context_tpl, users))
+			goto err;
+		if (write_userid_context(s, out, userid_context_tpl, users))
+			goto err;
 		if (write_user_context(s, out, user_context_tpl, users))
 			goto err;
 	}
@@ -1033,14 +1067,27 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 			s->fallback->home = NULL;
 		}
 	}
-	if (user_context_tpl) {
+	if (user_context_tpl || userid_context_tpl || username_context_tpl) {
+		if (write_username_context(s, out, username_context_tpl,
+					   s->fallback) != STATUS_SUCCESS) {
+			retval = STATUS_ERR;
+			goto done;
+		}
+
+		if (write_userid_context(s, out, userid_context_tpl,
+					 s->fallback) != STATUS_SUCCESS) {
+			retval = STATUS_ERR;
+			goto done;
+		}
+
 		if (write_user_context(s, out, user_context_tpl,
 				       s->fallback) != STATUS_SUCCESS) {
 			retval = STATUS_ERR;
 			goto done;
 		}
 
-		if (write_gen_home_dir_context(s, out, user_context_tpl,
+		if (write_gen_home_dir_context(s, out, username_context_tpl,
+					       userid_context_tpl, user_context_tpl,
 					       homedir_context_tpl) != STATUS_SUCCESS) {
 			retval = STATUS_ERR;
 		}
-- 
2.7.3

Re: genhomedircon USERID and USERNAME patches

[Nicolas Iooss]

[-- Attachment #1: Type: text/plain, Size: 2933 bytes --]

On Fri, Apr 8, 2016 at 6:05 PM, Jason Zaman <jason@perfinion.com> wrote:

> Hi all,
>
> I finally finished adding more templates to genhomedircon and a lot of
> general
> cleanups.
>
> The first few patches refactor the templating functions so they are
> smaller and
> easier to add new. All the common bits were taken out and they all take the
> user_entry_t struct instead of passing args one by one.
>
> The last three patches add the new templating types. I went with %{USERID}
> and
> %{USERNAME}. They now have a clear start and end unlike USER in the past
> and $
> is end of line in regexes so % seems safer. The matcher for USER now
> specifically excludes any line that has the new patterns in it too so
> there can
> be no conflict. It appears to work in the testing I have done with adding
> strange fcontexts. make test passes in the repo too but i have not run the
> full
> selinux-testsuite.
>

Hi,
Thanks for your work. Your patches are very well built and I have been able
to test them without any trouble. Here are some comments:
* In Patch 1, the last parameter of write_replacements() can be made a
const pointer: "const replacement_pair_t *repl" (parameters s and tpl too
but there are not currently const pointers).
* Patch 5 introduces a "gid" field in "struct user_entry", which is not
used in the templates. Why did you introduce it?
* Patch 7 introduces two functions, write_username_context
and write_userid_context, which handle lines containing %{USERNAME}
and %{USERID} separately. If a line includes both patterns, like
"%{USERID}-%{USERNAME}", the generated file will have for root user two
lines: one with "%{USERID}-root" and the other "0-%{USERNAME}". As a user I
would have expected both templates to be replaced. I believe this may be
achieved by merging the two predicate functions together (in patch 6), and
the substitutions functions too (in patch 7).


>
> %{USERNAME} defaults to ".*" in the fallback just like USER originally did
> %{USERID} defaults to "[0-9]+" for the fallback.
>
> Another thing I noticed was that HOME_DIR's fallback is "[^/]*", should it
> be +
> instead of *? I dont think it makes a huge difference because then it
> should
> match HOME_ROOT but it still seems wrong.
>

I agree. Moreover empty usernames or usernames with / look wrong to me too.
As refpolicy seems to use "USER" as if there was no slash in it, IMHO I
would suggest using "[^/]+" for %{USERNAME} default value too.

By the way, by grep'ing HOME_DIR in refpolicy I got a hit in a support
script, "support/genhomedircon" [1]. This script is invoked only when
building modular policy and I have not found an easy way to
invoke semanage_genhomedircon() from either the command line or a Python
script. Does this script needs to be updated or is there a way to use
libsemanage implementation instead?

Cheers,
Nicolas

[1]
https://github.com/TresysTechnology/refpolicy/blob/master/support/genhomedircon

[-- Attachment #2: Type: text/html, Size: 3923 bytes --]

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/11/2016 11:44 PM, Nicolas Iooss wrote:
<snip>

> 
> By the way, by grep'ing HOME_DIR in refpolicy I got a hit in a
> support script, "support/genhomedircon" [1]. This script is invoked
> only when building modular policy and I have not found an easy way
> to invoke semanage_genhomedircon() from either the command line or
> a Python script. Does this script needs to be updated or is there a
> way to use libsemanage implementation instead?

That script no longer works on modern systems AFAIK.

genhomedircon should not be part of libsemanage in my humble view.
Instead it should be standalone

my 0.02$


- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXDKoTAAoJECV0jlU3+UdpOswL/3tubV9Aa1IW/wqLbEfFdJYL
eT/BwBxqn9HoyR6I3EsvNrQ5rBJ5DNATWbCC08APW+U1Av8GvtlM2XPzzrYlr4e0
AY2e3Br1RXqOlwlYxdqjR+RvGpbhV5ZqI3zV2ZKSvy1z4S9qJ03mUyhd8FxhqACY
bAMNtim7C/CI9Fy0i5KYZE/bRozoVrix+0T1xKUuZDtrsxKxBcAMc+VN2yEA0c9Z
l54tmVQjKkTyqnA5YXGw/sHwVEfedvMmS+P7ZQzNOQAAIY+EFeHal+clhPo9eUI0
NYQflvmrnkHCKfWMCulWfKJIDwCifY/boO4Bk92IzY2bANR4KO/UgVxVG+R/Zmd+
jcadUeK/JuJkjSaZcwf5uja1/aCFsdsZu5IiskhLJpmbv/myTC8/uXJEs8HZ4S3D
gdUygCO0gjlqBJFN+6mGWps05fV9Zgmfn3VmdjWzne7AiESFOUfKlGf2MQFDR1oQ
d7W75bXZxiBdE+MIfZSwIsyQZgDyeCw87zKaW/co/g==
=hMAL
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Jason Zaman]

On Mon, Apr 11, 2016 at 11:44:20PM +0200, Nicolas Iooss wrote:
> On Fri, Apr 8, 2016 at 6:05 PM, Jason Zaman <jason@perfinion.com> wrote:
> > Hi all,
> >
> > I finally finished adding more templates to genhomedircon and a lot of
> > general
> > cleanups.
> >
> > The first few patches refactor the templating functions so they are
> > smaller and
> > easier to add new. All the common bits were taken out and they all take the
> > user_entry_t struct instead of passing args one by one.
> >
> > The last three patches add the new templating types. I went with %{USERID}
> > and
> > %{USERNAME}. They now have a clear start and end unlike USER in the past
> > and $
> > is end of line in regexes so % seems safer. The matcher for USER now
> > specifically excludes any line that has the new patterns in it too so
> > there can
> > be no conflict. It appears to work in the testing I have done with adding
> > strange fcontexts. make test passes in the repo too but i have not run the
> > full
> > selinux-testsuite.
> >
> 
> Hi,
> Thanks for your work. Your patches are very well built and I have been able
> to test them without any trouble. Here are some comments:
> * In Patch 1, the last parameter of write_replacements() can be made a
> const pointer: "const replacement_pair_t *repl" (parameters s and tpl too
> but there are not currently const pointers).
Good catch, I'll mark as many as I can const.

> * Patch 5 introduces a "gid" field in "struct user_entry", which is not
> used in the templates. Why did you introduce it?
I was thinking about adding a %{GROUPID} in the future so added gid. It does
not get stored in the fcontext files so isnt much overhead. I can remove
it tho if that is better.

> * Patch 7 introduces two functions, write_username_context
> and write_userid_context, which handle lines containing %{USERNAME}
> and %{USERID} separately. If a line includes both patterns, like
> "%{USERID}-%{USERNAME}", the generated file will have for root user two
> lines: one with "%{USERID}-root" and the other "0-%{USERNAME}". As a user I
> would have expected both templates to be replaced. I believe this may be
> achieved by merging the two predicate functions together (in patch 6), and
> the substitutions functions too (in patch 7).
I had missed this. Yeah I think just merging all the replacements
together would work well.

> > %{USERNAME} defaults to ".*" in the fallback just like USER originally did
> > %{USERID} defaults to "[0-9]+" for the fallback.
> >
> > Another thing I noticed was that HOME_DIR's fallback is "[^/]*", should it
> > be +
> > instead of *? I dont think it makes a huge difference because then it
> > should
> > match HOME_ROOT but it still seems wrong.
> >
> 
> I agree. Moreover empty usernames or usernames with / look wrong to me too.
> As refpolicy seems to use "USER" as if there was no slash in it, IMHO I
> would suggest using "[^/]+" for %{USERNAME} default value too.
I was hesitant to change this since I did not want to deviate from the
old behaviour much but I dont see any obvious issues changing it would
cause so I'll add a new patch fixing this too.

Regarding the default for %{USERNAME} too, there is this fcontext in
refpol: /tmp/gconfd-USER -d gen_context(system_u:object_r:user_tmp_t,s0)
which expands to: /tmp/gconfd-.* -d user_u:object_r:user_tmp_t:s0
I originally had it like how you suggest but changed it back so this
fallback fcontext remains unchanged. gconfd-[^/]+ is probably more correct
anyway so i'll change it.

> By the way, by grep'ing HOME_DIR in refpolicy I got a hit in a support
> script, "support/genhomedircon" [1]. This script is invoked only when
> building modular policy and I have not found an easy way to
> invoke semanage_genhomedircon() from either the command line or a Python
> script. Does this script needs to be updated or is there a way to use
> libsemanage implementation instead?

As far as I understand, the genhomedircon inside refpolicy was used
originally before it became part of semanage. We might want to just
remove that from refpolicy completely so there is no confusion but I
will leave that up to Chris.

As for running this to test, I did not find a super easy way either.
genhomedircon is a symlink to semodule and accepts no arguments. It
would be nice to have a -o outputfile or something for testing at least.
For testing I just ran either "genhomedircon" or "semodule -nB" and
looked at the /etc/selinux/mcs/contexts/files/file_contexts.homedirs
file directly.

I will wait a bit for more comments then send out v2. Thanks for
reviewing :D

-- Jason

Re: genhomedircon USERID and USERNAME patches

[Stephen Smalley]

On 04/11/2016 05:44 PM, Nicolas Iooss wrote:
> On Fri, Apr 8, 2016 at 6:05 PM, Jason Zaman <jason@perfinion.com
> <mailto:jason@perfinion.com>> wrote:
> 
>     Hi all,
> 
>     I finally finished adding more templates to genhomedircon and a lot
>     of general
>     cleanups.
> 
>     The first few patches refactor the templating functions so they are
>     smaller and
>     easier to add new. All the common bits were taken out and they all
>     take the
>     user_entry_t struct instead of passing args one by one.
> 
>     The last three patches add the new templating types. I went with
>     %{USERID} and
>     %{USERNAME}. They now have a clear start and end unlike USER in the
>     past and $
>     is end of line in regexes so % seems safer. The matcher for USER now
>     specifically excludes any line that has the new patterns in it too
>     so there can
>     be no conflict. It appears to work in the testing I have done with
>     adding
>     strange fcontexts. make test passes in the repo too but i have not
>     run the full
>     selinux-testsuite.
> 
> 
> Hi,
> Thanks for your work. Your patches are very well built and I have been
> able to test them without any trouble. Here are some comments:
> * In Patch 1, the last parameter of write_replacements() can be made a
> const pointer: "const replacement_pair_t *repl" (parameters s and tpl
> too but there are not currently const pointers).
> * Patch 5 introduces a "gid" field in "struct user_entry", which is not
> used in the templates. Why did you introduce it?
> * Patch 7 introduces two functions, write_username_context
> and write_userid_context, which handle lines containing %{USERNAME}
> and %{USERID} separately. If a line includes both patterns, like
> "%{USERID}-%{USERNAME}", the generated file will have for root user two
> lines: one with "%{USERID}-root" and the other "0-%{USERNAME}". As a
> user I would have expected both templates to be replaced. I believe this
> may be achieved by merging the two predicate functions together (in
> patch 6), and the substitutions functions too (in patch 7).
>  
> 
> 
>     %{USERNAME} defaults to ".*" in the fallback just like USER
>     originally did
>     %{USERID} defaults to "[0-9]+" for the fallback.
> 
>     Another thing I noticed was that HOME_DIR's fallback is "[^/]*",
>     should it be +
>     instead of *? I dont think it makes a huge difference because then
>     it should
>     match HOME_ROOT but it still seems wrong.
> 
> 
> I agree. Moreover empty usernames or usernames with / look wrong to me
> too. As refpolicy seems to use "USER" as if there was no slash in it,
> IMHO I would suggest using "[^/]+" for %{USERNAME} default value too.
> 
> By the way, by grep'ing HOME_DIR in refpolicy I got a hit in a support
> script, "support/genhomedircon" [1]. This script is invoked only when
> building modular policy and I have not found an easy way to
> invoke semanage_genhomedircon() from either the command line or a Python
> script. Does this script needs to be updated or is there a way to use
> libsemanage implementation instead?
> 
> Cheers,
> Nicolas
> 
> [1] https://github.com/TresysTechnology/refpolicy/blob/master/support/genhomedircon

I believe the script is obsolete under modular policy (only used on
monolithic builds), and you can trigger semanage_genhomedircon() just by
running semodule -Bn.  /usr/sbin/genhomedircon is a symlink to semodule,
and semodule runs as if invoked with -Bn when called under that name.

Re: genhomedircon USERID and USERNAME patches

[Christopher J. PeBenito]

On 4/12/2016 8:57 AM, Stephen Smalley wrote:
> On 04/11/2016 05:44 PM, Nicolas Iooss wrote:
>> On Fri, Apr 8, 2016 at 6:05 PM, Jason Zaman <jason@perfinion.com
>> <mailto:jason@perfinion.com>> wrote:

>> By the way, by grep'ing HOME_DIR in refpolicy I got a hit in a support
>> script, "support/genhomedircon" [1]. This script is invoked only when
>> building modular policy and I have not found an easy way to
>> invoke semanage_genhomedircon() from either the command line or a Python
>> script. Does this script needs to be updated or is there a way to use
>> libsemanage implementation instead?

>> [1] https://github.com/TresysTechnology/refpolicy/blob/master/support/genhomedircon
> 
> I believe the script is obsolete under modular policy (only used on
> monolithic builds), and you can trigger semanage_genhomedircon() just by
> running semodule -Bn.  /usr/sbin/genhomedircon is a symlink to semodule,
> and semodule runs as if invoked with -Bn when called under that name.

Right, the script that refpolicy has is only used on monolithic builds.

-- 
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/12/2016 04:35 PM, Christopher J. PeBenito wrote:

> Right, the script that refpolicy has is only used on monolithic
> builds.
> 

As I recall, last time i tried it, it was not working on a
redhat-based system. Probably bit-rot has set in. There is also some
hard-coding going on in there i believe.

Someone with c-skills should rewrite it in c, make it truly portable.
That way it can be used with both monolithic as well as modular policy
(i don't mean binary-based module policy here but text-based module
policy that otherwise does not require the presence of semodule)


- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXDnT0AAoJECV0jlU3+UdpemoL/1gd68W5tPnsbgHbZev28M4Z
d6hhjMJne1uMeANuvPUfPeGQpamd0+dhpJrROyLDe9V1KjbDENbu9PlHIfFncdOX
VyJNj+o+YEHx34nKOZRQAvWcjp1Kgn6JztjA6yfb2mf0D93BNkHQWadRllUzp7Lt
kUyFgZFzr1GbsEyK77hEr2/4k4JBWb2idf8XlMIyg6+51JX11eVjAmhXQn9uaJgF
rvyYvfUnr8DK+MVH2BMRIBNM10gPpVQvNlYrwsPfhXW+HyoS9ZE1HtXn9h3gr58A
sWg655PKTBtUU0fkrXJzRLCcmPWekOmh1lK82V23qfT2xcEG44XakBLlGlIQuX+k
l7R0DDE79QdLVDv7Joh+nVtiFuVYTOqtDWLyzq7RoDVMfuGU+Ni2c2LKONbYwKf4
J3YoS5KbMUr4IjQVlMo2+iGvsOLeI4g6SeppA2taZz2K01fdw2fYRD7chIN5ghqS
+EEhM8veTPpEq87ifpZQMIcBdKTXDqJRU9ppVUzoxQ==
=uIXm
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Stephen Smalley]

On 04/13/2016 12:34 PM, Dominick Grift wrote:
> On 04/12/2016 04:35 PM, Christopher J. PeBenito wrote:
> 
>> Right, the script that refpolicy has is only used on monolithic
>> builds.
> 
> 
> As I recall, last time i tried it, it was not working on a
> redhat-based system. Probably bit-rot has set in. There is also some
> hard-coding going on in there i believe.
> 
> Someone with c-skills should rewrite it in c, make it truly portable.
> That way it can be used with both monolithic as well as modular policy
> (i don't mean binary-based module policy here but text-based module
> policy that otherwise does not require the presence of semodule)

Even CIL-based modules are installed via semodule and managed via
libsemanage, and libsemanage already contains a C implementation of
genhomedircon.

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/13/2016 07:00 PM, Stephen Smalley wrote:
> On 04/13/2016 12:34 PM, Dominick Grift wrote:
>> On 04/12/2016 04:35 PM, Christopher J. PeBenito wrote:
>> 
>>> Right, the script that refpolicy has is only used on
>>> monolithic builds.
>> 
>> 
>> As I recall, last time i tried it, it was not working on a 
>> redhat-based system. Probably bit-rot has set in. There is also
>> some hard-coding going on in there i believe.
>> 
>> Someone with c-skills should rewrite it in c, make it truly
>> portable. That way it can be used with both monolithic as well as
>> modular policy (i don't mean binary-based module policy here but
>> text-based module policy that otherwise does not require the
>> presence of semodule)
> 
> Even CIL-based modules are installed via semodule and managed via 
> libsemanage, and libsemanage already contains a C implementation
> of genhomedircon.
> 
> 

Let me correct that:

"Even CIL-based modules can be installed via semodule and managed via
libsemanage, and libsemanage already contains a C implementation of
genhomedircon."


that is not my point though. My point is: since we need a working
genhomedircon for monolithic policy, we might cease the opportunity to
also support text-based module policy which can be installed via
semodule and managed via libsemanage, but does not strictly require that
.


- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXDn1vAAoJECV0jlU3+UdpkP4MAIhmW/ghl9/pVBVSX+56lZEB
ns9znLpwUSv50kcYDh9e8lfWrIOlJGgXD86D2pnCU2AjJbxvBWaEQpPw0Z0c+Vmy
M8NvBTqm763mTZ2ANgGFjWpE0cAOAFIHBmfNhacl6fyUwVLfxZA/locUh9SJIsqd
AhSx+lt9t2uMxB7W2PoCf6Bwpz+fQDbnD+3vaU5ZIcu/sNYGYd4Pmf4jYl38QR1P
vxCIrj4OTJrT7w0A0/J98d7jBnLsvvb63Jjm+5HP3PP5mHn0DGu6/qEBKJcuYWPP
tyRIVzlvreRrrVoF6u9BtgDD7+/1XXco20yoQanXCr2fhox5OkW8/TerK9KEcbwE
a4MZyDPlTrpthprznvSSaCo5uK6xCoaIBR9GjBlU3L/CUzI6ZioEoVwX9wxMpskA
TGtYRJyCVlrxYCy7ec/qEBY3vTg61DsTwq2fL84F+11NoJofIBuY2AwqQ5WpJmwN
WRtkEVQWqtxE1cybXhiQX0UIlBt3uqcHO6+Xr5TYSQ==
=u/g3
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/13/2016 07:10 PM, Dominick Grift wrote:
> On 04/13/2016 07:00 PM, Stephen Smalley wrote:
>> On 04/13/2016 12:34 PM, Dominick Grift wrote:
>>> On 04/12/2016 04:35 PM, Christopher J. PeBenito wrote:
>>> 
>>>> Right, the script that refpolicy has is only used on 
>>>> monolithic builds.
>>> 
>>> 
>>> As I recall, last time i tried it, it was not working on a 
>>> redhat-based system. Probably bit-rot has set in. There is
>>> also some hard-coding going on in there i believe.
>>> 
>>> Someone with c-skills should rewrite it in c, make it truly 
>>> portable. That way it can be used with both monolithic as well
>>> as modular policy (i don't mean binary-based module policy here
>>> but text-based module policy that otherwise does not require
>>> the presence of semodule)
> 
>> Even CIL-based modules are installed via semodule and managed via
>>  libsemanage, and libsemanage already contains a C
>> implementation of genhomedircon.
> 
> 
> 
> Let me correct that:
> 
> "Even CIL-based modules can be installed via semodule and managed
> via libsemanage, and libsemanage already contains a C
> implementation of genhomedircon."
> 
> 
> that is not my point though. My point is: since we need a working 
> genhomedircon for monolithic policy, we might cease the opportunity
> to also support text-based module policy which can be installed
> via semodule and managed via libsemanage, but does not strictly
> require that .
> 

Make that "seize" instead (i think)


> 
> 

- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXDn9WAAoJECV0jlU3+UdpivkL/RjKc+zFM5OAY6tt9Ev0fMr4
hSOgfzjrGjjzbX8j/orAz+gk0IJ7+2GDYzA4i9EUH8cFIE04Nd7ldEWRDnhxJJ0C
zHmKt2YOTLCgRW38RCQlX9cUU8ABuqjPlv/PUXPAspGbvnkAnH0SwRwQBJTlTH7w
usaGy+3Dl0rnrGhPvAs2fA1kBL0KlxQRIf+fcqJlU9Nt+OCOj7ufv13IulttMk1b
6HGq5jzhhhpSWhLwO/Mn7NSXmSobzl/U5xdjWyqEpsN0urcm1JIqwZIrsBSkn0+r
hc/5VIf5ckjLZepLTCV/FF1MisuZHcepgYCtjjVo9h1PuweB/Y5QlLCyJFyAofD/
e9TRhiCyzCb3cmfytKo8hxngAjOBq/h2fdFbpYz1d0meBXl4YKwHpSr3xmx+bIBX
pDP7dHewwPcqSq12J7eszAtnw9NbIxUhOZF8ERyOi82pZR/TlXW05GBr1OKjDz8D
w83DYzURyIh14xfqYPzDSxMgdzncLEgShx0jQR6SxQ==
=TpcH
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/13/2016 07:18 PM, Dominick Grift wrote:

>> Let me correct that:
> 
>> "Even CIL-based modules can be installed via semodule and
>> managed via libsemanage, and libsemanage already contains a C 
>> implementation of genhomedircon."
> 
> 
>> that is not my point though. My point is: since we need a working
>>  genhomedircon for monolithic policy, we might cease the
>> opportunity to also support text-based module policy which can be
>> installed via semodule and managed via libsemanage, but does not
>> strictly require that .
> 
> 
> Make that "seize" instead (i think)
> 

I just realized that i do not have to bring CIL into the equation
here. We can keep it nice, simple and to the point. Refpolicy
genhomedircon needs to be updated (and needs just a general review to
make it work again on modern distributions) as well to make this new
functionality also work with monolithic policy.

Also since were on the discussion of genhomedircon, I might be wrong
here, but I believe genhomedircon cannot currently deal with %group
entries in seusers.

- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXDo8yAAoJECV0jlU3+Udp7MYL/RjPxkBJMaueFJPP9rl93b/v
Jna5u8DdZLBXOCXNxViu7RKVEWgQ/l0c/DjousjnxsZ3RHAVPQ+p54OJLioyJmxj
PUrfp/xpeNkp/w/J2pKv/ElFl1hikQg8jqJsSJYLowTdP24cjgwNnXSWLOdsQdBH
mRPramRrTx/34SWcl2WMRC4MbVSnTEFM9S3OvjQWKHzImEqmgfEVR8d6XU9qe+oy
4qasZOT6sFNq/p9/lpHv7HuyjlysuVfKRnp6N4m/xgZ7/zOkLfTy2ozsVWRcRUHl
vsNGq/RHkiqNgozNZ6QjUfyOwv18wZrO3gFc/g4DSjJs97Iteej4vJw4/TrI1h4L
EiC31l2JU57Hq2ySVxK6BPxyMfLvxkE8dsGwJ/H1ABMvkQfqVCSW0ih2EmvI1jvo
fH1DWPPxwc1bBBtaWGr5MovdRF6vtud4Md+zufMqW0j3lLZTWVqASy+DuLFeRsIy
ZHjLSCNa9QHuc97QwCxuVRZJAWWSe9FvpAStNH/tAQ==
=nbjE
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/13/2016 08:25 PM, Dominick Grift wrote:
> On 04/13/2016 07:18 PM, Dominick Grift wrote:
> 
>>> Let me correct that:
> 
>>> "Even CIL-based modules can be installed via semodule and 
>>> managed via libsemanage, and libsemanage already contains a C 
>>> implementation of genhomedircon."
> 
> 
>>> that is not my point though. My point is: since we need a
>>> working genhomedircon for monolithic policy, we might cease
>>> the opportunity to also support text-based module policy which
>>> can be installed via semodule and managed via libsemanage, but
>>> does not strictly require that .
> 
> 
>> Make that "seize" instead (i think)
> 
> 
> I just realized that i do not have to bring CIL into the equation 
> here. We can keep it nice, simple and to the point. Refpolicy 
> genhomedircon needs to be updated (and needs just a general review
> to make it work again on modern distributions) as well to make this
> new functionality also work with monolithic policy.
> 
> Also since were on the discussion of genhomedircon, I might be
> wrong here, but I believe genhomedircon cannot currently deal with
> %group entries in seusers.
> 
> 

I think we should also generate file context specs for user mail spool
(is currently not done AFAIK in common module policy).

But if genhomedircon would not hardcode keywords and identifiers, then
i could accept them through the command line options and map it to
some hardcoded initial identifiers where/if needed

then one could specify on the command line the keyword, and any other
identifiers. So that is a bit more flexible so that when in the future
we get other instances where we need to generate contexts for some
location we dont have to edit the script we can just pass it some
additional new options. But yes then i suppose it would need to be a
standalone script. Added value there is that we then no longer have to
maintain to genhomedircon instances, one for libsemanage and one for
monolitic policy and text-based module policy.

- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXE2GXAAoJECV0jlU3+UdpHjML/0WTMGgEjjkgPuLduoOTxGjA
VBiiAOSIrrwh//TxXbbVxy1G46AHAD8SxBfC8TRCnAYMP01EVHpst7lLwXP4aDvH
a0Nhv7NV3nfdKCMuYKcXmGjPvkMD2u5v2JOpQjAM4s1AHxCm5irKq7rHZLeWfuVO
Ry4s0tQNRme8niHcr7I9jk50+ijxrUyKgh2CZQw0dmJ80nGCDXrbmGRYQYVY6Aw7
yNGhu7eoMbXd9yN5k9UAtT+kvLuD9gxahfokKECNqqLwWzf7ttP5idkDS8GQLYVM
OfogT6TKeQznqnU5y3ojTEVHy+2tjwjntVX4OSeBVietWH1Sulrv0SX0YhqjiYbQ
NE4GZi67JvExGXbGKSitMg6kSmivguUVIb7DAVviLW5FrHPWGZMybRFf/LtyZ3Ay
+GCgIFpeMn0933B4T6O6fZozcqpD66aFF9DWNvtQ2p9hIWzFNdxGwgg0B1gJJ0CO
eVReqMaR+15C1xiAoZz/C6Pa0r+qzTM8+0GorwEQ5A==
=ggrX
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/17/2016 12:12 PM, Dominick Grift wrote:
> On 04/13/2016 08:25 PM, Dominick Grift wrote:
>> On 04/13/2016 07:18 PM, Dominick Grift wrote:
> 
>>>> Let me correct that:
> 
>>>> "Even CIL-based modules can be installed via semodule and 
>>>> managed via libsemanage, and libsemanage already contains a C
>>>>  implementation of genhomedircon."
> 
> 
>>>> that is not my point though. My point is: since we need a 
>>>> working genhomedircon for monolithic policy, we might cease 
>>>> the opportunity to also support text-based module policy
>>>> which can be installed via semodule and managed via
>>>> libsemanage, but does not strictly require that .
> 
> 
>>> Make that "seize" instead (i think)
> 
> 
>> I just realized that i do not have to bring CIL into the equation
>>  here. We can keep it nice, simple and to the point. Refpolicy 
>> genhomedircon needs to be updated (and needs just a general
>> review to make it work again on modern distributions) as well to
>> make this new functionality also work with monolithic policy.
> 
>> Also since were on the discussion of genhomedircon, I might be 
>> wrong here, but I believe genhomedircon cannot currently deal
>> with %group entries in seusers.
> 
> 
> 
> I think we should also generate file context specs for user mail
> spool (is currently not done AFAIK in common module policy).
> 
> But if genhomedircon would not hardcode keywords and identifiers,
> then i could accept them through the command line options and map
> it to some hardcoded initial identifiers where/if needed
> 
> then one could specify on the command line the keyword, and any
> other identifiers. So that is a bit more flexible so that when in
> the future we get other instances where we need to generate
> contexts for some location we dont have to edit the script we can
> just pass it some additional new options. But yes then i suppose it
> would need to be a standalone script. Added value there is that we
> then no longer have to maintain to genhomedircon instances, one for
> libsemanage and one for monolitic policy and text-based module
> policy.
> 
> 

strangely the generated homedir_template does include my "USER_SPOOL"
entries, i cant find any reference to it in the code (maybe its a
fedora patch??)

nonetheless genhomedircon doesnt generate any contexts for me. I think
it might not be able to deal with the namespaced user
identities(sys.id instead of system_u for example)

And then there is the issue that even though the genhomedircon seems
to indicate that it supports USER and ROLE keywords. cil filecon
statements do not allow me to specify USER and ROLE in filecon it seems.

I use RBACSEP (defaultrole source) so i need to be able to associate
the appropriate roles with filecon for user file cons

All-in-all semodule/libsemanage is not an option for me. Too limited
and too much assumptions/hard coding

- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXE3ukAAoJECV0jlU3+Udpd9EL/jsi7srLMFRqaVXgtRMjWV0c
2GkG5GIAgsuKkEGhcibl9UM+j6z2wjQFU+h6MYnNak8NwiMYQINsuSsRWQpDkeHE
kih3Xnd8ziIrDdgnY3e+S3KzG9NAIFARVS0y+7Ynb+dAyc+EqtqBPW3Fcp0NTHQ0
BeE3WUTRiScNTk3tJJ3IPyGLwYVBlMYQUqF+PwlKevaRWquzluI0GyyqZeNr9hi+
t+OyjDSFETChI81Uu777artDaJWDN9jC9BgeU/C9/4k9yplv2Qje4XE4eCbo5YXA
wfjIGpo6WKr5j7mmeMTdRZHXmE9H9tH6WNRbIqvt8qs/+wBscORnt7HF0M5hxb4d
smZiyjFVprUS3B7TWnRMMWKgtshxVvokb7MZRHCrv3e/eMLfnfXxbKyC0J/qkyKr
xyZ9UXvMgr63T5QbZzgBciA5oAsoNi6IJi3A7hx+9Q1FJC0x5rRpjXc1MoGpYPfx
ptAtpnFqs3Wq8znanexMIAMLyBrVgWoAiau+qxdbAA==
=gDLV
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/17/2016 02:03 PM, Dominick Grift wrote:
> On 04/17/2016 12:12 PM, Dominick Grift wrote:
>> On 04/13/2016 08:25 PM, Dominick Grift wrote:
>>> On 04/13/2016 07:18 PM, Dominick Grift wrote:
> 
>>>>> Let me correct that:
> 
>>>>> "Even CIL-based modules can be installed via semodule and 
>>>>> managed via libsemanage, and libsemanage already contains a
>>>>> C implementation of genhomedircon."
> 
> 
>>>>> that is not my point though. My point is: since we need a 
>>>>> working genhomedircon for monolithic policy, we might cease
>>>>>  the opportunity to also support text-based module policy 
>>>>> which can be installed via semodule and managed via 
>>>>> libsemanage, but does not strictly require that .
> 
> 
>>>> Make that "seize" instead (i think)
> 
> 
>>> I just realized that i do not have to bring CIL into the
>>> equation here. We can keep it nice, simple and to the point.
>>> Refpolicy genhomedircon needs to be updated (and needs just a
>>> general review to make it work again on modern distributions)
>>> as well to make this new functionality also work with
>>> monolithic policy.
> 
>>> Also since were on the discussion of genhomedircon, I might be
>>>  wrong here, but I believe genhomedircon cannot currently deal 
>>> with %group entries in seusers.
> 
> 
> 
>> I think we should also generate file context specs for user mail 
>> spool (is currently not done AFAIK in common module policy).
> 
>> But if genhomedircon would not hardcode keywords and
>> identifiers, then i could accept them through the command line
>> options and map it to some hardcoded initial identifiers where/if
>> needed
> 
>> then one could specify on the command line the keyword, and any 
>> other identifiers. So that is a bit more flexible so that when
>> in the future we get other instances where we need to generate 
>> contexts for some location we dont have to edit the script we
>> can just pass it some additional new options. But yes then i
>> suppose it would need to be a standalone script. Added value
>> there is that we then no longer have to maintain to genhomedircon
>> instances, one for libsemanage and one for monolitic policy and
>> text-based module policy.
> 
> 
> 
> strangely the generated homedir_template does include my
> "USER_SPOOL" entries, i cant find any reference to it in the code
> (maybe its a fedora patch??)
> 
> nonetheless genhomedircon doesnt generate any contexts for me. I
> think it might not be able to deal with the namespaced user 
> identities(sys.id instead of system_u for example)
> 
> And then there is the issue that even though the genhomedircon
> seems to indicate that it supports USER and ROLE keywords. cil
> filecon statements do not allow me to specify USER and ROLE in
> filecon it seems.
> 
> I use RBACSEP (defaultrole source) so i need to be able to
> associate the appropriate roles with filecon for user file cons
> 
> All-in-all semodule/libsemanage is not an option for me. Too
> limited and too much assumptions/hard coding
> 
> 

I managed to get something semi-acceptable with this spec:

https://github.com/DefenSec/selinux-rpm-spec/blob/master/dssp-mcs-norbac
sep.spec

(its awesome how i dont need any fancy make files, just semodule and
sed to tweak some tunables if needed)

rbacsep disabled. genhomedircon now generates contexts for __default__
and since it can't deal with %group it leaves wheel users home dir
contexts as __default__. it does not replace the sys.id because it
can't deal with that, but thats not a big deal since i dont enforce
ubacsep. The mcs part also doesnt hurt.

what is a problem though is that genhomedircon can't generate contexts
for user mail spool files. So useradd and userdel might not work since
they may exit when they determine that they can't create user mail
spool files.

It is cool to see though how every module can be exported and be
replaced. Theres no concept of base. Everything is tweakable,
exportable and replaceable.

But genhomedircon should really be revisited with an open mind.

- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXE+G5AAoJECV0jlU3+Udp03gL/3TJmekH7hK+CxIPjFVT+HXp
tIEhKSEYeXasmXg5dgTctaEjIKiRHfAv/PTGVJ0lKWjPyxf5KaEDMYArfSgxVLK+
tNVJ5E3FaYLQPsXNl/4cTx+ttVo+ArGUR7o9RLzWnHmrMVMSlDM6foO6tnQntmR6
0CDBbe0Jx/HbTyWBWHf9Q6zGdoapcEhPJnkfx8Zd1rEcRWgFNBrYeaS9aWO/VPDO
b7xM0GilLSL6lG13sg1sY8HpkJwQ6jMo8nbYOhKpIkisZq+v7GprbNCrqTLHt7JD
nTWAOl6yUZGkIvP2luypDcCIGOahI8z9y32xf7E/1nxlQyEGhR4mrhFUctOsadra
//YrLcqac+ehnhWdgDkHw6vGUPmA3bEZaXl1DF1UFJX77RyUhD/00/dWYCa3WKmo
jVJZozAQkbuZZOcjuvu1mgE6hkp03Mz0/CeQDo/W9Ph5XuMN9XfbiGcvcozZ1q7W
xMBF6hRSice8cXhDz58eC1S8p5cgzKTWAMZRExAz0g==
=inQF
-----END PGP SIGNATURE-----

Re: genhomedircon USERID and USERNAME patches

[Dominick Grift]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/17/2016 09:19 PM, Dominick Grift wrote:
> On 04/17/2016 02:03 PM, Dominick Grift wrote:
>> On 04/17/2016 12:12 PM, Dominick Grift wrote:
>>> On 04/13/2016 08:25 PM, Dominick Grift wrote:
>>>> On 04/13/2016 07:18 PM, Dominick Grift wrote:
> 
>>>>>> Let me correct that:
> 
>>>>>> "Even CIL-based modules can be installed via semodule and
>>>>>>  managed via libsemanage, and libsemanage already
>>>>>> contains a C implementation of genhomedircon."
> 
> 
>>>>>> that is not my point though. My point is: since we need a
>>>>>>  working genhomedircon for monolithic policy, we might
>>>>>> cease the opportunity to also support text-based module
>>>>>> policy which can be installed via semodule and managed
>>>>>> via libsemanage, but does not strictly require that .
> 
> 
>>>>> Make that "seize" instead (i think)
> 
> 
>>>> I just realized that i do not have to bring CIL into the 
>>>> equation here. We can keep it nice, simple and to the point. 
>>>> Refpolicy genhomedircon needs to be updated (and needs just
>>>> a general review to make it work again on modern
>>>> distributions) as well to make this new functionality also
>>>> work with monolithic policy.
> 
>>>> Also since were on the discussion of genhomedircon, I might
>>>> be wrong here, but I believe genhomedircon cannot currently
>>>> deal with %group entries in seusers.
> 
> 
> 
>>> I think we should also generate file context specs for user
>>> mail spool (is currently not done AFAIK in common module
>>> policy).
> 
>>> But if genhomedircon would not hardcode keywords and 
>>> identifiers, then i could accept them through the command line 
>>> options and map it to some hardcoded initial identifiers
>>> where/if needed
> 
>>> then one could specify on the command line the keyword, and any
>>>  other identifiers. So that is a bit more flexible so that
>>> when in the future we get other instances where we need to
>>> generate contexts for some location we dont have to edit the
>>> script we can just pass it some additional new options. But yes
>>> then i suppose it would need to be a standalone script. Added
>>> value there is that we then no longer have to maintain to
>>> genhomedircon instances, one for libsemanage and one for
>>> monolitic policy and text-based module policy.
> 
> 
> 
>> strangely the generated homedir_template does include my 
>> "USER_SPOOL" entries, i cant find any reference to it in the
>> code (maybe its a fedora patch??)
> 
>> nonetheless genhomedircon doesnt generate any contexts for me. I 
>> think it might not be able to deal with the namespaced user 
>> identities(sys.id instead of system_u for example)
> 
>> And then there is the issue that even though the genhomedircon 
>> seems to indicate that it supports USER and ROLE keywords. cil 
>> filecon statements do not allow me to specify USER and ROLE in 
>> filecon it seems.
> 
>> I use RBACSEP (defaultrole source) so i need to be able to 
>> associate the appropriate roles with filecon for user file cons
> 
>> All-in-all semodule/libsemanage is not an option for me. Too 
>> limited and too much assumptions/hard coding
> 
> 
> 
> I managed to get something semi-acceptable with this spec:
> 
> https://github.com/DefenSec/selinux-rpm-spec/blob/master/dssp-mcs-norb
ac
>
> 
sep.spec
> 
> (its awesome how i dont need any fancy make files, just semodule
> and sed to tweak some tunables if needed)
> 
> rbacsep disabled. genhomedircon now generates contexts for
> __default__ and since it can't deal with %group it leaves wheel
> users home dir contexts as __default__. it does not replace the
> sys.id because it can't deal with that, but thats not a big deal
> since i dont enforce ubacsep. The mcs part also doesnt hurt.
> 
> what is a problem though is that genhomedircon can't generate
> contexts for user mail spool files. So useradd and userdel might
> not work since they may exit when they determine that they can't
> create user mail spool files.
> 
> It is cool to see though how every module can be exported and be 
> replaced. Theres no concept of base. Everything is tweakable, 
> exportable and replaceable.
> 
> But genhomedircon should really be revisited with an open mind.
> 
> 

BTW i think there is a bug in semodule man page:

# Write the HLL version of puppet and the CIL version of wireshark
# modules at priority 400 to the current working directory
$ semodule \-X 400 \-g wireshark \-\-cil \-g puppet \-\-hll


There is no -g, i think -E is meant here

- -- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQGcBAEBCAAGBQJXFH1GAAoJECV0jlU3+UdpNo8L/1GXBJF3of611my7gYh//feU
bcRf3Gt2tgBe1ld+IAapbHtfwXDvfGcUdklbJTJVZ8kkHN7eCZuTEmgxNssv68k5
uPrB+lC+4k5kHMtx3gQda2znai9+PmouBD14B5m1tqFJ8UEK/J+qA76Yogae+B/A
dBZWk5I3voo0j9W2hoVFUoVBVuywgaUe07BaSzju6VIYbbTkSJCp+GDeP1k41civ
yxe7C4I9lbha3UwTmNhI372RNOjY4jrf+q3h268GfMG/1YKzqXqR2KJDQQ/rwfYn
lQBylsnqoskJuO6sE36dbmhnSto3eKdUm47Ef97Ae1ZRi8P9lCscg/aUnLsKgN0c
andLdz15ewQEvIbBGJ8y8jpuif3fMUSIVugsIbkVYceHAIIDDRFaDDM23ekKFoSI
iemP7hVeFtxXZ+Km0D0Hm/W6ChR/GSRUf/3/XyeNq+bfFhxdTWwpt9bCbvL+iM2P
hrbAr6SElfnuCPhu+DFM/IUHc0B3+ZgtTvRPwLRK7Q==
=kDdm
-----END PGP SIGNATURE-----

genhomedircon USERID and USERNAME patches v2

[Jason Zaman]

Version 2 of the genhomedircon patches. These fix a few issues
that Nicolas Iooss found.

Changes from v1:

- Changed the fallback for the username to "[^/]+" instead of
    "[^/]*" or ".*"
- Made args const in write_replacements()
- Combined the %{USERNAME} and %{USERID} replacements into one
    method so they both get called together. This means that
    fcontexts like "%{USERNAME}-%{USERID}" become eg "root-0"
    as expected.
- I left the gid param in the struct for now. genhomedircon
    does not generate things for eg "%wheel" which I will look
    into later.

-- Jason

[PATCH v2 1/8] genhomedircon: factor out common replacement code

[Jason Zaman]

All the write_*_contexts() methods use exactly the same code.
This splits it off into a common helper function.

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 70 ++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 46 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 1a9e87e..09c2a10 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -548,23 +548,12 @@ static int check_line(genhomedircon_settings_t * s, Ustr *line)
 	return result;
 }
 
-static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
-				  semanage_list_t * tpl, const char *user,
-				  const char *seuser, const char *home,
-				  const char *role_prefix, const char *level)
+static int write_replacements(genhomedircon_settings_t * s, FILE * out,
+			      const semanage_list_t * tpl,
+			      const replacement_pair_t *repl)
 {
-	replacement_pair_t repl[] = {
-		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
-		{.search_for = TEMPLATE_HOME_DIR,.replace_with = home},
-		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
-		{.search_for = TEMPLATE_LEVEL,.replace_with = level},
-		{NULL, NULL}
-	};
 	Ustr *line = USTR_NULL;
 
-	if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user) < 0)
-		return STATUS_ERR;
-
 	for (; tpl; tpl = tpl->next) {
 		line = replace_all(tpl->data, repl);
 		if (!line)
@@ -582,6 +571,25 @@ static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
 	return STATUS_ERR;
 }
 
+static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
+				  semanage_list_t * tpl, const char *user,
+				  const char *seuser, const char *home,
+				  const char *role_prefix, const char *level)
+{
+	replacement_pair_t repl[] = {
+		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
+		{.search_for = TEMPLATE_HOME_DIR,.replace_with = home},
+		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
+		{.search_for = TEMPLATE_LEVEL,.replace_with = level},
+		{NULL, NULL}
+	};
+
+	if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user) < 0)
+		return STATUS_ERR;
+
+	return write_replacements(s, out, tpl, repl);
+}
+
 static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 				   semanage_list_t * tpl, char *homedir)
 {
@@ -589,23 +597,8 @@ static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 		{.search_for = TEMPLATE_HOME_ROOT,.replace_with = homedir},
 		{NULL, NULL}
 	};
-	Ustr *line = USTR_NULL;
-
-	for (; tpl; tpl = tpl->next) {
-		line = replace_all(tpl->data, repl);
-		if (!line)
-			goto fail;
-		if (check_line(s, line) == STATUS_SUCCESS) {
-			if (!ustr_io_putfileline(&line, out))
-				goto fail;
-		}
-		ustr_sc_free(&line);
-	}
-	return STATUS_SUCCESS;
 
-      fail:
-	ustr_sc_free(&line);
-	return STATUS_ERR;
+	return write_replacements(s, out, tpl, repl);
 }
 
 static int write_user_context(genhomedircon_settings_t * s, FILE * out,
@@ -618,23 +611,8 @@ static int write_user_context(genhomedircon_settings_t * s, FILE * out,
 		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
 		{NULL, NULL}
 	};
-	Ustr *line = USTR_NULL;
-
-	for (; tpl; tpl = tpl->next) {
-		line = replace_all(tpl->data, repl);
-		if (!line)
-			goto fail;
-		if (check_line(s, line) == STATUS_SUCCESS) {
-			if (!ustr_io_putfileline(&line, out))
-				goto fail;
-		}
-		ustr_sc_free(&line);
-	}
-	return STATUS_SUCCESS;
 
-      fail:
-	ustr_sc_free(&line);
-	return STATUS_ERR;
+	return write_replacements(s, out, tpl, repl);
 }
 
 static int user_sort_func(semanage_user_t ** arg1, semanage_user_t ** arg2)
-- 
2.7.3

[PATCH v2 2/8] genhomedircon: move fallback user to genhomedircon_user_entry_t

[Jason Zaman]

The fallback user is used in all the write functions, making it use a
struct allows us to have everything consistent between normal and
fallback users.

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 09c2a10..1e35b7e 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -83,17 +83,6 @@
 #define FALLBACK_USER_LEVEL "s0"
 #define DEFAULT_LOGIN "__default__"
 
-typedef struct {
-	const char *fcfilepath;
-	int usepasswd;
-	const char *homedir_template_path;
-	char *fallback_user;
-	char *fallback_user_prefix;
-	char *fallback_user_level;
-	semanage_handle_t *h_semanage;
-	sepol_policydb_t *policydb;
-} genhomedircon_settings_t;
-
 typedef struct user_entry {
 	char *name;
 	char *sename;
@@ -104,6 +93,15 @@ typedef struct user_entry {
 } genhomedircon_user_entry_t;
 
 typedef struct {
+	const char *fcfilepath;
+	int usepasswd;
+	const char *homedir_template_path;
+	genhomedircon_user_entry_t *fallback;
+	semanage_handle_t *h_semanage;
+	sepol_policydb_t *policydb;
+} genhomedircon_settings_t;
+
+typedef struct {
 	const char *search_for;
 	const char *replace_with;
 } replacement_pair_t;
@@ -1046,10 +1044,16 @@ int semanage_genhomedircon(semanage_handle_t * sh,
 	s.fcfilepath = semanage_final_path(SEMANAGE_FINAL_TMP,
 					   SEMANAGE_FC_HOMEDIRS);
 
-	s.fallback_user = strdup(FALLBACK_USER);
-	s.fallback_user_prefix = strdup(FALLBACK_USER_PREFIX);
-	s.fallback_user_level = strdup(FALLBACK_USER_LEVEL);
-	if (s.fallback_user == NULL || s.fallback_user_prefix == NULL || s.fallback_user_level == NULL) {
+	s.fallback = calloc(1, sizeof(genhomedircon_user_entry_t));
+	if (s.fallback == NULL) {
+		retval = STATUS_ERR;
+		goto done;
+	}
+
+	s.fallback->sename = strdup(FALLBACK_USER);
+	s.fallback->prefix = strdup(FALLBACK_USER_PREFIX);
+	s.fallback->level = strdup(FALLBACK_USER_LEVEL);
+	if (s.fallback->sename == NULL || s.fallback->prefix == NULL || s.fallback->level == NULL) {
 		retval = STATUS_ERR;
 		goto done;
 	}
@@ -1073,9 +1077,7 @@ done:
 	if (out != NULL)
 		fclose(out);
 
-	free(s.fallback_user);
-	free(s.fallback_user_prefix);
-	free(s.fallback_user_level);
+	pop_user_entry(&(s.fallback));
 	ignore_free();
 
 	return retval;
-- 
2.7.3

[PATCH v2 3/8] genhomedircon: rename FALLBACK #defines consistent with struct

[Jason Zaman]

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 1e35b7e..9529020 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -78,9 +78,10 @@
 #define TEMPLATE_SEUSER "system_u"
 #define TEMPLATE_LEVEL "s0"
 
-#define FALLBACK_USER "user_u"
-#define FALLBACK_USER_PREFIX "user"
-#define FALLBACK_USER_LEVEL "s0"
+#define FALLBACK_SENAME "user_u"
+#define FALLBACK_PREFIX "user"
+#define FALLBACK_LEVEL "s0"
+#define FALLBACK_NAME ".*"
 #define DEFAULT_LOGIN "__default__"
 
 typedef struct user_entry {
@@ -751,14 +752,14 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
 			if (semanage_user_query(s->h_semanage, key, &u) < 0)
 			{
 				prefix = name;
-				level = FALLBACK_USER_LEVEL;
+				level = FALLBACK_LEVEL;
 			}
 			else
 			{
 				prefix = semanage_user_get_prefix(u);
 				level = semanage_user_get_mlslevel(u);
 				if (!level)
-					level = FALLBACK_USER_LEVEL;
+					level = FALLBACK_LEVEL;
 			}
 
 			if (set_fallback_user(s, seuname, prefix, level) != 0)
@@ -843,10 +844,10 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 			prefix = semanage_user_get_prefix(*u);
 			level = semanage_user_get_mlslevel(*u);
 			if (!level)
-				level = FALLBACK_USER_LEVEL;
+				level = FALLBACK_LEVEL;
 		} else {
 			prefix = name;
-			level = FALLBACK_USER_LEVEL;
+			level = FALLBACK_LEVEL;
 		}
 
 		retval = getpwnam_r(name, &pwstorage, rbuf, rbuflen, &pwent);
@@ -1050,10 +1051,14 @@ int semanage_genhomedircon(semanage_handle_t * sh,
 		goto done;
 	}
 
-	s.fallback->sename = strdup(FALLBACK_USER);
-	s.fallback->prefix = strdup(FALLBACK_USER_PREFIX);
-	s.fallback->level = strdup(FALLBACK_USER_LEVEL);
-	if (s.fallback->sename == NULL || s.fallback->prefix == NULL || s.fallback->level == NULL) {
+	s.fallback->name = strdup(FALLBACK_NAME);
+	s.fallback->sename = strdup(FALLBACK_SENAME);
+	s.fallback->prefix = strdup(FALLBACK_PREFIX);
+	s.fallback->level = strdup(FALLBACK_LEVEL);
+	if (s.fallback->name == NULL
+	 || s.fallback->sename == NULL
+	 || s.fallback->prefix == NULL
+	 || s.fallback->level == NULL) {
 		retval = STATUS_ERR;
 		goto done;
 	}
-- 
2.7.3

[PATCH v2 4/8] genhomedircon: make all write context funcs take user_entry struct

[Jason Zaman]

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 86 ++++++++++++++---------------------------
 1 file changed, 30 insertions(+), 56 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 9529020..1a7882c 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -571,20 +571,23 @@ static int write_replacements(genhomedircon_settings_t * s, FILE * out,
 }
 
 static int write_home_dir_context(genhomedircon_settings_t * s, FILE * out,
-				  semanage_list_t * tpl, const char *user,
-				  const char *seuser, const char *home,
-				  const char *role_prefix, const char *level)
+				  semanage_list_t * tpl, const genhomedircon_user_entry_t *user)
 {
 	replacement_pair_t repl[] = {
-		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
-		{.search_for = TEMPLATE_HOME_DIR,.replace_with = home},
-		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
-		{.search_for = TEMPLATE_LEVEL,.replace_with = level},
+		{.search_for = TEMPLATE_SEUSER,.replace_with = user->sename},
+		{.search_for = TEMPLATE_HOME_DIR,.replace_with = user->home},
+		{.search_for = TEMPLATE_ROLE,.replace_with = user->prefix},
+		{.search_for = TEMPLATE_LEVEL,.replace_with = user->level},
 		{NULL, NULL}
 	};
 
-	if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user) < 0)
-		return STATUS_ERR;
+	if (strcmp(user->name, FALLBACK_NAME) == 0) {
+		if (fprintf(out, COMMENT_USER_HOME_CONTEXT, FALLBACK_SENAME) < 0)
+			return STATUS_ERR;
+	} else {
+		if (fprintf(out, COMMENT_USER_HOME_CONTEXT, user->name) < 0)
+			return STATUS_ERR;
+	}
 
 	return write_replacements(s, out, tpl, repl);
 }
@@ -601,13 +604,12 @@ static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 }
 
 static int write_user_context(genhomedircon_settings_t * s, FILE * out,
-			      semanage_list_t * tpl, const char *user,
-			      const char *seuser, const char *role_prefix)
+			      semanage_list_t * tpl, const genhomedircon_user_entry_t *user)
 {
 	replacement_pair_t repl[] = {
-		{.search_for = TEMPLATE_USER,.replace_with = user},
-		{.search_for = TEMPLATE_ROLE,.replace_with = role_prefix},
-		{.search_for = TEMPLATE_SEUSER,.replace_with = seuser},
+		{.search_for = TEMPLATE_USER,.replace_with = user->name},
+		{.search_for = TEMPLATE_ROLE,.replace_with = user->prefix},
+		{.search_for = TEMPLATE_SEUSER,.replace_with = user->sename},
 		{NULL, NULL}
 	};
 
@@ -692,32 +694,6 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
 	free(temp);
 }
 
-static int set_fallback_user(genhomedircon_settings_t *s, const char *user,
-			     const char *prefix, const char *level)
-{
-	char *fallback_user = strdup(user);
-	char *fallback_user_prefix = strdup(prefix);
-	char *fallback_user_level = NULL;
-	if (level) 
-		fallback_user_level = strdup(level);
-
-	if (fallback_user == NULL || fallback_user_prefix == NULL ||
-	    (fallback_user_level == NULL && level != NULL)) {
-		free(fallback_user);
-		free(fallback_user_prefix);
-		free(fallback_user_level);
-		return STATUS_ERR;
-	}
-
-	free(s->fallback_user);
-	free(s->fallback_user_prefix);
-	free(s->fallback_user_level);
-	s->fallback_user = fallback_user;
-	s->fallback_user_prefix = fallback_user_prefix;
-	s->fallback_user_level = fallback_user_level;
-	return STATUS_SUCCESS;
-}
-
 static int setup_fallback_user(genhomedircon_settings_t * s)
 {
 	semanage_seuser_t **seuser_list = NULL;
@@ -762,7 +738,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
 					level = FALLBACK_LEVEL;
 			}
 
-			if (set_fallback_user(s, seuname, prefix, level) != 0)
+			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
+					    seuname, prefix, "", level) != 0)
 				errors = STATUS_ERR;
 			semanage_user_key_free(key);
 			if (u)
@@ -823,7 +800,7 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 		seuname = semanage_seuser_get_sename(seuser_list[i]);
 		name = semanage_seuser_get_name(seuser_list[i]);
 
-		if (strcmp(name,"root") && strcmp(seuname, s->fallback_user) == 0)
+		if (strcmp(name,"root") && strcmp(seuname, s->fallback->sename) == 0)
 			continue;
 
 		if (strcmp(name, DEFAULT_LOGIN) == 0)
@@ -916,13 +893,9 @@ static int write_gen_home_dir_context(genhomedircon_settings_t * s, FILE * out,
 	}
 
 	for (; users; pop_user_entry(&users)) {
-		if (write_home_dir_context(s, out, homedir_context_tpl,
-					   users->name,
-					   users->sename, users->home,
-					   users->prefix, users->level))
+		if (write_home_dir_context(s, out, homedir_context_tpl, users))
 			goto err;
-		if (write_user_context(s, out, user_context_tpl, users->name,
-				       users->sename, users->prefix))
+		if (write_user_context(s, out, user_context_tpl, users))
 			goto err;
 	}
 
@@ -984,13 +957,13 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 				goto done;
 			}
 
-			if (write_home_dir_context(s, out,
-						   homedir_context_tpl,
-						   s->fallback_user, s->fallback_user,
-						   ustr_cstr(temp),
-						   s->fallback_user_prefix, s->fallback_user_level) !=
-			    STATUS_SUCCESS) {
+			free(s->fallback->home);
+			s->fallback->home = (char*) ustr_cstr(temp);
+
+			if (write_home_dir_context(s, out, homedir_context_tpl,
+						   s->fallback) != STATUS_SUCCESS) {
 				ustr_sc_free(&temp);
+				s->fallback->home = NULL;
 				retval = STATUS_ERR;
 				goto done;
 			}
@@ -998,17 +971,18 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 						    homeroot_context_tpl,
 						    h->data) != STATUS_SUCCESS) {
 				ustr_sc_free(&temp);
+				s->fallback->home = NULL;
 				retval = STATUS_ERR;
 				goto done;
 			}
 
 			ustr_sc_free(&temp);
+			s->fallback->home = NULL;
 		}
 	}
 	if (user_context_tpl) {
 		if (write_user_context(s, out, user_context_tpl,
-				       ".*", s->fallback_user,
-				       s->fallback_user_prefix) != STATUS_SUCCESS) {
+				       s->fallback) != STATUS_SUCCESS) {
 			retval = STATUS_ERR;
 			goto done;
 		}
-- 
2.7.3

[PATCH v2 5/8] genhomedircon: Add uid and gid to struct user_entry

[Jason Zaman]

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 1a7882c..56c58e0 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -82,10 +82,13 @@
 #define FALLBACK_PREFIX "user"
 #define FALLBACK_LEVEL "s0"
 #define FALLBACK_NAME ".*"
+#define FALLBACK_UIDGID "[0-9]+"
 #define DEFAULT_LOGIN "__default__"
 
 typedef struct user_entry {
 	char *name;
+	char *uid;
+	char *gid;
 	char *sename;
 	char *prefix;
 	char *home;
@@ -628,11 +631,13 @@ static int name_user_cmp(char *key, semanage_user_t ** val)
 }
 
 static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
-			   const char *sen, const char *pre, const char *h,
-			   const char *l)
+			   const char *u, const char *g, const char *sen,
+			   const char *pre, const char *h, const char *l)
 {
 	genhomedircon_user_entry_t *temp = NULL;
 	char *name = NULL;
+	char *uid = NULL;
+	char *gid = NULL;
 	char *sename = NULL;
 	char *prefix = NULL;
 	char *home = NULL;
@@ -644,6 +649,12 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 	name = strdup(n);
 	if (!name)
 		goto cleanup;
+	uid = strdup(u);
+	if (!uid)
+		goto cleanup;
+	gid = strdup(g);
+	if (!gid)
+		goto cleanup;
 	sename = strdup(sen);
 	if (!sename)
 		goto cleanup;
@@ -658,6 +669,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 		goto cleanup;
 
 	temp->name = name;
+	temp->uid = uid;
+	temp->gid = gid;
 	temp->sename = sename;
 	temp->prefix = prefix;
 	temp->home = home;
@@ -669,6 +682,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 
       cleanup:
 	free(name);
+	free(uid);
+	free(gid);
 	free(sename);
 	free(prefix);
 	free(home);
@@ -687,6 +702,8 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
 	temp = *list;
 	*list = temp->next;
 	free(temp->name);
+	free(temp->uid);
+	free(temp->gid);
 	free(temp->sename);
 	free(temp->prefix);
 	free(temp->home);
@@ -738,7 +755,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
 					level = FALLBACK_LEVEL;
 			}
 
-			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
+			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
+					    FALLBACK_UIDGID, FALLBACK_UIDGID,
 					    seuname, prefix, "", level) != 0)
 				errors = STATUS_ERR;
 			semanage_user_key_free(key);
@@ -768,6 +786,8 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 	const char *seuname = NULL;
 	const char *prefix = NULL;
 	const char *level = NULL;
+	char uid[10];
+	char gid[10];
 	struct passwd pwstorage, *pwent = NULL;
 	unsigned int i;
 	long rbuflen;
@@ -852,7 +872,13 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 		}
 		if (ignore(pwent->pw_dir))
 			continue;
-		if (push_user_entry(&head, name, seuname,
+
+		if (snprintf(uid, sizeof(uid), "%d", pwent->pw_uid) < 0
+		 || snprintf(gid, sizeof(gid), "%d", pwent->pw_gid) < 0) {
+			*errors = STATUS_ERR;
+			goto cleanup;
+		}
+		if (push_user_entry(&head, name, uid, gid, seuname,
 				    prefix, pwent->pw_dir, level) != STATUS_SUCCESS) {
 			*errors = STATUS_ERR;
 			break;
-- 
2.7.3

[PATCH v2 6/8] genhomedircon: make USERID, USERNAME context lists

[Jason Zaman]

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 56c58e0..e69b311 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -73,8 +73,13 @@
    which are searched for and replaced */
 #define TEMPLATE_HOME_ROOT "HOME_ROOT"
 #define TEMPLATE_HOME_DIR "HOME_DIR"
+/* these are legacy */
 #define TEMPLATE_USER "USER"
 #define TEMPLATE_ROLE "ROLE"
+/* new names */
+#define TEMPLATE_USERNAME "%{USERNAME}"
+#define TEMPLATE_USERID "%{USERID}"
+
 #define TEMPLATE_SEUSER "system_u"
 #define TEMPLATE_LEVEL "s0"
 
@@ -463,8 +468,21 @@ static int HOME_DIR_PRED(const char *string)
 	return semanage_is_prefix(string, TEMPLATE_HOME_DIR);
 }
 
+/* new names */
+static int USERNAME_CONTEXT_PRED(const char *string)
+{
+	return (int)(
+		(strstr(string, TEMPLATE_USERNAME) != NULL) ||
+		(strstr(string, TEMPLATE_USERID) != NULL)
+	);
+}
+
+/* This will never match USER if USERNAME or USERID are found. */
 static int USER_CONTEXT_PRED(const char *string)
 {
+	if (USERNAME_CONTEXT_PRED(string))
+		return 0;
+
 	return (int)(strstr(string, TEMPLATE_USER) != NULL);
 }
 
@@ -944,16 +962,21 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 {
 	semanage_list_t *homedirs = NULL;
 	semanage_list_t *h = NULL;
-	semanage_list_t *user_context_tpl = NULL;
 	semanage_list_t *homedir_context_tpl = NULL;
 	semanage_list_t *homeroot_context_tpl = NULL;
+	semanage_list_t *username_context_tpl = NULL;
+	semanage_list_t *user_context_tpl = NULL;
 	int retval = STATUS_SUCCESS;
 
 	homedir_context_tpl = make_template(s, &HOME_DIR_PRED);
 	homeroot_context_tpl = make_template(s, &HOME_ROOT_PRED);
+	username_context_tpl = make_template(s, &USERNAME_CONTEXT_PRED);
 	user_context_tpl = make_template(s, &USER_CONTEXT_PRED);
 
-	if (!homedir_context_tpl && !homeroot_context_tpl && !user_context_tpl)
+	if (!homedir_context_tpl
+	 && !homeroot_context_tpl
+	 && !username_context_tpl
+	 && !user_context_tpl)
 		goto done;
 
 	if (write_file_context_header(out) != STATUS_SUCCESS) {
@@ -1022,6 +1045,7 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 done:
 	/* Cleanup */
 	semanage_list_destroy(&homedirs);
+	semanage_list_destroy(&username_context_tpl);
 	semanage_list_destroy(&user_context_tpl);
 	semanage_list_destroy(&homedir_context_tpl);
 	semanage_list_destroy(&homeroot_context_tpl);
-- 
2.7.3

[PATCH v2 7/8] genhomedircon: write contexts for username and userid

[Jason Zaman]

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index e69b311..60a4def 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -624,6 +624,21 @@ static int write_home_root_context(genhomedircon_settings_t * s, FILE * out,
 	return write_replacements(s, out, tpl, repl);
 }
 
+static int write_username_context(genhomedircon_settings_t * s, FILE * out,
+				  semanage_list_t * tpl,
+				  const genhomedircon_user_entry_t *user)
+{
+	replacement_pair_t repl[] = {
+		{.search_for = TEMPLATE_USERNAME,.replace_with = user->name},
+		{.search_for = TEMPLATE_USERID,.replace_with = user->uid},
+		{.search_for = TEMPLATE_ROLE,.replace_with = user->prefix},
+		{.search_for = TEMPLATE_SEUSER,.replace_with = user->sename},
+		{NULL, NULL}
+	};
+
+	return write_replacements(s, out, tpl, repl);
+}
+
 static int write_user_context(genhomedircon_settings_t * s, FILE * out,
 			      semanage_list_t * tpl, const genhomedircon_user_entry_t *user)
 {
@@ -925,6 +940,7 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 }
 
 static int write_gen_home_dir_context(genhomedircon_settings_t * s, FILE * out,
+				      semanage_list_t * username_context_tpl,
 				      semanage_list_t * user_context_tpl,
 				      semanage_list_t * homedir_context_tpl)
 {
@@ -939,6 +955,8 @@ static int write_gen_home_dir_context(genhomedircon_settings_t * s, FILE * out,
 	for (; users; pop_user_entry(&users)) {
 		if (write_home_dir_context(s, out, homedir_context_tpl, users))
 			goto err;
+		if (write_username_context(s, out, username_context_tpl, users))
+			goto err;
 		if (write_user_context(s, out, user_context_tpl, users))
 			goto err;
 	}
@@ -1029,15 +1047,22 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 			s->fallback->home = NULL;
 		}
 	}
-	if (user_context_tpl) {
+	if (user_context_tpl || username_context_tpl) {
+		if (write_username_context(s, out, username_context_tpl,
+					   s->fallback) != STATUS_SUCCESS) {
+			retval = STATUS_ERR;
+			goto done;
+		}
+
 		if (write_user_context(s, out, user_context_tpl,
 				       s->fallback) != STATUS_SUCCESS) {
 			retval = STATUS_ERR;
 			goto done;
 		}
 
-		if (write_gen_home_dir_context(s, out, user_context_tpl,
-					       homedir_context_tpl) != STATUS_SUCCESS) {
+		if (write_gen_home_dir_context(s, out, username_context_tpl,
+					       user_context_tpl, homedir_context_tpl)
+				!= STATUS_SUCCESS) {
 			retval = STATUS_ERR;
 		}
 	}
-- 
2.7.3

[PATCH v2 8/8] genhomedircon: fix FALLBACK_NAME regex

[Jason Zaman]

Originally the fallback regex for the username was either ".*" or
"[^/]*". The second is more correct but still can match nothing.
changing the * to + means that it must match at least one character.

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 60a4def..fab6c30 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -86,7 +86,7 @@
 #define FALLBACK_SENAME "user_u"
 #define FALLBACK_PREFIX "user"
 #define FALLBACK_LEVEL "s0"
-#define FALLBACK_NAME ".*"
+#define FALLBACK_NAME "[^/]+"
 #define FALLBACK_UIDGID "[0-9]+"
 #define DEFAULT_LOGIN "__default__"
 
@@ -1018,7 +1018,7 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 		for (h = homedirs; h; h = h->next) {
 			Ustr *temp = ustr_dup_cstr(h->data);
 
-			if (!temp || !ustr_add_cstr(&temp, "/[^/]*")) {
+			if (!temp || !ustr_add_cstr(&temp, "/" FALLBACK_NAME)) {
 				ustr_sc_free(&temp);
 				retval = STATUS_ERR;
 				goto done;
-- 
2.7.3

Re: genhomedircon USERID and USERNAME patches v2

[Nicolas Iooss]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

On Sat, Apr 23, 2016 at 8:04 AM, Jason Zaman <jason@perfinion.com> wrote:

> Version 2 of the genhomedircon patches. These fix a few issues
> that Nicolas Iooss found.
>
> Changes from v1:
>
> - Changed the fallback for the username to "[^/]+" instead of
>     "[^/]*" or ".*"
> - Made args const in write_replacements()
> - Combined the %{USERNAME} and %{USERID} replacements into one
>     method so they both get called together. This means that
>     fcontexts like "%{USERNAME}-%{USERID}" become eg "root-0"
>     as expected.
> - I left the gid param in the struct for now. genhomedircon
>     does not generate things for eg "%wheel" which I will look
>     into later.
>

Hello,
I have read these patches and tested them on my system. They seem to work
fine: I added some corner-case file context patterns to my policy (mixing
USER, %{USERNAME}, %{USERID}, ROLE and system_u) and checked the content of
/etc/selinux/.../contexts/files/file_contexts.homedirs, the result of
"matchpathcon" (on existing and non-existing paths) and the behavior of
systemd-logind (which labels correctly the files). This set of patches
looks good to me.

Thanks,
Nicolas

[-- Attachment #2: Type: text/html, Size: 1705 bytes --]

Re: [PATCH v2 5/8] genhomedircon: Add uid and gid to struct user_entry

[Stephen Smalley]

On 04/23/2016 02:04 AM, Jason Zaman wrote:
> Signed-off-by: Jason Zaman <jason@perfinion.com>
> ---
>  libsemanage/src/genhomedircon.c | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
> index 1a7882c..56c58e0 100644
> --- a/libsemanage/src/genhomedircon.c
> +++ b/libsemanage/src/genhomedircon.c
> @@ -82,10 +82,13 @@
>  #define FALLBACK_PREFIX "user"
>  #define FALLBACK_LEVEL "s0"
>  #define FALLBACK_NAME ".*"
> +#define FALLBACK_UIDGID "[0-9]+"
>  #define DEFAULT_LOGIN "__default__"
>  
>  typedef struct user_entry {
>  	char *name;
> +	char *uid;
> +	char *gid;
>  	char *sename;
>  	char *prefix;
>  	char *home;
> @@ -628,11 +631,13 @@ static int name_user_cmp(char *key, semanage_user_t ** val)
>  }
>  
>  static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
> -			   const char *sen, const char *pre, const char *h,
> -			   const char *l)
> +			   const char *u, const char *g, const char *sen,
> +			   const char *pre, const char *h, const char *l)
>  {
>  	genhomedircon_user_entry_t *temp = NULL;
>  	char *name = NULL;
> +	char *uid = NULL;
> +	char *gid = NULL;
>  	char *sename = NULL;
>  	char *prefix = NULL;
>  	char *home = NULL;
> @@ -644,6 +649,12 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
>  	name = strdup(n);
>  	if (!name)
>  		goto cleanup;
> +	uid = strdup(u);
> +	if (!uid)
> +		goto cleanup;
> +	gid = strdup(g);
> +	if (!gid)
> +		goto cleanup;
>  	sename = strdup(sen);
>  	if (!sename)
>  		goto cleanup;
> @@ -658,6 +669,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
>  		goto cleanup;
>  
>  	temp->name = name;
> +	temp->uid = uid;
> +	temp->gid = gid;
>  	temp->sename = sename;
>  	temp->prefix = prefix;
>  	temp->home = home;
> @@ -669,6 +682,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
>  
>        cleanup:
>  	free(name);
> +	free(uid);
> +	free(gid);
>  	free(sename);
>  	free(prefix);
>  	free(home);
> @@ -687,6 +702,8 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
>  	temp = *list;
>  	*list = temp->next;
>  	free(temp->name);
> +	free(temp->uid);
> +	free(temp->gid);
>  	free(temp->sename);
>  	free(temp->prefix);
>  	free(temp->home);
> @@ -738,7 +755,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>  					level = FALLBACK_LEVEL;
>  			}
>  
> -			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
> +			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
> +					    FALLBACK_UIDGID, FALLBACK_UIDGID,
>  					    seuname, prefix, "", level) != 0)
>  				errors = STATUS_ERR;
>  			semanage_user_key_free(key);
> @@ -768,6 +786,8 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
>  	const char *seuname = NULL;
>  	const char *prefix = NULL;
>  	const char *level = NULL;
> +	char uid[10];
> +	char gid[10];

You need to allow space for the NUL terminator.

>  	struct passwd pwstorage, *pwent = NULL;
>  	unsigned int i;
>  	long rbuflen;
> @@ -852,7 +872,13 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
>  		}
>  		if (ignore(pwent->pw_dir))
>  			continue;
> -		if (push_user_entry(&head, name, seuname,
> +
> +		if (snprintf(uid, sizeof(uid), "%d", pwent->pw_uid) < 0
> +		 || snprintf(gid, sizeof(gid), "%d", pwent->pw_gid) < 0) {

Should you be using %u instead of %d?
Also, snprintf returns >= size if the output was truncated, not < 0.

> +			*errors = STATUS_ERR;
> +			goto cleanup;
> +		}
> +		if (push_user_entry(&head, name, uid, gid, seuname,
>  				    prefix, pwent->pw_dir, level) != STATUS_SUCCESS) {
>  			*errors = STATUS_ERR;
>  			break;
> 

Re: [PATCH v2 5/8] genhomedircon: Add uid and gid to struct user_entry

[Jason Zaman]

On Wed, Apr 27, 2016 at 01:04:25PM -0400, Stephen Smalley wrote:
> On 04/23/2016 02:04 AM, Jason Zaman wrote:
> > Signed-off-by: Jason Zaman <jason@perfinion.com>
> > ---
> >  libsemanage/src/genhomedircon.c | 34 ++++++++++++++++++++++++++++++----
> >  1 file changed, 30 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
> > index 1a7882c..56c58e0 100644
> > --- a/libsemanage/src/genhomedircon.c
> > +++ b/libsemanage/src/genhomedircon.c
> > @@ -82,10 +82,13 @@
> >  #define FALLBACK_PREFIX "user"
> >  #define FALLBACK_LEVEL "s0"
> >  #define FALLBACK_NAME ".*"
> > +#define FALLBACK_UIDGID "[0-9]+"
> >  #define DEFAULT_LOGIN "__default__"
> >  
> >  typedef struct user_entry {
> >  	char *name;
> > +	char *uid;
> > +	char *gid;
> >  	char *sename;
> >  	char *prefix;
> >  	char *home;
> > @@ -628,11 +631,13 @@ static int name_user_cmp(char *key, semanage_user_t ** val)
> >  }
> >  
> >  static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
> > -			   const char *sen, const char *pre, const char *h,
> > -			   const char *l)
> > +			   const char *u, const char *g, const char *sen,
> > +			   const char *pre, const char *h, const char *l)
> >  {
> >  	genhomedircon_user_entry_t *temp = NULL;
> >  	char *name = NULL;
> > +	char *uid = NULL;
> > +	char *gid = NULL;
> >  	char *sename = NULL;
> >  	char *prefix = NULL;
> >  	char *home = NULL;
> > @@ -644,6 +649,12 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
> >  	name = strdup(n);
> >  	if (!name)
> >  		goto cleanup;
> > +	uid = strdup(u);
> > +	if (!uid)
> > +		goto cleanup;
> > +	gid = strdup(g);
> > +	if (!gid)
> > +		goto cleanup;
> >  	sename = strdup(sen);
> >  	if (!sename)
> >  		goto cleanup;
> > @@ -658,6 +669,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
> >  		goto cleanup;
> >  
> >  	temp->name = name;
> > +	temp->uid = uid;
> > +	temp->gid = gid;
> >  	temp->sename = sename;
> >  	temp->prefix = prefix;
> >  	temp->home = home;
> > @@ -669,6 +682,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
> >  
> >        cleanup:
> >  	free(name);
> > +	free(uid);
> > +	free(gid);
> >  	free(sename);
> >  	free(prefix);
> >  	free(home);
> > @@ -687,6 +702,8 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
> >  	temp = *list;
> >  	*list = temp->next;
> >  	free(temp->name);
> > +	free(temp->uid);
> > +	free(temp->gid);
> >  	free(temp->sename);
> >  	free(temp->prefix);
> >  	free(temp->home);
> > @@ -738,7 +755,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
> >  					level = FALLBACK_LEVEL;
> >  			}
> >  
> > -			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
> > +			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
> > +					    FALLBACK_UIDGID, FALLBACK_UIDGID,
> >  					    seuname, prefix, "", level) != 0)
> >  				errors = STATUS_ERR;
> >  			semanage_user_key_free(key);
> > @@ -768,6 +786,8 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
> >  	const char *seuname = NULL;
> >  	const char *prefix = NULL;
> >  	const char *level = NULL;
> > +	char uid[10];
> > +	char gid[10];
> 
> You need to allow space for the NUL terminator.

2^32 = 4294967296 so 10 digits + null. i'll send an updated patch.
> 
> >  	struct passwd pwstorage, *pwent = NULL;
> >  	unsigned int i;
> >  	long rbuflen;
> > @@ -852,7 +872,13 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
> >  		}
> >  		if (ignore(pwent->pw_dir))
> >  			continue;
> > -		if (push_user_entry(&head, name, seuname,
> > +
> > +		if (snprintf(uid, sizeof(uid), "%d", pwent->pw_uid) < 0
> > +		 || snprintf(gid, sizeof(gid), "%d", pwent->pw_gid) < 0) {
> 
> Should you be using %u instead of %d?
yes, its unsigned, will fix.

> Also, snprintf returns >= size if the output was truncated, not < 0.

>From the man page:
RETURN VALUE
[...] Thus, a return value of size or more means that the output was truncated.
If an output error is encountered, a negative value is returned.

I definitely need to check <0. but do I *also* need to check >= size? I
dont think that can ever happen since 10chars+NULL fits fine.

-- Jason

> > +			*errors = STATUS_ERR;
> > +			goto cleanup;
> > +		}
> > +		if (push_user_entry(&head, name, uid, gid, seuname,
> >  				    prefix, pwent->pw_dir, level) != STATUS_SUCCESS) {
> >  			*errors = STATUS_ERR;
> >  			break;

Re: [PATCH v2 5/8] genhomedircon: Add uid and gid to struct user_entry

[Stephen Smalley]

On 04/28/2016 01:53 PM, Jason Zaman wrote:
> On Wed, Apr 27, 2016 at 01:04:25PM -0400, Stephen Smalley wrote:
>> On 04/23/2016 02:04 AM, Jason Zaman wrote:
>>> Signed-off-by: Jason Zaman <jason@perfinion.com>
>>> ---
>>>  libsemanage/src/genhomedircon.c | 34 ++++++++++++++++++++++++++++++----
>>>  1 file changed, 30 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
>>> index 1a7882c..56c58e0 100644
>>> --- a/libsemanage/src/genhomedircon.c
>>> +++ b/libsemanage/src/genhomedircon.c
>>> @@ -82,10 +82,13 @@
>>>  #define FALLBACK_PREFIX "user"
>>>  #define FALLBACK_LEVEL "s0"
>>>  #define FALLBACK_NAME ".*"
>>> +#define FALLBACK_UIDGID "[0-9]+"
>>>  #define DEFAULT_LOGIN "__default__"
>>>  
>>>  typedef struct user_entry {
>>>  	char *name;
>>> +	char *uid;
>>> +	char *gid;
>>>  	char *sename;
>>>  	char *prefix;
>>>  	char *home;
>>> @@ -628,11 +631,13 @@ static int name_user_cmp(char *key, semanage_user_t ** val)
>>>  }
>>>  
>>>  static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
>>> -			   const char *sen, const char *pre, const char *h,
>>> -			   const char *l)
>>> +			   const char *u, const char *g, const char *sen,
>>> +			   const char *pre, const char *h, const char *l)
>>>  {
>>>  	genhomedircon_user_entry_t *temp = NULL;
>>>  	char *name = NULL;
>>> +	char *uid = NULL;
>>> +	char *gid = NULL;
>>>  	char *sename = NULL;
>>>  	char *prefix = NULL;
>>>  	char *home = NULL;
>>> @@ -644,6 +649,12 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
>>>  	name = strdup(n);
>>>  	if (!name)
>>>  		goto cleanup;
>>> +	uid = strdup(u);
>>> +	if (!uid)
>>> +		goto cleanup;
>>> +	gid = strdup(g);
>>> +	if (!gid)
>>> +		goto cleanup;
>>>  	sename = strdup(sen);
>>>  	if (!sename)
>>>  		goto cleanup;
>>> @@ -658,6 +669,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
>>>  		goto cleanup;
>>>  
>>>  	temp->name = name;
>>> +	temp->uid = uid;
>>> +	temp->gid = gid;
>>>  	temp->sename = sename;
>>>  	temp->prefix = prefix;
>>>  	temp->home = home;
>>> @@ -669,6 +682,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
>>>  
>>>        cleanup:
>>>  	free(name);
>>> +	free(uid);
>>> +	free(gid);
>>>  	free(sename);
>>>  	free(prefix);
>>>  	free(home);
>>> @@ -687,6 +702,8 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
>>>  	temp = *list;
>>>  	*list = temp->next;
>>>  	free(temp->name);
>>> +	free(temp->uid);
>>> +	free(temp->gid);
>>>  	free(temp->sename);
>>>  	free(temp->prefix);
>>>  	free(temp->home);
>>> @@ -738,7 +755,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
>>>  					level = FALLBACK_LEVEL;
>>>  			}
>>>  
>>> -			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
>>> +			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
>>> +					    FALLBACK_UIDGID, FALLBACK_UIDGID,
>>>  					    seuname, prefix, "", level) != 0)
>>>  				errors = STATUS_ERR;
>>>  			semanage_user_key_free(key);
>>> @@ -768,6 +786,8 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
>>>  	const char *seuname = NULL;
>>>  	const char *prefix = NULL;
>>>  	const char *level = NULL;
>>> +	char uid[10];
>>> +	char gid[10];
>>
>> You need to allow space for the NUL terminator.
> 
> 2^32 = 4294967296 so 10 digits + null. i'll send an updated patch.
>>
>>>  	struct passwd pwstorage, *pwent = NULL;
>>>  	unsigned int i;
>>>  	long rbuflen;
>>> @@ -852,7 +872,13 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
>>>  		}
>>>  		if (ignore(pwent->pw_dir))
>>>  			continue;
>>> -		if (push_user_entry(&head, name, seuname,
>>> +
>>> +		if (snprintf(uid, sizeof(uid), "%d", pwent->pw_uid) < 0
>>> +		 || snprintf(gid, sizeof(gid), "%d", pwent->pw_gid) < 0) {
>>
>> Should you be using %u instead of %d?
> yes, its unsigned, will fix.
> 
>> Also, snprintf returns >= size if the output was truncated, not < 0.
> 
>>From the man page:
> RETURN VALUE
> [...] Thus, a return value of size or more means that the output was truncated.
> If an output error is encountered, a negative value is returned.
> 
> I definitely need to check <0. but do I *also* need to check >= size? I
> dont think that can ever happen since 10chars+NULL fits fine.

I don't think either case is actually possible here (< 0 should only
occur with printf or fprintf variants, not s*printf, and as you note,
the truncation case should be covered).

Re: [PATCH v2 5/8] genhomedircon: Add uid and gid to struct user_entry

[Jason Zaman]

On Thu, Apr 28, 2016 at 02:13:30PM -0400, Stephen Smalley wrote:
> On 04/28/2016 01:53 PM, Jason Zaman wrote:
> > On Wed, Apr 27, 2016 at 01:04:25PM -0400, Stephen Smalley wrote:
> >> On 04/23/2016 02:04 AM, Jason Zaman wrote:
> >>> +		if (snprintf(uid, sizeof(uid), "%d", pwent->pw_uid) < 0
> >>> +		 || snprintf(gid, sizeof(gid), "%d", pwent->pw_gid) < 0) {
> >>
> >> Should you be using %u instead of %d?
> > yes, its unsigned, will fix.
> > 
> >> Also, snprintf returns >= size if the output was truncated, not < 0.
> > 
> >>From the man page:
> > RETURN VALUE
> > [...] Thus, a return value of size or more means that the output was truncated.
> > If an output error is encountered, a negative value is returned.
> > 
> > I definitely need to check <0. but do I *also* need to check >= size? I
> > dont think that can ever happen since 10chars+NULL fits fine.
> 
> I don't think either case is actually possible here (< 0 should only
> occur with printf or fprintf variants, not s*printf, and as you note,
> the truncation case should be covered).

So I think this is correct but i noticed a few more things in the man
page so I am just going to be cautious and check them all anyway.

1) glibc changed bahaviour:
"The glibc implementation of the functions snprintf() and vsnprintf()
conforms to the C99 standard, that is, behaves as described above, since
glibc  version  2.1.   Until  glibc 2.0.6, they would return -1 when the
output was truncated."

2) it looks like there might possibly be locale issues for some of the
stranger ones? i dont think it'd be an issue but having the check doesnt
exactly harm anything since genhomedircon is only run once when building
a policy. This also raises the issue of if there are locale issues
should semodule and friends be checking/resetting LANG/LC_NUMERIC for
sanity early on?

I'm going to send v3 of this patch with these fixes. Do you want me to
re-send the whole set or is just this one enough?

-- Jason

[PATCH v3 5/8] genhomedircon: Add uid and gid to struct user_entry

[Jason Zaman]

Signed-off-by: Jason Zaman <jason@perfinion.com>
---
 libsemanage/src/genhomedircon.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 1a7882c..41fd7ff 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -82,10 +82,13 @@
 #define FALLBACK_PREFIX "user"
 #define FALLBACK_LEVEL "s0"
 #define FALLBACK_NAME ".*"
+#define FALLBACK_UIDGID "[0-9]+"
 #define DEFAULT_LOGIN "__default__"
 
 typedef struct user_entry {
 	char *name;
+	char *uid;
+	char *gid;
 	char *sename;
 	char *prefix;
 	char *home;
@@ -628,11 +631,13 @@ static int name_user_cmp(char *key, semanage_user_t ** val)
 }
 
 static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
-			   const char *sen, const char *pre, const char *h,
-			   const char *l)
+			   const char *u, const char *g, const char *sen,
+			   const char *pre, const char *h, const char *l)
 {
 	genhomedircon_user_entry_t *temp = NULL;
 	char *name = NULL;
+	char *uid = NULL;
+	char *gid = NULL;
 	char *sename = NULL;
 	char *prefix = NULL;
 	char *home = NULL;
@@ -644,6 +649,12 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 	name = strdup(n);
 	if (!name)
 		goto cleanup;
+	uid = strdup(u);
+	if (!uid)
+		goto cleanup;
+	gid = strdup(g);
+	if (!gid)
+		goto cleanup;
 	sename = strdup(sen);
 	if (!sename)
 		goto cleanup;
@@ -658,6 +669,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 		goto cleanup;
 
 	temp->name = name;
+	temp->uid = uid;
+	temp->gid = gid;
 	temp->sename = sename;
 	temp->prefix = prefix;
 	temp->home = home;
@@ -669,6 +682,8 @@ static int push_user_entry(genhomedircon_user_entry_t ** list, const char *n,
 
       cleanup:
 	free(name);
+	free(uid);
+	free(gid);
 	free(sename);
 	free(prefix);
 	free(home);
@@ -687,6 +702,8 @@ static void pop_user_entry(genhomedircon_user_entry_t ** list)
 	temp = *list;
 	*list = temp->next;
 	free(temp->name);
+	free(temp->uid);
+	free(temp->gid);
 	free(temp->sename);
 	free(temp->prefix);
 	free(temp->home);
@@ -738,7 +755,8 @@ static int setup_fallback_user(genhomedircon_settings_t * s)
 					level = FALLBACK_LEVEL;
 			}
 
-			if (push_user_entry(&(s->fallback), FALLBACK_NAME, 0, 0,
+			if (push_user_entry(&(s->fallback), FALLBACK_NAME,
+					    FALLBACK_UIDGID, FALLBACK_UIDGID,
 					    seuname, prefix, "", level) != 0)
 				errors = STATUS_ERR;
 			semanage_user_key_free(key);
@@ -768,6 +786,8 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 	const char *seuname = NULL;
 	const char *prefix = NULL;
 	const char *level = NULL;
+	char uid[11];
+	char gid[11];
 	struct passwd pwstorage, *pwent = NULL;
 	unsigned int i;
 	long rbuflen;
@@ -852,7 +872,19 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
 		}
 		if (ignore(pwent->pw_dir))
 			continue;
-		if (push_user_entry(&head, name, seuname,
+
+		len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid);
+		if (len < 0 || len >= sizeof(uid)) {
+			*errors = STATUS_ERR;
+			goto cleanup;
+		}
+		len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid);
+		if (len < 0 || len >= sizeof(gid)) {
+			*errors = STATUS_ERR;
+			goto cleanup;
+		}
+
+		if (push_user_entry(&head, name, uid, gid, seuname,
 				    prefix, pwent->pw_dir, level) != STATUS_SUCCESS) {
 			*errors = STATUS_ERR;
 			break;
-- 
2.7.3

Re: [PATCH v2 2/8] genhomedircon: move fallback user to genhomedircon_user_entry_t

[Stephen Smalley]

On 04/23/2016 02:04 AM, Jason Zaman wrote:
> The fallback user is used in all the write functions, making it use a
> struct allows us to have everything consistent between normal and
> fallback users.
> 
> Signed-off-by: Jason Zaman <jason@perfinion.com>

When you split a patch into a series, the goal is to ensure that the
code remains in a working state after each patch in the series.
Otherwise, git bisect will often find broken states in the future.
So, we have two options:
1. You can refactor this patch set so that it does compile after each
patch (currently breaks on this one at least).
2. I can squash them all into one logical change.

> ---
>  libsemanage/src/genhomedircon.c | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
> index 09c2a10..1e35b7e 100644
> --- a/libsemanage/src/genhomedircon.c
> +++ b/libsemanage/src/genhomedircon.c
> @@ -83,17 +83,6 @@
>  #define FALLBACK_USER_LEVEL "s0"
>  #define DEFAULT_LOGIN "__default__"
>  
> -typedef struct {
> -	const char *fcfilepath;
> -	int usepasswd;
> -	const char *homedir_template_path;
> -	char *fallback_user;
> -	char *fallback_user_prefix;
> -	char *fallback_user_level;
> -	semanage_handle_t *h_semanage;
> -	sepol_policydb_t *policydb;
> -} genhomedircon_settings_t;
> -
>  typedef struct user_entry {
>  	char *name;
>  	char *sename;
> @@ -104,6 +93,15 @@ typedef struct user_entry {
>  } genhomedircon_user_entry_t;
>  
>  typedef struct {
> +	const char *fcfilepath;
> +	int usepasswd;
> +	const char *homedir_template_path;
> +	genhomedircon_user_entry_t *fallback;
> +	semanage_handle_t *h_semanage;
> +	sepol_policydb_t *policydb;
> +} genhomedircon_settings_t;
> +
> +typedef struct {
>  	const char *search_for;
>  	const char *replace_with;
>  } replacement_pair_t;
> @@ -1046,10 +1044,16 @@ int semanage_genhomedircon(semanage_handle_t * sh,
>  	s.fcfilepath = semanage_final_path(SEMANAGE_FINAL_TMP,
>  					   SEMANAGE_FC_HOMEDIRS);
>  
> -	s.fallback_user = strdup(FALLBACK_USER);
> -	s.fallback_user_prefix = strdup(FALLBACK_USER_PREFIX);
> -	s.fallback_user_level = strdup(FALLBACK_USER_LEVEL);
> -	if (s.fallback_user == NULL || s.fallback_user_prefix == NULL || s.fallback_user_level == NULL) {
> +	s.fallback = calloc(1, sizeof(genhomedircon_user_entry_t));
> +	if (s.fallback == NULL) {
> +		retval = STATUS_ERR;
> +		goto done;
> +	}
> +
> +	s.fallback->sename = strdup(FALLBACK_USER);
> +	s.fallback->prefix = strdup(FALLBACK_USER_PREFIX);
> +	s.fallback->level = strdup(FALLBACK_USER_LEVEL);
> +	if (s.fallback->sename == NULL || s.fallback->prefix == NULL || s.fallback->level == NULL) {
>  		retval = STATUS_ERR;
>  		goto done;
>  	}
> @@ -1073,9 +1077,7 @@ done:
>  	if (out != NULL)
>  		fclose(out);
>  
> -	free(s.fallback_user);
> -	free(s.fallback_user_prefix);
> -	free(s.fallback_user_level);
> +	pop_user_entry(&(s.fallback));
>  	ignore_free();
>  
>  	return retval;
> 

Re: [PATCH v2 2/8] genhomedircon: move fallback user to genhomedircon_user_entry_t

[Jason Zaman]

On Fri, Apr 29, 2016 at 12:54:44PM -0400, Stephen Smalley wrote:
> On 04/23/2016 02:04 AM, Jason Zaman wrote:
> > The fallback user is used in all the write functions, making it use a
> > struct allows us to have everything consistent between normal and
> > fallback users.
> > 
> > Signed-off-by: Jason Zaman <jason@perfinion.com>
> 
> When you split a patch into a series, the goal is to ensure that the
> code remains in a working state after each patch in the series.
> Otherwise, git bisect will often find broken states in the future.
> So, we have two options:
> 1. You can refactor this patch set so that it does compile after each
> patch (currently breaks on this one at least).
> 2. I can squash them all into one logical change.

The best is squashing 2-4 into one then. The others are pretty self
contained. 2-4 was a really big change so I split it up even tho it
broke the build but if you'd prefer squashing them thats not a problem.

-- Jason

Re: [PATCH v3 5/8] genhomedircon: Add uid and gid to struct user_entry

[Stephen Smalley]

On 04/29/2016 08:04 AM, Jason Zaman wrote:
> Signed-off-by: Jason Zaman <jason@perfinion.com>
> ---
>  libsemanage/src/genhomedircon.c | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
> index 1a7882c..41fd7ff 100644
> --- a/libsemanage/src/genhomedircon.c
> +++ b/libsemanage/src/genhomedircon.c
> @@ -852,7 +872,19 @@ static genhomedircon_user_entry_t *get_users(genhomedircon_settings_t * s,
>  		}
>  		if (ignore(pwent->pw_dir))
>  			continue;
> -		if (push_user_entry(&head, name, seuname,
> +
> +		len = snprintf(uid, sizeof(uid), "%u", pwent->pw_uid);
> +		if (len < 0 || len >= sizeof(uid)) {
> +			*errors = STATUS_ERR;
> +			goto cleanup;
> +		}
> +		len = snprintf(gid, sizeof(gid), "%u", pwent->pw_gid);
> +		if (len < 0 || len >= sizeof(gid)) {
> +			*errors = STATUS_ERR;
> +			goto cleanup;
> +		}
> +
> +		if (push_user_entry(&head, name, uid, gid, seuname,
>  				    prefix, pwent->pw_dir, level) != STATUS_SUCCESS) {
>  			*errors = STATUS_ERR;
>  			break;
> 

$ make DESTDIR=~/obj clean install > make.out
genhomedircon.c: In function ‘get_users’:
genhomedircon.c:910:22: error: comparison between signed and unsigned
integer expressions [-Werror=sign-compare]
   if (len < 0 || len >= sizeof(uid)) {
                      ^
genhomedircon.c:915:22: error: comparison between signed and unsigned
integer expressions [-Werror=sign-compare]
   if (len < 0 || len >= sizeof(gid)) {
                      ^
cc1: all warnings being treated as errors
make[2]: *** [genhomedircon.o] Error 1
make[1]: *** [install] Error 2
make: *** [install] Error 1

Re: [PATCH v2 2/8] genhomedircon: move fallback user to genhomedircon_user_entry_t

[Stephen Smalley]

On 04/29/2016 03:23 PM, Jason Zaman wrote:
> On Fri, Apr 29, 2016 at 12:54:44PM -0400, Stephen Smalley wrote:
>> On 04/23/2016 02:04 AM, Jason Zaman wrote:
>>> The fallback user is used in all the write functions, making it use a
>>> struct allows us to have everything consistent between normal and
>>> fallback users.
>>>
>>> Signed-off-by: Jason Zaman <jason@perfinion.com>
>>
>> When you split a patch into a series, the goal is to ensure that the
>> code remains in a working state after each patch in the series.
>> Otherwise, git bisect will often find broken states in the future.
>> So, we have two options:
>> 1. You can refactor this patch set so that it does compile after each
>> patch (currently breaks on this one at least).
>> 2. I can squash them all into one logical change.
> 
> The best is squashing 2-4 into one then. The others are pretty self
> contained. 2-4 was a really big change so I split it up even tho it
> broke the build but if you'd prefer squashing them thats not a problem.

I have to squash 2-5 to keep it buildable at each step.