No description
48412c3930
Email: guido@trentalancia.com Subject: Contributed manual pages for libselinux Date: Sat, 21 Nov 2009 20:51:17 +0100 Hello Eamon ! On Fri, 2009-11-20 at 21:42 -0500, Eamon Walsh wrote: > Hi, thanks for doing this. Some quick review below. You are welcome, I suppose it was a boring task for many... Thanks very much for reviewing the changes. And please accept my apologies for not placing "[PATCH]" in the subject of the original post. I had just subscribed to the list. I left you cc address intact here... > There is too much in matchpathcon(3) now. It's going to need to be > split up into different pages, perhaps the init/fini/teardown stuff in > one page, the lookup calls in another, and the non-matchpathcon prefixed > calls in a third page. > > Also, .so manpage links are needed for all the calls here. Yes, matchpathcon is a mess. Following your guidelines, I have now splitted the huge and messy page in several different man pages. It's easier to consult and easier to maintain. The first part (page) is strictly related to _init, its variant _init_index, _fini, matchpathcon and its variant matchpathcon_index. Nice and concise. References are provided in the "SEE ALSO" section to the rest. The second page describes the auxiliary lookup calls (matchpathcon_checkmatches) and the inode associations functions (matchpathcon_filespec_{add,destroy,eval}). The reference section points to the main matchpathcon page. A third page has been created for the functions that are used to set the flags (set_matchpathcon_flags) or to configure the behaviour of the main matchpathcon functions (set_matchpathcon_invalidcon and set_matchpathcon_printf). A fourth and fifth page is devoted to functions that should never had ended up in matchpathcon (selinux_file_context_cmp and selinux_file_context_verify in one page and selinux_lsetfilecon_default in another one): we do not really need to save electrons needed for new pages... > > > > * print_access_vector > > > > Looks good. No modifications. > > * security_disable > > > > See the selinux.h comments for this. It needs to be documented that > this function can only be called at startup time. Ok. I have stressed that now and also mentioned that after the policy has been loaded at startup, then only "setenforce" can be used to alter (not disable) the mode of the SELinux kernel code (for example by placing it into "permissive" mode). > > * security_set_boolean_list > > > > a RETURN VALUE section is needed in this page, documenting at least this > call if not the others in that page. I have now added a "RETURN VALUE" section. Also, to avoid confusion, I have rephrased the word "returns" in "provides" when not strictly referring the to the return value of the function (take for example security_get_boolean_names(), strictly speaking the function returns an integer representing 0=success or -1=failure, although from a conceptual point of view it also returns a list trough modification of one of its parameters passed by reference). Usually when an application developer looks at the "RETURN VALUE" section it is because he/she has already planned/coded the call to the function (and thus also the handling to parameters passed by reference) and only needs to check for the function exit status so that it can be handled properly at the call point. > > * selinux_check_passwd_access > > > > This is a replacement for the inconsistently named "checkPasswdAccess" > function. So, the existing description of checkPasswdAccess should be > moved to this function, and checkPasswdAccess should be changed to "this > is a deprecated alias for selinux_check_passwd_access". Yes, I have now mentioned that checkPasswdAccess is deprecated. We are referring to file security_compute_av.3 as the description of these two functions lives there... By the way, it has been pointed out that this function should not hard-code a string. I also agree with him, there is a generic constant for such "passwd" object class, it is defined in flask.h could be used instead of the string, thus avoiding hard-coding and also allowing to save a few cycles and be theoretically future-proof (if ever the name would change, say to "password", "auth-token" or anything else). libselinux/src/checkAccess.c.orig 2009-11-21 20:07:21.000000000 libselinux/src/checkAccess.c 2009-11-21 20:08:36.000000000 @@ -13,17 +13,12 @@ int selinux_check_passwd_access(access_v if (is_selinux_enabled() == 0) return 0; if (getprevcon_raw(&user_context) == 0) { - security_class_t passwd_class; struct av_decision avd; int retval; - passwd_class = string_to_security_class("passwd"); - if (passwd_class == 0) - return 0; - retval = security_compute_av_raw(user_context, user_context, - passwd_class, + SECCLASS_PASSWD, requested, &avd); Note that the above code, should really live in the application and not in the selinux library. It used to be like that, then for some reason it has been introduced. Redhat's passwd and cronie are calling the library function and thus at the moment they rely on it. But for example, util-linux-ng has the code in it and does not call this function, as I believe it should be. A very minor issue anyway... > > * selinux_init_load_policy > > > > A paragraph break is needed in the DESCRIPTION section before this function. Done. I have also added a note to the already mentioned fact that after initial policy load, SELinux cannot be anymore disabled using calls to security_disable(3). > > * selinux_lsetfilecon_default > > > > See notes above about the matchpathcon manpage. Yes, separate man page now. > > * selinux_mkload_policy > > > > Looks good. No modifications. > > * set_selinuxmnt > > > > This manpage includes two static functions that are not part of the > libselinux API (at least, not anymore) and should be removed. > > Also, I'm not comfortable with the description given. Instead, use the > comments in selinux.h, which are more accurate and verbose. > Please let me know if things are any better now. I did also provide on the same day a patch for beautifying and improving the command-line option parsing of a few utilities (a ticket had been created by somebody). That patch provides those improvement according to GNU-style parsing of "help" and "version" options (including long-option variants). I think it also fixes a couple of typos here and there. Feel free to include that patch too if you like it, so that the ticket can be closed ! I will attach it again in another separate message: it has been slightly modified in order to apply cleanly to the latest git snapshot. More important, I was also thinking about fingerprinting (and subsequently checking) the libraries with some cryptographic hash function such as the NIST-recommended SHA2. It is beginning to be done for security-related projects like OpenSSL, so I believe it is even more essential for SELinux. Ever thought about anything like that ? Best regards, Guido Signed-off-By: Joshua Brindle <method@manicmethod.com> |
||
|---|---|---|
| checkpolicy | ||
| libselinux | ||
| libsemanage | ||
| libsepol | ||
| policycoreutils | ||
| scripts | ||
| sepolgen | ||
| .gitignore | ||
| Makefile | ||