| Message ID | 20191224235939.7483-24-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | LSM: Module stacking for AppArmor | expand |
On 12/24/19 6:59 PM, Casey Schaufler wrote: > The getsockopt SO_PEERSEC provides the LSM based security > information for a single module, but for reasons of backward > compatibility cannot include the information for multiple > modules. A new option SO_PEERCONTEXT is added to report the > security "context" of multiple modules using a "compound" format > > lsm1\0value\0lsm2\0value\0 > > This is expected to be used by system services, including dbus-daemon. > The exact format of a compound context has been the subject of > considerable debate. This format was suggested by Simon McVittie, > a dbus maintainer with a significant stake in the format being > usable. Since upstream AA does not currently ever set the peer label info, there is no need for this support for stacking upstream AA today, and there is no way to test this functionality with more than one module present currently in an upstream kernel. Either fix AA to actually implement the functionality so it can be tested properly, or drop it from this series please. I don't understand why AA continues to keep this kind of basic and longstanding downstream functionality out of tree. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > cc: netdev@vger.kernel.org > cc: linux-api@vger.kernel.org > --- > Documentation/security/lsm.rst | 15 ++++ > arch/alpha/include/uapi/asm/socket.h | 1 + > arch/mips/include/uapi/asm/socket.h | 1 + > arch/parisc/include/uapi/asm/socket.h | 1 + > arch/sparc/include/uapi/asm/socket.h | 1 + > include/linux/lsm_hooks.h | 9 +- > include/linux/security.h | 10 ++- > include/uapi/asm-generic/socket.h | 1 + > net/core/sock.c | 7 +- > security/apparmor/lsm.c | 20 ++--- > security/security.c | 115 +++++++++++++++++++++++--- > security/selinux/hooks.c | 20 ++--- > security/smack/smack_lsm.c | 31 +++---- > 13 files changed, 170 insertions(+), 62 deletions(-) > > diff --git a/Documentation/security/lsm.rst b/Documentation/security/lsm.rst > index aadf47c808c0..77cc326a52cc 100644 > --- a/Documentation/security/lsm.rst > +++ b/Documentation/security/lsm.rst > @@ -199,3 +199,18 @@ capability-related fields: > - ``fs/nfsd/auth.c``::c:func:`nfsd_setuser()` > > - ``fs/proc/array.c``::c:func:`task_cap()` > + > +LSM External Interfaces > +======================= > + > +The LSM infrastructure does not generally provide external interfaces. > +The individual security modules provide what external interfaces they > +require. The infrastructure does provide two interfaces for the special > +case where multiple security modules provide a process context. This > +is provided in compound context format. > + > +- `lsm1\0value\0lsm2\0value\0` > + > +The special file ``/proc/pid/attr/context`` provides the security > +context of the identified process. The socket option SO_PEERCONTEXT > +provides the security context of a packet. > diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h > index de6c4df61082..b26fb34e4226 100644 > --- a/arch/alpha/include/uapi/asm/socket.h > +++ b/arch/alpha/include/uapi/asm/socket.h > @@ -123,6 +123,7 @@ > #define SO_SNDTIMEO_NEW 67 > > #define SO_DETACH_REUSEPORT_BPF 68 > +#define SO_PEERCONTEXT 69 > > #if !defined(__KERNEL__) > > diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h > index d0a9ed2ca2d6..10e03507b1ed 100644 > --- a/arch/mips/include/uapi/asm/socket.h > +++ b/arch/mips/include/uapi/asm/socket.h > @@ -134,6 +134,7 @@ > #define SO_SNDTIMEO_NEW 67 > > #define SO_DETACH_REUSEPORT_BPF 68 > +#define SO_PEERCONTEXT 69 > > #if !defined(__KERNEL__) > > diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h > index 10173c32195e..e11df59a84d1 100644 > --- a/arch/parisc/include/uapi/asm/socket.h > +++ b/arch/parisc/include/uapi/asm/socket.h > @@ -115,6 +115,7 @@ > #define SO_SNDTIMEO_NEW 0x4041 > > #define SO_DETACH_REUSEPORT_BPF 0x4042 > +#define SO_PEERCONTEXT 0x4043 > > #if !defined(__KERNEL__) > > diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h > index 8029b681fc7c..5b41ef778040 100644 > --- a/arch/sparc/include/uapi/asm/socket.h > +++ b/arch/sparc/include/uapi/asm/socket.h > @@ -116,6 +116,7 @@ > #define SO_SNDTIMEO_NEW 0x0045 > > #define SO_DETACH_REUSEPORT_BPF 0x0047 > +#define SO_PEERCONTEXT 0x0048 > > #if !defined(__KERNEL__) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 2bf82e1cf347..2ae10e7f81a7 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -880,8 +880,8 @@ > * SO_GETPEERSEC. For tcp sockets this can be meaningful if the > * socket is associated with an ipsec SA. > * @sock is the local socket. > - * @optval userspace memory where the security state is to be copied. > - * @optlen userspace int where the module should copy the actual length > + * @optval memory where the security state is to be copied. > + * @optlen int where the module should copy the actual length > * of the security state. > * @len as input is the maximum length to copy to userspace provided > * by the caller. > @@ -1724,9 +1724,8 @@ union security_list_options { > int (*socket_setsockopt)(struct socket *sock, int level, int optname); > int (*socket_shutdown)(struct socket *sock, int how); > int (*socket_sock_rcv_skb)(struct sock *sk, struct sk_buff *skb); > - int (*socket_getpeersec_stream)(struct socket *sock, > - char __user *optval, > - int __user *optlen, unsigned len); > + int (*socket_getpeersec_stream)(struct socket *sock, char **optval, > + int *optlen, unsigned len); > int (*socket_getpeersec_dgram)(struct socket *sock, > struct sk_buff *skb, u32 *secid); > int (*sk_alloc_security)(struct sock *sk, int family, gfp_t priority); > diff --git a/include/linux/security.h b/include/linux/security.h > index d7af2bbbc878..26967055a002 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -178,6 +178,7 @@ struct lsmblob { > #define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */ > #define LSMBLOB_DISPLAY -4 /* Use the "display" slot */ > #define LSMBLOB_FIRST -5 /* Use the default "display" slot */ > +#define LSMBLOB_COMPOUND -6 /* A compound "display" */ > > /** > * lsmblob_init - initialize an lsmblob structure. > @@ -1396,7 +1397,8 @@ int security_socket_setsockopt(struct socket *sock, int level, int optname); > int security_socket_shutdown(struct socket *sock, int how); > int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb); > int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, > - int __user *optlen, unsigned len); > + int __user *optlen, unsigned len, > + int display); > int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, > struct lsmblob *blob); > int security_sk_alloc(struct sock *sk, int family, gfp_t priority); > @@ -1530,8 +1532,10 @@ static inline int security_sock_rcv_skb(struct sock *sk, > return 0; > } > > -static inline int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, > - int __user *optlen, unsigned len) > +static inline int security_socket_getpeersec_stream(struct socket *sock, > + char __user *optval, > + int __user *optlen, > + unsigned len, int display) > { > return -ENOPROTOOPT; > } > diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h > index 77f7c1638eb1..e3a853d53705 100644 > --- a/include/uapi/asm-generic/socket.h > +++ b/include/uapi/asm-generic/socket.h > @@ -118,6 +118,7 @@ > #define SO_SNDTIMEO_NEW 67 > > #define SO_DETACH_REUSEPORT_BPF 68 > +#define SO_PEERCONTEXT 69 > > #if !defined(__KERNEL__) > > diff --git a/net/core/sock.c b/net/core/sock.c > index 043db3ce023e..63b7eda81a90 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -1411,7 +1411,12 @@ int sock_getsockopt(struct socket *sock, int level, int optname, > break; > > case SO_PEERSEC: > - return security_socket_getpeersec_stream(sock, optval, optlen, len); > + return security_socket_getpeersec_stream(sock, optval, optlen, > + len, LSMBLOB_DISPLAY); > + > + case SO_PEERCONTEXT: > + return security_socket_getpeersec_stream(sock, optval, optlen, > + len, LSMBLOB_COMPOUND); > > case SO_MARK: > v.val = sk->sk_mark; > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c > index 16b992235c11..34edfd29c32f 100644 > --- a/security/apparmor/lsm.c > +++ b/security/apparmor/lsm.c > @@ -1078,10 +1078,8 @@ static struct aa_label *sk_peer_label(struct sock *sk) > * > * Note: for tcp only valid if using ipsec or cipso on lan > */ > -static int apparmor_socket_getpeersec_stream(struct socket *sock, > - char __user *optval, > - int __user *optlen, > - unsigned int len) > +static int apparmor_socket_getpeersec_stream(struct socket *sock, char **optval, > + int *optlen, unsigned int len) > { > char *name; > int slen, error = 0; > @@ -1101,17 +1099,11 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock, > if (slen < 0) { > error = -ENOMEM; > } else { > - if (slen > len) { > + if (slen > len) > error = -ERANGE; > - } else if (copy_to_user(optval, name, slen)) { > - error = -EFAULT; > - goto out; > - } > - if (put_user(slen, optlen)) > - error = -EFAULT; > -out: > - kfree(name); > - > + else > + *optval = name; > + *optlen = slen; > } > > done: > diff --git a/security/security.c b/security/security.c > index 6d05222aac9c..80539dfd0245 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -723,6 +723,42 @@ static void __init lsm_early_task(struct task_struct *task) > panic("%s: Early task alloc failed.\n", __func__); > } > > +/** > + * append_ctx - append a lsm/context pair to a compound context > + * @ctx: the existing compound context > + * @ctxlen: size of the old context, including terminating nul byte > + * @lsm: new lsm name, nul terminated > + * @new: new context, possibly nul terminated > + * @newlen: maximum size of @new > + * > + * replace @ctx with a new compound context, appending @newlsm and @new > + * to @ctx. On exit the new data replaces the old, which is freed. > + * @ctxlen is set to the new size, which includes a trailing nul byte. > + * > + * Returns 0 on success, -ENOMEM if no memory is available. > + */ > +static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new, > + int newlen) > +{ > + char *final; > + int llen; > + > + llen = strlen(lsm) + 1; > + newlen = strnlen(new, newlen) + 1; > + > + final = kzalloc(*ctxlen + llen + newlen, GFP_KERNEL); > + if (final == NULL) > + return -ENOMEM; > + if (*ctxlen) > + memcpy(final, *ctx, *ctxlen); > + memcpy(final + *ctxlen, lsm, llen); > + memcpy(final + *ctxlen + llen, new, newlen); > + kfree(*ctx); > + *ctx = final; > + *ctxlen = *ctxlen + llen + newlen; > + return 0; > +} > + > /* > * Hook list operation macros. > * > @@ -2164,8 +2200,8 @@ int security_setprocattr(const char *lsm, const char *name, void *value, > hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { > if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm)) > continue; > - if (lsm == NULL && *display != LSMBLOB_INVALID && > - *display != hp->lsmid->slot) > + if (lsm == NULL && display != NULL && > + *display != LSMBLOB_INVALID && *display != hp->lsmid->slot) > continue; > return hp->hook.setprocattr(name, value, size); > } > @@ -2245,12 +2281,21 @@ void security_release_secctx(struct lsmcontext *cp) > { > struct security_hook_list *hp; > > + if (cp->slot == LSMBLOB_INVALID) > + return; > + > + if (cp->slot == LSMBLOB_COMPOUND) { > + kfree(cp->context); > + goto clear_out; > + } > + > hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list) > if (cp->slot == hp->lsmid->slot) { > hp->hook.release_secctx(cp->context, cp->len); > break; > } > > +clear_out: > memset(cp, 0, sizeof(*cp)); > } > EXPORT_SYMBOL(security_release_secctx); > @@ -2383,17 +2428,67 @@ int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) > EXPORT_SYMBOL(security_sock_rcv_skb); > > int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, > - int __user *optlen, unsigned len) > + int __user *optlen, unsigned len, > + int display) > { > - int display = lsm_task_display(current); > struct security_hook_list *hp; > + char *final = NULL; > + char *cp; > + int rc = 0; > + unsigned finallen = 0; > + unsigned clen = 0; > > - hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, > - list) > - if (display == LSMBLOB_INVALID || display == hp->lsmid->slot) > - return hp->hook.socket_getpeersec_stream(sock, optval, > - optlen, len); > - return -ENOPROTOOPT; > + switch (display) { > + case LSMBLOB_DISPLAY: > + rc = -ENOPROTOOPT; > + display = lsm_task_display(current); > + hlist_for_each_entry(hp, > + &security_hook_heads.socket_getpeersec_stream, > + list) > + if (display == LSMBLOB_INVALID || > + display == hp->lsmid->slot) { > + rc = hp->hook.socket_getpeersec_stream(sock, > + &final, &finallen, len); > + break; > + } > + break; > + case LSMBLOB_COMPOUND: > + /* > + * A compound context, in the form [lsm\0value\0]... > + */ > + hlist_for_each_entry(hp, > + &security_hook_heads.socket_getpeersec_stream, > + list) { > + rc = hp->hook.socket_getpeersec_stream(sock, &cp, &clen, > + len); > + if (rc == -EINVAL || rc == -ENOPROTOOPT) { > + rc = 0; > + continue; > + } > + if (rc) { > + kfree(final); > + return rc; > + } > + rc = append_ctx(&final, &finallen, hp->lsmid->lsm, > + cp, clen); > + } > + if (final == NULL) > + return -EINVAL; > + break; > + default: > + return -EINVAL; > + } > + > + if (finallen > len) > + rc = -ERANGE; > + else if (copy_to_user(optval, final, finallen)) > + rc = -EFAULT; > + > + if (put_user(finallen, optlen)) > + rc = -EFAULT; > + > + kfree(final); > + return rc; > } > > int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index cd4743331800..c3e6fd3f8c56 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -5056,10 +5056,8 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) > return err; > } > > -static int selinux_socket_getpeersec_stream(struct socket *sock, > - char __user *optval, > - int __user *optlen, > - unsigned int len) > +static int selinux_socket_getpeersec_stream(struct socket *sock, char **optval, > + int *optlen, unsigned int len) > { > int err = 0; > char *scontext; > @@ -5079,18 +5077,12 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, > if (err) > return err; > > - if (scontext_len > len) { > + if (scontext_len > len) > err = -ERANGE; > - goto out_len; > - } > - > - if (copy_to_user(optval, scontext, scontext_len)) > - err = -EFAULT; > + else > + *optval = scontext; > > -out_len: > - if (put_user(scontext_len, optlen)) > - err = -EFAULT; > - kfree(scontext); > + *optlen = scontext_len; > return err; > } > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 9ce67e03ac49..316c5faf9053 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -3957,28 +3957,29 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) > * > * returns zero on success, an error code otherwise > */ > -static int smack_socket_getpeersec_stream(struct socket *sock, > - char __user *optval, > - int __user *optlen, unsigned len) > +static int smack_socket_getpeersec_stream(struct socket *sock, char **optval, > + int *optlen, unsigned len) > { > - struct socket_smack *ssp; > - char *rcp = ""; > - int slen = 1; > + struct socket_smack *ssp = smack_sock(sock->sk); > + char *rcp; > + int slen; > int rc = 0; > > - ssp = smack_sock(sock->sk); > - if (ssp->smk_packet != NULL) { > - rcp = ssp->smk_packet->smk_known; > - slen = strlen(rcp) + 1; > + if (ssp->smk_packet == NULL) { > + *optlen = 0; > + return -EINVAL; > } > > + rcp = ssp->smk_packet->smk_known; > + slen = strlen(rcp) + 1; > if (slen > len) > rc = -ERANGE; > - else if (copy_to_user(optval, rcp, slen) != 0) > - rc = -EFAULT; > - > - if (put_user(slen, optlen) != 0) > - rc = -EFAULT; > + else { > + *optval = kstrdup(rcp, GFP_KERNEL); > + if (*optval == NULL) > + rc = -ENOMEM; > + } > + *optlen = slen; > > return rc; > } >
On Mon, 06 Jan 2020 at 12:15:57 -0500, Stephen Smalley wrote: > On 12/24/19 6:59 PM, Casey Schaufler wrote: > > The getsockopt SO_PEERSEC provides the LSM based security > > information for a single module, but for reasons of backward > > compatibility cannot include the information for multiple > > modules. A new option SO_PEERCONTEXT is added to report the > > security "context" of multiple modules using a "compound" format > > > > lsm1\0value\0lsm2\0value\0 > > > > This is expected to be used by system services, including dbus-daemon. > > The exact format of a compound context has been the subject of > > considerable debate. This format was suggested by Simon McVittie, > > a dbus maintainer with a significant stake in the format being > > usable. > > Since upstream AA does not currently ever set the peer label info, there is > no need for this support for stacking upstream AA today, and there is no way > to test this functionality with more than one module present currently in an > upstream kernel. Either fix AA to actually implement the functionality so > it can be tested properly, or drop it from this series please. I don't > understand why AA continues to keep this kind of basic and longstanding > downstream functionality out of tree. Alternatively, a pair of tiny in-tree or out-of-tree stackable LSMs that don't make any security decisions, and label every labellable process/socket/thing with something predictable, would make it really easy for both kernel and user-space developers to test this and the user-space code that uses it (D-Bus and others). For example, they could label process 1234 and all sockets created by process 1234 with "contexttest1\0pid1234\0contexttest2\0process1234" or something like that. I'd love to see AppArmor in upstream kernels support SO_PEERSEC and SO_PEERCONTEXT, but setting up a development machine to stack AppArmor and SELinux (and still be able to boot, without one or the other LSM forbidding something important) seems likely to be harder than setting it up to load some toy LSMs. smcv
On 1/6/2020 9:29 AM, Simon McVittie wrote: > On Mon, 06 Jan 2020 at 12:15:57 -0500, Stephen Smalley wrote: >> On 12/24/19 6:59 PM, Casey Schaufler wrote: >>> The getsockopt SO_PEERSEC provides the LSM based security >>> information for a single module, but for reasons of backward >>> compatibility cannot include the information for multiple >>> modules. A new option SO_PEERCONTEXT is added to report the >>> security "context" of multiple modules using a "compound" format >>> >>> lsm1\0value\0lsm2\0value\0 >>> >>> This is expected to be used by system services, including dbus-daemon. >>> The exact format of a compound context has been the subject of >>> considerable debate. This format was suggested by Simon McVittie, >>> a dbus maintainer with a significant stake in the format being >>> usable. >> Since upstream AA does not currently ever set the peer label info, there is >> no need for this support for stacking upstream AA today, and there is no way >> to test this functionality with more than one module present currently in an >> upstream kernel. Either fix AA to actually implement the functionality so >> it can be tested properly, or drop it from this series please. I agree that SO_PEERCONTEXT can be deferred until such time as we have AppArmor upstream support for SO_PEERSEC. >> I don't >> understand why AA continues to keep this kind of basic and longstanding >> downstream functionality out of tree. Not everyone has the resource commitments of the world's largest government. :( > Alternatively, a pair of tiny in-tree or out-of-tree stackable LSMs > that don't make any security decisions, and label every labellable > process/socket/thing with something predictable, would make it really > easy for both kernel and user-space developers to test this and the > user-space code that uses it (D-Bus and others). Sounds like a fun and educational project. Maybe one of our lurkers could do something clever. > > For example, they could label process 1234 and all sockets created by > process 1234 with "contexttest1\0pid1234\0contexttest2\0process1234" or > something like that. > > I'd love to see AppArmor in upstream kernels support SO_PEERSEC and > SO_PEERCONTEXT, but setting up a development machine to stack AppArmor > and SELinux (and still be able to boot, without one or the other LSM > forbidding something important) seems likely to be harder than setting > it up to load some toy LSMs. > > smcv
On 1/6/20 12:29 PM, Simon McVittie wrote: > On Mon, 06 Jan 2020 at 12:15:57 -0500, Stephen Smalley wrote: >> On 12/24/19 6:59 PM, Casey Schaufler wrote: >>> The getsockopt SO_PEERSEC provides the LSM based security >>> information for a single module, but for reasons of backward >>> compatibility cannot include the information for multiple >>> modules. A new option SO_PEERCONTEXT is added to report the >>> security "context" of multiple modules using a "compound" format >>> >>> lsm1\0value\0lsm2\0value\0 >>> >>> This is expected to be used by system services, including dbus-daemon. >>> The exact format of a compound context has been the subject of >>> considerable debate. This format was suggested by Simon McVittie, >>> a dbus maintainer with a significant stake in the format being >>> usable. >> >> Since upstream AA does not currently ever set the peer label info, there is >> no need for this support for stacking upstream AA today, and there is no way >> to test this functionality with more than one module present currently in an >> upstream kernel. Either fix AA to actually implement the functionality so >> it can be tested properly, or drop it from this series please. I don't >> understand why AA continues to keep this kind of basic and longstanding >> downstream functionality out of tree. > > Alternatively, a pair of tiny in-tree or out-of-tree stackable LSMs > that don't make any security decisions, and label every labellable > process/socket/thing with something predictable, would make it really > easy for both kernel and user-space developers to test this and the > user-space code that uses it (D-Bus and others). > > For example, they could label process 1234 and all sockets created by > process 1234 with "contexttest1\0pid1234\0contexttest2\0process1234" or > something like that. > > I'd love to see AppArmor in upstream kernels support SO_PEERSEC and > SO_PEERCONTEXT, but setting up a development machine to stack AppArmor > and SELinux (and still be able to boot, without one or the other LSM > forbidding something important) seems likely to be harder than setting > it up to load some toy LSMs. AA+SELinux with these patches boots fine for me with Fedora; it doesn't load any policy for AA but you still get a compound context from /proc/pid/attr/context. Should be similar for booting with a distro that only enables AA by default; you'll get "kernel" for the SELinux part of the compound label in the absence of any policy loaded.
On 1/6/20 1:03 PM, Casey Schaufler wrote: > On 1/6/2020 9:29 AM, Simon McVittie wrote: >> On Mon, 06 Jan 2020 at 12:15:57 -0500, Stephen Smalley wrote: >>> On 12/24/19 6:59 PM, Casey Schaufler wrote: >>>> The getsockopt SO_PEERSEC provides the LSM based security >>>> information for a single module, but for reasons of backward >>>> compatibility cannot include the information for multiple >>>> modules. A new option SO_PEERCONTEXT is added to report the >>>> security "context" of multiple modules using a "compound" format >>>> >>>> lsm1\0value\0lsm2\0value\0 >>>> >>>> This is expected to be used by system services, including dbus-daemon. >>>> The exact format of a compound context has been the subject of >>>> considerable debate. This format was suggested by Simon McVittie, >>>> a dbus maintainer with a significant stake in the format being >>>> usable. >>> Since upstream AA does not currently ever set the peer label info, there is >>> no need for this support for stacking upstream AA today, and there is no way >>> to test this functionality with more than one module present currently in an >>> upstream kernel. Either fix AA to actually implement the functionality so >>> it can be tested properly, or drop it from this series please. > > I agree that SO_PEERCONTEXT can be deferred until such time as we have > AppArmor upstream support for SO_PEERSEC. > >>> I don't >>> understand why AA continues to keep this kind of basic and longstanding >>> downstream functionality out of tree. > > Not everyone has the resource commitments of the world's largest > government. :( How hard is it to upstream code that is a) entirely contained within the AA security module, and b) already shipping in Ubuntu kernels for quite some time? Seems to be more of a lack of an upstream-first philosophy than a resources issue... > >> Alternatively, a pair of tiny in-tree or out-of-tree stackable LSMs >> that don't make any security decisions, and label every labellable >> process/socket/thing with something predictable, would make it really >> easy for both kernel and user-space developers to test this and the >> user-space code that uses it (D-Bus and others). > > Sounds like a fun and educational project. Maybe one of our lurkers > could do something clever. > >> >> For example, they could label process 1234 and all sockets created by >> process 1234 with "contexttest1\0pid1234\0contexttest2\0process1234" or >> something like that. >> >> I'd love to see AppArmor in upstream kernels support SO_PEERSEC and >> SO_PEERCONTEXT, but setting up a development machine to stack AppArmor >> and SELinux (and still be able to boot, without one or the other LSM >> forbidding something important) seems likely to be harder than setting >> it up to load some toy LSMs. > >> >> smcv >
diff --git a/Documentation/security/lsm.rst b/Documentation/security/lsm.rst index aadf47c808c0..77cc326a52cc 100644 --- a/Documentation/security/lsm.rst +++ b/Documentation/security/lsm.rst @@ -199,3 +199,18 @@ capability-related fields: - ``fs/nfsd/auth.c``::c:func:`nfsd_setuser()` - ``fs/proc/array.c``::c:func:`task_cap()` + +LSM External Interfaces +======================= + +The LSM infrastructure does not generally provide external interfaces. +The individual security modules provide what external interfaces they +require. The infrastructure does provide two interfaces for the special +case where multiple security modules provide a process context. This +is provided in compound context format. + +- `lsm1\0value\0lsm2\0value\0` + +The special file ``/proc/pid/attr/context`` provides the security +context of the identified process. The socket option SO_PEERCONTEXT +provides the security context of a packet. diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h index de6c4df61082..b26fb34e4226 100644 --- a/arch/alpha/include/uapi/asm/socket.h +++ b/arch/alpha/include/uapi/asm/socket.h @@ -123,6 +123,7 @@ #define SO_SNDTIMEO_NEW 67 #define SO_DETACH_REUSEPORT_BPF 68 +#define SO_PEERCONTEXT 69 #if !defined(__KERNEL__) diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h index d0a9ed2ca2d6..10e03507b1ed 100644 --- a/arch/mips/include/uapi/asm/socket.h +++ b/arch/mips/include/uapi/asm/socket.h @@ -134,6 +134,7 @@ #define SO_SNDTIMEO_NEW 67 #define SO_DETACH_REUSEPORT_BPF 68 +#define SO_PEERCONTEXT 69 #if !defined(__KERNEL__) diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h index 10173c32195e..e11df59a84d1 100644 --- a/arch/parisc/include/uapi/asm/socket.h +++ b/arch/parisc/include/uapi/asm/socket.h @@ -115,6 +115,7 @@ #define SO_SNDTIMEO_NEW 0x4041 #define SO_DETACH_REUSEPORT_BPF 0x4042 +#define SO_PEERCONTEXT 0x4043 #if !defined(__KERNEL__) diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h index 8029b681fc7c..5b41ef778040 100644 --- a/arch/sparc/include/uapi/asm/socket.h +++ b/arch/sparc/include/uapi/asm/socket.h @@ -116,6 +116,7 @@ #define SO_SNDTIMEO_NEW 0x0045 #define SO_DETACH_REUSEPORT_BPF 0x0047 +#define SO_PEERCONTEXT 0x0048 #if !defined(__KERNEL__) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 2bf82e1cf347..2ae10e7f81a7 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -880,8 +880,8 @@ * SO_GETPEERSEC. For tcp sockets this can be meaningful if the * socket is associated with an ipsec SA. * @sock is the local socket. - * @optval userspace memory where the security state is to be copied. - * @optlen userspace int where the module should copy the actual length + * @optval memory where the security state is to be copied. + * @optlen int where the module should copy the actual length * of the security state. * @len as input is the maximum length to copy to userspace provided * by the caller. @@ -1724,9 +1724,8 @@ union security_list_options { int (*socket_setsockopt)(struct socket *sock, int level, int optname); int (*socket_shutdown)(struct socket *sock, int how); int (*socket_sock_rcv_skb)(struct sock *sk, struct sk_buff *skb); - int (*socket_getpeersec_stream)(struct socket *sock, - char __user *optval, - int __user *optlen, unsigned len); + int (*socket_getpeersec_stream)(struct socket *sock, char **optval, + int *optlen, unsigned len); int (*socket_getpeersec_dgram)(struct socket *sock, struct sk_buff *skb, u32 *secid); int (*sk_alloc_security)(struct sock *sk, int family, gfp_t priority); diff --git a/include/linux/security.h b/include/linux/security.h index d7af2bbbc878..26967055a002 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -178,6 +178,7 @@ struct lsmblob { #define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */ #define LSMBLOB_DISPLAY -4 /* Use the "display" slot */ #define LSMBLOB_FIRST -5 /* Use the default "display" slot */ +#define LSMBLOB_COMPOUND -6 /* A compound "display" */ /** * lsmblob_init - initialize an lsmblob structure. @@ -1396,7 +1397,8 @@ int security_socket_setsockopt(struct socket *sock, int level, int optname); int security_socket_shutdown(struct socket *sock, int how); int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb); int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, - int __user *optlen, unsigned len); + int __user *optlen, unsigned len, + int display); int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, struct lsmblob *blob); int security_sk_alloc(struct sock *sk, int family, gfp_t priority); @@ -1530,8 +1532,10 @@ static inline int security_sock_rcv_skb(struct sock *sk, return 0; } -static inline int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, - int __user *optlen, unsigned len) +static inline int security_socket_getpeersec_stream(struct socket *sock, + char __user *optval, + int __user *optlen, + unsigned len, int display) { return -ENOPROTOOPT; } diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h index 77f7c1638eb1..e3a853d53705 100644 --- a/include/uapi/asm-generic/socket.h +++ b/include/uapi/asm-generic/socket.h @@ -118,6 +118,7 @@ #define SO_SNDTIMEO_NEW 67 #define SO_DETACH_REUSEPORT_BPF 68 +#define SO_PEERCONTEXT 69 #if !defined(__KERNEL__) diff --git a/net/core/sock.c b/net/core/sock.c index 043db3ce023e..63b7eda81a90 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1411,7 +1411,12 @@ int sock_getsockopt(struct socket *sock, int level, int optname, break; case SO_PEERSEC: - return security_socket_getpeersec_stream(sock, optval, optlen, len); + return security_socket_getpeersec_stream(sock, optval, optlen, + len, LSMBLOB_DISPLAY); + + case SO_PEERCONTEXT: + return security_socket_getpeersec_stream(sock, optval, optlen, + len, LSMBLOB_COMPOUND); case SO_MARK: v.val = sk->sk_mark; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 16b992235c11..34edfd29c32f 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1078,10 +1078,8 @@ static struct aa_label *sk_peer_label(struct sock *sk) * * Note: for tcp only valid if using ipsec or cipso on lan */ -static int apparmor_socket_getpeersec_stream(struct socket *sock, - char __user *optval, - int __user *optlen, - unsigned int len) +static int apparmor_socket_getpeersec_stream(struct socket *sock, char **optval, + int *optlen, unsigned int len) { char *name; int slen, error = 0; @@ -1101,17 +1099,11 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock, if (slen < 0) { error = -ENOMEM; } else { - if (slen > len) { + if (slen > len) error = -ERANGE; - } else if (copy_to_user(optval, name, slen)) { - error = -EFAULT; - goto out; - } - if (put_user(slen, optlen)) - error = -EFAULT; -out: - kfree(name); - + else + *optval = name; + *optlen = slen; } done: diff --git a/security/security.c b/security/security.c index 6d05222aac9c..80539dfd0245 100644 --- a/security/security.c +++ b/security/security.c @@ -723,6 +723,42 @@ static void __init lsm_early_task(struct task_struct *task) panic("%s: Early task alloc failed.\n", __func__); } +/** + * append_ctx - append a lsm/context pair to a compound context + * @ctx: the existing compound context + * @ctxlen: size of the old context, including terminating nul byte + * @lsm: new lsm name, nul terminated + * @new: new context, possibly nul terminated + * @newlen: maximum size of @new + * + * replace @ctx with a new compound context, appending @newlsm and @new + * to @ctx. On exit the new data replaces the old, which is freed. + * @ctxlen is set to the new size, which includes a trailing nul byte. + * + * Returns 0 on success, -ENOMEM if no memory is available. + */ +static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new, + int newlen) +{ + char *final; + int llen; + + llen = strlen(lsm) + 1; + newlen = strnlen(new, newlen) + 1; + + final = kzalloc(*ctxlen + llen + newlen, GFP_KERNEL); + if (final == NULL) + return -ENOMEM; + if (*ctxlen) + memcpy(final, *ctx, *ctxlen); + memcpy(final + *ctxlen, lsm, llen); + memcpy(final + *ctxlen + llen, new, newlen); + kfree(*ctx); + *ctx = final; + *ctxlen = *ctxlen + llen + newlen; + return 0; +} + /* * Hook list operation macros. * @@ -2164,8 +2200,8 @@ int security_setprocattr(const char *lsm, const char *name, void *value, hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) { if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm)) continue; - if (lsm == NULL && *display != LSMBLOB_INVALID && - *display != hp->lsmid->slot) + if (lsm == NULL && display != NULL && + *display != LSMBLOB_INVALID && *display != hp->lsmid->slot) continue; return hp->hook.setprocattr(name, value, size); } @@ -2245,12 +2281,21 @@ void security_release_secctx(struct lsmcontext *cp) { struct security_hook_list *hp; + if (cp->slot == LSMBLOB_INVALID) + return; + + if (cp->slot == LSMBLOB_COMPOUND) { + kfree(cp->context); + goto clear_out; + } + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list) if (cp->slot == hp->lsmid->slot) { hp->hook.release_secctx(cp->context, cp->len); break; } +clear_out: memset(cp, 0, sizeof(*cp)); } EXPORT_SYMBOL(security_release_secctx); @@ -2383,17 +2428,67 @@ int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) EXPORT_SYMBOL(security_sock_rcv_skb); int security_socket_getpeersec_stream(struct socket *sock, char __user *optval, - int __user *optlen, unsigned len) + int __user *optlen, unsigned len, + int display) { - int display = lsm_task_display(current); struct security_hook_list *hp; + char *final = NULL; + char *cp; + int rc = 0; + unsigned finallen = 0; + unsigned clen = 0; - hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream, - list) - if (display == LSMBLOB_INVALID || display == hp->lsmid->slot) - return hp->hook.socket_getpeersec_stream(sock, optval, - optlen, len); - return -ENOPROTOOPT; + switch (display) { + case LSMBLOB_DISPLAY: + rc = -ENOPROTOOPT; + display = lsm_task_display(current); + hlist_for_each_entry(hp, + &security_hook_heads.socket_getpeersec_stream, + list) + if (display == LSMBLOB_INVALID || + display == hp->lsmid->slot) { + rc = hp->hook.socket_getpeersec_stream(sock, + &final, &finallen, len); + break; + } + break; + case LSMBLOB_COMPOUND: + /* + * A compound context, in the form [lsm\0value\0]... + */ + hlist_for_each_entry(hp, + &security_hook_heads.socket_getpeersec_stream, + list) { + rc = hp->hook.socket_getpeersec_stream(sock, &cp, &clen, + len); + if (rc == -EINVAL || rc == -ENOPROTOOPT) { + rc = 0; + continue; + } + if (rc) { + kfree(final); + return rc; + } + rc = append_ctx(&final, &finallen, hp->lsmid->lsm, + cp, clen); + } + if (final == NULL) + return -EINVAL; + break; + default: + return -EINVAL; + } + + if (finallen > len) + rc = -ERANGE; + else if (copy_to_user(optval, final, finallen)) + rc = -EFAULT; + + if (put_user(finallen, optlen)) + rc = -EFAULT; + + kfree(final); + return rc; } int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index cd4743331800..c3e6fd3f8c56 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5056,10 +5056,8 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) return err; } -static int selinux_socket_getpeersec_stream(struct socket *sock, - char __user *optval, - int __user *optlen, - unsigned int len) +static int selinux_socket_getpeersec_stream(struct socket *sock, char **optval, + int *optlen, unsigned int len) { int err = 0; char *scontext; @@ -5079,18 +5077,12 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, if (err) return err; - if (scontext_len > len) { + if (scontext_len > len) err = -ERANGE; - goto out_len; - } - - if (copy_to_user(optval, scontext, scontext_len)) - err = -EFAULT; + else + *optval = scontext; -out_len: - if (put_user(scontext_len, optlen)) - err = -EFAULT; - kfree(scontext); + *optlen = scontext_len; return err; } diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 9ce67e03ac49..316c5faf9053 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3957,28 +3957,29 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) * * returns zero on success, an error code otherwise */ -static int smack_socket_getpeersec_stream(struct socket *sock, - char __user *optval, - int __user *optlen, unsigned len) +static int smack_socket_getpeersec_stream(struct socket *sock, char **optval, + int *optlen, unsigned len) { - struct socket_smack *ssp; - char *rcp = ""; - int slen = 1; + struct socket_smack *ssp = smack_sock(sock->sk); + char *rcp; + int slen; int rc = 0; - ssp = smack_sock(sock->sk); - if (ssp->smk_packet != NULL) { - rcp = ssp->smk_packet->smk_known; - slen = strlen(rcp) + 1; + if (ssp->smk_packet == NULL) { + *optlen = 0; + return -EINVAL; } + rcp = ssp->smk_packet->smk_known; + slen = strlen(rcp) + 1; if (slen > len) rc = -ERANGE; - else if (copy_to_user(optval, rcp, slen) != 0) - rc = -EFAULT; - - if (put_user(slen, optlen) != 0) - rc = -EFAULT; + else { + *optval = kstrdup(rcp, GFP_KERNEL); + if (*optval == NULL) + rc = -ENOMEM; + } + *optlen = slen; return rc; }
The getsockopt SO_PEERSEC provides the LSM based security information for a single module, but for reasons of backward compatibility cannot include the information for multiple modules. A new option SO_PEERCONTEXT is added to report the security "context" of multiple modules using a "compound" format lsm1\0value\0lsm2\0value\0 This is expected to be used by system services, including dbus-daemon. The exact format of a compound context has been the subject of considerable debate. This format was suggested by Simon McVittie, a dbus maintainer with a significant stake in the format being usable. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> cc: netdev@vger.kernel.org cc: linux-api@vger.kernel.org --- Documentation/security/lsm.rst | 15 ++++ arch/alpha/include/uapi/asm/socket.h | 1 + arch/mips/include/uapi/asm/socket.h | 1 + arch/parisc/include/uapi/asm/socket.h | 1 + arch/sparc/include/uapi/asm/socket.h | 1 + include/linux/lsm_hooks.h | 9 +- include/linux/security.h | 10 ++- include/uapi/asm-generic/socket.h | 1 + net/core/sock.c | 7 +- security/apparmor/lsm.c | 20 ++--- security/security.c | 115 +++++++++++++++++++++++--- security/selinux/hooks.c | 20 ++--- security/smack/smack_lsm.c | 31 +++---- 13 files changed, 170 insertions(+), 62 deletions(-)