2010-11-23 12:47:15

by Miloslav Trmac

[permalink] [raw]
Subject: RFC: AF_ALG auditing

Hello,
attached is an user-space patch that adds support for auditing uses of the AF_ALG protocol family developed by Herbert Xu to provide user-space access to kernel crypto accelerators. Kernel patches will follow.

One new record is defined: AUDIT_CRYPTO_USERSPACE_OP. An audited event is always caused by a syscall, and all other syscall-related data (process identity, syscall result) is audited in the usual records.

To disable auditing crypto by default and to allow the users to selectively enable them using filters, a new filter field AUDIT_CRYPTO_OP is defined; auditing of all crypto operations can thus be enabled using (auditctl -a exit,always -F crypto_op!=0).

In addition to the user-space patch, attached are also a few example audit entries.
Mirek


Attachments:
audit-2.0.5-AF_ALG.patch (8.95 kB)
audit-examples (4.10 kB)
Download all attachments

2010-11-23 12:50:54

by Miloslav Trmac

[permalink] [raw]
Subject: [PATCH 4/5] Audit type-independent events


Signed-off-by: Miloslav Trmač <[email protected]>
---
crypto/af_alg.c | 30 ++++++++++++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 490ae43..fc1b0f7 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -14,6 +14,7 @@

#include <asm/atomic.h>
#include <crypto/if_alg.h>
+#include <linux/audit.h>
#include <linux/crypto.h>
#include <linux/idr.h>
#include <linux/init.h>
@@ -160,6 +161,11 @@ static void alg_sk_destruct(struct sock *sk) {}
void af_alg_sk_destruct_child(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
+ struct alg_sock *parent_ask = alg_sk(ask->parent);
+
+ audit_log_crypto_op(AUDIT_CRYPTO_OP_CTX_DEL, parent_ask->id,
+ ask->id, -1,
+ ask->type->alg_name(parent_ask->private), NULL);

sock_put(ask->parent);
alg_sk_destruct(sk);
@@ -235,6 +241,11 @@ static int alg_setkey(struct sock *sk, char __user *ukey,
u8 *key;
int err;

+ err = audit_log_crypto_op(AUDIT_CRYPTO_OP_TFM_KEY_IMPORT, ask->id, -1,
+ -1, type->alg_name(ask->private), NULL);
+ if (err)
+ return err;
+
key = sock_kmalloc(sk, keylen, GFP_KERNEL);
if (!key)
return -ENOMEM;
@@ -315,6 +326,14 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
alg_sk(sk2)->parent = sk;
alg_sk(sk2)->type = type;

+ err = audit_log_crypto_op(AUDIT_CRYPTO_OP_CTX_NEW, ask->id,
+ alg_sk(sk2)->id, -1,
+ type->alg_name(ask->private), NULL);
+ if (err) {
+ sk_free(sk2);
+ return err;
+ }
+
newsock->ops = type->ops;
newsock->state = SS_CONNECTED;

@@ -359,6 +378,9 @@ static void alg_sock_destruct(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);

+ audit_log_crypto_op(AUDIT_CRYPTO_OP_TFM_DEL, ask->id, -1, -1, NULL,
+ NULL);
+
alg_do_release(ask->type, ask->private);
alg_sk_destruct(sk);
}
@@ -379,6 +401,14 @@ static int alg_create(struct net *net, struct socket *sock, int protocol,
if (!sk)
goto out;

+ err = audit_log_crypto_op(AUDIT_CRYPTO_OP_TFM_NEW, alg_sk(sk)->id, -1,
+ -1, NULL, NULL);
+ if (err) {
+ alg_sk_destruct(sk);
+ sk_free(sk);
+ goto out;
+ }
+
sock->ops = &alg_proto_ops;
sock_init_data(sock, sk);

--
1.7.3.2

2010-11-23 12:50:52

by Miloslav Trmac

[permalink] [raw]
Subject: [PATCH 1/5] Add general crypto auditing infrastructure

Collect audited crypto operations in a list, because a single _exit()
can cause several AF_ALG sockets to be closed, and each needs to be
audited.

Add the AUDIT_CRYPTO_OP field so that crypto operations are not audited
by default, but auditing can be enabled using a rule (probably
"-F crypto_op!=0").

Signed-off-by: Miloslav Trmač <[email protected]>
---
include/linux/audit.h | 22 +++++++++++
kernel/auditfilter.c | 2 +
kernel/auditsc.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+), 0 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index f391d45..a9516da 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -122,6 +122,8 @@
#define AUDIT_MAC_UNLBL_STCADD 1416 /* NetLabel: add a static label */
#define AUDIT_MAC_UNLBL_STCDEL 1417 /* NetLabel: del a static label */

+#define AUDIT_CRYPTO_USERSPACE_OP 1600 /* User-space crypto operation */
+
#define AUDIT_FIRST_KERN_ANOM_MSG 1700
#define AUDIT_LAST_KERN_ANOM_MSG 1799
#define AUDIT_ANOM_PROMISCUOUS 1700 /* Device changed promiscuous mode */
@@ -219,6 +221,7 @@
#define AUDIT_PERM 106
#define AUDIT_DIR 107
#define AUDIT_FILETYPE 108
+#define AUDIT_CRYPTO_OP 109

#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
@@ -314,6 +317,13 @@ enum {
#define AUDIT_PERM_READ 4
#define AUDIT_PERM_ATTR 8

+#define AUDIT_CRYPTO_OP_TFM_NEW 1
+#define AUDIT_CRYPTO_OP_TFM_KEY_IMPORT 2
+#define AUDIT_CRYPTO_OP_TFM_DEL 3
+#define AUDIT_CRYPTO_OP_CTX_NEW 4
+#define AUDIT_CRYPTO_OP_CTX_OP 5
+#define AUDIT_CRYPTO_OP_CTX_DEL 6
+
struct audit_status {
__u32 mask; /* Bit mask for valid entries */
__u32 enabled; /* 1 = enabled, 0 = disabled */
@@ -478,6 +488,8 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
const struct cred *new,
const struct cred *old);
extern void __audit_log_capset(pid_t pid, const struct cred *new, const struct cred *old);
+extern int __audit_log_crypto_op(int op, int tfm, int ctx, int ctx2,
+ const char *algorithm, const char *operation);

static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
@@ -531,6 +543,15 @@ static inline void audit_log_capset(pid_t pid, const struct cred *new,
__audit_log_capset(pid, new, old);
}

+static inline int audit_log_crypto_op(int op, int tfm, int ctx, int ctx2,
+ const char *algorithm,
+ const char *operation)
+{
+ if (unlikely(!audit_dummy_context()))
+ return __audit_log_crypto_op(op, tfm, ctx, ctx2, algorithm, operation);
+ return 0;
+}
+
extern int audit_n_rules;
extern int audit_signals;
#else
@@ -564,6 +585,7 @@ extern int audit_signals;
#define audit_mq_getsetattr(d,s) ((void)0)
#define audit_log_bprm_fcaps(b, ncr, ocr) ({ 0; })
#define audit_log_capset(pid, ncr, ocr) ((void)0)
+#define audit_log_crypto_op(op, tfm, ctx, ctx2, algorithm, operation) (0)
#define audit_ptrace(t) ((void)0)
#define audit_n_rules 0
#define audit_signals 0
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index ce08041..6f35eed 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -364,6 +364,7 @@ static struct audit_entry *audit_rule_to_entry(struct audit_rule *rule)
case AUDIT_DEVMINOR:
case AUDIT_EXIT:
case AUDIT_SUCCESS:
+ case AUDIT_CRYPTO_OP:
/* bit ops are only useful on syscall args */
if (f->op == Audit_bitmask || f->op == Audit_bittest)
goto exit_free;
@@ -454,6 +455,7 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
case AUDIT_DEVMINOR:
case AUDIT_EXIT:
case AUDIT_SUCCESS:
+ case AUDIT_CRYPTO_OP:
case AUDIT_ARG0:
case AUDIT_ARG1:
case AUDIT_ARG2:
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 3828ad5..3894713 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -158,6 +158,16 @@ struct audit_aux_data_capset {
struct audit_cap_data cap;
};

+struct audit_crypto_op {
+ struct list_head list;
+ int op;
+ int tfm;
+ int ctx; /* -1 means N/A */
+ int ctx2; /* -1 means N/A */
+ const char *algorithm; /* NULL means N/A */
+ const char *operation; /* NULL means N/A */
+};
+
struct audit_tree_refs {
struct audit_tree_refs *next;
struct audit_chunk *c[31];
@@ -182,6 +192,7 @@ struct audit_context {
struct audit_context *previous; /* For nested syscalls */
struct audit_aux_data *aux;
struct audit_aux_data *aux_pids;
+ struct list_head crypto;
struct sockaddr_storage *sockaddr;
size_t sockaddr_len;
/* Save things to print about task_struct */
@@ -633,6 +644,19 @@ static int audit_filter_rules(struct task_struct *tsk,
case AUDIT_FILETYPE:
result = audit_match_filetype(ctx, f->val);
break;
+ case AUDIT_CRYPTO_OP: {
+ struct audit_crypto_op *ax;
+
+ if (!ctx)
+ break;
+ list_for_each_entry(ax, &ctx->crypto, list) {
+ result = audit_comparator(ax->op, f->op,
+ f->val);
+ if (result)
+ break;
+ }
+ break;
+ }
}

if (!result) {
@@ -828,6 +852,7 @@ static inline void audit_free_names(struct audit_context *context)
static inline void audit_free_aux(struct audit_context *context)
{
struct audit_aux_data *aux;
+ struct audit_crypto_op *crypto, *tmp;

while ((aux = context->aux)) {
context->aux = aux->next;
@@ -837,6 +862,10 @@ static inline void audit_free_aux(struct audit_context *context)
context->aux_pids = aux->next;
kfree(aux);
}
+ list_for_each_entry_safe(crypto, tmp, &context->crypto, list) {
+ list_del(&crypto->list);
+ kfree(crypto);
+ }
}

static inline void audit_zero_context(struct audit_context *context,
@@ -854,6 +883,7 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
if (!(context = kmalloc(sizeof(*context), GFP_KERNEL)))
return NULL;
audit_zero_context(context, state);
+ INIT_LIST_HEAD(&context->crypto);
INIT_LIST_HEAD(&context->killed_trees);
return context;
}
@@ -1311,12 +1341,50 @@ static void show_special(struct audit_context *context, int *call_panic)
audit_log_end(ab);
}

+static void log_crypto_op(struct audit_context *context,
+ const struct audit_crypto_op *crypto)
+{
+ static const char *const ops[] = {
+ [AUDIT_CRYPTO_OP_TFM_NEW] = "tfm_new",
+ [AUDIT_CRYPTO_OP_TFM_KEY_IMPORT] = "tfm_key_import",
+ [AUDIT_CRYPTO_OP_TFM_DEL] = "tfm_del",
+ [AUDIT_CRYPTO_OP_CTX_NEW] = "ctx_new",
+ [AUDIT_CRYPTO_OP_CTX_OP] = "ctx_op",
+ [AUDIT_CRYPTO_OP_CTX_DEL] = "ctx_del",
+ };
+
+ struct audit_buffer *ab;
+
+ ab = audit_log_start(context, GFP_KERNEL, AUDIT_CRYPTO_USERSPACE_OP);
+ if (!ab)
+ return;
+ if (crypto->op < ARRAY_SIZE(ops) && ops[crypto->op] != NULL)
+ audit_log_format(ab, "crypto_op=%s", ops[crypto->op]);
+ else
+ audit_log_format(ab, "crypto_op=%d", crypto->op);
+ audit_log_format(ab, " tfm=%d", crypto->tfm);
+ if (crypto->ctx != -1)
+ audit_log_format(ab, " ctx=%d", crypto->ctx);
+ if (crypto->ctx2 != -1)
+ audit_log_format(ab, " ctx2=%d", crypto->ctx2);
+ if (crypto->algorithm != NULL) {
+ audit_log_format(ab, " algorithm=");
+ audit_log_string(ab, crypto->algorithm);
+ }
+ if (crypto->operation != NULL) {
+ audit_log_format(ab, " operation=");
+ audit_log_string(ab, crypto->operation);
+ }
+ audit_log_end(ab);
+}
+
static void audit_log_exit(struct audit_context *context, struct task_struct *tsk)
{
const struct cred *cred;
int i, call_panic = 0;
struct audit_buffer *ab;
struct audit_aux_data *aux;
+ struct audit_crypto_op *crypto;
const char *tty;

/* tsk == current */
@@ -1443,6 +1511,9 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
call_panic = 1;
}

+ list_for_each_entry(crypto, &context->crypto, list)
+ log_crypto_op(context, crypto);
+
if (context->target_pid &&
audit_log_pid_context(context, context->target_pid,
context->target_auid, context->target_uid,
@@ -2484,6 +2555,35 @@ void __audit_log_capset(pid_t pid,
}

/**
+ * __audit_log_crypto_op - store information about an user-space crypto op
+ * @op: AUDIT_CRYPTO_OP_*
+ * @tfm: unique transform ID
+ * @ctx: unique context ID, or -1
+ * @ctx2: secondary context ID, or -1
+ * @algorithm: algorithm (crypto API transform) name, or NULL
+ * @operation: more detailed operation description, or NULL
+ */
+int __audit_log_crypto_op(int op, int tfm, int ctx, int ctx2,
+ const char *algorithm, const char *operation)
+{
+ struct audit_crypto_op *ax;
+
+ ax = kmalloc(sizeof(*ax), GFP_KERNEL);
+ if (!ax)
+ return -ENOMEM;
+
+ ax->op = op;
+ ax->tfm = tfm;
+ ax->ctx = ctx;
+ ax->ctx2 = ctx2;
+ ax->algorithm = algorithm;
+ ax->operation = operation;
+ list_add_tail(&ax->list, &current->audit_context->crypto);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__audit_log_crypto_op);
+
+/**
* audit_core_dumps - record information about processes that end abnormally
* @signr: signal value
*
--
1.7.3.2

2010-11-23 12:50:56

by Miloslav Trmac

[permalink] [raw]
Subject: [PATCH 5/5] Audit type-specific crypto operations


Signed-off-by: Miloslav Trmač <[email protected]>
---
crypto/af_alg.c | 14 ++++++++++++++
crypto/algif_hash.c | 27 +++++++++++++++++++++++----
crypto/algif_skcipher.c | 15 +++++++++++++++
include/crypto/if_alg.h | 6 ++++++
4 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index fc1b0f7..450d51a 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -542,6 +542,20 @@ void af_alg_complete(struct crypto_async_request *req, int err)
}
EXPORT_SYMBOL_GPL(af_alg_complete);

+#ifdef CONFIG_AUDIT
+int af_alg_audit_crypto_op(struct sock *sk, const char *operation, int ctx2)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ struct alg_sock *parent_ask = alg_sk(ask->parent);
+ const char *alg_name;
+
+ alg_name = parent_ask->type->alg_name(parent_ask->private);
+ return audit_log_crypto_op(AUDIT_CRYPTO_OP_CTX_OP, parent_ask->id,
+ ask->id, ctx2, alg_name, operation);
+}
+EXPORT_SYMBOL_GPL(af_alg_audit_crypto_op);
+#endif
+
static int __init af_alg_init(void)
{
int err = proto_register(&alg_proto, 0);
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 3a61e9d..7e3ffd1 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -46,6 +46,10 @@ static int hash_sendmsg(struct kiocb *unused, struct socket *sock,
long copied = 0;
int err;

+ err = af_alg_audit_crypto_op(sk, "hash-input", -1);
+ if (err)
+ return err;
+
if (limit > sk->sk_sndbuf)
limit = sk->sk_sndbuf;

@@ -112,6 +116,10 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;

+ err = af_alg_audit_crypto_op(sk, "hash-input", -1);
+ if (err)
+ return err;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
@@ -154,6 +162,10 @@ static int hash_recvmsg(struct kiocb *unused, struct socket *sock,
unsigned ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req));
int err;

+ err = af_alg_audit_crypto_op(sk, "hash-output", -1);
+ if (err)
+ return err;
+
if (len > ds)
len = ds;
else if (len < ds)
@@ -202,12 +214,19 @@ static int hash_accept(struct socket *sock, struct socket *newsock, int flags)
ctx2 = ask2->private;
ctx2->more = 1;

+ err = af_alg_audit_crypto_op(sk, "hash-clone", ask2->id);
+ if (err)
+ goto error_sk2;
+
err = crypto_ahash_import(&ctx2->req, state);
- if (err) {
- sock_orphan(sk2);
- sock_put(sk2);
- }
+ if (err)
+ goto error_sk2;
+
+ return err;

+error_sk2:
+ sock_orphan(sk2);
+ sock_put(sk2);
return err;
}

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index e14c8be..f69a109 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -283,6 +283,11 @@ static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
memcpy(ctx->iv, con.iv->iv, ivsize);
}

+ err = af_alg_audit_crypto_op(sk, ctx->enc ? "encrypt-input"
+ : "decrypt-input", -1);
+ if (err)
+ goto unlock;
+
limit = max_t(int, sk->sk_sndbuf, PAGE_SIZE);
limit -= ctx->used;

@@ -384,6 +389,11 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
int err = -EINVAL;
int limit;

+ err = af_alg_audit_crypto_op(sk, ctx->enc ? "encrypt-input"
+ : "decrypt-input", -1);
+ if (err)
+ return err;
+
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
@@ -443,6 +453,11 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
int used;
long copied = 0;

+ err = af_alg_audit_crypto_op(sk, ctx->enc ? "encrypt-output"
+ : "decrypt-output", -1);
+ if (err)
+ return err;
+
lock_sock(sk);
for (iov = msg->msg_iov, iovlen = msg->msg_iovlen; iovlen > 0;
iovlen--, iov++) {
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 092c599..6650ae5 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -80,6 +80,12 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con);
int af_alg_wait_for_completion(int err, struct af_alg_completion *completion);
void af_alg_complete(struct crypto_async_request *req, int err);

+#ifdef CONFIG_AUDIT
+int af_alg_audit_crypto_op(struct sock *sk, const char *operation, int ctx2);
+#else
+#define af_alg_audit_crypto_op(sk, operation, ctx2) (0)
+#endif
+
static inline struct alg_sock *alg_sk(struct sock *sk)
{
return (struct alg_sock *)sk;
--
1.7.3.2

2010-11-23 12:50:51

by Miloslav Trmac

[permalink] [raw]
Subject: [PATCH 2/5] Add unique IDs to AF_ALG sockets

Ideally we should be able to use i_ino of the inode associated with the
socket, but i_ino can have duplicate values if the static counter inside
new_inode() wraps around.

Signed-off-by: Miloslav Trmač <[email protected]>
---
crypto/af_alg.c | 66 +++++++++++++++++++++++++++++++++++++++++++++-
crypto/algif_hash.c | 2 +-
crypto/algif_skcipher.c | 2 +-
include/crypto/if_alg.h | 10 +++---
4 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index cabed0e..490ae43 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -15,10 +15,12 @@
#include <asm/atomic.h>
#include <crypto/if_alg.h>
#include <linux/crypto.h>
+#include <linux/idr.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/net.h>
#include <linux/rwsem.h>

@@ -38,6 +40,10 @@ static struct proto alg_proto = {

static LIST_HEAD(alg_types);
static DECLARE_RWSEM(alg_types_sem);
+#ifdef CONFIG_AUDIT
+static DEFINE_MUTEX(alg_ida_mutex);
+static DEFINE_IDA(alg_ida);
+#endif

static const struct af_alg_type *alg_get_type(const char *name)
{
@@ -107,6 +113,59 @@ int af_alg_unregister_type(const struct af_alg_type *type)
}
EXPORT_SYMBOL_GPL(af_alg_unregister_type);

+static struct sock *alg_sk_alloc(struct net *net)
+{
+ struct sock *sk;
+
+ sk = sk_alloc(net, PF_ALG, GFP_KERNEL, &alg_proto);
+ if (!sk)
+ goto out;
+
+#ifdef CONFIG_AUDIT
+ mutex_lock(&alg_ida_mutex);
+ /* ida_pre_get() should preallocate enough, and, due to
+ lists_ida_mutex, nobody else can use the preallocated data.
+ Therefore the loop recommended in idr_get_new() documentation is not
+ necessary. */
+ if (ida_pre_get(&alg_ida, GFP_KERNEL) == 0 ||
+ ida_get_new(&alg_ida, &alg_sk(sk)->id) != 0) {
+ mutex_unlock(&alg_ida_mutex);
+ alg_sk(sk)->id = -1;
+ sk_free(sk);
+ sk = NULL;
+ goto out;
+ }
+ mutex_unlock(&alg_ida_mutex);
+#endif
+
+out:
+ return sk;
+}
+
+#ifdef CONFIG_AUDIT
+static void alg_sk_destruct(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+
+ if (ask->id != -1) {
+ mutex_lock(&alg_ida_mutex);
+ ida_remove(&alg_ida, ask->id);
+ mutex_unlock(&alg_ida_mutex);
+ }
+}
+#else
+static void alg_sk_destruct(struct sock *sk) {}
+#endif
+
+void af_alg_sk_destruct_child(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+
+ sock_put(ask->parent);
+ alg_sk_destruct(sk);
+}
+EXPORT_SYMBOL_GPL(af_alg_sk_destruct_child);
+
static void alg_do_release(const struct af_alg_type *type, void *private)
{
if (!type)
@@ -236,7 +295,7 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
if (!type)
goto unlock;

- sk2 = sk_alloc(sock_net(sk), PF_ALG, GFP_KERNEL, &alg_proto);
+ sk2 = alg_sk_alloc(sock_net(sk));
err = -ENOMEM;
if (!sk2)
goto unlock;
@@ -245,6 +304,7 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)

err = type->accept(ask->private, sk2);
if (err) {
+ alg_sk_destruct(sk2);
sk_free(sk2);
goto unlock;
}
@@ -300,6 +360,7 @@ static void alg_sock_destruct(struct sock *sk)
struct alg_sock *ask = alg_sk(sk);

alg_do_release(ask->type, ask->private);
+ alg_sk_destruct(sk);
}

static int alg_create(struct net *net, struct socket *sock, int protocol,
@@ -314,7 +375,7 @@ static int alg_create(struct net *net, struct socket *sock, int protocol,
return -EPROTONOSUPPORT;

err = -ENOMEM;
- sk = sk_alloc(net, PF_ALG, GFP_KERNEL, &alg_proto);
+ sk = alg_sk_alloc(net);
if (!sk)
goto out;

@@ -474,6 +535,7 @@ static void __exit af_alg_exit(void)
{
sock_unregister(PF_ALG);
proto_unregister(&alg_proto);
+ ida_destroy(&alg_ida);
}

module_init(af_alg_init);
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 62122a1..f08a42c 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -256,7 +256,7 @@ static void hash_sock_destruct(struct sock *sk)
sock_kfree_s(sk, ctx->result,
crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)));
sock_kfree_s(sk, ctx, ctx->len);
- af_alg_release_parent(sk);
+ af_alg_sk_destruct_child(sk);
}

static int hash_accept_parent(void *private, struct sock *sk)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index d8d3ddf..4069460 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -575,7 +575,7 @@ static void skcipher_sock_destruct(struct sock *sk)
skcipher_free_sgl(sk);
sock_kfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));
sock_kfree_s(sk, ctx, ctx->len);
- af_alg_release_parent(sk);
+ af_alg_sk_destruct_child(sk);
}

static int skcipher_accept_parent(void *private, struct sock *sk)
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index c5813c8..336b9f2 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -31,6 +31,9 @@ struct alg_sock {

const struct af_alg_type *type;
void *private;
+#ifdef CONFIG_AUDIT
+ int id;
+#endif
};

struct af_alg_completion {
@@ -62,6 +65,8 @@ struct af_alg_sgl {
int af_alg_register_type(const struct af_alg_type *type);
int af_alg_unregister_type(const struct af_alg_type *type);

+void af_alg_sk_destruct_child(struct sock *sk);
+
int af_alg_release(struct socket *sock);
int af_alg_accept(struct sock *sk, struct socket *newsock);

@@ -79,11 +84,6 @@ static inline struct alg_sock *alg_sk(struct sock *sk)
return (struct alg_sock *)sk;
}

-static inline void af_alg_release_parent(struct sock *sk)
-{
- sock_put(alg_sk(sk)->parent);
-}
-
static inline void af_alg_init_completion(struct af_alg_completion *completion)
{
init_completion(&completion->completion);
--
1.7.3.2

2010-11-23 12:50:52

by Miloslav Trmac

[permalink] [raw]
Subject: [PATCH 3/5] Add "alg_name" operation to af_alg_type.


Signed-off-by: Miloslav Trmač <[email protected]>
---
crypto/algif_hash.c | 6 ++++++
crypto/algif_skcipher.c | 6 ++++++
include/crypto/if_alg.h | 1 +
3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index f08a42c..3a61e9d 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -243,6 +243,11 @@ static void hash_release(void *private)
crypto_free_ahash(private);
}

+static const char *hash_alg_name(void *private)
+{
+ return crypto_tfm_alg_name(crypto_ahash_tfm(private));
+}
+
static int hash_setkey(void *private, const u8 *key, unsigned int keylen)
{
return crypto_ahash_setkey(private, key, keylen);
@@ -296,6 +301,7 @@ static int hash_accept_parent(void *private, struct sock *sk)
static const struct af_alg_type algif_type_hash = {
.bind = hash_bind,
.release = hash_release,
+ .alg_name = hash_alg_name,
.setkey = hash_setkey,
.accept = hash_accept_parent,
.ops = &algif_hash_ops,
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 4069460..e14c8be 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -561,6 +561,11 @@ static void skcipher_release(void *private)
crypto_free_ablkcipher(private);
}

+static const char *skcipher_alg_name(void *private)
+{
+ return crypto_tfm_alg_name(crypto_ablkcipher_tfm(private));
+}
+
static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
{
return crypto_ablkcipher_setkey(private, key, keylen);
@@ -619,6 +624,7 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
static const struct af_alg_type algif_type_skcipher = {
.bind = skcipher_bind,
.release = skcipher_release,
+ .alg_name = skcipher_alg_name,
.setkey = skcipher_setkey,
.accept = skcipher_accept_parent,
.ops = &algif_skcipher_ops,
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 336b9f2..092c599 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -49,6 +49,7 @@ struct af_alg_control {
struct af_alg_type {
void *(*bind)(const char *name, u32 type, u32 mask);
void (*release)(void *private);
+ const char *(*alg_name)(void *private);
int (*setkey)(void *private, const u8 *key, unsigned int keylen);
int (*accept)(void *private, struct sock *sk);

--
1.7.3.2

2010-11-23 15:12:41

by Eric Paris

[permalink] [raw]
Subject: Re: [PATCH 1/5] Add general crypto auditing infrastructure

On Tue, 2010-11-23 at 13:50 +0100, Miloslav Trmač wrote:
> Collect audited crypto operations in a list, because a single _exit()
> can cause several AF_ALG sockets to be closed, and each needs to be
> audited.
>
> Add the AUDIT_CRYPTO_OP field so that crypto operations are not audited
> by default, but auditing can be enabled using a rule (probably
> "-F crypto_op!=0").

Just an implementation question, why a new list instead of finding a way
to reuse struct audit_aux_data?

-Eric

2010-11-23 18:25:55

by Miloslav Trmac

[permalink] [raw]
Subject: Re: [PATCH 1/5] Add general crypto auditing infrastructure

----- "Eric Paris" <[email protected]> wrote:
> On Tue, 2010-11-23 at 13:50 +0100, Miloslav Trmač wrote:
> > Collect audited crypto operations in a list, because a single _exit()
> > can cause several AF_ALG sockets to be closed, and each needs to be
> > audited.
> >
> > Add the AUDIT_CRYPTO_OP field so that crypto operations are not
> audited
> > by default, but auditing can be enabled using a rule (probably
> > "-F crypto_op!=0").
>
> Just an implementation question, why a new list instead of finding a way
> to reuse struct audit_aux_data?
This remained in the code from an earlier version where the relative order of crypto records was meaningful. In the current version the only difference is that an AUDIT_CRYPTO_OP filter has to traverse fewer entries.

Thanks for pointing this out, I'll drop the list.
Mirek

2010-11-23 18:37:21

by Eric Paris

[permalink] [raw]
Subject: Re: [PATCH 1/5] Add general crypto auditing infrastructure

On Tue, 2010-11-23 at 13:25 -0500, Miloslav Trmac wrote:
> ----- "Eric Paris" <[email protected]> wrote:
> > On Tue, 2010-11-23 at 13:50 +0100, Miloslav Trmač wrote:
> > > Collect audited crypto operations in a list, because a single _exit()
> > > can cause several AF_ALG sockets to be closed, and each needs to be
> > > audited.
> > >
> > > Add the AUDIT_CRYPTO_OP field so that crypto operations are not
> > audited
> > > by default, but auditing can be enabled using a rule (probably
> > > "-F crypto_op!=0").
> >
> > Just an implementation question, why a new list instead of finding a
> way
> > to reuse struct audit_aux_data?
> This remained in the code from an earlier version where the relative
> order of crypto records was meaningful. In the current version the
> only difference is that an AUDIT_CRYPTO_OP filter has to traverse
> fewer entries.

It probably won't actually have to traverse extra entries. We shouldn't
(at least that I can think of) ever have a single syscall which is going
to have crypto, execve, signal, fcaps, etc. records simultaneously. In
any case, if you send another round, I'd suggest reuse or aux.

-Eric