Re: genhomedircon USERID and USERNAME patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 8, 2016 at 6:05 PM, Jason Zaman <jason@xxxxxxxxxxxxx> 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
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.

[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux