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
On Thu, 26 Mar 2009, Pavel Machek wrote:
> Hi!
>
> I don't think merging that is good idea.
It's already merged in Linus' tree.
> Security should be doable
> without making shell-like glob matching...
The TOMOYO developers have already responded to your feedback on this
issue. It's also an inherent aspect of pathname security, an issue which
has been resolved in favour of inclusion in the kernel.
As for the rest of the feedback, please work with the developers to fix
any bugs or lack of documentation.
- James
--
James Morris
<[email protected]>
On Fri 2009-03-27 11:15:28, James Morris wrote:
> On Thu, 26 Mar 2009, Pavel Machek wrote:
>
> > Hi!
> >
> > I don't think merging that is good idea.
>
> It's already merged in Linus' tree.
Too bad.
> > Security should be doable
> > without making shell-like glob matching...
>
> The TOMOYO developers have already responded to your feedback on this
> issue. It's also an inherent aspect of pathname security, an issue which
> has been resolved in favour of inclusion in the kernel.
Do you have any references? My memory claims otherwise on this.
> As for the rest of the feedback, please work with the developers to fix
> any bugs or lack of documentation.
Which brings a question: given that kernel<->user interface is
undocumented, how was this reviewed?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> As for the rest of the feedback, please work with the developers to fix
> any bugs or lack of documentation.
Apparently not even its user<->kernel interface was reviewed. This
violates "one value per file in sysfs":
[root@tomoyo]# cat /sys/kernel/security/tomoyo/meminfo
Shared: 61440
Private: 69632
Dynamic: 768
Total: 131840
You can set memory quota by writing to this file.
(Example)
[root@tomoyo]# echo Shared: 2097152 > /sys/kernel/security/tomoyo/meminfo
[root@tomoyo]# echo Private: 2097152 > /sys/kernel/security/tomoyo/meminfo
(not to mention being ugly as hell).
This is totaly useless once tomoyo is merged:
** /sys/kernel/security/tomoyo/version **
This file is used for getting TOMOYO Linux's version.
(Example)
[root@etch]# cat /sys/kernel/security/tomoyo/version
2.2.0-pre
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Fri, 27 Mar 2009, Pavel Machek wrote:
> Hi!
>
> > As for the rest of the feedback, please work with the developers to fix
> > any bugs or lack of documentation.
>
> Apparently not even its user<->kernel interface was reviewed. This
> violates "one value per file in sysfs":
>
> [root@tomoyo]# cat /sys/kernel/security/tomoyo/meminfo
> Shared: 61440
> Private: 69632
> Dynamic: 768
> Total: 131840
>
> You can set memory quota by writing to this file.
> (Example)
> [root@tomoyo]# echo Shared: 2097152 > /sys/kernel/security/tomoyo/meminfo
> [root@tomoyo]# echo Private: 2097152 > /sys/kernel/security/tomoyo/meminfo
This is not sysfs, it's securityfs, with their documentation showing it
mounted on /sys.
- James
--
James Morris
<[email protected]>
On Fri, 27 Mar 2009, Pavel Machek wrote:
> > > Security should be doable
> > > without making shell-like glob matching...
> >
> > The TOMOYO developers have already responded to your feedback on this
> > issue. It's also an inherent aspect of pathname security, an issue which
> > has been resolved in favour of inclusion in the kernel.
>
> Do you have any references? My memory claims otherwise on this.
Al Viro merged the LSM pathname hooks.
> > As for the rest of the feedback, please work with the developers to fix
> > any bugs or lack of documentation.
>
> Which brings a question: given that kernel<->user interface is
> undocumented, how was this reviewed?
By 15 iterative posts to lkml and LSM, with extensive discussion and
feedback, as well as presentations by the TOMOYO developers at various
conferences around the world.
- James
--
James Morris
<[email protected]>
Hello.
James Morris wrote:
> As for the rest of the feedback, please work with the developers to fix
> any bugs or lack of documentation.
Thanks.
Pavel Machek wrote:
> Are those interfaces documented somewhere?
They are documented at http://tomoyo.sourceforge.jp/en/2.2.x/policy-reference.html .
> This is quite nasty. I don't think turning off enforcement in
> interrupt is good idea. ("fails open").
This is not "fails open". TOMOYO deals only operations which are allowed to
sleep (e.g. opening files, making directories). This in_interrupt() check is
for safety in case somebody who are not allowed to sleep called TOMOYO's
function by error.
> I'm not sure basing security on pids is good idea...
PID is used for reaching a domain which that PID is in, not for access control
decisions.
> 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:
Oh, typo, thanks.
I think there is no need to use #define here, for nobody accesses
barrier1/barrier2.
> +#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.)
This "undelete domain" feature was introduced to allow administrators switch
domain policy periodically.
> If it contains copyright, it should contain copyright. It probably
> should not contain version numbers.
TOMOYO's management tools want /sys/kernel/security/tomoyo/version .
> Can we get an interface that does not need as many strings/ as much
> string parsing?
A plain text interface splitted by ' ' and '\n' is cleaner than introducing
binary interface. (TOMOYO uses \040 for ' ' and \012 for '\n'. No worry for
' ' and '\n' in pathnames.)
> 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.
These user<->kernel interface are for TOMOYO's policy management tools,
not for general userland applications like /bin/bash /usr/sbin/sshd etc.
Regards.
----------------------------------------
TOMOYO: Fix a typo.
Signed-off-by: Kentaro Takeda <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
Signed-off-by: Toshiharu Harada <[email protected]>
---
security/tomoyo/common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.29-git1.orig/security/tomoyo/common.h
+++ linux-2.6.29-git1/security/tomoyo/common.h
@@ -55,7 +55,7 @@ struct tomoyo_path_info {
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. */
+ char barrier1[16]; /* Safeguard for overrun. */
char body[TOMOYO_MAX_PATHNAME_LEN];
char barrier2[16]; /* Safeguard for overrun. */
};
On Fri, 27 Mar 2009, Tetsuo Handa wrote:
> ----------------------------------------
> TOMOYO: Fix a typo.
>
> Signed-off-by: Kentaro Takeda <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
> Signed-off-by: Toshiharu Harada <[email protected]>
> ---
> security/tomoyo/common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-2.6.29-git1.orig/security/tomoyo/common.h
> +++ linux-2.6.29-git1/security/tomoyo/common.h
> @@ -55,7 +55,7 @@ struct tomoyo_path_info {
> 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. */
> + char barrier1[16]; /* Safeguard for overrun. */
> char body[TOMOYO_MAX_PATHNAME_LEN];
> char barrier2[16]; /* Safeguard for overrun. */
> };
>
Applied.
--
James Morris
<[email protected]>
Pavel Machek <[email protected]> wrote:
> I don't think merging that is good idea. Security should be doable
> without making shell-like glob matching...
How do you suppose a security system should handle mozilla modifying
~/.bashrc differently from downloading something to ~/pr0n.jpg?
On Fri 2009-03-27 11:46:42, James Morris wrote:
> On Fri, 27 Mar 2009, Pavel Machek wrote:
>
> > Hi!
> >
> > > As for the rest of the feedback, please work with the developers to fix
> > > any bugs or lack of documentation.
> >
> > Apparently not even its user<->kernel interface was reviewed. This
> > violates "one value per file in sysfs":
> >
> > [root@tomoyo]# cat /sys/kernel/security/tomoyo/meminfo
> > Shared: 61440
> > Private: 69632
> > Dynamic: 768
> > Total: 131840
> >
> > You can set memory quota by writing to this file.
> > (Example)
> > [root@tomoyo]# echo Shared: 2097152 > /sys/kernel/security/tomoyo/meminfo
> > [root@tomoyo]# echo Private: 2097152 > /sys/kernel/security/tomoyo/meminfo
>
> This is not sysfs, it's securityfs, with their documentation showing it
> mounted on /sys.
Well, that's a bit better, but it still does not justify turning
securityfs into procfs-like mess... when solution is so easy. Just use
one value per file.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> > > As for the rest of the feedback, please work with the developers to fix
> > > any bugs or lack of documentation.
> >
> > Which brings a question: given that kernel<->user interface is
> > undocumented, how was this reviewed?
>
> By 15 iterative posts to lkml and LSM, with extensive discussion and
> feedback, as well as presentations by the TOMOYO developers at various
> conferences around the world.
Yeah, 15 posts to lkml, largely ignored becuase the design problems
seemed unfixable...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> > This is quite nasty. I don't think turning off enforcement in
> > interrupt is good idea. ("fails open").
> This is not "fails open". TOMOYO deals only operations which are allowed to
> sleep (e.g. opening files, making directories). This in_interrupt() check is
> for safety in case somebody who are not allowed to sleep called TOMOYO's
> function by error.
If it never happens, why not fail closed?
> > I'm not sure basing security on pids is good idea...
> PID is used for reaching a domain which that PID is in, not for access control
> decisions.
Can I get some documentation about domains etc? How will it interact
with containers?
> > 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:
> Oh, typo, thanks.
> I think there is no need to use #define here, for nobody accesses
> barrier1/barrier2.
I do believe that those barriers should be deleted. You should just
avoid buffer overruns; there's no reason 16 bytes should be enough to
protect you.
> > +#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.)
> This "undelete domain" feature was introduced to allow administrators switch
> domain policy periodically.
255 needs a constant at the very least.
> > If it contains copyright, it should contain copyright. It probably
> > should not contain version numbers.
> TOMOYO's management tools want /sys/kernel/security/tomoyo/version .
So fix them. It is better than carrying "version" forever.
> > Can we get an interface that does not need as many strings/ as much
> > string parsing?
> A plain text interface splitted by ' ' and '\n' is cleaner than introducing
> binary interface. (TOMOYO uses \040 for ' ' and \012 for '\n'. No worry for
> ' ' and '\n' in pathnames.)
\0 terminated strings?
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> > 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.
> These user<->kernel interface are for TOMOYO's policy management tools,
> not for general userland applications like /bin/bash /usr/sbin/sshd etc.
I do believe all user<->kernel interfaces should be properly
reviewed/documented. We'll have to maintain them forever. Just because
you don't intend them to be used by /bin/bash does not mean people
will not use them; and it does not mean we will not have to carry them
forever.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Fri 2009-03-27 10:28:07, Bodo Eggert wrote:
> Pavel Machek <[email protected]> wrote:
>
> > I don't think merging that is good idea. Security should be doable
> > without making shell-like glob matching...
>
> How do you suppose a security system should handle mozilla modifying
> ~/.bashrc differently from downloading something to ~/pr0n.jpg?
How does shell-like glob matching help there? You'd need to parse
/etc/passwd to find all ~ directories...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Fri, 27 Mar 2009, Pavel Machek wrote:
> On Fri 2009-03-27 10:28:07, Bodo Eggert wrote:
> > Pavel Machek <[email protected]> wrote:
> > > I don't think merging that is good idea. Security should be doable
> > > without making shell-like glob matching...
> >
> > How do you suppose a security system should handle mozilla modifying
> > ~/.bashrc differently from downloading something to ~/pr0n.jpg?
>
> How does shell-like glob matching help there? You'd need to parse
> /etc/passwd to find all ~ directories...
That is, if you'd use HOME=`dd if=/dev/urandom ...`.
If you have your users in /home/user, you can tell /home/*/.*
is bad, /home/*/[^.]* is OK.
How would you exclude mozilla from writing to .* then? ".a" is bad,
".b" is bad ...? or "A" is OK, "a" is OK, "zzzzzzzzzzzzz" is OK"?
Either way, you'd need several universes to store the security profile.
--
The enemy diversion you have been ignoring will be the main attack.
On Fri 2009-03-27 14:04:32, Bodo Eggert wrote:
> On Fri, 27 Mar 2009, Pavel Machek wrote:
> > On Fri 2009-03-27 10:28:07, Bodo Eggert wrote:
> > > Pavel Machek <[email protected]> wrote:
>
> > > > I don't think merging that is good idea. Security should be doable
> > > > without making shell-like glob matching...
> > >
> > > How do you suppose a security system should handle mozilla modifying
> > > ~/.bashrc differently from downloading something to ~/pr0n.jpg?
> >
> > How does shell-like glob matching help there? You'd need to parse
> > /etc/passwd to find all ~ directories...
>
> That is, if you'd use HOME=`dd if=/dev/urandom ...`.
>
> If you have your users in /home/user, you can tell /home/*/.*
> is bad, /home/*/[^.]* is OK.
On the common systems I know of, homes are spread over different
volumes and different directories. TOMOYO's wildcards do _not_ solve
this particular problems.
> How would you exclude mozilla from writing to .* then? ".a" is bad,
> ".b" is bad ...? or "A" is OK, "a" is OK, "zzzzzzzzzzzzz" is OK"?
> Either way, you'd need several universes to store the security profile.
What is magic about .* files? I want mozilla to store the pictures as
.naughty.picture.jpg -- I don't see anything wrong with that.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sun, 29 Mar 2009, Pavel Machek wrote:
> On Fri 2009-03-27 14:04:32, Bodo Eggert wrote:
>> On Fri, 27 Mar 2009, Pavel Machek wrote:
>>> On Fri 2009-03-27 10:28:07, Bodo Eggert wrote:
>>>> Pavel Machek <[email protected]> wrote:
>>>>> I don't think merging that is good idea. Security should be doable
>>>>> without making shell-like glob matching...
>>>>
>>>> How do you suppose a security system should handle mozilla modifying
>>>> ~/.bashrc differently from downloading something to ~/pr0n.jpg?
>>>
>>> How does shell-like glob matching help there? You'd need to parse
>>> /etc/passwd to find all ~ directories...
>>
>> That is, if you'd use HOME=`dd if=/dev/urandom ...`.
>>
>> If you have your users in /home/user, you can tell /home/*/.*
>> is bad, /home/*/[^.]* is OK.
>
> On the common systems I know of, homes are spread over different
> volumes and different directories. TOMOYO's wildcards do _not_ solve
> this particular problems.
Don't do that then. If you start having user's homes at
/usr/local/sbin/something/, your systenm is FUBAR anyway.
Put your homes into /home/volume/group/user (=~ /home/*/*/*/.*).
>> How would you exclude mozilla from writing to .* then? ".a" is bad,
>> ".b" is bad ...? or "A" is OK, "a" is OK, "zzzzzzzzzzzzz" is OK"?
>> Either way, you'd need several universes to store the security profile.
>
> What is magic about .* files? I want mozilla to store the pictures as
> .naughty.picture.jpg -- I don't see anything wrong with that.
As long as you have a guaranteed-to-be-complete list of config files, you
can get along without wildcards. And still if you do, I'll write a program
to make it incomplete.
>>> How would you exclude mozilla from writing to .* then? ".a" is bad,
>>> ".b" is bad ...? or "A" is OK, "a" is OK, "zzzzzzzzzzzzz" is OK"?
>>> Either way, you'd need several universes to store the security profile.
>>
>> What is magic about .* files? I want mozilla to store the pictures as
>> .naughty.picture.jpg -- I don't see anything wrong with that.
>
> As long as you have a guaranteed-to-be-complete list of config files, you
> can get along without wildcards. And still if you do, I'll write a
> program to make it incomplete.
Not all config files match .* pattern. I have at least hugo.ini
mxmap.ini in my ~.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sun, 29 Mar 2009, Pavel Machek wrote:
>
> >>> How would you exclude mozilla from writing to .* then? ".a" is bad,
> >>> ".b" is bad ...? or "A" is OK, "a" is OK, "zzzzzzzzzzzzz" is OK"?
> >>> Either way, you'd need several universes to store the security profile.
> >>
> >> What is magic about .* files? I want mozilla to store the pictures as
> >> .naughty.picture.jpg -- I don't see anything wrong with that.
> >
> > As long as you have a guaranteed-to-be-complete list of config files, you
> > can get along without wildcards. And still if you do, I'll write a
> > program to make it incomplete.
>
> Not all config files match .* pattern. I have at least hugo.ini
> mxmap.ini in my ~.
^^^^
I see a pattern there.
IMO there is no use in a security system if it allows you to modify
something like ~/.bashrc, and a security system not allowing mozilla to
create ~/.mozilla or ~/pr0n.jpg is not usable at all.
You must handle different files in one directory diffrerently, and since
they are not there yet, you can't label them. Instead, you'll have to label
them at runtime, and you have to do it based on the filename. At the same
time, you have a HUGE number of problematic filenames and a HUGE number of
safe filenames. Unless you have about 500 universes, you can't implement a
bitmap of allowed an non-allowed filenames.
What will you do? Give up and let mozilla modify all the config files you
didn't think of? Or not let mozilla store tux.png in ~?
--
Artificial Intelligence usually beats real stupidity.
Hello.
> > +#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.)
> This "undelete domain" feature was introduced to allow administrators switch
> domain policy periodically.
>
We reconsidered this feature and concluded that we won't need this feature.
Thanks.
----------
Subject: tomoyo: remove "undelete domain" command.
Since TOMOYO's policy management tools does not use the "undelete domain"
command, we decided to remove that command.
Signed-off-by: Kentaro Takeda <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
Signed-off-by: Toshiharu Harada <[email protected]>
---
security/tomoyo/common.c | 7 ---
security/tomoyo/common.h | 8 ----
security/tomoyo/domain.c | 90 +----------------------------------------------
3 files changed, 5 insertions(+), 100 deletions(-)
--- linux-2.6.29-git13.orig/security/tomoyo/common.c
+++ linux-2.6.29-git13/security/tomoyo/common.c
@@ -1252,15 +1252,12 @@ static int tomoyo_write_domain_policy(st
struct tomoyo_domain_info *domain = head->write_var1;
bool is_delete = false;
bool is_select = false;
- bool is_undelete = false;
unsigned int profile;
if (tomoyo_str_starts(&data, TOMOYO_KEYWORD_DELETE))
is_delete = true;
else if (tomoyo_str_starts(&data, TOMOYO_KEYWORD_SELECT))
is_select = true;
- else if (tomoyo_str_starts(&data, TOMOYO_KEYWORD_UNDELETE))
- is_undelete = true;
if (is_select && tomoyo_is_select_one(head, data))
return 0;
/* Don't allow updating policies by non manager programs. */
@@ -1274,9 +1271,7 @@ static int tomoyo_write_domain_policy(st
down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(data);
up_read(&tomoyo_domain_list_lock);
- } else if (is_undelete)
- domain = tomoyo_undelete_domain(data);
- else
+ } else
domain = tomoyo_find_or_assign_new_domain(data, 0);
head->write_var1 = domain;
return 0;
--- linux-2.6.29-git13.orig/security/tomoyo/common.h
+++ linux-2.6.29-git13/security/tomoyo/common.h
@@ -88,10 +88,7 @@ struct tomoyo_domain_info {
/* Name of this domain. Never NULL. */
const struct tomoyo_path_info *domainname;
u8 profile; /* Profile number to use. */
- u8 is_deleted; /* Delete flag.
- 0 = active.
- 1 = deleted but undeletable.
- 255 = deleted and no longer undeletable. */
+ bool is_deleted; /* Delete flag. */
bool quota_warned; /* Quota warnning flag. */
/* DOMAIN_FLAGS_*. Use tomoyo_set_domain_flag() to modify. */
u8 flags;
@@ -144,7 +141,6 @@ struct tomoyo_double_path_acl_record {
#define TOMOYO_KEYWORD_NO_INITIALIZE_DOMAIN "no_initialize_domain "
#define TOMOYO_KEYWORD_NO_KEEP_DOMAIN "no_keep_domain "
#define TOMOYO_KEYWORD_SELECT "select "
-#define TOMOYO_KEYWORD_UNDELETE "undelete "
#define TOMOYO_KEYWORD_USE_PROFILE "use_profile "
#define TOMOYO_KEYWORD_IGNORE_GLOBAL_ALLOW_READ "ignore_global_allow_read"
/* A domain definition starts with <kernel>. */
@@ -267,8 +263,6 @@ struct tomoyo_domain_info *tomoyo_find_d
struct tomoyo_domain_info *tomoyo_find_or_assign_new_domain(const char *
domainname,
const u8 profile);
-/* Undelete a domain. */
-struct tomoyo_domain_info *tomoyo_undelete_domain(const char *domainname);
/* Check mode for specified functionality. */
unsigned int tomoyo_check_flags(const struct tomoyo_domain_info *domain,
const u8 index);
--- linux-2.6.29-git13.orig/security/tomoyo/domain.c
+++ linux-2.6.29-git13/security/tomoyo/domain.c
@@ -551,9 +551,7 @@ int tomoyo_write_alias_policy(char *data
return tomoyo_update_alias_entry(data, cp, is_delete);
}
-/* Domain create/delete/undelete handler. */
-
-/* #define TOMOYO_DEBUG_DOMAIN_UNDELETE */
+/* Domain create/delete handler. */
/**
* tomoyo_delete_domain - Delete a domain.
@@ -571,41 +569,15 @@ int tomoyo_delete_domain(char *domainnam
tomoyo_fill_path_info(&name);
/***** EXCLUSIVE SECTION START *****/
down_write(&tomoyo_domain_list_lock);
-#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
- printk(KERN_DEBUG "tomoyo_delete_domain %s\n", domainname);
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
- if (tomoyo_pathcmp(domain->domainname, &name))
- continue;
- printk(KERN_DEBUG "List: %p %u\n", domain, domain->is_deleted);
- }
-#endif
/* Is there an active domain? */
list_for_each_entry(domain, &tomoyo_domain_list, list) {
- struct tomoyo_domain_info *domain2;
/* Never delete tomoyo_kernel_domain */
if (domain == &tomoyo_kernel_domain)
continue;
if (domain->is_deleted ||
tomoyo_pathcmp(domain->domainname, &name))
continue;
- /* Mark already deleted domains as non undeletable. */
- list_for_each_entry(domain2, &tomoyo_domain_list, list) {
- if (!domain2->is_deleted ||
- tomoyo_pathcmp(domain2->domainname, &name))
- continue;
-#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;
- }
- /* Delete and mark active domain as undeletable. */
- domain->is_deleted = 1;
-#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
- printk(KERN_DEBUG "Marked %p as undeletable\n", domain);
-#endif
+ domain->is_deleted = true;
break;
}
up_write(&tomoyo_domain_list_lock);
@@ -614,58 +586,6 @@ int tomoyo_delete_domain(char *domainnam
}
/**
- * tomoyo_undelete_domain - Undelete a domain.
- *
- * @domainname: The name of domain.
- *
- * Returns pointer to "struct tomoyo_domain_info" on success, NULL otherwise.
- */
-struct tomoyo_domain_info *tomoyo_undelete_domain(const char *domainname)
-{
- struct tomoyo_domain_info *domain;
- struct tomoyo_domain_info *candidate_domain = NULL;
- struct tomoyo_path_info name;
-
- name.name = domainname;
- tomoyo_fill_path_info(&name);
- /***** EXCLUSIVE SECTION START *****/
- down_write(&tomoyo_domain_list_lock);
-#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
- printk(KERN_DEBUG "tomoyo_undelete_domain %s\n", domainname);
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
- if (tomoyo_pathcmp(domain->domainname, &name))
- continue;
- printk(KERN_DEBUG "List: %p %u\n", domain, domain->is_deleted);
- }
-#endif
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
- if (tomoyo_pathcmp(&name, domain->domainname))
- continue;
- if (!domain->is_deleted) {
- /* This domain is active. I can't undelete. */
- candidate_domain = NULL;
-#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
- printk(KERN_DEBUG "%p is active. I can't undelete.\n",
- domain);
-#endif
- break;
- }
- /* Is this domain undeletable? */
- if (domain->is_deleted == 1)
- candidate_domain = domain;
- }
- if (candidate_domain) {
- candidate_domain->is_deleted = 0;
-#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
- printk(KERN_DEBUG "%p was undeleted.\n", candidate_domain);
-#endif
- }
- up_write(&tomoyo_domain_list_lock);
- /***** EXCLUSIVE SECTION END *****/
- return candidate_domain;
-}
-
-/**
* tomoyo_find_or_assign_new_domain - Create a domain.
*
* @domainname: The name of domain.
@@ -711,10 +631,6 @@ struct tomoyo_domain_info *tomoyo_find_o
/***** CRITICAL SECTION END *****/
if (flag)
continue;
-#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
- printk(KERN_DEBUG "Reusing %p %s\n", domain,
- domain->domainname->name);
-#endif
list_for_each_entry(ptr, &domain->acl_info_list, list) {
ptr->type |= TOMOYO_ACL_DELETED;
}
@@ -722,7 +638,7 @@ struct tomoyo_domain_info *tomoyo_find_o
domain->profile = profile;
domain->quota_warned = false;
mb(); /* Avoid out-of-order execution. */
- domain->is_deleted = 0;
+ domain->is_deleted = false;
goto out;
}
/* No memory reusable. Create using new memory. */
On Mon 2009-04-06 20:49:14, Tetsuo Handa wrote:
> Hello.
>
> > > +#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.)
> > This "undelete domain" feature was introduced to allow administrators switch
> > domain policy periodically.
> >
> We reconsidered this feature and concluded that we won't need this feature.
Thanks.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Mon, 6 Apr 2009, Tetsuo Handa wrote:
> Thanks.
> ----------
> Subject: tomoyo: remove "undelete domain" command.
>
> Since TOMOYO's policy management tools does not use the "undelete domain"
> command, we decided to remove that command.
>
> Signed-off-by: Kentaro Takeda <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
> Signed-off-by: Toshiharu Harada <[email protected]>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
--
James Morris
<[email protected]>