From 2216c6933cf09bd4922b49e9d5fd9a7e48b056a5 Mon Sep 17 00:00:00 2001 From: Ed Maste Date: Mon, 30 Apr 2018 17:31:06 +0000 Subject: [PATCH] Disable connectat/bindat with AT_FDCWD in capmode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously it was possible to connect a socket (which had the CAP_CONNECT right) by calling "connectat(AT_FDCWD, ...)" even in capabilties mode. This combination should be treated the same as a call to connect (i.e. forbidden in capabilities mode). Similarly for bindat. Disable connectat/bindat with AT_FDCWD in capabilities mode, fix up the documentation and add tests. PR: 222632 Submitted by: Jan Kokemüller Reviewed by: Domagoj Stolfa MFC after: 1 week Relnotes: Yes Differential Revision: https://reviews.freebsd.org/D15221 --- share/man/man4/rights.4 | 26 ++- sys/kern/uipc_syscalls.c | 10 ++ tests/sys/capsicum/Makefile | 3 + tests/sys/capsicum/bindat_connectat.c | 233 ++++++++++++++++++++++++++ 4 files changed, 266 insertions(+), 6 deletions(-) create mode 100644 tests/sys/capsicum/bindat_connectat.c diff --git a/share/man/man4/rights.4 b/share/man/man4/rights.4 index da9a5aa8a980..b2ecf61eccc3 100644 --- a/share/man/man4/rights.4 +++ b/share/man/man4/rights.4 @@ -32,7 +32,7 @@ .\" .\" $FreeBSD$ .\" -.Dd August 17, 2016 +.Dd April 30, 2018 .Dt RIGHTS 4 .Os .Sh NAME @@ -94,8 +94,15 @@ Permit and .Xr acl_set_fd_np 3 . .It Dv CAP_BIND -Permit -.Xr bind 2 . +When not in capabilities mode, permit +.Xr bind 2 +and +.Xr bindat 2 +with special value +.Dv AT_FDCWD +in the +.Fa fd +parameter. Note that sockets can also become bound implicitly as a result of .Xr connect 2 or @@ -116,9 +123,16 @@ An alias to and .Dv CAP_LOOKUP . .It Dv CAP_CONNECT -Permit -.Xr connect 2 ; -also required for +When not in capabilities mode, permit +.Xr connect 2 +and +.Xr connectat 2 +with special value +.Dv AT_FDCWD +in the +.Fa fd +parameter. +This right is also required for .Xr sendto 2 with a non-NULL destination address. .It Dv CAP_CONNECTAT diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c index 4f1e362155e7..2de9ada463c3 100644 --- a/sys/kern/uipc_syscalls.c +++ b/sys/kern/uipc_syscalls.c @@ -187,6 +187,11 @@ kern_bindat(struct thread *td, int dirfd, int fd, struct sockaddr *sa) cap_rights_t rights; int error; +#ifdef CAPABILITY_MODE + if (IN_CAPABILITY_MODE(td) && (dirfd == AT_FDCWD)) + return (ECAPMODE); +#endif + AUDIT_ARG_FD(fd); AUDIT_ARG_SOCKADDR(td, dirfd, sa); error = getsock_cap(td, fd, cap_rights_init(&rights, CAP_BIND), @@ -483,6 +488,11 @@ kern_connectat(struct thread *td, int dirfd, int fd, struct sockaddr *sa) cap_rights_t rights; int error, interrupted = 0; +#ifdef CAPABILITY_MODE + if (IN_CAPABILITY_MODE(td) && (dirfd == AT_FDCWD)) + return (ECAPMODE); +#endif + AUDIT_ARG_FD(fd); AUDIT_ARG_SOCKADDR(td, dirfd, sa); error = getsock_cap(td, fd, cap_rights_init(&rights, CAP_CONNECT), diff --git a/tests/sys/capsicum/Makefile b/tests/sys/capsicum/Makefile index d0b0aaeba28c..5c62931ee89e 100644 --- a/tests/sys/capsicum/Makefile +++ b/tests/sys/capsicum/Makefile @@ -2,8 +2,11 @@ TESTSDIR= ${TESTSBASE}/sys/capsicum +ATF_TESTS_C+= bindat_connectat ATF_TESTS_C+= ioctls_test +CFLAGS.bindat_connectat.c+= -I${SRCTOP}/tests + WARNS?= 6 .include diff --git a/tests/sys/capsicum/bindat_connectat.c b/tests/sys/capsicum/bindat_connectat.c new file mode 100644 index 000000000000..03695128a675 --- /dev/null +++ b/tests/sys/capsicum/bindat_connectat.c @@ -0,0 +1,233 @@ +/* + * Copyright (c) 2017 Jan Kokemüller + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include +__FBSDID("$FreeBSD$"); + +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "freebsd_test_suite/macros.h" + +static int rootfd = -1; + +/* circumvent bug 215690 */ +int +open(const char *path, int flags, ...) +{ + mode_t mode = 0; + + if (flags & O_CREAT) { + va_list ap; + va_start(ap, flags); + mode = (mode_t) va_arg(ap, int); + va_end(ap); + } + + if (path && path[0] == '/' && rootfd >= 0) { + return (openat(rootfd, path + 1, flags, mode)); + } else { + return (openat(AT_FDCWD, path, flags, mode)); + } +} + +static void +check_capsicum(void) +{ + ATF_REQUIRE_FEATURE("security_capabilities"); + ATF_REQUIRE_FEATURE("security_capability_mode"); + + ATF_REQUIRE((rootfd = open("/", O_EXEC | O_CLOEXEC)) >= 0); +} + +typedef int (*socket_fun)(int, const struct sockaddr *, socklen_t); + +static int +connectat_fdcwd(int s, const struct sockaddr *name, socklen_t namelen) +{ + + return (connectat(AT_FDCWD, s, name, namelen)); +} + +static int +bindat_fdcwd(int s, const struct sockaddr *name, socklen_t namelen) +{ + + return (bindat(AT_FDCWD, s, name, namelen)); +} + + +ATF_TC(bindat_connectat_1); +ATF_TC_HEAD(bindat_connectat_1, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Verify that connect/bind work in normal case"); +} + +static void +check_1(socket_fun f, int s, const struct sockaddr_in *name) +{ + + ATF_REQUIRE((s = socket(AF_INET, SOCK_STREAM, 0)) >= 0); + ATF_REQUIRE_ERRNO(EAFNOSUPPORT, + f(s, (const struct sockaddr *)(name), + sizeof(struct sockaddr_in)) < 0); +} + +ATF_TC_BODY(bindat_connectat_1, tc) +{ + struct sockaddr_in sin; + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = htons(0); + sin.sin_addr.s_addr = htonl(0xE0000000); + + check_1(bindat_fdcwd, 0, &sin); + check_1(bind, 0, &sin); + check_1(connectat_fdcwd, 0, &sin); + check_1(connect, 0, &sin); +} + + +ATF_TC(bindat_connectat_2); +ATF_TC_HEAD(bindat_connectat_2, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Verify that connect/bind are disabled in cap-mode"); +} + +static void +check_2(socket_fun f, int s, const struct sockaddr_in *name) +{ + + ATF_REQUIRE_ERRNO(ECAPMODE, + f(s, (const struct sockaddr *)name, + sizeof(struct sockaddr_in)) < 0); +} + +ATF_TC_BODY(bindat_connectat_2, tc) +{ + int sock; + struct sockaddr_in sin; + + check_capsicum(); + + ATF_REQUIRE(cap_enter() >= 0); + + /* note: sock is created _after_ cap_enter() and contains all rights */ + ATF_REQUIRE((sock = socket(AF_INET, SOCK_STREAM, 0)) >= 0); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + /* dummy port and multicast address (224.0.0.0) to distinguish two + * cases: + * - ECAPMODE/ENOTCAPABLE --> call blocked by capsicum + * - EAFNOSUPPORT --> call went through to protocol layer + */ + sin.sin_port = htons(0); + sin.sin_addr.s_addr = htonl(0xE0000000); + + check_2(bindat_fdcwd, sock, &sin); + check_2(bind, sock, &sin); + check_2(connectat_fdcwd, sock, &sin); + check_2(connect, sock, &sin); +} + + +ATF_TC(bindat_connectat_3); +ATF_TC_HEAD(bindat_connectat_3, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Check that taking away CAP_BIND/CAP_CONNECT " + "sabotages bind/connect"); +} + +static void +check_3(socket_fun f, int s, const struct sockaddr_in *name, + cap_rights_t *rights, cap_rights_t *sub_rights) +{ + + ATF_REQUIRE((s = socket(AF_INET, SOCK_STREAM, 0)) >= 0); + ATF_REQUIRE(cap_rights_limit(s, rights) >= 0); + ATF_REQUIRE_ERRNO(EAFNOSUPPORT, + f(s, (const struct sockaddr *)name, + sizeof(struct sockaddr_in)) < 0); + ATF_REQUIRE(cap_rights_limit(s, + cap_rights_remove(rights, sub_rights)) >= 0); + ATF_REQUIRE_ERRNO(ENOTCAPABLE, + f(s, (const struct sockaddr *)name, + sizeof(struct sockaddr_in)) < 0); +} + +ATF_TC_BODY(bindat_connectat_3, tc) +{ + struct sockaddr_in sin; + cap_rights_t rights, sub_rights; + + check_capsicum(); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = htons(0); + sin.sin_addr.s_addr = htonl(0xE0000000); + + check_3(bindat_fdcwd, 0, &sin, + cap_rights_init(&rights, CAP_SOCK_SERVER), + cap_rights_init(&sub_rights, CAP_BIND)); + check_3(bind, 0, &sin, + cap_rights_init(&rights, CAP_SOCK_SERVER), + cap_rights_init(&sub_rights, CAP_BIND)); + check_3(connectat_fdcwd, 0, &sin, + cap_rights_init(&rights, CAP_SOCK_CLIENT), + cap_rights_init(&sub_rights, CAP_CONNECT)); + check_3(connect, 0, &sin, + cap_rights_init(&rights, CAP_SOCK_CLIENT), + cap_rights_init(&sub_rights, CAP_CONNECT)); +} + + +ATF_TP_ADD_TCS(tp) +{ + + ATF_TP_ADD_TC(tp, bindat_connectat_1); + ATF_TP_ADD_TC(tp, bindat_connectat_2); + ATF_TP_ADD_TC(tp, bindat_connectat_3); + + return (atf_no_error()); +}