Fix the SELinux dynamic network access controls
Subsystems (5)
cover Fix the SELinux dynamic network access controls
For some time now SELinux has had dynamic network access controls for labeled networking: when labeled networking was configured, the controls were active, when labeled networking wasn't configured the controls were disabled. The dynamic controls simplified security policy and improved performance in the common case; life was good.
Unfortunately, there is a bit of a problem in that the labeled IPsec portion of the dynamic network access controls is broken such that it acts as a one way street. Once you load a labeled IPsec configuration into the kernel the SELinux network access controls are enabled and stay enabled, even after the configuration is removed. Resolving this involves two fixes:
#1 - Fixing the SELinux labeled IPsec configuration detection so that we track things on alloc/free and not alloc/delete. (patch 1/2)
#2 - Force a IPsec garbage collection when we delete a IPsec policy so we don't have IPsec state lying unused and dormant for extended periods of time. (patch 2/2)
I'm posting these patches as an RFC because I'm not 100% certain that the core XFRM patches are "The Right Thing" so I'd like some commentary from the netdev folks on that (patch 2/2). I'm confident that The SELinux patch is "The Right Thing", but as usual, comments are welcome if you see something. However, if everybody is happy with both patches then merge away!
-Paul
---
Paul Moore (2):
selinux: fix the labeled xfrm/IPsec reference count handling
xfrm: force a garbage collection after deleting a policy
include/linux/security.h | 26 ++- include/net/xfrm.h | 6 + net/key/af_key.c | 4 net/xfrm/xfrm_policy.c | 3 net/xfrm/xfrm_user.c | 2 security/capability.c | 15 +- security/security.c | 13 - security/selinux/hooks.c | 5 - security/selinux/include/xfrm.h | 8 + security/selinux/xfrm.c | 384 ++++++++++++++++++--------------------- 10 files changed, 231 insertions(+), 235 deletions(-)
RFC PATCH 1/2 selinux: fix the labeled xfrm/IPsec reference count handling
The SELinux labeled IPsec code was improperly handling its reference counting, dropping a reference on a delete operation instead of on a free/release operation. This patch resolves this issue as well as some other related issues:
* Change the name of the xfrm LSM security_operations field members to match the LSM hook names, like most everywhere else
* Don't reuse security_operations member functions across security_xfrm_state_alloc() and security_xfrm_state_alloc_acquire()
* Move the xfrm_sec_ctx check into the LSM specific function
* General cleanup/janitorial work in security/selinux/xfrm.c
--- include/linux/security.h | 26 ++- security/capability.c | 15 +- security/security.c | 13 - security/selinux/hooks.c | 5 - security/selinux/include/xfrm.h | 8 + security/selinux/xfrm.c | 384 ++++++++++++++++++--------------------- 6 files changed, 217 insertions(+), 234 deletions(-)
include/linux/security.h +18 -8
@@ -1039,17 +1039,25 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)* @xfrm_policy_delete_security: * @ctx contains the xfrm_sec_ctx. * Authorize deletion of xp->security. -* @xfrm_state_alloc_security: +* @xfrm_state_alloc: * @x contains the xfrm_state being added to the Security Association * Database by the XFRM system. * @sec_ctx contains the security context information being provided by * the user-level SA generation program (e.g., setkey or racoon). -* @secid contains the secid from which to take the mls portion of the context. * Allocate a security structure to the x->security field; the security * field is initialized to NULL when the xfrm_state is allocated. Set the -* context to correspond to either sec_ctx or polsec, with the mls portion -* taken from secid in the latter case. -* Return 0 if operation was successful (memory to allocate, legal context). +* context to correspond to sec_ctx. Return 0 if operation was successful +* (memory to allocate, legal context). +* @xfrm_state_alloc_acquire: +* @x contains the xfrm_state being added to the Security Association +* Database by the XFRM system. +* @polsec contains the policy's security context. +* @secid contains the secid from which to take the mls portion of the +* context. +* Allocate a security structure to the x->security field; the security +* field is initialized to NULL when the xfrm_state is allocated. Set the +* context to correspond to secid. Return 0 if operation was successful +* (memory to allocate, legal context). * @xfrm_state_free_security: * @x contains the xfrm_state. * Deallocate x->security.
@@ -1651,9 +1659,11 @@ struct security_operations {int (*xfrm_policy_clone_security) (struct xfrm_sec_ctx *old_ctx,struct xfrm_sec_ctx **new_ctx);void (*xfrm_policy_free_security) (struct xfrm_sec_ctx *ctx);int (*xfrm_policy_delete_security) (struct xfrm_sec_ctx *ctx); -int (*xfrm_state_alloc_security) (struct xfrm_state *x, -struct xfrm_user_sec_ctx *sec_ctx, -u32 secid); +int (*xfrm_state_alloc) (struct xfrm_state *x, +struct xfrm_user_sec_ctx *sec_ctx); +int (*xfrm_state_alloc_acquire) (struct xfrm_state *x, +struct xfrm_sec_ctx *polsec, +u32 secid);void (*xfrm_state_free_security) (struct xfrm_state *x);int (*xfrm_state_delete_security) (struct xfrm_state *x);int (*xfrm_policy_lookup) (struct xfrm_sec_ctx *ctx,u32 fl_secid,u8 dir);
security/capability.c +11 -4
@@ -767,9 +767,15 @@ static int cap_xfrm_policy_delete_security(struct xfrm_sec_ctx *ctx)return 0 ; } -static int cap_xfrm_state_alloc_security (struct xfrm_state *x, -struct xfrm_user_sec_ctx *sec_ctx, -u32 secid) +static int cap_xfrm_state_alloc (struct xfrm_state *x, +struct xfrm_user_sec_ctx *sec_ctx) +{ +return 0 ; +} + +static int cap_xfrm_state_alloc_acquire (struct xfrm_state *x, +struct xfrm_sec_ctx *polsec, +u32 secid) {return 0 ; }
@@ -1084,7 +1090,8 @@ void __init security_fixup_ops(struct security_operations *ops)set_to_cap_if_null (ops, xfrm_policy_clone_security);set_to_cap_if_null (ops, xfrm_policy_free_security);set_to_cap_if_null (ops, xfrm_policy_delete_security); -set_to_cap_if_null (ops, xfrm_state_alloc_security); +set_to_cap_if_null (ops, xfrm_state_alloc); +set_to_cap_if_null (ops, xfrm_state_alloc_acquire);set_to_cap_if_null (ops, xfrm_state_free_security);set_to_cap_if_null (ops, xfrm_state_delete_security);set_to_cap_if_null (ops, xfrm_policy_lookup);
security/security.c +4 -9
@@ -1322,22 +1322,17 @@ int security_xfrm_policy_delete(struct xfrm_sec_ctx *ctx)return security_ops->xfrm_policy_delete_security (ctx); } -int security_xfrm_state_alloc (struct xfrm_state *x,struct xfrm_user_sec_ctx *sec_ctx) +int security_xfrm_state_alloc (struct xfrm_state *x, +struct xfrm_user_sec_ctx *sec_ctx) { -return security_ops->xfrm_state_alloc_security (x, sec_ctx,0 ); +return security_ops->xfrm_state_alloc (x, sec_ctx); }EXPORT_SYMBOL (security_xfrm_state_alloc);int security_xfrm_state_alloc_acquire (struct xfrm_state *x,struct xfrm_sec_ctx *polsec,u32 secid) { -if (!polsec) -return 0 ; -/* -* We want the context to be taken from secid which is usually -* from the sock. -*/ -return security_ops->xfrm_state_alloc_security (x,NULL , secid); +return security_ops->xfrm_state_alloc_acquire (x, polsec, secid); }int security_xfrm_state_delete (struct xfrm_state *x)
security/selinux/hooks.c +3 -2
@@ -5705,10 +5705,11 @@ static struct security_operations selinux_ops = {#ifdef CONFIG_SECURITY_NETWORK_XFRM .xfrm_policy_alloc_security = selinux_xfrm_policy_alloc, - .xfrm_policy_clone_security = selinux_xfrm_policy_clone, + .xfrm_policy_clone_security = selinux_xfrm_clone, .xfrm_policy_free_security = selinux_xfrm_policy_free, .xfrm_policy_delete_security = selinux_xfrm_policy_delete, - .xfrm_state_alloc_security = selinux_xfrm_state_alloc, + .xfrm_state_alloc = selinux_xfrm_state_alloc, + .xfrm_state_alloc_acquire = selinux_xfrm_state_alloc_acquire, .xfrm_state_free_security = selinux_xfrm_state_free, .xfrm_state_delete_security = selinux_xfrm_state_delete, .xfrm_policy_lookup = selinux_xfrm_policy_lookup,
security/selinux/include/xfrm.h +5 -3
@@ -9,14 +9,16 @@#include <net/flow.h> +int selinux_xfrm_clone (struct xfrm_sec_ctx *old_ctx, +struct xfrm_sec_ctx **new_ctxp);int selinux_xfrm_policy_alloc (struct xfrm_sec_ctx **ctxp,struct xfrm_user_sec_ctx *sec_ctx); -int selinux_xfrm_policy_clone (struct xfrm_sec_ctx *old_ctx, -struct xfrm_sec_ctx **new_ctxp);void selinux_xfrm_policy_free (struct xfrm_sec_ctx *ctx);int selinux_xfrm_policy_delete (struct xfrm_sec_ctx *ctx);int selinux_xfrm_state_alloc (struct xfrm_state *x, -struct xfrm_user_sec_ctx *sec_ctx,u32 secid); +struct xfrm_user_sec_ctx *sec_ctx); +int selinux_xfrm_state_alloc_acquire (struct xfrm_state *x, +struct xfrm_sec_ctx *polsec,u32 secid);void selinux_xfrm_state_free (struct xfrm_state *x);int selinux_xfrm_state_delete (struct xfrm_state *x);int selinux_xfrm_policy_lookup (struct xfrm_sec_ctx *ctx,u32 fl_secid,u8 dir);
security/selinux/xfrm.c +176 -208
505 lines not on this page · raw message
--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.
RFC PATCH 2/2 xfrm: force a garbage collection after deleting a policy
In reply to Paul MooreIn some cases after deleting a policy from the SPD the policy would remain in the dst/flow/route cache for an extended period of time which caused problems for SELinux as its dynamic network access controls key off of the number of XFRM policy and state entries. This patch corrects this problem by forcing a XFRM garbage collection whenever a policy is sucessfully removed.
--- include/net/xfrm.h | 6 ++++++ net/key/af_key.c | 4 ++++ net/xfrm/xfrm_policy.c | 3 ++- net/xfrm/xfrm_user.c | 2 ++ 4 files changed, 14 insertions(+), 1 deletion(-)
include/net/xfrm.h +6 -0
@@ -1160,6 +1160,8 @@ static inline void xfrm_sk_free_policy(struct sock *sk) } } +extern void xfrm_garbage_collect (struct net *net); +#else static inline void xfrm_sk_free_policy (struct sock *sk) {}
@@ -1194,6 +1196,10 @@ static inline int xfrm6_policy_check_reverse(struct sock *sk, int dir, {return 1 ; } +static inline void xfrm_garbage_collect (struct net *net) +{ +return ; +}#endif static __inline__
net/key/af_key.c +4 -0
@@ -2366,6 +2366,8 @@ static int pfkey_spddelete(struct sock *sk, struct sk_buff *skb, const struct sa out:xfrm_pol_put (xp); +if (err ==0 ) +xfrm_garbage_collect (net);return err; }
@@ -2615,6 +2617,8 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, const struct sadb_ out:xfrm_pol_put (xp); +if (delete && err ==0 ) +xfrm_garbage_collect (net);return err; }
net/xfrm/xfrm_policy.c +2 -1
@@ -2557,11 +2557,12 @@ static void __xfrm_garbage_collect(struct net *net) } } -static void xfrm_garbage_collect (struct net *net) +void xfrm_garbage_collect (struct net *net) {flow_cache_flush ();__xfrm_garbage_collect (net); } +EXPORT_SYMBOL (xfrm_garbage_collect);static void xfrm_garbage_collect_deferred (struct net *net) {
net/xfrm/xfrm_user.c +2 -0
@@ -1681,6 +1681,8 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh, out:xfrm_pol_put (xp); +if (delete && err ==0 ) +xfrm_garbage_collect (net);return err; }
--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.
Re: [RFC PATCH 2/2] xfrm: force a garbage collection after deleting a policy
Hello.
Quoted 20 lines · Paul Moore
In some cases after deleting a policy from the SPD the policy would remain in the dst/flow/route cache for an extended period of time which caused problems for SELinux as its dynamic network access controls key off of the number of XFRM policy and state entries. This patch corrects this problem by forcing a XFRM garbage collection whenever a policy is sucessfully removed.
Reported-by Ondrej Moris <[email protected]>Signed-off-by Paul Moore <[email protected]>--- include/net/xfrm.h | 6 ++++++ net/key/af_key.c | 4 ++++ net/xfrm/xfrm_policy.c | 3 ++- net/xfrm/xfrm_user.c | 2 ++ 4 files changed, 14 insertions(+), 1 deletion(-)include/net/xfrm.h +0
-0
[...]
include/net/xfrm.h static inline int xfrm6_policy_check_reverse(struct sock *sk, int dir, · line 1201
Quoted 7 lines
include/net/xfrm.h +3
-0@@ -1194,6 +1196,10 @@ static inline int xfrm6_policy_check_reverse(struct sock *sk, int dir, {return 1 ; } +static inline void xfrm_garbage_collect (struct net *net) +{ +return ;
Not needed.
include/net/xfrm.h static inline int xfrm6_policy_check_reverse(struct sock *sk, int dir, · line 1205
Quoted 5 lines
include/net/xfrm.h +1
-0+}#endif static __inline__
WBR, Sergei
Re: [RFC PATCH 2/2] xfrm: force a garbage collection after deleting a policy
Quoted 33 lines · Sergei Shtylyov
Hello.
Quoted 21 lines · Paul Moore
In some cases after deleting a policy from the SPD the policy would remain in the dst/flow/route cache for an extended period of time which caused problems for SELinux as its dynamic network access controls key off of the number of XFRM policy and state entries. This patch corrects this problem by forcing a XFRM garbage collection whenever a policy is sucessfully removed.
Reported-by Ondrej Moris <[email protected]>Signed-off-by Paul Moore <[email protected]>---
include/net/xfrm.h | 6 ++++++ net/key/af_key.c | 4 ++++ net/xfrm/xfrm_policy.c | 3 ++- net/xfrm/xfrm_user.c | 2 ++ 4 files changed, 14 insertions(+), 1 deletion(-)include/net/xfrm.h +0
-0[...]
Quoted 6 lines
include/net/xfrm.h +0
-0@@ -1194,6 +1196,10 @@ static inline int xfrm6_policy_check_reverse(structsock *sk, int dir,>
{return 1;
Quoted 9 lines
include/net/xfrm.h static inline int xfrm6_policy_check_reverse(struct sock *sk, int dir, · line 1201
Quoted 5 lines
}include/net/xfrm.h +3
-0+static inline void xfrm_garbage_collect (struct net *net) +{ +return ;Not needed.
Quoted 1 line
+}
True, I added it for the sake of completeness, but I'll go ahead and remove it.
Signature · 8 lines
paul moore security and virtualization @ redhat -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.