On Wed, Feb 22, 2012 at 09:10:37AM -0800, John Johansen wrote:
> Base support for network mediation.
> 
> Signed-off-by: John Johansen <[email protected]>
> [...]
> diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile
> index 86103ce..96cf725 100644
> --- a/security/apparmor/Makefile
> +++ b/security/apparmor/Makefile
> [...]
> @@ -62,3 +64,5 @@ $(obj)/capability_names.h : 
> $(srctree)/include/linux/capability.h \
>  $(obj)/rlim_names.h : $(srctree)/include/asm-generic/resource.h \
>                     $(src)/Makefile
>       $(call cmd,make-rlim)
> +$(obj)/af_names.h : $(srctree)/include/linux/socket.h
> +     $(call cmd,make-af)

Please have this depend on $(src)/Makefile (like the others) so that when
the Makefile rules change, a rebuild of the .h file is triggered. Also,
please include comments at the generation to show what the sed mess
produces (like the others have).

> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 38d6262..4267401 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -198,9 +198,15 @@ static struct aa_fs_entry aa_fs_entry_domain[] = {
>       { }
>  };
>  
> +static struct aa_fs_entry aa_fs_entry_network[] = {
> +     AA_FS_FILE_BOOLEAN("af_masking",        1),

You just want this to be a boolean, not a list?

> diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
> new file mode 100644
> index 0000000..3c7d599
> --- /dev/null
> +++ b/security/apparmor/include/net.h
> @@ -0,0 +1,40 @@
> +/*
> + * AppArmor security module
> + *
> + * This file contains AppArmor network mediation definitions.
> + *
> + * Copyright (C) 1998-2008 Novell/SUSE
> + * Copyright 2009-2010 Canonical Ltd.

Should update this to 2012.

> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + */
> +
> +#ifndef __AA_NET_H
> +#define __AA_NET_H
> +
> +#include <net/sock.h>
> +
> +/* struct aa_net - network confinement data
> + * @allowed: basic network families permissions
> + * @audit_network: which network permissions to force audit
> + * @quiet_network: which network permissions to quiet rejects
> + */
> +struct aa_net {
> +     u16 allow[AF_MAX];
> +     u16 audit[AF_MAX];
> +     u16 quiet[AF_MAX];
> +};
> +
> +extern int aa_net_perm(int op, struct aa_profile *profile, u16 family,
> +                    int type, int protocol, struct sock *sk);
> +extern int aa_revalidate_sk(int op, struct sock *sk);
> +
> +static inline void aa_free_net_rules(struct aa_net *new)
> +{
> +     /* NOP */
> +}
> +
> +#endif /* __AA_NET_H */
> diff --git a/security/apparmor/include/policy.h 
> b/security/apparmor/include/policy.h
> index 9e18e96..3f582a7 100644
> --- a/security/apparmor/include/policy.h
> +++ b/security/apparmor/include/policy.h
> @@ -27,6 +27,7 @@
>  #include "capability.h"
>  #include "domain.h"
>  #include "file.h"
> +#include "net.h"
>  #include "resource.h"
>  
>  extern const char *profile_mode_names[];
> @@ -157,6 +158,7 @@ struct aa_policydb {
>   * @policy: general match rules governing policy
>   * @file: The set of rules governing basic file access and domain transitions
>   * @caps: capabilities for the profile
> + * @net: network controls for the profile
>   * @rlimits: rlimits for the profile
>   *
>   * The AppArmor profile contains the basic confinement data.  Each profile
> @@ -194,6 +196,7 @@ struct aa_profile {
>       struct aa_policydb policy;
>       struct aa_file_rules file;
>       struct aa_caps caps;
> +     struct aa_net net;
>       struct aa_rlimit rlimits;
>  };
>  
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 3783202..7459547 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -32,6 +32,7 @@
>  #include "include/context.h"
>  #include "include/file.h"
>  #include "include/ipc.h"
> +#include "include/net.h"
>  #include "include/path.h"
>  #include "include/policy.h"
>  #include "include/procattr.h"
> @@ -621,6 +622,104 @@ static int apparmor_task_setrlimit(struct task_struct 
> *task,
>       return error;
>  }
>  
> +static int apparmor_socket_create(int family, int type, int protocol, int 
> kern)
> +{
> +     struct aa_profile *profile;
> +     int error = 0;
> +
> +     if (kern)
> +             return 0;
> +
> +     profile = __aa_current_profile();
> +     if (!unconfined(profile))
> +             error = aa_net_perm(OP_CREATE, profile, family, type, protocol,
> +                                 NULL);
> +     return error;
> +}
> +
> +static int apparmor_socket_bind(struct socket *sock,
> +                             struct sockaddr *address, int addrlen)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_BIND, sk);
> +}
> +
> +static int apparmor_socket_connect(struct socket *sock,
> +                                struct sockaddr *address, int addrlen)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_CONNECT, sk);
> +}
> +
> +static int apparmor_socket_listen(struct socket *sock, int backlog)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_LISTEN, sk);
> +}
> +
> +static int apparmor_socket_accept(struct socket *sock, struct socket 
> *newsock)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_ACCEPT, sk);
> +}
> +
> +static int apparmor_socket_sendmsg(struct socket *sock,
> +                                struct msghdr *msg, int size)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_SENDMSG, sk);
> +}
> +
> +static int apparmor_socket_recvmsg(struct socket *sock,
> +                                struct msghdr *msg, int size, int flags)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_RECVMSG, sk);
> +}
> +
> +static int apparmor_socket_getsockname(struct socket *sock)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_GETSOCKNAME, sk);
> +}
> +
> +static int apparmor_socket_getpeername(struct socket *sock)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_GETPEERNAME, sk);
> +}
> +
> +static int apparmor_socket_getsockopt(struct socket *sock, int level,
> +                                   int optname)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_GETSOCKOPT, sk);
> +}
> +
> +static int apparmor_socket_setsockopt(struct socket *sock, int level,
> +                                   int optname)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_SETSOCKOPT, sk);
> +}
> +
> +static int apparmor_socket_shutdown(struct socket *sock, int how)
> +{
> +     struct sock *sk = sock->sk;
> +
> +     return aa_revalidate_sk(OP_SOCK_SHUTDOWN, sk);
> +}
> +
>  static struct security_operations apparmor_ops = {
>       .name =                         "apparmor",
>  
> @@ -652,6 +751,19 @@ static struct security_operations apparmor_ops = {
>       .getprocattr =                  apparmor_getprocattr,
>       .setprocattr =                  apparmor_setprocattr,
>  
> +     .socket_create =                apparmor_socket_create,
> +     .socket_bind =                  apparmor_socket_bind,
> +     .socket_connect =               apparmor_socket_connect,
> +     .socket_listen =                apparmor_socket_listen,
> +     .socket_accept =                apparmor_socket_accept,
> +     .socket_sendmsg =               apparmor_socket_sendmsg,
> +     .socket_recvmsg =               apparmor_socket_recvmsg,
> +     .socket_getsockname =           apparmor_socket_getsockname,
> +     .socket_getpeername =           apparmor_socket_getpeername,
> +     .socket_getsockopt =            apparmor_socket_getsockopt,
> +     .socket_setsockopt =            apparmor_socket_setsockopt,
> +     .socket_shutdown =              apparmor_socket_shutdown,
> +
>       .cred_alloc_blank =             apparmor_cred_alloc_blank,
>       .cred_free =                    apparmor_cred_free,
>       .cred_prepare =                 apparmor_cred_prepare,
> diff --git a/security/apparmor/net.c b/security/apparmor/net.c
> new file mode 100644
> index 0000000..1765901
> --- /dev/null
> +++ b/security/apparmor/net.c
> @@ -0,0 +1,170 @@
> +/*
> + * AppArmor security module
> + *
> + * This file contains AppArmor network mediation
> + *
> + * Copyright (C) 1998-2008 Novell/SUSE
> + * Copyright 2009-2010 Canonical Ltd.

2012 again.

> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + */
> +
> +#include "include/apparmor.h"
> +#include "include/audit.h"
> +#include "include/context.h"
> +#include "include/net.h"
> +#include "include/policy.h"
> +
> +#include "af_names.h"
> +
> +static const char *sock_type_names[] = {
> +     "unknown(0)",
> +     "stream",
> +     "dgram",
> +     "raw",
> +     "rdm",
> +     "seqpacket",
> +     "dccp",
> +     "unknown(7)",
> +     "unknown(8)",
> +     "unknown(9)",
> +     "packet",
> +};

Should sock_type_names be generated too? Everything else is. The
users of it can check for NULL entries and generate the "unknown(NUM)"
stuff instead.

> +
> +/* audit callback for net specific fields */
> +static void audit_cb(struct audit_buffer *ab, void *va)
> +{
> +     struct common_audit_data *sa = va;
> +
> +     audit_log_format(ab, " family=");
> +     if (address_family_names[sa->u.net.family]) {
> +             audit_log_string(ab, address_family_names[sa->u.net.family]);
> +     } else {
> +             audit_log_format(ab, " \"unknown(%d)\"", sa->u.net.family);

This probably shouldn't have the leading " " since it follows "=".

> +     }
> +
> +     audit_log_format(ab, " sock_type=");
> +     if (sock_type_names[sa->aad.net.type]) {
> +             audit_log_string(ab, sock_type_names[sa->aad.net.type]);
> +     } else {
> +             audit_log_format(ab, "\"unknown(%d)\"", sa->aad.net.type);
> +     }
> +
> +     audit_log_format(ab, " protocol=%d", sa->aad.net.protocol);
> +}
> +
> +/**
> + * audit_net - audit network access
> + * @profile: profile being enforced  (NOT NULL)
> + * @op: operation being checked
> + * @family: network family
> + * @type:   network type
> + * @protocol: network protocol
> + * @sk: socket auditing is being applied to
> + * @error: error code for failure else 0
> + *
> + * Returns: %0 or sa->error else other errorcode on failure
> + */
> +static int audit_net(struct aa_profile *profile, int op, u16 family, int 
> type,
> +                  int protocol, struct sock *sk, int error)
> +{
> +     int audit_type = AUDIT_APPARMOR_AUTO;
> +     struct common_audit_data sa;
> +     if (sk) {
> +             COMMON_AUDIT_DATA_INIT(&sa, NET);
> +     } else {
> +             COMMON_AUDIT_DATA_INIT(&sa, NONE);
> +     }
> +     /* todo fill in socket addr info */
> +
> +     sa.aad.op = op,
> +     sa.u.net.family = family;
> +     sa.u.net.sk = sk;
> +     sa.aad.net.type = type;
> +     sa.aad.net.protocol = protocol;
> +     sa.aad.error = error;
> +
> +     if (likely(!sa.aad.error)) {
> +             u16 audit_mask = profile->net.audit[sa.u.net.family];
> +             if (likely((AUDIT_MODE(profile) != AUDIT_ALL) &&
> +                        !(1 << sa.aad.net.type & audit_mask)))
> +                     return 0;
> +             audit_type = AUDIT_APPARMOR_AUDIT;
> +     } else {
> +             u16 quiet_mask = profile->net.quiet[sa.u.net.family];
> +             u16 kill_mask = 0;
> +             u16 denied = (1 << sa.aad.net.type) & ~quiet_mask;
> +
> +             if (denied & kill_mask)
> +                     audit_type = AUDIT_APPARMOR_KILL;
> +
> +             if ((denied & quiet_mask) &&
> +                 AUDIT_MODE(profile) != AUDIT_NOQUIET &&
> +                 AUDIT_MODE(profile) != AUDIT_ALL)
> +                     return COMPLAIN_MODE(profile) ? 0 : sa.aad.error;
> +     }
> +
> +     return aa_audit(audit_type, profile, GFP_KERNEL, &sa, audit_cb);
> +}
> +
> +/**
> + * aa_net_perm - very course network access check
> + * @op: operation being checked
> + * @profile: profile being enforced  (NOT NULL)
> + * @family: network family
> + * @type:   network type
> + * @protocol: network protocol
> + *
> + * Returns: %0 else error if permission denied
> + */
> +int aa_net_perm(int op, struct aa_profile *profile, u16 family, int type,
> +             int protocol, struct sock *sk)
> +{
> +     u16 family_mask;
> +     int error;
> +
> +     if ((family < 0) || (family >= AF_MAX))
> +             return -EINVAL;
> +
> +     if ((type < 0) || (type >= SOCK_MAX))
> +             return -EINVAL;
> +
> +     /* unix domain and netlink sockets are handled by ipc */
> +     if (family == AF_UNIX || family == AF_NETLINK)
> +             return 0;
> +
> +     family_mask = profile->net.allow[family];
> +
> +     error = (family_mask & (1 << type)) ? 0 : -EACCES;
> +
> +     return audit_net(profile, op, family, type, protocol, sk, error);
> +}
> +
> +/**
> + * aa_revalidate_sk - Revalidate access to a sock
> + * @op: operation being checked
> + * @sk: sock being revalidated  (NOT NULL)
> + *
> + * Returns: %0 else error if permission denied
> + */
> +int aa_revalidate_sk(int op, struct sock *sk)
> +{
> +     struct aa_profile *profile;
> +     int error = 0;
> +
> +     /* aa_revalidate_sk should not be called from interrupt context
> +      * don't mediate these calls as they are not task related
> +      */
> +     if (in_interrupt())
> +             return 0;
> +
> +     profile = __aa_current_profile();
> +     if (!unconfined(profile))
> +             error = aa_net_perm(op, profile, sk->sk_family, sk->sk_type,
> +                                 sk->sk_protocol, sk);
> +
> +     return error;
> +}
> diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
> index 8b7febb..4be20c3 100644
> --- a/security/apparmor/policy.c
> +++ b/security/apparmor/policy.c
> @@ -748,6 +748,7 @@ static void free_profile(struct aa_profile *profile)
>  
>       aa_free_file_rules(&profile->file);
>       aa_free_cap_rules(&profile->caps);
> +     aa_free_net_rules(&profile->net);
>       aa_free_rlimit_rules(&profile->rlimits);
>  
>       aa_free_sid(profile->sid);
> diff --git a/security/apparmor/policy_unpack.c 
> b/security/apparmor/policy_unpack.c
> index c7a6d03..ec03f42 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -191,6 +191,19 @@ fail:
>       return 0;
>  }
>  
> +static bool unpack_u16(struct aa_ext *e, u16 *data, const char *name)
> +{
> +     if (unpack_nameX(e, AA_U16, name)) {
> +             if (!inbounds(e, sizeof(u16)))
> +                     return 0;
> +             if (data)
> +                     *data = le16_to_cpu(get_unaligned((u16 *) e->pos));
> +             e->pos += sizeof(u16);
> +             return 1;
> +     }
> +     return 0;
> +}
> +
>  static bool unpack_u32(struct aa_ext *e, u32 *data, const char *name)
>  {
>       if (unpack_nameX(e, AA_U32, name)) {
> @@ -469,7 +482,8 @@ static struct aa_profile *unpack_profile(struct aa_ext *e)
>  {
>       struct aa_profile *profile = NULL;
>       const char *name = NULL;
> -     int error = -EPROTO;
> +     size_t size = 0;
> +     int i, error = -EPROTO;
>       kernel_cap_t tmpcap;
>       u32 tmp;
>  
> @@ -562,6 +576,38 @@ static struct aa_profile *unpack_profile(struct aa_ext 
> *e)
>       if (!unpack_rlimits(e, profile))
>               goto fail;
>  
> +     size = unpack_array(e, "net_allowed_af");
> +     if (size) {
> +
> +             for (i = 0; i < size; i++) {
> +                     /* discard extraneous rules that this kernel will
> +                      * never request
> +                      */
> +                     if (i >= AF_MAX) {
> +                             u16 tmp;
> +                             if (!unpack_u16(e, &tmp, NULL) ||
> +                                 !unpack_u16(e, &tmp, NULL) ||
> +                                 !unpack_u16(e, &tmp, NULL))
> +                                     goto fail;
> +                             continue;
> +                     }
> +                     if (!unpack_u16(e, &profile->net.allow[i], NULL))
> +                             goto fail;
> +                     if (!unpack_u16(e, &profile->net.audit[i], NULL))
> +                             goto fail;
> +                     if (!unpack_u16(e, &profile->net.quiet[i], NULL))
> +                             goto fail;
> +             }
> +             if (!unpack_nameX(e, AA_ARRAYEND, NULL))
> +                     goto fail;
> +             /*
> +              * allow unix domain and netlink sockets they are handled
> +              * by IPC
> +              */

Should this comment move below the current indent level?

> +     }
> +     profile->net.allow[AF_UNIX] = 0xffff;
> +     profile->net.allow[AF_NETLINK] = 0xffff;
> +
>       if (unpack_nameX(e, AA_STRUCT, "policydb")) {
>               /* generic policy dfa - optional and may be NULL */
>               profile->policy.dfa = unpack_dfa(e);
> -- 
> 1.7.9
> 
> 
> -- 
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor
-- 
Kees Cook

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to