[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [busybox:00323] Re: [PATCH 4/8] busybox -- libselinux utilities
From: Bernhard Fischer <rep.dot.nop () gmail ! com>
Date: 2007-01-31 14:04:19
Message-ID: 20070131140419.GA11967 () aon ! at
[Download RAW message or body]
On Wed, Jan 31, 2007 at 09:13:47PM +0900, KaiGai Kohei wrote:
> Bernhard, Thanks for your comments.
>
> The attached patch fixes following items:
> - avcstat and togglesebool applet were removed
> - xis_selinux_enabled() was added at libbb/xfuncs.c
> - unneccesary headers were removed.
> - bb_error_msg_and_die() + strerror() were replaced
> with bb_perror_msg_and_die()
> - "Selinux Utilities" at menuconfig got dependency with CONFIG_SELINUX
> - some cleanups.
>
[snip]
> Index: sebusybox-libselinux-0131/libbb/xfuncs.c
> ===================================================================
> --- sebusybox-libselinux-0131/libbb/xfuncs.c (revision 17684)
> +++ sebusybox-libselinux-0131/libbb/xfuncs.c (working copy)
> @@ -574,6 +574,17 @@
> bb_perror_msg_and_die("can't stat '%s'", name);
> }
>
> +// xis_selinux_enabled() - die if SELinux is disabled.
> +void xis_selinux_enabled(void)
> +{
> +#ifdef CONFIG_SELINUX
Please rather use #if ENABLE_SELINUX
> + if (!is_selinux_enabled())
> + bb_error_msg_and_die("SELinux is disabled");
> +#else
> + bb_error_msg_and_die("SELinux support is disabled");
> +#endif
> +}
> +
> /* It is perfectly ok to pass in a NULL for either width or for
> * height, in which case that value will not be set. */
> int get_terminal_width_height(const int fd, int *width, int *height)
> Index: sebusybox-libselinux-0131/Makefile
> ===================================================================
> --- sebusybox-libselinux-0131/Makefile (revision 17684)
> +++ sebusybox-libselinux-0131/Makefile (working copy)
> @@ -442,6 +442,7 @@
> networking/udhcp/ \
> procps/ \
> runit/ \
> + selinux/ \
> shell/ \
> sysklogd/ \
> util-linux/ \
> Index: sebusybox-libselinux-0131/include/libbb.h
> ===================================================================
> --- sebusybox-libselinux-0131/include/libbb.h (revision 17684)
> +++ sebusybox-libselinux-0131/include/libbb.h (working copy)
> @@ -571,6 +571,7 @@
> extern void renew_current_security_context(void);
> extern void set_current_security_context(security_context_t sid);
> #endif
> +extern void xis_selinux_enabled(void);
> extern int restricted_shell(const char *shell);
> extern void setup_environment(const char *shell, int loginshell, int changeenv, \
> const struct passwd *pw); extern int correct_password(const struct passwd *pw);
> Index: sebusybox-libselinux-0131/include/usage.h
> ===================================================================
> --- sebusybox-libselinux-0131/include/usage.h (revision 17684)
> +++ sebusybox-libselinux-0131/include/usage.h (working copy)
> @@ -1013,6 +1013,9 @@
> " -6 When using port/proto only search IPv6 space\n" \
> " -SIGNAL When used with -k, this signal will be used to kill"
>
> +#define getenforce_trivial_usage
> +#define getenforce_full_usage
> +
> #define getopt_trivial_usage \
> "[OPTIONS]..."
> #define getopt_full_usage \
> @@ -1047,6 +1050,11 @@
> " esac\n" \
> "done\n"
>
> +#define getsebool_trivial_usage \
> + "-a or getsebool boolean..."
> +#define getsebool_full_usage \
> + "-a Show all SELinux booleans."
> +
> #define getty_trivial_usage \
> "[OPTIONS]... baud_rate,... line [termtype]"
> #define getty_full_usage \
> @@ -1896,6 +1904,15 @@
> "/dev/hda[0-15]\n"
> #endif
>
> +#define matchpathcon_trivial_usage \
> + "[-n] [-N] [-f file_contexts_file] [-p prefix] [-V]"
> +#define matchpathcon_full_usage \
> + "\t-n Do not display path.\n" \
> + "\t-N Do not use translations.\n" \
> + "\t-f file_context_file Use alternate file_context file\n" \
> + "\t-p prefix Use prefix to speed translations\n" \
> + "\t-V Verify file context on disk matches defaults"
This isn't easily readable. Perhaps put some \t\t before the explanation
> +
> #define md5sum_trivial_usage \
> "[OPTION] [FILEs...]" \
> USE_FEATURE_MD5_SHA1_SUM_CHECK("\n or: md5sum [OPTION] -c [FILE]")
> @@ -2714,6 +2731,9 @@
> "$ echo \"foo\" | sed -e 's/f[a-zA-Z]o/bar/g'\n" \
> "bar\n"
>
> +#define selinuxenabled_trivial_usage
> +#define selinuxenabled_full_usage
> +
> #define seq_trivial_usage \
> "[first [increment]] last"
> #define seq_full_usage \
> @@ -2731,6 +2751,10 @@
> "\n\nOptions:\n" \
> " -r Reset output to /dev/console"
>
> +#define setenforce_trivial_usage \
> + "[ Enforcing | Permissive | 1 | 0 ]"
> +#define setenforce_full_usage
> +
> #define setkeycodes_trivial_usage \
> "SCANCODE KEYCODE ..."
> #define setkeycodes_full_usage \
> Index: sebusybox-libselinux-0131/include/applets.h
> ===================================================================
> --- sebusybox-libselinux-0131/include/applets.h (revision 17684)
> +++ sebusybox-libselinux-0131/include/applets.h (working copy)
> @@ -133,7 +133,9 @@
> USE_FTPGET(APPLET_ODDNAME(ftpget, ftpgetput, _BB_DIR_USR_BIN, \
> _BB_SUID_NEVER,ftpget)) USE_FTPPUT(APPLET_ODDNAME(ftpput, ftpgetput, \
> _BB_DIR_USR_BIN, _BB_SUID_NEVER,ftpput)) USE_FUSER(APPLET(fuser, _BB_DIR_USR_BIN, \
> _BB_SUID_NEVER)) +USE_GETENFORCE(APPLET(getenforce, _BB_DIR_USR_SBIN, \
> _BB_SUID_NEVER)) USE_GETOPT(APPLET(getopt, _BB_DIR_BIN, _BB_SUID_NEVER))
> +USE_GETSEBOOL(APPLET(getsebool, _BB_DIR_USR_SBIN, _BB_SUID_NEVER))
> USE_GETTY(APPLET(getty, _BB_DIR_SBIN, _BB_SUID_NEVER))
> USE_GREP(APPLET(grep, _BB_DIR_BIN, _BB_SUID_NEVER))
> USE_GUNZIP(APPLET(gunzip, _BB_DIR_BIN, _BB_SUID_NEVER))
> @@ -187,6 +189,7 @@
> USE_LSATTR(APPLET(lsattr, _BB_DIR_BIN, _BB_SUID_NEVER))
> USE_LSMOD(APPLET(lsmod, _BB_DIR_SBIN, _BB_SUID_NEVER))
> USE_UNLZMA(APPLET_ODDNAME(lzmacat, unlzma, _BB_DIR_USR_BIN, _BB_SUID_NEVER, \
> lzmacat)) +USE_MATCHPATHCON(APPLET(matchpathcon, _BB_DIR_USR_SBIN, _BB_SUID_NEVER))
> USE_MAKEDEVS(APPLET(makedevs, _BB_DIR_SBIN, _BB_SUID_NEVER))
> USE_MD5SUM(APPLET_ODDNAME(md5sum, md5_sha1_sum, _BB_DIR_USR_BIN, _BB_SUID_NEVER, \
> md5sum)) USE_MDEV(APPLET(mdev, _BB_DIR_SBIN, _BB_SUID_NEVER))
> @@ -249,10 +252,12 @@
> USE_RUNSV(APPLET(runsv, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> USE_RUNSVDIR(APPLET(runsvdir, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> USE_RX(APPLET(rx, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> +USE_SELINUXENABLED(APPLET(selinuxenabled, _BB_DIR_USR_SBIN, _BB_SUID_NEVER))
> USE_SED(APPLET(sed, _BB_DIR_BIN, _BB_SUID_NEVER))
> USE_SEQ(APPLET(seq, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> USE_SETARCH(APPLET(setarch, _BB_DIR_BIN, _BB_SUID_NEVER))
> USE_SETCONSOLE(APPLET(setconsole, _BB_DIR_SBIN, _BB_SUID_NEVER))
> +USE_SETENFORCE(APPLET(setenforce, _BB_DIR_USR_SBIN, _BB_SUID_NEVER))
> USE_SETKEYCODES(APPLET(setkeycodes, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> USE_SETLOGCONS(APPLET(setlogcons, _BB_DIR_USR_SBIN, _BB_SUID_NEVER))
> USE_SETSID(APPLET(setsid, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
> Index: sebusybox-libselinux-0131/selinux/getenforce.c
> ===================================================================
> --- sebusybox-libselinux-0131/selinux/getenforce.c (revision 0)
> +++ sebusybox-libselinux-0131/selinux/getenforce.c (revision 0)
> @@ -0,0 +1,33 @@
> +/*
> + * getenforce
> + *
> + * Based on libselinux 1.33.1
> + * Port to BusyBox Hiroshi Shinji <shiroshi@my.email.ne.jp>
> + *
> + */
> +
> +#include "busybox.h"
> +
> +int getenforce_main(int argc, char **argv)
> +{
> + int rc;
> +
->+ rc = is_selinux_enabled();
->+ if (rc < 0)
->+ bb_error_msg_and_die("is_selinux_enabled() failed");
That's exactly the place where you should use xis_selinux_enabled(). It
obviously has to return an int due to this :)
xis_selinux_enabled();
> +
->+ if (rc == 1) {
> + rc = security_getenforce();
> + if (rc < 0)
> + bb_error_msg_and_die("getenforce() failed");
> +
> + if (rc)
> + puts("Enforcing");
> + else
> + puts("Permissive");
->+ } else {
->+ puts("Disabled");
->+ }
> +
> + return 0;
> +}
> Index: sebusybox-libselinux-0131/selinux/selinuxenabled.c
> ===================================================================
> --- sebusybox-libselinux-0131/selinux/selinuxenabled.c (revision 0)
> +++ sebusybox-libselinux-0131/selinux/selinuxenabled.c (revision 0)
> @@ -0,0 +1,13 @@
> +/*
> + * selinuxenabled
> + *
> + * Based on libselinux 1.33.1
> + * Port to BusyBox Hiroshi Shinji <shiroshi@my.email.ne.jp>
> + *
> + */
> +#include "busybox.h"
> +
> +int selinuxenabled_main(int argc, char **argv)
> +{
> + return !is_selinux_enabled();
> +}
> Index: sebusybox-libselinux-0131/selinux/getsebool.c
> ===================================================================
> --- sebusybox-libselinux-0131/selinux/getsebool.c (revision 0)
> +++ sebusybox-libselinux-0131/selinux/getsebool.c (revision 0)
> @@ -0,0 +1,73 @@
> +/*
> + * getsebool
> + *
> + * Based on libselinux 1.33.1
> + * Port to BusyBox Hiroshi Shinji <shiroshi@my.email.ne.jp>
> + *
> + */
> +
> +#include "busybox.h"
> +
> +#define GETSEBOOL_OPT_ALL 1
> +
> +int getsebool_main(int argc, char **argv)
> +{
> + int i, rc = 0, active, pending, len = 0;
> + char **names;
> + unsigned long opt;
> +
> + opt = getopt32(argc, argv, "a");
> +
> + xis_selinux_enabled();
> +
> + if (opt & GETSEBOOL_OPT_ALL) {
> + if (argc > 2)
> + bb_show_usage();
> +
> + rc = security_get_boolean_names(&names, &len);
> + if (rc)
> + bb_perror_msg_and_die("cannot get boolean names: ");
> +
> + if (!len) {
> + puts("No booleans");
> + return 0;
> + }
> + }
> +
> + if (!len) {
> + if (argc < 2)
> + bb_show_usage();
> + len = argc - 1;
> + names = xmalloc(sizeof(char *) * len);
> + for (i = 0; i < len; i++)
> + names[i] = xstrdup(argv[i + 1]);
> + }
I still think that the whole applet above can be written smaller.
Perhaps you can reuse i instead of "opt" without a size penalty?
Also, var & val is bloat there, i guess. Smaller if you just
if (i) { /* GETSEBOOL_OPT_ALL set */
> +
> + for (i = 0; i < len; i++) {
> + active = security_get_boolean_active(names[i]);
> + if (active < 0) {
> + bb_error_msg("error getting active value for %s", names[i]);
> + rc = -1;
> + goto out;
> + }
> + pending = security_get_boolean_pending(names[i]);
> + if (pending < 0) {
> + bb_error_msg("error getting pending value for %s", names[i]);
> + rc = -1;
> + goto out;
> + }
> + printf("%s --> %s", names[i], (active ? "on" : "off"));
> + if (pending != active)
> + printf(" pending: %s", (pending ? "on" : "off"));
> + putchar('\n');
> + }
> +
> + out:
Please put this at the start of the line as it's easier to find labels
there.
> + if (ENABLE_FEATURE_CLEAN_UP) {
> + for (i = 0; i < len; i++)
> + free(names[i]);
> + free(names);
> + }
> +
> + return rc;
> +}
[snip Kbuild and make stuff which is ok]
> Index: sebusybox-libselinux-0131/selinux/matchpathcon.c
> ===================================================================
> --- sebusybox-libselinux-0131/selinux/matchpathcon.c (revision 0)
> +++ sebusybox-libselinux-0131/selinux/matchpathcon.c (revision 0)
> @@ -0,0 +1,98 @@
> +/* matchpathcon - get the default security context for the specified
> + * path from the file contexts configuration.
> + * based on libselinux-1.32
> + * Port to busybox: KaiGai Kohei <kaigai@kaigai.gr.jp>
> + *
> + */
> +#include "busybox.h"
> +
> +static int printmatchpathcon(char *path, int header)
> +{
> + char *buf;
> + int rc = matchpathcon(path, 0, &buf);
> + if (rc < 0) {
> + fprintf(stderr, "matchpathcon(%s) failed: %s\n",
> + path, strerror(errno));
> + return 1;
> + }
Nah.. Please fix. (bb_perror_msg_and_die("%s", path);)
> + if (header)
> + printf("%s\t%s\n", path, buf);
> + else
> + printf("%s\n", buf);
> +
> + freecon(buf);
> + return 0;
> +}
> +
> +#define MATCHPATHCON_OPT_NOT_PRINT (1<<0) /* -n */
> +#define MATCHPATHCON_OPT_NOT_TRANS (1<<1) /* -N */
> +#define MATCHPATHCON_OPT_FCONTEXT (1<<2) /* -f */
> +#define MATCHPATHCON_OPT_PREFIX (1<<3) /* -p */
> +#define MATCHPATHCON_OPT_VERIFY (1<<4) /* -V */
> +
> +int matchpathcon_main(int argc, char **argv)
> +{
> + int i;
> + int header = 1;
> + int verify = 0;
> + int notrans = 0;
> + int error = 0;
It's very likely smaller to use one (smallu)int and mask those bits -- or use
opts or remove opts altogether and use option_mask32. You need to see
which one creates smaller code.
> + unsigned long opts;
See above.
> + char *fcontext, *prefix;
> +
->+ if (argc < 2)
->+ bb_show_usage();
> +
> + opt_complementary = "?:f--p:p--f";
from libbb/getopt32.c:
"-N" A dash as the first char in a opt_complementary group followed
by a single digit (0-9) means that at least N non-option
arguments must be present on the command line
so we want (don't need '?', i think):
opt_complementary = "-1:f--p:p--f";
> + opts = getopt32(argc, argv, "nNf:p:V", &fcontext, &prefix);
> +
> + if (opts & MATCHPATHCON_OPT_NOT_PRINT)
> + header = 0;
> + if (opts & MATCHPATHCON_OPT_NOT_TRANS) {
> + notrans = 1;
> + set_matchpathcon_flags(MATCHPATHCON_NOTRANS);
> + }
> + if (opts & MATCHPATHCON_OPT_FCONTEXT) {
> + if (matchpathcon_init(fcontext))
> + bb_error_msg_and_die("error while processing %s: %s",
> + fcontext, errno ? strerror(errno) : "invalid");
bb_perror_msg_and_die
> + }
> + if (opts & MATCHPATHCON_OPT_PREFIX) {
> + if (matchpathcon_init_prefix(NULL, prefix))
> + bb_error_msg_and_die("error while processing %s: %s",
> + prefix, errno ? strerror(errno) : "invalid");
bb_perror_msg_and_die
> + }
> + if (opts & MATCHPATHCON_OPT_VERIFY)
> + verify = 1;
> +
> + for (i = optind; i < argc; i++) {
> + security_context_t con;
> + int rc;
Is caching argv[i] here smaller?
> +
> + if (!verify) {
> + error += printmatchpathcon(argv[i], header);
> + continue;
> + }
> +
> + if (selinux_file_context_verify(argv[i], 0)) {
> + printf("%s verified.\n", argv[i]);
> + continue;
> + }
> +
> + if (notrans)
> + rc = lgetfilecon_raw(argv[i], &con);
> + else
> + rc = lgetfilecon(argv[i], &con);
> +
> + if (rc >= 0) {
> + printf("%s has context %s, should be ", argv[i], con);
> + error += printmatchpathcon(argv[i], 0);
> + freecon(con);
> + } else {
> + printf("actual context unknown: %s, should be ", strerror(errno));
> + error += printmatchpathcon(argv[i], 0);
> + }
> + }
> + matchpathcon_fini();
> + return error;
> +}
> Index: sebusybox-libselinux-0131/selinux/setenforce.c
> ===================================================================
> --- sebusybox-libselinux-0131/selinux/setenforce.c (revision 0)
> +++ sebusybox-libselinux-0131/selinux/setenforce.c (revision 0)
> @@ -0,0 +1,33 @@
> +/*
> + * setenforce
> + *
> + * Based on libselinux 1.33.1
> + * Port to BusyBox Hiroshi Shinji <shiroshi@my.email.ne.jp>
> + *
> + */
> +
> +#include "busybox.h"
> +
> +int setenforce_main(int argc, char **argv)
> +{
> + int rc = 0;
> + if (argc != 2)
> + bb_show_usage();
> +
> + xis_selinux_enabled();
> +
> + if ((argv[1][0] == '0' || argv[1][0] == '1') && argv[1][1] == '\0') {
> + rc = security_setenforce(atoi(argv[1]));
> + } else {
> + if (strcasecmp(argv[1], "enforcing") == 0) {
> + rc = security_setenforce(1);
> + } else if (strcasecmp(argv[1], "permissive") == 0) {
rc = security_setenforce(index_in_substr_array);
Isn't case-insensitive, but would suffice IMO and would make that applet
considerably smaller. What do you think?
> + rc = security_setenforce(0);
> + } else
> + bb_show_usage();
> + }
> + if (rc < 0)
> + bb_perror_msg_and_die("setenforce() failed : ");
> +
> + return 0;
> +}
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic