A new fd generated from accept() or sctp_peeloff() should inherit the rights of the original fd (cf. https://lists.cam.ac.uk/pipermail/cl-capsicum-discuss/2014-February/msg00001.html) Test case available in https://github.com/google/capsicum-test
A commit references this bug: Author: oshogbo Date: Thu Sep 22 09:58:47 UTC 2016 New revision: 306174 URL: https://svnweb.freebsd.org/changeset/base/306174 Log: capsicum: propagate rights on accept(2) Descriptor returned by accept(2) should inherits capabilities rights from the listening socket. PR: 201052 Reviewed by: emaste, jonathan Discussed with: many Differential Revision: https://reviews.freebsd.org/D7724 Changes: head/sys/compat/cloudabi/cloudabi_sock.c head/sys/compat/linux/linux_socket.c head/sys/kern/kern_sendfile.c head/sys/kern/uipc_syscalls.c head/sys/netinet/sctp_syscalls.c head/sys/sys/socketvar.h
A commit references this bug: Author: dchagin Date: Wed Mar 15 16:38:40 UTC 2017 New revision: 315312 URL: https://svnweb.freebsd.org/changeset/base/315312 Log: MFC r305093 (by mjg@): fd: add fdeget_locked and use in kern_descrip MFC r305756 (by oshogbo@): fd: add fget_cap and fget_cap_locked primitives. They can be used to obtain capabilities along with a referenced fp. MFC r306174 (by oshogbo@): capsicum: propagate rights on accept(2) Descriptor returned by accept(2) should inherits capabilities rights from the listening socket. PR: 201052 MFC r306184 (by oshogbo@): fd: simplify fgetvp_rights by using fget_cap_locked. MFC r306225 (by mjg@): fd: fix up fgetvp_rights after r306184 fget_cap_locked returns a referenced file, but the fgetvp_rights does not need it. Instead, due to the filedesc lock being held, it can ref the vnode after the file was looked up. Fix up fget_cap_locked to be consistent with other _locked helpers and not ref the file. This plugs a leak introduced in r306184. MFC r306232 (by oshogbo@): fd: fix up fget_cap If the kernel is not compiled with the CAPABILITIES kernel options fget_unlocked doesn't return the sequence number so fd_modify will always report modification, in that case we got infinity loop. MFC r311474 (by glebius@): Use getsock_cap() instead of fgetsock(). MFC r312079 (by glebius@): Use getsock_cap() instead of deprecated fgetsock(). MFC r312081 (by glebius@): Use getsock_cap() instead of deprecated fgetsock(). MFC r312087 (by glebius@): Remove deprecated fgetsock() and fputsock(). Bump __FreeBSD_version as getsock_cap changed and fgetsock/fputsock pair removed. Changes: _U stable/11/ stable/11/sys/compat/cloudabi/cloudabi_sock.c stable/11/sys/compat/linux/linux_socket.c stable/11/sys/dev/iscsi_initiator/isc_soc.c stable/11/sys/dev/iscsi_initiator/iscsi.c stable/11/sys/kern/kern_descrip.c stable/11/sys/kern/kern_sendfile.c stable/11/sys/kern/uipc_syscalls.c stable/11/sys/netinet/sctp_syscalls.c stable/11/sys/sys/file.h stable/11/sys/sys/filedesc.h stable/11/sys/sys/param.h stable/11/sys/sys/socketvar.h
Mariusz, can this be closed now?
We still don't have support for sctp_peeloff.
(In reply to Mariusz Zaborski from comment #4) Ah, indeed. I've reset the asignee for now.
For bugs matching the following conditions: - Status == In Progress - Assignee == "bugs@FreeBSD.org" - Last Modified Year <= 2017 Do - Set Status to "Open"
^Triage: clear stale flags. To submitter: is this aging PR still relevant?
(In reply to Mark Linimon from comment #7) I guess so. Unfortunately, I have no knowledge about capsicum.
The change should be similar to the one for accept(), i.e. https://github.com/freebsd/freebsd-src/commit/85b0f9de11c3988f53f899cd171b685037da03a8 getsock_cap gained a new arg `struct filecaps *havecapsp` which is not used in most cases but in kern_accept4() we pass fcaps to get the existing capabilities, and then pass that to falloc_caps to obtain the new fd. I think the diff would look like: diff --git a/sys/netinet/sctp_syscalls.c b/sys/netinet/sctp_syscalls.c index d67e260b6f99..1bd6f2707d5d 100644 --- a/sys/netinet/sctp_syscalls.c +++ b/sys/netinet/sctp_syscalls.c @@ -141,13 +141,14 @@ sys_sctp_peeloff(struct thread *td, struct sctp_peeloff_args *uap) { struct file *headfp, *nfp = NULL; struct socket *head, *so; + struct filecaps fcaps; cap_rights_t rights; u_int fflag; int error, fd; AUDIT_ARG_FD(uap->sd); - error = getsock(td, uap->sd, cap_rights_init_one(&rights, CAP_PEELOFF), - &headfp); + error = getsock_cap(td, uap->sd, + cap_rights_init_one(&rights, CAP_PEELOFF), &headfp, NULL, &fcaps); if (error != 0) goto done2; fflag = atomic_load_int(&headfp->f_flag); @@ -165,7 +166,7 @@ sys_sctp_peeloff(struct thread *td, struct sctp_peeloff_args *uap) * but that is ok. */ - error = falloc(td, &nfp, &fd, 0); + error = falloc_cap(td, &nfp, &fd, 0, &fcaps); if (error != 0) goto done; td->td_retval[0] = fd;
(In reply to Ed Maste from comment #9) I've put this change in https://reviews.freebsd.org/D46884. It's not yet even build-tested. When I have time I'll see if we can add/modify capsicum-test tests to cover this.
^Triage: feedback received. Apparently this bug is still relevant.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6684779b321590c71f162390bcf28feee2a3b967 commit 6684779b321590c71f162390bcf28feee2a3b967 Author: Ed Maste <emaste@FreeBSD.org> AuthorDate: 2024-10-07 18:26:58 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-10-07 19:40:06 +0000 capsicum-test: rights are propagated on accept(2) As of commit 85b0f9de11c3 ("capsicum: propagate rights on accept(2)") a capability is generated from accept(cap_fd,...). Enable the corresponding test code. PR: 201052 Reviewed by: oshogbo Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D46994 contrib/capsicum-test/capsicum-freebsd.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
(In reply to Ed Maste from comment #0) Note that the cl-capsicum-discuss mailing list does not appear to be available now. Looking at my own mail archive I see some back-and-forth on this topic with a general consensus on the approach. Robert Watson's comment: > The main merit of the current approach is that it is simple and easy-to-understand, > even if we don't like all of its consequences. When combining the properties of a > capability system with a conventional UNIX OS design, that's about as good as it > gets. We pondered for quite a while how to deal with "descriptors inherited from > descriptors" and most of the variations aren't much fun, as they require additional > rights masks and hence increase the potential for developer confusion leading to > security surprises. I'm happy to keep having the conversation as I also agree that > the current API isn't great, though.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=91a9e4e01dab7a740b8e3b7c39c59a537e71e5d2 commit 91a9e4e01dab7a740b8e3b7c39c59a537e71e5d2 Author: Ed Maste <emaste@FreeBSD.org> AuthorDate: 2024-10-03 11:54:44 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-10-09 00:36:50 +0000 sctp: propagate cap rights on sctp_peeloff PR: 201052 Reviewed by: oshogbo, tuexen Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D46884 sys/netinet/sctp_syscalls.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=38518fda66cda6c57af0aa655d19c1897c0ab15d commit 38518fda66cda6c57af0aa655d19c1897c0ab15d Author: Ed Maste <emaste@FreeBSD.org> AuthorDate: 2024-10-07 18:28:20 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-10-09 00:41:45 +0000 capsicum-test: rights are propagated on sctp_peeloff(2) As of commit 91a9e4e01dab ("capsicum: propagate rights on sctp_peeloff") a capability is generated from sctp_peeloff(cap_fd,...). Enable the corresponding test code. PR: 201052 Reviewed by: oshogbo, tuexen Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D47000 contrib/capsicum-test/capsicum-freebsd.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=fbc593342cbf1ba962cf3266a245402169c0ffcf commit fbc593342cbf1ba962cf3266a245402169c0ffcf Author: Ed Maste <emaste@FreeBSD.org> AuthorDate: 2024-10-07 18:26:58 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-10-11 15:24:55 +0000 capsicum-test: rights are propagated on accept(2) As of commit 85b0f9de11c3 ("capsicum: propagate rights on accept(2)") a capability is generated from accept(cap_fd,...). Enable the corresponding test code. PR: 201052 Reviewed by: oshogbo Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D46994 (cherry picked from commit 6684779b321590c71f162390bcf28feee2a3b967) contrib/capsicum-test/capsicum-freebsd.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ae3d7e27abc98d7325d506a55af6a3ea2e028738 commit ae3d7e27abc98d7325d506a55af6a3ea2e028738 Author: Ed Maste <emaste@FreeBSD.org> AuthorDate: 2024-10-03 11:54:44 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-10-17 16:29:21 +0000 sctp: propagate cap rights on sctp_peeloff PR: 201052 Reviewed by: oshogbo, tuexen Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D46884 (cherry picked from commit 91a9e4e01dab7a740b8e3b7c39c59a537e71e5d2) sys/netinet/sctp_syscalls.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=202bbdcb1a2b67ddfad967db38969af5caa668d7 commit 202bbdcb1a2b67ddfad967db38969af5caa668d7 Author: Ed Maste <emaste@FreeBSD.org> AuthorDate: 2024-10-07 18:28:20 +0000 Commit: Ed Maste <emaste@FreeBSD.org> CommitDate: 2024-10-17 16:29:21 +0000 capsicum-test: rights are propagated on sctp_peeloff(2) As of commit 91a9e4e01dab ("capsicum: propagate rights on sctp_peeloff") a capability is generated from sctp_peeloff(cap_fd,...). Enable the corresponding test code. PR: 201052 Reviewed by: oshogbo, tuexen Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D47000 (cherry picked from commit 38518fda66cda6c57af0aa655d19c1897c0ab15d) contrib/capsicum-test/capsicum-freebsd.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)