Hello,
Thanks for the comments.
----- "Eric Paris" <epa...@redhat.com> wrote:
> A couple functions I think you can safely drop a level of indentation
> include audit_log_crypto_op(), audit_filter_rules(), and maybe
> log_crypto_op() needs a helper function to cut down the indentation?
> Maybe not.
Fixed all of these.
> I really don't like %s in audit_log_format(). So unless its easy to
> prove that the string meets all the rules and always will meet the
> rules, please use audit_log_string() (and in this code I noticed that I
> could not verify 'operation' in this patch, which makes me very
> nervous.
The callers ensure that the inputs are trusted, but I did have untrusted input
there at least once, so it is indeed safer.
Attached is an updated patch; in addition to the above changes, it also splits
struct audit_crypto_op to three to avoid an union, making the code easier to
read and more similar to other auxiliary data structures in auditsc.c.
Mirekdiff --git a/include/linux/audit.h b/include/linux/audit.h
index 3c7a358..cfb3363 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -122,6 +122,11 @@
#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_STORAGE_KEY 1600 /* Key storage key configured */
+#define AUDIT_CRYPTO_USERSPACE_OP 1601 /* User-space crypto operation */
+#define AUDIT_CRYPTO_KEY_VALUE 1602 /* Public values of a key, immediatelly
+ follows USERSPACE_OP. */
+
#define AUDIT_FIRST_KERN_ANOM_MSG 1700
#define AUDIT_LAST_KERN_ANOM_MSG 1799
#define AUDIT_ANOM_PROMISCUOUS 1700 /* Device changed promiscuous mode */
@@ -207,6 +212,7 @@
#define AUDIT_OBJ_TYPE 21
#define AUDIT_OBJ_LEV_LOW 22
#define AUDIT_OBJ_LEV_HIGH 23
+#define AUDIT_CRYPTO_OP 24
/* These are ONLY useful when checking
* at syscall exit time (AUDIT_AT_EXIT). */
@@ -314,6 +320,20 @@ enum {
#define AUDIT_PERM_READ 4
#define AUDIT_PERM_ATTR 8
+#define AUDIT_CRYPTO_OP_CONTEXT_NEW 1
+#define AUDIT_CRYPTO_OP_CONTEXT_DEL 2
+#define AUDIT_CRYPTO_OP_SESSION_INIT 3
+#define AUDIT_CRYPTO_OP_SESSION_OP 4
+#define AUDIT_CRYPTO_OP_SESSION_FINAL 5
+#define AUDIT_CRYPTO_OP_KEY_IMPORT 6
+#define AUDIT_CRYPTO_OP_KEY_EXPORT 7
+#define AUDIT_CRYPTO_OP_KEY_WRAP 8
+#define AUDIT_CRYPTO_OP_KEY_UNWRAP 9
+#define AUDIT_CRYPTO_OP_KEY_GEN 10
+#define AUDIT_CRYPTO_OP_KEY_DERIVE 11
+#define AUDIT_CRYPTO_OP_KEY_ZEROIZE 12
+#define AUDIT_CRYPTO_OP_KEY_GET_INFO 13
+
struct audit_status {
__u32 mask; /* Bit mask for valid entries */
__u32 enabled; /* 1 = enabled, 0 = disabled */
@@ -404,6 +424,12 @@ struct audit_field {
void *lsm_rule;
};
+struct audit_crypto_value {
+ char name;
+ void *value;
+ size_t value_size;
+};
+
#define AUDITSC_INVALID 0
#define AUDITSC_SUCCESS 1
#define AUDITSC_FAILURE 2
@@ -479,6 +505,12 @@ 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 context, int session,
+ const char *operation, const char *algorithm,
+ int key1, void *key1_id, size_t key1_id_size,
+ int key2, void *key2_id, size_t key2_id_size);
+extern void __audit_log_crypto_values(const struct audit_crypto_value *values,
+ size_t num_values);
static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
@@ -532,6 +564,27 @@ 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 context, int session,
+ const char *operation,
+ const char *algorithm, int key1,
+ void *key1_id, size_t key1_id_size,
+ int key2, void *key2_id,
+ size_t key2_id_size)
+{
+ if (likely(audit_dummy_context()))
+ return 0;
+ return __audit_log_crypto_op(op, context, session, operation, algorithm,
+ key1, key1_id, key1_id_size, key2, key2_id,
+ key2_id_size);
+}
+
+static inline void audit_log_crypto_values(const struct audit_crypto_value *a,
+ size_t num_values)
+{
+ if (unlikely(!audit_dummy_context()))
+ __audit_log_crypto_values(a, num_values);
+}
+
extern int audit_n_rules;
extern int audit_signals;
#else
@@ -565,6 +618,8 @@ 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, ctx, sess, k1, id1, size1, k2, id2, size2) (0)
+#define audit_log_crypto_values(a, values, num_values) ((void)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 a706040..a25a587 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -363,6 +363,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;
@@ -457,6 +458,7 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
case AUDIT_ARG1:
case AUDIT_ARG2:
case AUDIT_ARG3:
+ case AUDIT_CRYPTO_OP:
break;
case AUDIT_ARCH:
entry->rule.arch_f = f;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index fc0f928..3085398 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -50,6 +50,7 @@
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/mount.h>
+#include <linux/ncr.h>
#include <linux/socket.h>
#include <linux/mqueue.h>
#include <linux/audit.h>
@@ -80,6 +81,9 @@
/* no execve audit message should be longer than this (userspace limits) */
#define MAX_EXECVE_AUDIT_LEN 7500
+/* Number of public key values that can be audited */
+#define AUDIT_MAX_CRYPTO_KEY_VALUES 4
+
/* number of audit rules */
int audit_n_rules;
@@ -157,6 +161,32 @@ struct audit_aux_data_capset {
struct audit_cap_data cap;
};
+#define AUDIT_CRYPTO_OP_KEY_VALUE -1
+struct audit_crypto_header {
+ struct list_head list;
+ int op; /* AUDIT_CRYPTO_OP_KEY_VALUE for key values */
+};
+
+struct audit_crypto_op {
+ struct audit_crypto_header hdr;
+ int context;
+ int session;
+ const char *operation;
+ const char *algorithm;
+ int key1;
+ unsigned char key1_id[MAX_KEY_ID_SIZE];
+ size_t key1_id_size;
+ int key2;
+ unsigned char key2_id[MAX_KEY_ID_SIZE];
+ size_t key2_id_size;
+};
+
+struct audit_crypto_values {
+ struct audit_crypto_header hdr;
+ size_t num_values;
+ struct audit_crypto_value values[AUDIT_MAX_CRYPTO_KEY_VALUES];
+};
+
struct audit_tree_refs {
struct audit_tree_refs *next;
struct audit_chunk *c[31];
@@ -181,6 +211,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 */
@@ -632,6 +663,21 @@ 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_header *ax;
+
+ if (!ctx)
+ break;
+ list_for_each_entry(ax, &ctx->crypto, list) {
+ if (ax->op == AUDIT_CRYPTO_OP_KEY_VALUE)
+ continue;
+ result = audit_comparator(ax->op, f->op,
+ f->val);
+ if (result)
+ break;
+ }
+ break;
+ }
}
if (!result) {
@@ -824,9 +870,23 @@ static inline void audit_free_names(struct audit_context *context)
context->pwd.mnt = NULL;
}
+static void audit_free_crypto_op(struct audit_crypto_header *crypto)
+{
+ if (crypto->op == AUDIT_CRYPTO_OP_KEY_VALUE) {
+ struct audit_crypto_values *val;
+ size_t i;
+
+ val = (struct audit_crypto_values *)crypto;
+ for (i = 0; i < val->num_values; i++)
+ kfree(val->values[i].value);
+ }
+ kfree(crypto);
+}
+
static inline void audit_free_aux(struct audit_context *context)
{
struct audit_aux_data *aux;
+ struct audit_crypto_header *crypto, *tmp;
while ((aux = context->aux)) {
context->aux = aux->next;
@@ -836,6 +896,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);
+ audit_free_crypto_op(crypto);
+ }
}
static inline void audit_zero_context(struct audit_context *context,
@@ -853,6 +917,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;
}
@@ -1310,12 +1375,91 @@ 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_header *header)
+{
+ static const char *const ops[] = {
+ [AUDIT_CRYPTO_OP_CONTEXT_NEW] = "context_new",
+ [AUDIT_CRYPTO_OP_CONTEXT_DEL] = "context_del",
+ [AUDIT_CRYPTO_OP_SESSION_INIT] = "session_init",
+ [AUDIT_CRYPTO_OP_SESSION_OP] = "session_op",
+ [AUDIT_CRYPTO_OP_SESSION_FINAL] = "session_final",
+ [AUDIT_CRYPTO_OP_KEY_IMPORT] = "key_import",
+ [AUDIT_CRYPTO_OP_KEY_EXPORT] = "key_export",
+ [AUDIT_CRYPTO_OP_KEY_WRAP] = "key_wrap",
+ [AUDIT_CRYPTO_OP_KEY_UNWRAP] = "key_unwrap",
+ [AUDIT_CRYPTO_OP_KEY_GEN] = "key_gen",
+ [AUDIT_CRYPTO_OP_KEY_DERIVE] = "key_derive",
+ [AUDIT_CRYPTO_OP_KEY_ZEROIZE] = "key_zeroize",
+ [AUDIT_CRYPTO_OP_KEY_GET_INFO] = "key_get_info",
+ };
+
+ const struct audit_crypto_op *crypto;
+ struct audit_buffer *ab;
+
+ crypto = (struct audit_crypto_op *)header;
+ ab = audit_log_start(context, GFP_KERNEL, AUDIT_CRYPTO_USERSPACE_OP);
+ if (!ab)
+ return;
+ if (header->op < ARRAY_SIZE(ops) && ops[header->op] != NULL)
+ audit_log_format(ab, "crypto_op=%s", ops[header->op]);
+ else
+ audit_log_format(ab, "crypto_op=%d", header->op);
+ audit_log_format(ab, " ctx=%d", crypto->context);
+ if (crypto->session != -1)
+ audit_log_format(ab, " session=%d", crypto->session);
+ if (crypto->operation != NULL) {
+ audit_log_format(ab, " operation=");
+ audit_log_string(ab, crypto->operation);
+ }
+ if (crypto->algorithm != NULL) {
+ audit_log_format(ab, " algo=");
+ audit_log_string(ab, crypto->algorithm);
+ }
+ if (crypto->key1 != -1) {
+ audit_log_format(ab, " key1=%d", crypto->key1);
+ if (crypto->key1_id_size > 0) {
+ audit_log_format(ab, " key1_id=");
+ audit_log_n_untrustedstring(ab, crypto->key1_id,
+ crypto->key1_id_size);
+ }
+ }
+ if (crypto->key2 != -1) {
+ audit_log_format(ab, " key2=%d", crypto->key2);
+ if (crypto->key2_id_size > 0) {
+ audit_log_format(ab, " key2_id=");
+ audit_log_n_untrustedstring(ab, crypto->key2_id,
+ crypto->key2_id_size);
+ }
+ }
+ audit_log_end(ab);
+}
+
+static void log_crypto_values(struct audit_context *context,
+ const struct audit_crypto_header *header)
+{
+ struct audit_buffer *ab;
+ const struct audit_crypto_values *crypto;
+ const struct audit_crypto_value *v;
+
+ crypto = (struct audit_crypto_values *)header;
+ ab = audit_log_start(context, GFP_KERNEL, AUDIT_CRYPTO_KEY_VALUE);
+ if (!ab)
+ return;
+ for (v = crypto->values; v < crypto->values + crypto->num_values; v++) {
+ audit_log_format(ab, " key_value_%c=", v->name);
+ audit_log_n_hex(ab, v->value, v->value_size);
+ }
+ 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_header *crypto;
const char *tty;
/* tsk == current */
@@ -1442,6 +1586,13 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
call_panic = 1;
}
+ list_for_each_entry(crypto, &context->crypto, list) {
+ if (crypto->op != AUDIT_CRYPTO_OP_KEY_VALUE)
+ log_crypto_op(context, crypto);
+ else
+ log_crypto_values(context, crypto);
+ }
+
if (context->target_pid &&
audit_log_pid_context(context, context->target_pid,
context->target_auid, context->target_uid,
@@ -2486,6 +2637,89 @@ void __audit_log_capset(pid_t pid,
}
/**
+ * __audit_log_crypto_op - store information about an user-space crypto op
+ * @op: AUDIT_CRYPTO_OP_*
+ * @context: user-space context ID
+ * @session: session ID within @context, or -1
+ * @operation: more detailed operation description, or NULL
+ * @algorithm: algorithm (crypto API transform) name, or NULL
+ * @key1: ID of key 1 within @context, or -1
+ * @key1_id: user-space ID of key 1 set from user-space if @key1 != -1
+ * @key1_id_size: Size of @key1_id
+ * @key2: ID of key 2 within @context, or -1
+ * @key2_id: user-space ID of key 2 set from user-space if @key2 != -1
+ * @key2_id_size: Size of @key2_id
+ */
+int __audit_log_crypto_op(int op, int context, int session,
+ const char *operation, const char *algorithm,
+ int key1, void *key1_id, size_t key1_id_size,
+ int key2, void *key2_id, size_t key2_id_size)
+{
+ struct audit_crypto_op *ax;
+ struct audit_context *ctx = current->audit_context;
+
+ ax = kmalloc(sizeof(*ax), GFP_KERNEL);
+ if (!ax)
+ return -ENOMEM;
+
+ ax->hdr.op = op;
+ ax->context = context;
+ ax->session = session;
+ ax->operation = operation;
+ ax->algorithm = algorithm;
+ ax->key1 = key1;
+ if (key1 != -1) {
+ ax->key1_id_size = min(key1_id_size, sizeof(ax->key1_id));
+ memcpy(ax->key1_id, key1_id, ax->key1_id_size);
+ } else
+ ax->key1_id_size = 0;
+ ax->key2 = key2;
+ if (key2 != -1) {
+ ax->key2_id_size = min(key2_id_size, sizeof(ax->key2_id));
+ memcpy(ax->key2_id, key2_id, ax->key2_id_size);
+ } else
+ ax->key2_id_size = 0;
+ list_add_tail(&ax->hdr.list, &ctx->crypto);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__audit_log_crypto_op);
+
+/**
+ * __audit_log_crypto_op - store information about values of crypto keys
+ * @values: individual byte arrays describing the key. The "value" members
+ * must be allocated using kmalloc(), this function will take care of freeing
+ * them.
+ * @num_values: number of elements in @values
+ */
+void __audit_log_crypto_values(const struct audit_crypto_value *values,
+ size_t num_values)
+{
+ struct audit_crypto_values *ax;
+ struct audit_context *ctx = current->audit_context;
+ size_t i;
+
+ ax = kmalloc(sizeof(*ax), GFP_KERNEL);
+ if (!ax)
+ goto free_values;
+
+ ax->hdr.op = AUDIT_CRYPTO_OP_KEY_VALUE;
+ BUG_ON(num_values > ARRAY_SIZE(ax->values));
+ ax->num_values = num_values;
+ for (i = 0; i < num_values; i++) {
+ ax->values[i].name = values[i].name;
+ ax->values[i].value = values[i].value;
+ ax->values[i].value_size = values[i].value_size;
+ }
+ list_add_tail(&ax->hdr.list, &ctx->crypto);
+ return;
+
+free_values:
+ for (i = 0; i < num_values; i++)
+ kfree(values[i].value);
+}
+EXPORT_SYMBOL_GPL(__audit_log_crypto_values);
+
+/**
* audit_core_dumps - record information about processes that end abnormally
* @signr: signal value
*
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
