On Tue, Jul 22, 2014 at 05:07:20PM -0700, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ovs-vswitchd typically runs as root, but it doesn't need most of the powers
> of root. This commit makes it drop all the capabilities that it doesn't
> typically need.
>
> I don't know how capabilities are really used in practice. Something
> equivalent could be achieved through the file system. I don't know the
> preferred way to do it.
I am no expert on the matter, but yeah, you can use setcap to adjust
the file capabilities. As far as I can tell setcap is used when the
program doesn't do that itself.
> diff --git a/configure.ac b/configure.ac
> index 971c7b3..9665516 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -55,6 +55,7 @@ OVS_CHECK_COVERAGE
> OVS_CHECK_NDEBUG
> OVS_CHECK_NETLINK
> OVS_CHECK_OPENSSL
> +OVS_CHECK_LIBCAP
> OVS_CHECK_LOGDIR
> OVS_CHECK_PYTHON
> OVS_CHECK_PYTHON_COMPAT
> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
> index 26b8058..6017c74 100644
> --- a/m4/openvswitch.m4
> +++ b/m4/openvswitch.m4
> @@ -192,6 +192,15 @@ AC_DEFUN([OVS_CHECK_BACKTRACE],
> [AC_DEFINE([HAVE_BACKTRACE], [1],
> [Define to 1 if you have backtrace(3).])])])
>
> +dnl Defines HAVE_LIBCAP if libcap and <sys/capability.h> are available.
> +AC_DEFUN([OVS_CHECK_LIBCAP],
> + [AC_CHECK_HEADER([sys/capability.h],
> + [AC_SEARCH_LIBS([cap_get_proc], [cap])
> + if test $ac_cv_search_cap_get_proc != no; then
> + AC_DEFINE([HAVE_LIBCAP], [1],
not important on RFC, but there is a trailing space here.
> + [Define to 1 if you have libcap and <sys/capability.h>.])
> + fi])])
> +
> dnl Checks for valgrind/valgrind.h.
> AC_DEFUN([OVS_CHECK_VALGRIND],
> [AC_CHECK_HEADERS([valgrind/valgrind.h])])
> diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> index 3c843e1..6625cd7 100644
> --- a/vswitchd/ovs-vswitchd.c
> +++ b/vswitchd/ovs-vswitchd.c
> @@ -49,6 +49,9 @@
> #include "vlog.h"
> #include "lib/vswitch-idl.h"
> #include "lib/netdev-dpdk.h"
> +#ifdef HAVE_LIBCAP
> +#include <sys/capability.h>
> +#endif
>
> VLOG_DEFINE_THIS_MODULE(vswitchd);
>
> @@ -60,6 +63,7 @@ static unixctl_cb_func ovs_vswitchd_exit;
>
> static char *parse_options(int argc, char *argv[], char **unixctl_path);
> static void usage(void) NO_RETURN;
> +static void drop_capabilities(void);
>
> int
> main(int argc, char *argv[])
> @@ -75,6 +79,8 @@ main(int argc, char *argv[])
> argc -= retval;
> argv += retval;
>
> + drop_capabilities();
> +
> proctitle_init(argc, argv);
> service_start(&argc, &argv);
> remote = parse_options(argc, argv, &unixctl_path);
> @@ -265,3 +271,77 @@ ovs_vswitchd_exit(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> *exiting = true;
> unixctl_command_reply(conn, NULL);
> }
> +
> +static void
> +drop_capabilities(void)
> +{
> +#ifdef HAVE_LIBCAP
> + /* Capabilities we'd like to retain, if we have them. */
> + static const cap_value_t keep[] = {
> + CAP_IPC_LOCK, CAP_NET_ADMIN, CAP_NET_BIND_SERVICE, CAP_NET_RAW
> + };
> +
> + cap_t cur, next;
> + char *s;
> + int i;
> +
> + /* Get current capabilities. */
> + cur = cap_get_proc();
> + if (!cur) {
> + VLOG_FATAL("failed to retrieve process capabilities (%s)",
> + ovs_strerror(errno));
> + }
> +
> + /* Get empty capability. */
> + next = cap_init();
> + if (!next) {
> + VLOG_FATAL("failed to create capability set (%s)",
> + ovs_strerror(errno));
> + }
> +
> + /* Copy the capabilities that we want to keep from 'cur' to 'next'.
> + * Leave other capabilities unset.
> + * Thus, 'keep' is effectively a whitelist; we drop all other
> + * capabilities. */
> + for (i = 0; i < ARRAY_SIZE(keep); i++) {
> + static const int flags[] = {
> + CAP_EFFECTIVE, CAP_INHERITABLE, CAP_PERMITTED
> + };
> + cap_value_t value = keep[i];
> + int j;
> +
> + for (j = 0; j < ARRAY_SIZE(flags); j++) {
> + cap_flag_t flag = flags[j];
> + cap_flag_value_t flag_value;
> +
> + if (cap_get_flag(cur, value, flags[j], &flag_value)) {
> + VLOG_FATAL("failed to get capability flag (%s)",
> + ovs_strerror(errno));
> + }
> + if (cap_set_flag(next, flag, 1, &value, flag_value)) {
I am not sure if the above loop is correct. You're allowed to clear
or even set capabilities on EFFECTIVE if it's in PERMITTED. If it's
not there, you can't set it. Here you check if the capability is
supported and then set whatever the state it was before.
Maybe the loop could be:
for (i, i < cap_matrix_size; i++) {
cap_value_t value = keep[i];
err = cap_get_flag(cur, value, CAP_PERMITTED, &flag_value));
/* keep the capability */
if (err != 1 && flag_value) {
cap_set_flag(next, CAP_EFFECTIVE, 1, &value, CAP_SET);
cap_set_flag(next, CAP_PERMITTED, 1, &value, CAP_SET);
} else {
VLOG_FATAL("failed to get capability...")
}
Since you are with minimal set of capabilities, the lack of any of them
means that the program won't function correctly. So my suggestion is to
use CAP_SET and fail with fatal error when the cap is not available.
fbl
> + VLOG_FATAL("failed to set capability flag (%s)",
> + ovs_strerror(errno));
> + }
> + }
> + }
> +
> + /* Set our capability set to 'next'. */
> + if (cap_set_proc(next)) {
> + VLOG_FATAL("failed to set process capabilities (%s)",
> + ovs_strerror(errno));
> + }
> +
> + /* Log that we dropped capabilities. */
> + s = cap_to_text(next, NULL);
> + if (!s) {
> + VLOG_FATAL("failed to convert capabilities to text (%s)",
> + ovs_strerror(errno));
> + }
> + VLOG_INFO("reduced capability set to: %s", s);
> +
> + /* Clean up. */
> + cap_free(s);
> + cap_free(cur);
> + cap_free(next);
> +#endif /* HAVE_LIBCAP */
> +}
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
