[apparmor] [PATCH 12/16] AppArmor: basic networking rules
John Johansen
john.johansen at canonical.com
Wed Feb 22 21:06:21 UTC 2012
On 02/22/2012 12:59 PM, Kees Cook wrote:
> On Wed, Feb 22, 2012 at 09:10:37AM -0800, John Johansen wrote:
>> Base support for network mediation.
>>
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>> [...]
>> 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).
>
heh yeah, /me just did a quick merge on this patch and resolved conflicts. I should
have spent a little time updating it.
>> 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?
>
Hrmm we could do that, I really didn't put a lot of thought into it as this is
the older patch set being moved forward, instead of the newer one.
But since we are going to need compatibility it is probably best to make this
a directory.
>> 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.
>
yeah autogenerating this would be good
>> +
>> +/* 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?
>
yep
>> + }
>> + 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
>> AppArmor at lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
More information about the AppArmor
mailing list