| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 26 Mar 2009 22:50:31 +0100
From: Pavel Machek <pavel@....cz>
To: James Morris <jmorris@...ei.org>,
kernel list <linux-kernel@...r.kernel.org>
Subject: TOMOYO in linux-next
Hi!
I don't think merging that is good idea. Security should be doable
without making shell-like glob matching...
+/**
+ * tomoyo_file_matches_to_pattern2 - Pattern matching without '/' character
+ * and "\-" pattern.
+ *
+ * @filename: The start of string to check.
+ * @filename_end: The end of string to check.
+ * @pattern: The start of pattern to compare.
+ * @pattern_end: The end of pattern to compare.
+ *
+ * Returns true if @filename matches @pattern, false otherwise.
+ */
+static bool tomoyo_file_matches_to_pattern2(const char *filename,
...or code like...
+/**
+ * tomoyo_path_depth - Evaluate the number of '/' in a string.
+ *
+ * @pathname: The string to evaluate.
+ *
+ * Returns path depth of the string.
+ *
+ * I score 2 for each of the '/' in the @pathname
+ * and score 1 if the @pathname ends with '/'.
+ */
+static int tomoyo_path_depth(const char *pathname)
...or code like...
+/* String table for functionality that takes 2 modes. */
+static const char *tomoyo_mode_2[4] = {
+ "disabled", "enabled", "enabled", "enabled"
+};
Are those interfaces documented somewhere?
This is quite nasty. I don't think turning off enforcement in
interrupt is good idea. ("fails open").
+/**
+ * tomoyo_check_flags - Check mode for specified functionality.
+ *
+ * @domain: Pointer to "struct tomoyo_domain_info".
+ * @index: The functionality to check mode.
+ *
+ * TOMOYO checks only process context.
+ * This code disables TOMOYO's enforcement in case the function is
called from
+ * interrupt context.
+ */
+unsigned int tomoyo_check_flags(const struct tomoyo_domain_info
*domain,
+ const u8 index)
+{
+ const u8 profile = domain->profile;
+
+ if (WARN_ON(in_interrupt()))
+ return 0;
Comments leave something to be desired:
+ /***** EXCLUSIVE SECTION START *****/
+/**
+ * tomoyo_close_control - close() for /sys/kernel/security/tomoyo/
interface.
+ *
+ * @file: Pointer to "struct file".
I'm not sure basing security on pids is good idea...
+ /* Don't allow updating policies by non manager programs. */
+ if (head->write != tomoyo_write_pid &&
Hmm, barrier is spelled otherwise, and I'm not sure I'd trust this:
+struct tomoyo_path_info_with_data {
+ /* Keep "head" first, for this pointer is passed to tomoyo_free(). */
+ struct tomoyo_path_info head;
+ char bariier1[16]; /* Safeguard for overrun. */
I guess constants should be used here:
+#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
+ if (domain2->is_deleted != 255)
+ printk(KERN_DEBUG
+ "Marked %p as non undeletable\n",
+ domain2);
+#endif
+ domain2->is_deleted = 255;
(I don't know why we want undelete in tomoyo.)
If it contains copyright, it should contain copyright. It probably
should not contain version numbers.
+/*
+ * security/tomoyo/file.c
+ *
+ * Implementation of the Domain-Based Mandatory Access Control.
+ *
+ * Copyright (C) 2005-2009 NTT DATA CORPORATION
+ *
+ * Version: 2.2.0-pre 2009/02/01
Code is full of tables such as:
+/* Keyword array for single path operations. */
+static const char
*tomoyo_sp_keyword[TOMOYO_MAX_SINGLE_PATH_OPERATION] = {
+ [TOMOYO_TYPE_READ_WRITE_ACL] = "read/write",
+ [TOMOYO_TYPE_EXECUTE_ACL] = "execute",
+ [TOMOYO_TYPE_READ_ACL] = "read",
+ [TOMOYO_TYPE_WRITE_ACL] = "write",
+ [TOMOYO_TYPE_CREATE_ACL] = "create",
+ [TOMOYO_TYPE_UNLINK_ACL] = "unlink",
+ [TOMOYO_TYPE_MKDIR_ACL] = "mkdir",
+ [TOMOYO_TYPE_RMDIR_ACL] = "rmdir",
+ [TOMOYO_TYPE_MKFIFO_ACL] = "mkfifo",
+ [TOMOYO_TYPE_MKSOCK_ACL] = "mksock",
+ [TOMOYO_TYPE_MKBLOCK_ACL] = "mkblock",
+ [TOMOYO_TYPE_MKCHAR_ACL] = "mkchar",
+ [TOMOYO_TYPE_TRUNCATE_ACL] = "truncate",
+ [TOMOYO_TYPE_SYMLINK_ACL] = "symlink",
+ [TOMOYO_TYPE_REWRITE_ACL] = "rewrite",
+};
Can we get an interface that does not need as many strings/ as much
string parsing?
That's my main complaint: Documentation.*tomoyo nor
Documentation.*TOMOYO does not exist, still this adds a *lot* of new
user<->kernel interfaces.
New user<->kernel interaces should be documented and very carefuly
reviewed; I don't think that happened here.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists