[PATCH] allow disabling IMA at runtime

[PATCH] allow disabling IMA at runtime

[Kyle McMartin]

From: Kyle McMartin <kyle@redhat.com>

Due to a memory leak in IMA that we're currently debugging in Fedora
rawhide, it would be nice to be able to disable that support at runtime.
Currently it's only able to be built in, and there's no toggle to avoid
initializing it.

Provide one, in order to enhance debuggability. If a user can reboot a
machine and edit its command line, one can do a far sight worse things
than disabling a security precaution.

Signed-off-by: Kyle McMartin <kyle@redhat.com>

---
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7936b80..0d1b1ed 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -926,6 +926,11 @@ and is between 256 and 4096 characters. It is defined in the file
 	ihash_entries=	[KNL]
 			Set number of hash buckets for inode cache.
 
+	ima=		[IMA]
+			Format: { "0" | "1" }
+			0 -- disable IMA.
+			1 -- enable IMA. (default)
+
 	ima_audit=	[IMA]
 			Format: { "0" | "1" }
 			0 -- integrity auditing messages. (Default)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 101c512..cc7603e 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -339,10 +339,27 @@ int ima_bprm_check(struct linux_binprm *bprm)
 	return 0;
 }
 
+static int ima_disabled = 0;
+static int __init ima_enabled(char *str)
+{
+	unsigned long enabled;
+
+	if (!strict_strtoul(str, 0, &enabled))
+		ima_disabled = enabled ? 0 : 1;
+
+	return 1;
+}
+__setup("ima=", ima_enabled);
+
 static int __init init_ima(void)
 {
 	int error;
 
+	if (ima_disabled) {
+		pr_info("IMA disabled at user request.\n");
+		return 0;
+	}
+
 	ima_iintcache_init();
 	error = ima_init();
 	ima_initialized = 1;

Re: [PATCH] allow disabling IMA at runtime

[Mimi Zohar]

Kyle McMartin <kyle@infradead.org> wrote on 08/25/2009 10:10:05 PM:

> From: Kyle McMartin <kyle@redhat.com>
> 
> Due to a memory leak in IMA that we're currently debugging in Fedora
> rawhide, it would be nice to be able to disable that support at runtime.
> Currently it's only able to be built in, and there's no toggle to avoid
> initializing it.
> 
> Provide one, in order to enhance debuggability. If a user can reboot a
> machine and edit its command line, one can do a far sight worse things
> than disabling a security precaution.
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>

Have you tried using Eric's patch
"IMA: Minimal IMA policy and boot param for TCB IMA policy"
(commit 5789ba3bd0a3cd20df5980ebf03358f2eb44fd67), which introduced
the ima_tcb=1 command line option?  It wasn't backported to 2.6.30.

Mimi

> ---
> diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
> index 7936b80..0d1b1ed 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -926,6 +926,11 @@ and is between 256 and 4096 characters. It is 
defined in the file
>     ihash_entries=   [KNL]
>           Set number of hash buckets for inode cache.
> 
> +   ima=      [IMA]
> +         Format: { "0" | "1" }
> +         0 -- disable IMA.
> +         1 -- enable IMA. (default)
> +
>     ima_audit=   [IMA]
>           Format: { "0" | "1" }
>           0 -- integrity auditing messages. (Default)
> diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
> index 101c512..cc7603e 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -339,10 +339,27 @@ int ima_bprm_check(struct linux_binprm *bprm)
>     return 0;
>  }
> 
> +static int ima_disabled = 0;
> +static int __init ima_enabled(char *str)
> +{
> +   unsigned long enabled;
> +
> +   if (!strict_strtoul(str, 0, &enabled))
> +      ima_disabled = enabled ? 0 : 1;
> +
> +   return 1;
> +}
> +__setup("ima=", ima_enabled);
> +
>  static int __init init_ima(void)
>  {
>     int error;
> 
> +   if (ima_disabled) {
> +      pr_info("IMA disabled at user request.\n");
> +      return 0;
> +   }
> +
>     ima_iintcache_init();
>     error = ima_init();
>     ima_initialized = 1;

Re: [PATCH] allow disabling IMA at runtime

[Eric Paris]

On Wed, 2009-08-26 at 08:07 -0400, Mimi Zohar wrote:
> Kyle McMartin <kyle@infradead.org> wrote on 08/25/2009 10:10:05 PM:
> 
> > From: Kyle McMartin <kyle@redhat.com>
> > 
> > Due to a memory leak in IMA that we're currently debugging in Fedora
> > rawhide, it would be nice to be able to disable that support at runtime.
> > Currently it's only able to be built in, and there's no toggle to avoid
> > initializing it.
> > 
> > Provide one, in order to enhance debuggability. If a user can reboot a
> > machine and edit its command line, one can do a far sight worse things
> > than disabling a security precaution.
> > 
> > Signed-off-by: Kyle McMartin <kyle@redhat.com>
> 
> Have you tried using Eric's patch
> "IMA: Minimal IMA policy and boot param for TCB IMA policy"
> (commit 5789ba3bd0a3cd20df5980ebf03358f2eb44fd67), which introduced
> the ima_tcb=1 command line option?  It wasn't backported to 2.6.30.

Hey Mimi, I was going to get in touch with you today, I don't really
think this patch is necessary.  Kyle hacked it together because it was a
quick and dirty 'fix' for a memory leak that he didn't want to hunt down
and he knows I won't let him compile IMA out *smile*.  Intended to try
to track it down this morning, but I'm getting swamped already, maybe
you can try to figure out what's going on before I get a chance to come
back to it this afternoon?

nfs_inode_cache       34     34   1824   17    8 : tunables    0    0    0 : slabdata      2      2      0
fuse_inode            22     22   1472   22    8 : tunables    0    0    0 : slabdata      1      1      0
rpc_inode_cache       40     40   1600   20    8 : tunables    0    0    0 : slabdata      2      2      0
btrfs_inode_cache  10622  10668   2328   14    8 : tunables    0    0    0 : slabdata    762    762      0
iint_cache        369714 369720    312   26    2 : tunables    0    0    0 : slabdata  14220  14220      0
mqueue_inode_cache     19     19   1664   19    8 : tunables    0    0    0 : slabdata      1      1      0
isofs_inode_cache      0      0   1288   25    8 : tunables    0    0    0 : slabdata      0      0      0
hugetlbfs_inode_cache     24     24   1312   24    8 : tunables    0    0    0 : slabdata      1      1      0
ext4_inode_cache       0      0   1864   17    8 : tunables    0    0    0 : slabdata      0      0      0
ext3_inode_cache      19     19   1656   19    8 : tunables    0    0    0 : slabdata      1      1      0
inotify_inode_mark_entry    253    255    240   17    1 : tunables    0    0    0 : slabdata     15     15      0
shmem_inode_cache   2740   3003   1560   21    8 : tunables    0    0    0 : slabdata    143    143      0
sock_inode_cache     902    920   1408   23    8 : tunables    0    0    0 : slabdata     40     40      0
proc_inode_cache    3060   3075   1288   25    8 : tunables    0    0    0 : slabdata    123    123      0
inode_cache         9943  10192   1240   26    8 : tunables    0    0    0 : slabdata    392    392      0
selinux_inode_security  27237  27838    264   31    2 : tunables    0    0    0 : slabdata    898    898      0

So the iint_cache is a LOT larger than all of the inode caches put
together.  This is a 2.6.31-0.167.rc6.git6.fc12.x86_64 kernel without
any kernel options.

-Eric

-Eric

Re: [PATCH] allow disabling IMA at runtime

[Andrew Morton]

On Tue, 25 Aug 2009 22:10:05 -0400
Kyle McMartin <kyle@mcmartin.ca> wrote:

> From: Kyle McMartin <kyle@redhat.com>
> 
> Due to a memory leak in IMA that we're currently debugging in Fedora
> rawhide, it would be nice to be able to disable that support at runtime.
> Currently it's only able to be built in, and there's no toggle to avoid
> initializing it.
> 
> Provide one, in order to enhance debuggability. If a user can reboot a
> machine and edit its command line, one can do a far sight worse things
> than disabling a security precaution.
> 

nits:

> 
> ---
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 7936b80..0d1b1ed 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -926,6 +926,11 @@ and is between 256 and 4096 characters. It is defined in the file
>  	ihash_entries=	[KNL]
>  			Set number of hash buckets for inode cache.
>  
> +	ima=		[IMA]
> +			Format: { "0" | "1" }
> +			0 -- disable IMA.
> +			1 -- enable IMA. (default)
> +
>  	ima_audit=	[IMA]
>  			Format: { "0" | "1" }
>  			0 -- integrity auditing messages. (Default)
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 101c512..cc7603e 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -339,10 +339,27 @@ int ima_bprm_check(struct linux_binprm *bprm)
>  	return 0;
>  }
>  
> +static int ima_disabled = 0;
> +static int __init ima_enabled(char *str)
> +{
> +	unsigned long enabled;
> +
> +	if (!strict_strtoul(str, 0, &enabled))
> +		ima_disabled = enabled ? 0 : 1;
> +	return 1;
> +}

- documentation says "0 or 1" but the implementation says "0 or non-zero".

- implementation returns `1' whether or not the argument was
  successfully parsed.

  What happens if a __setup() functions returns non-1?

  <spends a while fossicking through prehistory and RustyCode>

  From my reading of kernel/params.c:parse_args(), every __setup()
  function which returns `1' should result in printk("%s: `%s' invalid
  for parameter `%s'), so I'm all confused and giving up.

> +__setup("ima=", ima_enabled);

Are we supposed to use core_param() nowadays?

>  static int __init init_ima(void)
>  {
>  	int error;
>  
> +	if (ima_disabled) {
> +		pr_info("IMA disabled at user request.\n");
> +		return 0;
> +	}
> +
>  	ima_iintcache_init();
>  	error = ima_init();
>  	ima_initialized = 1;

Re: [PATCH] allow disabling IMA at runtime

[James Morris]

Also, do we still need this now the memory leak is fixed upstream?

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=53a7197aff20e341487fca8575275056fe1c63e5


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH] allow disabling IMA at runtime

[Rusty Russell]

On Thu, 27 Aug 2009 09:32:25 am Andrew Morton wrote:
>   From my reading of kernel/params.c:parse_args(), every __setup()
>   function which returns `1' should result in printk("%s: `%s' invalid
>   for parameter `%s'), so I'm all confused and giving up.

Nope, it's an "obsolete_param", so non-zero means success.

> > +__setup("ima=", ima_enabled);
> 
> Are we supposed to use core_param() nowadays?

Yeah, if this had backwards compatibility requirements.  But for new,
non-core params like this:

1) make sure the module is called "ima" (even if not a module: make the
   object ima.o)

2) use module_param(), and make your parameters "disable" and "audit"
   (ima_disable and ima_audit if must be nonstatic and use module_param_named).
   You don't need to write any parse functions at all!

3) Tell users to use ima.disable and ima.audit.

Thanks,
Rusty.

Re: [PATCH] allow disabling IMA at runtime

[David Safford]

>Hey Mimi, I was going to get in touch with you today, I don't really
>think this patch is necessary.  Kyle hacked it together because it was a
>quick and dirty 'fix' for a memory leak that he didn't want to hunt down
>and he knows I won't let him compile IMA out *smile*.  Intended to try
>to track it down this morning, but I'm getting swamped already, maybe
>you can try to figure out what's going on before I get a chance to come
>back to it this afternoon?
>
>nfs_inode_cache       34     34   1824   17    8 : tunables    0    0    0 : slabdata      2      2      0
>fuse_inode            22     22   1472   22    8 : tunables    0    0    0 : slabdata      1      1      0
>rpc_inode_cache       40     40   1600   20    8 : tunables    0    0    0 : slabdata      2      2      0
>btrfs_inode_cache  10622  10668   2328   14    8 : tunables    0    0    0 : slabdata    762    762      0
>iint_cache        369714 369720    312   26    2 : tunables    0    0    0 : slabdata  14220  14220      0
>mqueue_inode_cache     19     19   1664   19    8 : tunables    0    0    0 : slabdata      1      1      0
>isofs_inode_cache      0      0   1288   25    8 : tunables    0    0    0 : slabdata      0      0      0
>hugetlbfs_inode_cache     24     24   1312   24    8 : tunables    0    0    0 : slabdata      1      1      0
>ext4_inode_cache       0      0   1864   17    8 : tunables    0    0    0 : slabdata      0      0      0
>ext3_inode_cache      19     19   1656   19    8 : tunables    0    0    0 : slabdata      1      1      0
>inotify_inode_mark_entry    253    255    240   17    1 : tunables    0    0    0 : slabdata     15     15      0
>shmem_inode_cache   2740   3003   1560   21    8 : tunables    0    0    0 : slabdata    143    143      0
>sock_inode_cache     902    920   1408   23    8 : tunables    0    0    0 : slabdata     40     40      0
>proc_inode_cache    3060   3075   1288   25    8 : tunables    0    0    0 : slabdata    123    123      0
>inode_cache         9943  10192   1240   26    8 : tunables    0    0    0 : slabdata    392    392      0
>selinux_inode_security  27237  27838    264   31    2 : tunables    0    0    0 : slabdata    898    898      0
>
>So the iint_cache is a LOT larger than all of the inode caches put
>together.  This is a 2.6.31-0.167.rc6.git6.fc12.x86_64 kernel without
>any kernel options.
>
>-Eric
>

Sorry about the delay - we had a major fiber cut in Hawthorne yesterday.
I'm running 2.6.30.4, and here are my numbers, which look more reasonable.
I'm guessing there may be a IMA free imbalance in btrfs, which we have
not really tested. Are you getting imbalance messages?

I'll try to look at it today...

dave safford

fat_inode_cache       20     20    408   20    2 : tunables    0    0
  0 : slabdata      1      1      0
fat_cache              0      0     24  170    1 : tunables    0    0
  0 : slabdata      0      0      0
iint_cache         71720  73797     80   51    1 : tunables    0    0
  0 : slabdata   1447   1447      0
mqueue_inode_cache     14     14    576   14    2 : tunables    0    0
   0 : slabdata      1      1      0
isofs_inode_cache      0      0    384   21    2 : tunables    0    0
  0 : slabdata      0      0      0
hugetlbfs_inode_cache     23     23    352   23    2 : tunables    0
 0    0 : slabdata      1      1      0
ext4_inode_cache   53826  53830    584   14    2 : tunables    0    0
  0 : slabdata   3845   3845      0
ext3_inode_cache   13999  14080    512   16    2 : tunables    0    0
  0 : slabdata    880    880      0
shmem_inode_cache   1723   1734    456   17    2 : tunables    0    0
  0 : slabdata    102    102      0
sock_inode_cache     800    846    448   18    2 : tunables    0    0
  0 : slabdata     47     47      0
skbuff_fclone_cache     42     42    384   21    2 : tunables    0
0    0 : slabdata      2      2      0
file_lock_cache       78     78    104   39    1 : tunables    0    0
  0 : slabdata      2      2      0
proc_inode_cache     607    903    376   21    2 : tunables    0    0
  0 : slabdata     43     43      0
bdev_cache            48     48    512   16    2 : tunables    0    0
  0 : slabdata      3      3      0
sysfs_dir_cache    11474  11475     48   85    1 : tunables    0    0
  0 : slabdata    135    135      0
inode_cache          813   1449    352   23    2 : tunables    0    0
  0 : slabdata     63     63      0
signal_cache         190    196    576   14    2 : tunables    0    0
  0 : slabdata     14     14      0
sighand_cache        186    192   1344   12    4 : tunables    0    0
  0 : slabdata     16     16      0
idr_layer_cache      739    780    152   26    1 : tunables    0    0
  0 : slabdata     30     30      0

Re: [PATCH] allow disabling IMA at runtime

[Eric Paris]

On Thu, 2009-08-27 at 08:30 -0400, David Safford wrote:
> >Hey Mimi, I was going to get in touch with you today, I don't really
> >think this patch is necessary.  Kyle hacked it together because it was a
> >quick and dirty 'fix' for a memory leak that he didn't want to hunt down
> >and he knows I won't let him compile IMA out *smile*.  Intended to try
> >to track it down this morning, but I'm getting swamped already, maybe
> >you can try to figure out what's going on before I get a chance to come
> >back to it this afternoon?
> >
> >nfs_inode_cache       34     34   1824   17    8 : tunables    0    0    0 : slabdata      2      2      0
> >fuse_inode            22     22   1472   22    8 : tunables    0    0    0 : slabdata      1      1      0
> >rpc_inode_cache       40     40   1600   20    8 : tunables    0    0    0 : slabdata      2      2      0
> >btrfs_inode_cache  10622  10668   2328   14    8 : tunables    0    0    0 : slabdata    762    762      0
> >iint_cache        369714 369720    312   26    2 : tunables    0    0    0 : slabdata  14220  14220      0
> >mqueue_inode_cache     19     19   1664   19    8 : tunables    0    0    0 : slabdata      1      1      0
> >isofs_inode_cache      0      0   1288   25    8 : tunables    0    0    0 : slabdata      0      0      0
> >hugetlbfs_inode_cache     24     24   1312   24    8 : tunables    0    0    0 : slabdata      1      1      0
> >ext4_inode_cache       0      0   1864   17    8 : tunables    0    0    0 : slabdata      0      0      0
> >ext3_inode_cache      19     19   1656   19    8 : tunables    0    0    0 : slabdata      1      1      0
> >inotify_inode_mark_entry    253    255    240   17    1 : tunables    0    0    0 : slabdata     15     15      0
> >shmem_inode_cache   2740   3003   1560   21    8 : tunables    0    0    0 : slabdata    143    143      0
> >sock_inode_cache     902    920   1408   23    8 : tunables    0    0    0 : slabdata     40     40      0
> >proc_inode_cache    3060   3075   1288   25    8 : tunables    0    0    0 : slabdata    123    123      0
> >inode_cache         9943  10192   1240   26    8 : tunables    0    0    0 : slabdata    392    392      0
> >selinux_inode_security  27237  27838    264   31    2 : tunables    0    0    0 : slabdata    898    898      0
> >
> >So the iint_cache is a LOT larger than all of the inode caches put
> >together.  This is a 2.6.31-0.167.rc6.git6.fc12.x86_64 kernel without
> >any kernel options.
> >
> >-Eric
> >
> 
> Sorry about the delay - we had a major fiber cut in Hawthorne yesterday.
> I'm running 2.6.30.4, and here are my numbers, which look more reasonable.
> I'm guessing there may be a IMA free imbalance in btrfs, which we have
> not really tested. Are you getting imbalance messages?

I think I found it.
http://git.kernel.org/?p=linux/kernel/git/jmorris/security-testing-2.6.git;a=commitdiff;h=53a7197aff20e341487fca8575275056fe1c63e5

thanks!

-Eric