On Thu, Oct 06, 2011 at 11:38:27AM -0400, Richa Marwaha wrote:
The ideal way to use qemu-bridge-helper is to give it an fscap of using:
setcap cap_net_admin=ep qemu-bridge-helper
Unfortunately, most distros still do not have a mechanism to package files
with fscaps applied. This means they'll have to SUID the qemu-bridge-helper
binary.
To improve security, use libcap to reduce our capability set to just
cap_net_admin, then reduce privileges down to the calling user. This is
hopefully close to equivalent to fscap support from a security perspective.
+#ifdef CONFIG_LIBCAP
+static int drop_privileges(void)
+{
+ cap_t cap;
+ cap_value_t new_caps[] = {CAP_NET_ADMIN};
+
+ cap = cap_init();
Check for NULL ?
+
+ /* set capabilities to be permitted and inheritable. we don't need the
+ * caps to be effective right now as they'll get reset when we seteuid
+ * anyway */
+ cap_set_flag(cap, CAP_PERMITTED, 1, new_caps, CAP_SET);
+ cap_set_flag(cap, CAP_INHERITABLE, 1, new_caps, CAP_SET);
Check for failure ?
+
+ if (cap_set_proc(cap) == -1) {
+ return -1;
+ }
+
+ cap_free(cap);
Check for failure ?
+
+ /* reduce our privileges to a normal user */
+ setegid(getgid());
+ seteuid(getuid());
Check for failure ?
+ cap = cap_init();
Check for NULL ?
+
+ /* enable the our capabilities. we marked them as inheritable earlier
+ * which is what allows this to work. */
+ cap_set_flag(cap, CAP_EFFECTIVE, 1, new_caps, CAP_SET);
+ cap_set_flag(cap, CAP_PERMITTED, 1, new_caps, CAP_SET);
Check for failure ?
+
+ if (cap_set_proc(cap) == -1) {
+ return -1;
+ }
+
+ cap_free(cap);
Check for failure ?
+
+ return 0;
+}
+#endif
It may seem like checking for failure on cap_free/cap_set_flag is
not required because they can only return EINVAL for invalid
args, but since this is missing the check for NULL on cap_init
you can actually see errors from those latter functions in an
OOM cenario.
I think I'd suggest not using libcap, instead try libcap-ng [1] whose
APIs are designed with safety in mind& result in much simpler and
clearer code:
eg, that entire function above can be expressed using capng with
something approximating:
capng_clear(CAPNG_SELECT_BOTH);
if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE|CAPNG_PERMITTED,
CAP_NET_ADMIN)< 0)
error(...);
if (capng_change_id(getuid(), getgid(), CAPNG_DROP_SUPP_GRP |
CAPNG_CLEAR_BOUNDING))
error(...);