+ Features

- switch policy hash fro sha1 to sha256
 
 + Bug Fixes
   - Fix refcount leak in task_kill
   - Fix leak of pdb objects and trans_table
   - avoid crash when parse profie name is empty
 
 + Cleanups
   - add static to stack_msg and nulldfa
   - more kernel-doc cleanups
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCgAdFiEE7cSDD705q2rFEEf7BS82cBjVw9gFAmWps1IACgkQBS82cBjV
 w9haNg//U0EcUFS1BrF1urJKhmYc4AhGOA7GQfQ+s2Ra0GcDQakU4gGHie3Nlh/k
 KzriVKCkoAwsffEBosLSeoPD/5890XIDephTrEjQOe35BVjs2jrh+q1WdqSRlpc+
 fAa2ObETxXFZ0PsYfU7cSBh6JHEiyPCqunGoI7+Deb513jvAMTpg0V/YbZdepRt9
 EwvQuNYVR7SUK6+EKK+dWM/O3UkedSaVDIEQljY41JREWG4FIwpbvj4lDoQnBw85
 g3iWvHGw4IPwYyyCSf2vJYnfS5bOF+J33TtXRzqYAPN5KSBf5EooAz23nD/pYEKV
 fbLzz5UNTqyFohhsmfo4f/FgP/myfDp0LBol3QYTFPnmkvPXDpGtn3elBPepDc+z
 /KfaP8+8qajTc3rLvEucOgBdpvvAtJGUE0X571VuYWIC9jNYV0XrAq9Wvr2KXfJq
 NPWy1m+fzZ/URh5slBhRfsfupN3JM7DVeI/TAvr8Vdy+0EYH7Q60x7cJcrPWW1Vw
 eHy/QNPI5+4VisGsuXwiri9iUqI/kS5y3ONQOS1lFBaRIGhztHmIMdk0esYSuJ+W
 3RHJln99lziRyagdoiR1hr0N764X0xKQ0s7oV7hIoo/h9eC0RED8ot+TPqg7utZj
 +5Kpy0P2MqBN0mun9e/fxmcbDV7ChOWueGFc2JZcdYU83SrAZf4=
 =qYLj
 -----END PGP SIGNATURE-----

Merge tag 'apparmor-pr-2024-01-18' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor

Pull AppArmor updates from John Johansen:
 "This adds a single feature, switch the hash used to check policy from
  sha1 to sha256

  There are fixes for two memory leaks, and refcount bug and a potential
  crash when a profile name is empty. Along with a couple minor code
  cleanups.

  Summary:

  Features
   - switch policy hash from sha1 to sha256

  Bug Fixes
   - Fix refcount leak in task_kill
   - Fix leak of pdb objects and trans_table
   - avoid crash when parse profie name is empty

  Cleanups
   - add static to stack_msg and nulldfa
   - more kernel-doc cleanups"

* tag 'apparmor-pr-2024-01-18' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
  apparmor: Fix memory leak in unpack_profile()
  apparmor: avoid crash when parsed profile name is empty
  apparmor: fix possible memory leak in unpack_trans_table
  apparmor: free the allocated pdb objects
  apparmor: Fix ref count leak in task_kill
  apparmor: cleanup network hook comments
  apparmor: add missing params to aa_may_ptrace kernel-doc comments
  apparmor: declare nulldfa as static
  apparmor: declare stack_msg as static
  apparmor: switch SECURITY_APPARMOR_HASH from sha1 to sha256
This commit is contained in:
Linus Torvalds 2024-01-19 10:53:55 -08:00
commit 237c31cb5d
9 changed files with 54 additions and 74 deletions

View File

@ -57,10 +57,10 @@ config SECURITY_APPARMOR_INTROSPECT_POLICY
cpu is paramount. cpu is paramount.
config SECURITY_APPARMOR_HASH config SECURITY_APPARMOR_HASH
bool "Enable introspection of sha1 hashes for loaded profiles" bool "Enable introspection of sha256 hashes for loaded profiles"
depends on SECURITY_APPARMOR_INTROSPECT_POLICY depends on SECURITY_APPARMOR_INTROSPECT_POLICY
select CRYPTO select CRYPTO
select CRYPTO_SHA1 select CRYPTO_SHA256
default y default y
help help
This option selects whether introspection of loaded policy This option selects whether introspection of loaded policy
@ -74,10 +74,10 @@ config SECURITY_APPARMOR_HASH_DEFAULT
depends on SECURITY_APPARMOR_HASH depends on SECURITY_APPARMOR_HASH
default y default y
help help
This option selects whether sha1 hashing of loaded policy This option selects whether sha256 hashing of loaded policy
is enabled by default. The generation of sha1 hashes for is enabled by default. The generation of sha256 hashes for
loaded policy provide system administrators a quick way loaded policy provide system administrators a quick way to
to verify that policy in the kernel matches what is expected, verify that policy in the kernel matches what is expected,
however it can slow down policy load on some devices. In however it can slow down policy load on some devices. In
these cases policy hashing can be disabled by default and these cases policy hashing can be disabled by default and
enabled only if needed. enabled only if needed.

View File

@ -1474,7 +1474,7 @@ int __aa_fs_create_rawdata(struct aa_ns *ns, struct aa_loaddata *rawdata)
rawdata->dents[AAFS_LOADDATA_REVISION] = dent; rawdata->dents[AAFS_LOADDATA_REVISION] = dent;
if (aa_g_hash_policy) { if (aa_g_hash_policy) {
dent = aafs_create_file("sha1", S_IFREG | 0444, dir, dent = aafs_create_file("sha256", S_IFREG | 0444, dir,
rawdata, &seq_rawdata_hash_fops); rawdata, &seq_rawdata_hash_fops);
if (IS_ERR(dent)) if (IS_ERR(dent))
goto fail; goto fail;
@ -1643,11 +1643,11 @@ static const char *rawdata_get_link_base(struct dentry *dentry,
return target; return target;
} }
static const char *rawdata_get_link_sha1(struct dentry *dentry, static const char *rawdata_get_link_sha256(struct dentry *dentry,
struct inode *inode, struct inode *inode,
struct delayed_call *done) struct delayed_call *done)
{ {
return rawdata_get_link_base(dentry, inode, done, "sha1"); return rawdata_get_link_base(dentry, inode, done, "sha256");
} }
static const char *rawdata_get_link_abi(struct dentry *dentry, static const char *rawdata_get_link_abi(struct dentry *dentry,
@ -1664,8 +1664,8 @@ static const char *rawdata_get_link_data(struct dentry *dentry,
return rawdata_get_link_base(dentry, inode, done, "raw_data"); return rawdata_get_link_base(dentry, inode, done, "raw_data");
} }
static const struct inode_operations rawdata_link_sha1_iops = { static const struct inode_operations rawdata_link_sha256_iops = {
.get_link = rawdata_get_link_sha1, .get_link = rawdata_get_link_sha256,
}; };
static const struct inode_operations rawdata_link_abi_iops = { static const struct inode_operations rawdata_link_abi_iops = {
@ -1738,7 +1738,7 @@ int __aafs_profile_mkdir(struct aa_profile *profile, struct dentry *parent)
profile->dents[AAFS_PROF_ATTACH] = dent; profile->dents[AAFS_PROF_ATTACH] = dent;
if (profile->hash) { if (profile->hash) {
dent = create_profile_file(dir, "sha1", profile, dent = create_profile_file(dir, "sha256", profile,
&seq_profile_hash_fops); &seq_profile_hash_fops);
if (IS_ERR(dent)) if (IS_ERR(dent))
goto fail; goto fail;
@ -1748,9 +1748,9 @@ int __aafs_profile_mkdir(struct aa_profile *profile, struct dentry *parent)
#ifdef CONFIG_SECURITY_APPARMOR_EXPORT_BINARY #ifdef CONFIG_SECURITY_APPARMOR_EXPORT_BINARY
if (profile->rawdata) { if (profile->rawdata) {
if (aa_g_hash_policy) { if (aa_g_hash_policy) {
dent = aafs_create("raw_sha1", S_IFLNK | 0444, dir, dent = aafs_create("raw_sha256", S_IFLNK | 0444, dir,
profile->label.proxy, NULL, NULL, profile->label.proxy, NULL, NULL,
&rawdata_link_sha1_iops); &rawdata_link_sha256_iops);
if (IS_ERR(dent)) if (IS_ERR(dent))
goto fail; goto fail;
aa_get_proxy(profile->label.proxy); aa_get_proxy(profile->label.proxy);

View File

@ -106,16 +106,16 @@ static int __init init_profile_hash(void)
if (!apparmor_initialized) if (!apparmor_initialized)
return 0; return 0;
tfm = crypto_alloc_shash("sha1", 0, 0); tfm = crypto_alloc_shash("sha256", 0, 0);
if (IS_ERR(tfm)) { if (IS_ERR(tfm)) {
int error = PTR_ERR(tfm); int error = PTR_ERR(tfm);
AA_ERROR("failed to setup profile sha1 hashing: %d\n", error); AA_ERROR("failed to setup profile sha256 hashing: %d\n", error);
return error; return error;
} }
apparmor_tfm = tfm; apparmor_tfm = tfm;
apparmor_hash_size = crypto_shash_digestsize(apparmor_tfm); apparmor_hash_size = crypto_shash_digestsize(apparmor_tfm);
aa_info_message("AppArmor sha1 policy hashing enabled"); aa_info_message("AppArmor sha256 policy hashing enabled");
return 0; return 0;
} }

View File

@ -1311,7 +1311,7 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
return error; return error;
} }
const char *stack_msg = "change_profile unprivileged unconfined converted to stacking"; static const char *stack_msg = "change_profile unprivileged unconfined converted to stacking";
/** /**
* aa_change_profile - perform a one-way profile transition * aa_change_profile - perform a one-way profile transition

View File

@ -41,6 +41,7 @@ void aa_free_str_table(struct aa_str_table *t)
kfree_sensitive(t->table[i]); kfree_sensitive(t->table[i]);
kfree_sensitive(t->table); kfree_sensitive(t->table);
t->table = NULL; t->table = NULL;
t->size = 0;
} }
} }

View File

@ -1023,7 +1023,6 @@ static int apparmor_task_kill(struct task_struct *target, struct kernel_siginfo
cl = aa_get_newest_cred_label(cred); cl = aa_get_newest_cred_label(cred);
error = aa_may_signal(cred, cl, tc, tl, sig); error = aa_may_signal(cred, cl, tc, tl, sig);
aa_put_label(cl); aa_put_label(cl);
return error;
} else { } else {
cl = __begin_current_label_crit_section(); cl = __begin_current_label_crit_section();
error = aa_may_signal(current_cred(), cl, tc, tl, sig); error = aa_may_signal(current_cred(), cl, tc, tl, sig);
@ -1056,9 +1055,6 @@ static int apparmor_userns_create(const struct cred *cred)
return error; return error;
} }
/**
* apparmor_sk_alloc_security - allocate and attach the sk_security field
*/
static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags) static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags)
{ {
struct aa_sk_ctx *ctx; struct aa_sk_ctx *ctx;
@ -1072,9 +1068,6 @@ static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags)
return 0; return 0;
} }
/**
* apparmor_sk_free_security - free the sk_security field
*/
static void apparmor_sk_free_security(struct sock *sk) static void apparmor_sk_free_security(struct sock *sk)
{ {
struct aa_sk_ctx *ctx = aa_sock(sk); struct aa_sk_ctx *ctx = aa_sock(sk);
@ -1087,6 +1080,8 @@ static void apparmor_sk_free_security(struct sock *sk)
/** /**
* apparmor_sk_clone_security - clone the sk_security field * apparmor_sk_clone_security - clone the sk_security field
* @sk: sock to have security cloned
* @newsk: sock getting clone
*/ */
static void apparmor_sk_clone_security(const struct sock *sk, static void apparmor_sk_clone_security(const struct sock *sk,
struct sock *newsk) struct sock *newsk)
@ -1103,9 +1098,6 @@ static void apparmor_sk_clone_security(const struct sock *sk,
new->peer = aa_get_label(ctx->peer); new->peer = aa_get_label(ctx->peer);
} }
/**
* apparmor_socket_create - check perms before creating a new socket
*/
static int apparmor_socket_create(int family, int type, int protocol, int kern) static int apparmor_socket_create(int family, int type, int protocol, int kern)
{ {
struct aa_label *label; struct aa_label *label;
@ -1127,10 +1119,14 @@ static int apparmor_socket_create(int family, int type, int protocol, int kern)
/** /**
* apparmor_socket_post_create - setup the per-socket security struct * apparmor_socket_post_create - setup the per-socket security struct
* @sock: socket that is being setup
* @family: family of socket being created
* @type: type of the socket
* @ptotocol: protocol of the socket
* @kern: socket is a special kernel socket
* *
* Note: * Note:
* - kernel sockets currently labeled unconfined but we may want to * - kernel sockets labeled kernel_t used to use unconfined
* move to a special kernel label
* - socket may not have sk here if created with sock_create_lite or * - socket may not have sk here if created with sock_create_lite or
* sock_alloc. These should be accept cases which will be handled in * sock_alloc. These should be accept cases which will be handled in
* sock_graft. * sock_graft.
@ -1156,9 +1152,6 @@ static int apparmor_socket_post_create(struct socket *sock, int family,
return 0; return 0;
} }
/**
* apparmor_socket_bind - check perms before bind addr to socket
*/
static int apparmor_socket_bind(struct socket *sock, static int apparmor_socket_bind(struct socket *sock,
struct sockaddr *address, int addrlen) struct sockaddr *address, int addrlen)
{ {
@ -1172,9 +1165,6 @@ static int apparmor_socket_bind(struct socket *sock,
aa_sk_perm(OP_BIND, AA_MAY_BIND, sock->sk)); aa_sk_perm(OP_BIND, AA_MAY_BIND, sock->sk));
} }
/**
* apparmor_socket_connect - check perms before connecting @sock to @address
*/
static int apparmor_socket_connect(struct socket *sock, static int apparmor_socket_connect(struct socket *sock,
struct sockaddr *address, int addrlen) struct sockaddr *address, int addrlen)
{ {
@ -1188,9 +1178,6 @@ static int apparmor_socket_connect(struct socket *sock,
aa_sk_perm(OP_CONNECT, AA_MAY_CONNECT, sock->sk)); aa_sk_perm(OP_CONNECT, AA_MAY_CONNECT, sock->sk));
} }
/**
* apparmor_socket_listen - check perms before allowing listen
*/
static int apparmor_socket_listen(struct socket *sock, int backlog) static int apparmor_socket_listen(struct socket *sock, int backlog)
{ {
AA_BUG(!sock); AA_BUG(!sock);
@ -1202,9 +1189,7 @@ static int apparmor_socket_listen(struct socket *sock, int backlog)
aa_sk_perm(OP_LISTEN, AA_MAY_LISTEN, sock->sk)); aa_sk_perm(OP_LISTEN, AA_MAY_LISTEN, sock->sk));
} }
/** /*
* apparmor_socket_accept - check perms before accepting a new connection.
*
* Note: while @newsock is created and has some information, the accept * Note: while @newsock is created and has some information, the accept
* has not been done. * has not been done.
*/ */
@ -1233,18 +1218,12 @@ static int aa_sock_msg_perm(const char *op, u32 request, struct socket *sock,
aa_sk_perm(op, request, sock->sk)); aa_sk_perm(op, request, sock->sk));
} }
/**
* apparmor_socket_sendmsg - check perms before sending msg to another socket
*/
static int apparmor_socket_sendmsg(struct socket *sock, static int apparmor_socket_sendmsg(struct socket *sock,
struct msghdr *msg, int size) struct msghdr *msg, int size)
{ {
return aa_sock_msg_perm(OP_SENDMSG, AA_MAY_SEND, sock, msg, size); return aa_sock_msg_perm(OP_SENDMSG, AA_MAY_SEND, sock, msg, size);
} }
/**
* apparmor_socket_recvmsg - check perms before receiving a message
*/
static int apparmor_socket_recvmsg(struct socket *sock, static int apparmor_socket_recvmsg(struct socket *sock,
struct msghdr *msg, int size, int flags) struct msghdr *msg, int size, int flags)
{ {
@ -1263,17 +1242,11 @@ static int aa_sock_perm(const char *op, u32 request, struct socket *sock)
aa_sk_perm(op, request, sock->sk)); aa_sk_perm(op, request, sock->sk));
} }
/**
* apparmor_socket_getsockname - check perms before getting the local address
*/
static int apparmor_socket_getsockname(struct socket *sock) static int apparmor_socket_getsockname(struct socket *sock)
{ {
return aa_sock_perm(OP_GETSOCKNAME, AA_MAY_GETATTR, sock); return aa_sock_perm(OP_GETSOCKNAME, AA_MAY_GETATTR, sock);
} }
/**
* apparmor_socket_getpeername - check perms before getting remote address
*/
static int apparmor_socket_getpeername(struct socket *sock) static int apparmor_socket_getpeername(struct socket *sock)
{ {
return aa_sock_perm(OP_GETPEERNAME, AA_MAY_GETATTR, sock); return aa_sock_perm(OP_GETPEERNAME, AA_MAY_GETATTR, sock);
@ -1292,9 +1265,6 @@ static int aa_sock_opt_perm(const char *op, u32 request, struct socket *sock,
aa_sk_perm(op, request, sock->sk)); aa_sk_perm(op, request, sock->sk));
} }
/**
* apparmor_socket_getsockopt - check perms before getting socket options
*/
static int apparmor_socket_getsockopt(struct socket *sock, int level, static int apparmor_socket_getsockopt(struct socket *sock, int level,
int optname) int optname)
{ {
@ -1302,9 +1272,6 @@ static int apparmor_socket_getsockopt(struct socket *sock, int level,
level, optname); level, optname);
} }
/**
* apparmor_socket_setsockopt - check perms before setting socket options
*/
static int apparmor_socket_setsockopt(struct socket *sock, int level, static int apparmor_socket_setsockopt(struct socket *sock, int level,
int optname) int optname)
{ {
@ -1312,9 +1279,6 @@ static int apparmor_socket_setsockopt(struct socket *sock, int level,
level, optname); level, optname);
} }
/**
* apparmor_socket_shutdown - check perms before shutting down @sock conn
*/
static int apparmor_socket_shutdown(struct socket *sock, int how) static int apparmor_socket_shutdown(struct socket *sock, int how)
{ {
return aa_sock_perm(OP_SHUTDOWN, AA_MAY_SHUTDOWN, sock); return aa_sock_perm(OP_SHUTDOWN, AA_MAY_SHUTDOWN, sock);
@ -1323,6 +1287,8 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
#ifdef CONFIG_NETWORK_SECMARK #ifdef CONFIG_NETWORK_SECMARK
/** /**
* apparmor_socket_sock_rcv_skb - check perms before associating skb to sk * apparmor_socket_sock_rcv_skb - check perms before associating skb to sk
* @sk: sk to associate @skb with
* @skb: skb to check for perms
* *
* Note: can not sleep may be called with locks held * Note: can not sleep may be called with locks held
* *
@ -1354,6 +1320,11 @@ static struct aa_label *sk_peer_label(struct sock *sk)
/** /**
* apparmor_socket_getpeersec_stream - get security context of peer * apparmor_socket_getpeersec_stream - get security context of peer
* @sock: socket that we are trying to get the peer context of
* @optval: output - buffer to copy peer name to
* @optlen: output - size of copied name in @optval
* @len: size of @optval buffer
* Returns: 0 on success, -errno of failure
* *
* Note: for tcp only valid if using ipsec or cipso on lan * Note: for tcp only valid if using ipsec or cipso on lan
*/ */
@ -2182,7 +2153,7 @@ __initcall(apparmor_nf_ip_init);
static char nulldfa_src[] = { static char nulldfa_src[] = {
#include "nulldfa.in" #include "nulldfa.in"
}; };
struct aa_dfa *nulldfa; static struct aa_dfa *nulldfa;
static char stacksplitdfa_src[] = { static char stacksplitdfa_src[] = {
#include "stacksplitdfa.in" #include "stacksplitdfa.in"

View File

@ -99,13 +99,14 @@ const char *const aa_profile_mode_names[] = {
}; };
static void aa_free_pdb(struct aa_policydb *policy) static void aa_free_pdb(struct aa_policydb *pdb)
{ {
if (policy) { if (pdb) {
aa_put_dfa(policy->dfa); aa_put_dfa(pdb->dfa);
if (policy->perms) if (pdb->perms)
kvfree(policy->perms); kvfree(pdb->perms);
aa_free_str_table(&policy->trans); aa_free_str_table(&pdb->trans);
kfree(pdb);
} }
} }

View File

@ -478,6 +478,8 @@ static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
if (!table) if (!table)
goto fail; goto fail;
strs->table = table;
strs->size = size;
for (i = 0; i < size; i++) { for (i = 0; i < size; i++) {
char *str; char *str;
int c, j, pos, size2 = aa_unpack_strdup(e, &str, NULL); int c, j, pos, size2 = aa_unpack_strdup(e, &str, NULL);
@ -520,14 +522,11 @@ static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
goto fail; goto fail;
if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail; goto fail;
strs->table = table;
strs->size = size;
} }
return true; return true;
fail: fail:
kfree_sensitive(table); aa_free_str_table(strs);
e->pos = saved_pos; e->pos = saved_pos;
return false; return false;
} }
@ -833,6 +832,10 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
tmpname = aa_splitn_fqname(name, strlen(name), &tmpns, &ns_len); tmpname = aa_splitn_fqname(name, strlen(name), &tmpns, &ns_len);
if (tmpns) { if (tmpns) {
if (!tmpname) {
info = "empty profile name";
goto fail;
}
*ns_name = kstrndup(tmpns, ns_len, GFP_KERNEL); *ns_name = kstrndup(tmpns, ns_len, GFP_KERNEL);
if (!*ns_name) { if (!*ns_name) {
info = "out of memory"; info = "out of memory";
@ -1022,8 +1025,10 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
} }
} else if (rules->policy->dfa && } else if (rules->policy->dfa &&
rules->policy->start[AA_CLASS_FILE]) { rules->policy->start[AA_CLASS_FILE]) {
aa_put_pdb(rules->file);
rules->file = aa_get_pdb(rules->policy); rules->file = aa_get_pdb(rules->policy);
} else { } else {
aa_put_pdb(rules->file);
rules->file = aa_get_pdb(nullpdb); rules->file = aa_get_pdb(nullpdb);
} }
error = -EPROTO; error = -EPROTO;

View File

@ -278,7 +278,9 @@ static int profile_tracer_perm(const struct cred *cred,
/** /**
* aa_may_ptrace - test if tracer task can trace the tracee * aa_may_ptrace - test if tracer task can trace the tracee
* @tracer_cred: cred of task doing the tracing (NOT NULL)
* @tracer: label of the task doing the tracing (NOT NULL) * @tracer: label of the task doing the tracing (NOT NULL)
* @tracee_cred: cred of task to be traced
* @tracee: task label to be traced * @tracee: task label to be traced
* @request: permission request * @request: permission request
* *