From 3df98d79215ace13d1e91ddfc5a67a0f5acbd83f Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Sun, 27 Sep 2020 22:38:26 -0400 Subject: lsm,selinux: pass flowi_common instead of flowi to the LSM hooks As pointed out by Herbert in a recent related patch, the LSM hooks do not have the necessary address family information to use the flowi struct safely. As none of the LSMs currently use any of the protocol specific flowi information, replace the flowi pointers with pointers to the address family independent flowi_common struct. Reported-by: Herbert Xu Acked-by: James Morris Signed-off-by: Paul Moore --- include/linux/security.h | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'include/linux/security.h') diff --git a/include/linux/security.h b/include/linux/security.h index bc2725491560..469273e8b46d 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -167,7 +167,7 @@ struct sk_buff; struct sock; struct sockaddr; struct socket; -struct flowi; +struct flowi_common; struct dst_entry; struct xfrm_selector; struct xfrm_policy; @@ -1355,8 +1355,9 @@ int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u int security_sk_alloc(struct sock *sk, int family, gfp_t priority); void security_sk_free(struct sock *sk); void security_sk_clone(const struct sock *sk, struct sock *newsk); -void security_sk_classify_flow(struct sock *sk, struct flowi *fl); -void security_req_classify_flow(const struct request_sock *req, struct flowi *fl); +void security_sk_classify_flow(struct sock *sk, struct flowi_common *flic); +void security_req_classify_flow(const struct request_sock *req, + struct flowi_common *flic); void security_sock_graft(struct sock*sk, struct socket *parent); int security_inet_conn_request(struct sock *sk, struct sk_buff *skb, struct request_sock *req); @@ -1507,11 +1508,13 @@ static inline void security_sk_clone(const struct sock *sk, struct sock *newsk) { } -static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl) +static inline void security_sk_classify_flow(struct sock *sk, + struct flowi_common *flic) { } -static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl) +static inline void security_req_classify_flow(const struct request_sock *req, + struct flowi_common *flic) { } @@ -1638,9 +1641,9 @@ void security_xfrm_state_free(struct xfrm_state *x); int security_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir); int security_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp, - const struct flowi *fl); + const struct flowi_common *flic); int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid); -void security_skb_classify_flow(struct sk_buff *skb, struct flowi *fl); +void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic); #else /* CONFIG_SECURITY_NETWORK_XFRM */ @@ -1692,7 +1695,8 @@ static inline int security_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_s } static inline int security_xfrm_state_pol_flow_match(struct xfrm_state *x, - struct xfrm_policy *xp, const struct flowi *fl) + struct xfrm_policy *xp, + const struct flowi_common *flic) { return 1; } @@ -1702,7 +1706,8 @@ static inline int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid) return 0; } -static inline void security_skb_classify_flow(struct sk_buff *skb, struct flowi *fl) +static inline void security_skb_classify_flow(struct sk_buff *skb, + struct flowi_common *flic) { } -- cgit v1.3-14-g43fede From c7a5899eb26e2a4d516d53f65b6dd67be2228041 Mon Sep 17 00:00:00 2001 From: Antony Antony Date: Tue, 17 Nov 2020 17:47:23 +0100 Subject: xfrm: redact SA secret with lockdown confidentiality redact XFRM SA secret in the netlink response to xfrm_get_sa() or dumpall sa. Enable lockdown, confidentiality mode, at boot or at run time. e.g. when enabled: cat /sys/kernel/security/lockdown none integrity [confidentiality] ip xfrm state src 172.16.1.200 dst 172.16.1.100 proto esp spi 0x00000002 reqid 2 mode tunnel replay-window 0 aead rfc4106(gcm(aes)) 0x0000000000000000000000000000000000000000 96 note: the aead secret is redacted. Redacting secret is also a FIPS 140-2 requirement. v1->v2 - add size checks before memset calls v2->v3 - replace spaces with tabs for consistency v3->v4 - use kernel lockdown instead of a /proc setting v4->v5 - remove kconfig option Reviewed-by: Stephan Mueller Signed-off-by: Antony Antony Signed-off-by: Steffen Klassert --- include/linux/security.h | 1 + net/xfrm/xfrm_user.c | 74 +++++++++++++++++++++++++++++++++++++++++++----- security/security.c | 1 + 3 files changed, 69 insertions(+), 7 deletions(-) (limited to 'include/linux/security.h') diff --git a/include/linux/security.h b/include/linux/security.h index bc2725491560..1112a79a7dba 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -127,6 +127,7 @@ enum lockdown_reason { LOCKDOWN_PERF, LOCKDOWN_TRACEFS, LOCKDOWN_XMON_RW, + LOCKDOWN_XFRM_SECRET, LOCKDOWN_CONFIDENTIALITY_MAX, }; diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index d0c32a8fcc4a..0727ac853b55 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -848,21 +848,84 @@ static int copy_user_offload(struct xfrm_state_offload *xso, struct sk_buff *skb return 0; } +static bool xfrm_redact(void) +{ + return IS_ENABLED(CONFIG_SECURITY) && + security_locked_down(LOCKDOWN_XFRM_SECRET); +} + static int copy_to_user_auth(struct xfrm_algo_auth *auth, struct sk_buff *skb) { struct xfrm_algo *algo; + struct xfrm_algo_auth *ap; struct nlattr *nla; + bool redact_secret = xfrm_redact(); nla = nla_reserve(skb, XFRMA_ALG_AUTH, sizeof(*algo) + (auth->alg_key_len + 7) / 8); if (!nla) return -EMSGSIZE; - algo = nla_data(nla); strncpy(algo->alg_name, auth->alg_name, sizeof(algo->alg_name)); - memcpy(algo->alg_key, auth->alg_key, (auth->alg_key_len + 7) / 8); + + if (redact_secret && auth->alg_key_len) + memset(algo->alg_key, 0, (auth->alg_key_len + 7) / 8); + else + memcpy(algo->alg_key, auth->alg_key, + (auth->alg_key_len + 7) / 8); algo->alg_key_len = auth->alg_key_len; + nla = nla_reserve(skb, XFRMA_ALG_AUTH_TRUNC, xfrm_alg_auth_len(auth)); + if (!nla) + return -EMSGSIZE; + ap = nla_data(nla); + memcpy(ap, auth, sizeof(struct xfrm_algo_auth)); + if (redact_secret && auth->alg_key_len) + memset(ap->alg_key, 0, (auth->alg_key_len + 7) / 8); + else + memcpy(ap->alg_key, auth->alg_key, + (auth->alg_key_len + 7) / 8); + return 0; +} + +static int copy_to_user_aead(struct xfrm_algo_aead *aead, struct sk_buff *skb) +{ + struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_AEAD, aead_len(aead)); + struct xfrm_algo_aead *ap; + bool redact_secret = xfrm_redact(); + + if (!nla) + return -EMSGSIZE; + + ap = nla_data(nla); + memcpy(ap, aead, sizeof(*aead)); + + if (redact_secret && aead->alg_key_len) + memset(ap->alg_key, 0, (aead->alg_key_len + 7) / 8); + else + memcpy(ap->alg_key, aead->alg_key, + (aead->alg_key_len + 7) / 8); + return 0; +} + +static int copy_to_user_ealg(struct xfrm_algo *ealg, struct sk_buff *skb) +{ + struct xfrm_algo *ap; + bool redact_secret = xfrm_redact(); + struct nlattr *nla = nla_reserve(skb, XFRMA_ALG_CRYPT, + xfrm_alg_len(ealg)); + if (!nla) + return -EMSGSIZE; + + ap = nla_data(nla); + memcpy(ap, ealg, sizeof(*ealg)); + + if (redact_secret && ealg->alg_key_len) + memset(ap->alg_key, 0, (ealg->alg_key_len + 7) / 8); + else + memcpy(ap->alg_key, ealg->alg_key, + (ealg->alg_key_len + 7) / 8); + return 0; } @@ -906,20 +969,17 @@ static int copy_to_user_state_extra(struct xfrm_state *x, goto out; } if (x->aead) { - ret = nla_put(skb, XFRMA_ALG_AEAD, aead_len(x->aead), x->aead); + ret = copy_to_user_aead(x->aead, skb); if (ret) goto out; } if (x->aalg) { ret = copy_to_user_auth(x->aalg, skb); - if (!ret) - ret = nla_put(skb, XFRMA_ALG_AUTH_TRUNC, - xfrm_alg_auth_len(x->aalg), x->aalg); if (ret) goto out; } if (x->ealg) { - ret = nla_put(skb, XFRMA_ALG_CRYPT, xfrm_alg_len(x->ealg), x->ealg); + ret = copy_to_user_ealg(x->ealg, skb); if (ret) goto out; } diff --git a/security/security.c b/security/security.c index a28045dc9e7f..abff77c1c8a7 100644 --- a/security/security.c +++ b/security/security.c @@ -65,6 +65,7 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = { [LOCKDOWN_PERF] = "unsafe use of perf", [LOCKDOWN_TRACEFS] = "use of tracefs", [LOCKDOWN_XMON_RW] = "xmon read and write access", + [LOCKDOWN_XFRM_SECRET] = "xfrm SA secret", [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality", }; -- cgit v1.3-14-g43fede From 41dd9596d6b239a125c3d19f9d0ca90bdbfbf876 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Mon, 30 Nov 2020 16:36:29 +0100 Subject: security: add const qualifier to struct sock in various places A followup change to tcp_request_sock_op would have to drop the 'const' qualifier from the 'route_req' function as the 'security_inet_conn_request' call is moved there - and that function expects a 'struct sock *'. However, it turns out its also possible to add a const qualifier to security_inet_conn_request instead. Signed-off-by: Florian Westphal Acked-by: James Morris Signed-off-by: Jakub Kicinski --- include/linux/lsm_audit.h | 2 +- include/linux/lsm_hook_defs.h | 2 +- include/linux/security.h | 4 ++-- security/apparmor/include/net.h | 2 +- security/apparmor/lsm.c | 2 +- security/apparmor/net.c | 6 +++--- security/lsm_audit.c | 4 ++-- security/security.c | 2 +- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 4 ++-- 10 files changed, 15 insertions(+), 15 deletions(-) (limited to 'include/linux/security.h') diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index 28f23b341c1c..cd23355d2271 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -26,7 +26,7 @@ struct lsm_network_audit { int netif; - struct sock *sk; + const struct sock *sk; u16 family; __be16 dport; __be16 sport; diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 32a940117e7a..acc0494cceba 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -301,7 +301,7 @@ LSM_HOOK(void, LSM_RET_VOID, sk_clone_security, const struct sock *sk, struct sock *newsk) LSM_HOOK(void, LSM_RET_VOID, sk_getsecid, struct sock *sk, u32 *secid) LSM_HOOK(void, LSM_RET_VOID, sock_graft, struct sock *sk, struct socket *parent) -LSM_HOOK(int, 0, inet_conn_request, struct sock *sk, struct sk_buff *skb, +LSM_HOOK(int, 0, inet_conn_request, const struct sock *sk, struct sk_buff *skb, struct request_sock *req) LSM_HOOK(void, LSM_RET_VOID, inet_csk_clone, struct sock *newsk, const struct request_sock *req) diff --git a/include/linux/security.h b/include/linux/security.h index bc2725491560..0df62735651b 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1358,7 +1358,7 @@ void security_sk_clone(const struct sock *sk, struct sock *newsk); void security_sk_classify_flow(struct sock *sk, struct flowi *fl); void security_req_classify_flow(const struct request_sock *req, struct flowi *fl); void security_sock_graft(struct sock*sk, struct socket *parent); -int security_inet_conn_request(struct sock *sk, +int security_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req); void security_inet_csk_clone(struct sock *newsk, const struct request_sock *req); @@ -1519,7 +1519,7 @@ static inline void security_sock_graft(struct sock *sk, struct socket *parent) { } -static inline int security_inet_conn_request(struct sock *sk, +static inline int security_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req) { return 0; diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h index 2431c011800d..aadb4b29fb66 100644 --- a/security/apparmor/include/net.h +++ b/security/apparmor/include/net.h @@ -107,6 +107,6 @@ int aa_sock_file_perm(struct aa_label *label, const char *op, u32 request, struct socket *sock); int apparmor_secmark_check(struct aa_label *label, char *op, u32 request, - u32 secid, struct sock *sk); + u32 secid, const struct sock *sk); #endif /* __AA_NET_H */ diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index ffeaee5ed968..1b0aba8eb723 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1147,7 +1147,7 @@ static void apparmor_sock_graft(struct sock *sk, struct socket *parent) } #ifdef CONFIG_NETWORK_SECMARK -static int apparmor_inet_conn_request(struct sock *sk, struct sk_buff *skb, +static int apparmor_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req) { struct aa_sk_ctx *ctx = SK_CTX(sk); diff --git a/security/apparmor/net.c b/security/apparmor/net.c index fa0e85568450..e0c1b50d6edd 100644 --- a/security/apparmor/net.c +++ b/security/apparmor/net.c @@ -211,7 +211,7 @@ static int apparmor_secmark_init(struct aa_secmark *secmark) } static int aa_secmark_perm(struct aa_profile *profile, u32 request, u32 secid, - struct common_audit_data *sa, struct sock *sk) + struct common_audit_data *sa) { int i, ret; struct aa_perms perms = { }; @@ -244,13 +244,13 @@ static int aa_secmark_perm(struct aa_profile *profile, u32 request, u32 secid, } int apparmor_secmark_check(struct aa_label *label, char *op, u32 request, - u32 secid, struct sock *sk) + u32 secid, const struct sock *sk) { struct aa_profile *profile; DEFINE_AUDIT_SK(sa, op, sk); return fn_for_each_confined(label, profile, aa_secmark_perm(profile, request, secid, - &sa, sk)); + &sa)); } #endif diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 53d0d183db8f..078f9cdcd7f5 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -183,7 +183,7 @@ int ipv6_skb_to_auditdata(struct sk_buff *skb, static inline void print_ipv6_addr(struct audit_buffer *ab, - struct in6_addr *addr, __be16 port, + const struct in6_addr *addr, __be16 port, char *name1, char *name2) { if (!ipv6_addr_any(addr)) @@ -322,7 +322,7 @@ static void dump_common_audit_data(struct audit_buffer *ab, } case LSM_AUDIT_DATA_NET: if (a->u.net->sk) { - struct sock *sk = a->u.net->sk; + const struct sock *sk = a->u.net->sk; struct unix_sock *u; struct unix_address *addr; int len = 0; diff --git a/security/security.c b/security/security.c index a28045dc9e7f..6509f95d203f 100644 --- a/security/security.c +++ b/security/security.c @@ -2225,7 +2225,7 @@ void security_sock_graft(struct sock *sk, struct socket *parent) } EXPORT_SYMBOL(security_sock_graft); -int security_inet_conn_request(struct sock *sk, +int security_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req) { return call_int_hook(inet_conn_request, 0, sk, skb, req); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6b1826fc3658..6fa593006802 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5355,7 +5355,7 @@ static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk, selinux_netlbl_sctp_sk_clone(sk, newsk); } -static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb, +static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req) { struct sk_security_struct *sksec = sk->sk_security; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 5c90b9fa4d40..3a62d6aa74a6 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3864,7 +3864,7 @@ static inline struct smack_known *smack_from_skb(struct sk_buff *skb) * * Returns smack_known of the IP options or NULL if that won't work. */ -static struct smack_known *smack_from_netlbl(struct sock *sk, u16 family, +static struct smack_known *smack_from_netlbl(const struct sock *sk, u16 family, struct sk_buff *skb) { struct netlbl_lsm_secattr secattr; @@ -4114,7 +4114,7 @@ static void smack_sock_graft(struct sock *sk, struct socket *parent) * Returns 0 if a task with the packet label could write to * the socket, otherwise an error code */ -static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb, +static int smack_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req) { u16 family = sk->sk_family; -- cgit v1.3-14-g43fede From ed9b25d1970a4787ac6a39c2091e63b127ecbfc1 Mon Sep 17 00:00:00 2001 From: Serge Hallyn Date: Sun, 15 Nov 2020 21:55:31 -0600 Subject: [SECURITY] fix namespaced fscaps when !CONFIG_SECURITY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Namespaced file capabilities were introduced in 8db6c34f1dbc . When userspace reads an xattr for a namespaced capability, a virtualized representation of it is returned if the caller is in a user namespace owned by the capability's owning rootid. The function which performs this virtualization was not hooked up if CONFIG_SECURITY=n. Therefore in that case the original xattr was shown instead of the virtualized one. To test this using libcap-bin (*1), $ v=$(mktemp) $ unshare -Ur setcap cap_sys_admin-eip $v $ unshare -Ur setcap -v cap_sys_admin-eip $v /tmp/tmp.lSiIFRvt8Y: OK "setcap -v" verifies the values instead of setting them, and will check whether the rootid value is set. Therefore, with this bug un-fixed, and with CONFIG_SECURITY=n, setcap -v will fail: $ v=$(mktemp) $ unshare -Ur setcap cap_sys_admin=eip $v $ unshare -Ur setcap -v cap_sys_admin=eip $v nsowner[got=1000, want=0],/tmp/tmp.HHDiOOl9fY differs in [] Fix this bug by calling cap_inode_getsecurity() in security_inode_getsecurity() instead of returning -EOPNOTSUPP, when CONFIG_SECURITY=n. *1 - note, if libcap is too old for getcap to have the '-n' option, then use verify-caps instead. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209689 Cc: Hervé Guillemet Acked-by: Casey Schaufler Signed-off-by: Serge Hallyn Signed-off-by: Andrew G. Morgan Signed-off-by: James Morris --- include/linux/security.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux/security.h') diff --git a/include/linux/security.h b/include/linux/security.h index 0a0a03b36a3b..2befc0a25eb3 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -864,7 +864,7 @@ static inline int security_inode_killpriv(struct dentry *dentry) static inline int security_inode_getsecurity(struct inode *inode, const char *name, void **buffer, bool alloc) { - return -EOPNOTSUPP; + return cap_inode_getsecurity(inode, name, buffer, alloc); } static inline int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags) -- cgit v1.3-14-g43fede