Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

2015-10-24 Thread Petko Manolov 
On 15-10-23 14:43:53, Mimi Zohar wrote:
> On Fri, 2015-10-23 at 16:05 +0300, Petko Manolov wrote:
> > On 15-10-22 21:49:25, Dmitry Kasatkin wrote:
> 
> > > diff --git a/security/integrity/ima/Kconfig 
> > > b/security/integrity/ima/Kconfig
> > > index df30334..a292b88 100644
> > > --- a/security/integrity/ima/Kconfig
> > > +++ b/security/integrity/ima/Kconfig
> > > @@ -123,14 +123,17 @@ config IMA_APPRAISE
> > > If unsure, say N.
> > >  
> > >  config IMA_TRUSTED_KEYRING
> > > - bool "Require all keys on the .ima keyring be signed"
> > > + bool "Require all keys on the .ima keyring be signed (deprecated)"
> > >   depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
> > >   depends on INTEGRITY_ASYMMETRIC_KEYS
> > > + select INTEGRITY_TRUSTED_KEYRING
> > >   default y
> > >   help
> > >  This option requires that all keys added to the .ima
> > >  keyring be signed by a key on the system trusted keyring.
> > >  
> > > +This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> > > +
> > >  config IMA_LOAD_X509
> > >   bool "Load X509 certificate onto the '.ima' trusted keyring"
> > >   depends on IMA_TRUSTED_KEYRING
> > 
> > 
> > I guess we may as well remove this switch.  Otherwise somebody have to 
> > remember 
> > to post a patch that does so a few kernel releases after this one goes 
> > mainline.
> 
> There's no harm in leaving the "IMA_TRUSTED_KEYRING" Kconfig option for a 
> couple of releases (or perhaps until it is enabled in a long term release), 
> so 
> that the INTEGRITY_TRUSTED_KEYRING Kconfig option is enabled automatically.

I have no strong opinion either way.  Just saying.  Let it be for the moment.


Petko
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

2015-10-23 Thread Mimi Zohar 
On Fri, 2015-10-23 at 16:05 +0300, Petko Manolov wrote:
> On 15-10-22 21:49:25, Dmitry Kasatkin wrote:

> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index df30334..a292b88 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -123,14 +123,17 @@ config IMA_APPRAISE
> >   If unsure, say N.
> >  
> >  config IMA_TRUSTED_KEYRING
> > -   bool "Require all keys on the .ima keyring be signed"
> > +   bool "Require all keys on the .ima keyring be signed (deprecated)"
> > depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
> > depends on INTEGRITY_ASYMMETRIC_KEYS
> > +   select INTEGRITY_TRUSTED_KEYRING
> > default y
> > help
> >This option requires that all keys added to the .ima
> >keyring be signed by a key on the system trusted keyring.
> >  
> > +  This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> > +
> >  config IMA_LOAD_X509
> > bool "Load X509 certificate onto the '.ima' trusted keyring"
> > depends on IMA_TRUSTED_KEYRING
> 
> 
> I guess we may as well remove this switch.  Otherwise somebody have to 
> remember 
> to post a patch that does so a few kernel releases after this one goes 
> mainline.

There's no harm in leaving the "IMA_TRUSTED_KEYRING" Kconfig option for
a couple of releases (or perhaps until it is enabled in a long term
release), so that the INTEGRITY_TRUSTED_KEYRING Kconfig option is
enabled automatically.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

2015-10-23 Thread Dmitry Kasatkin 


From: Petko Manolov [pet...@mip-labs.com]
Sent: Friday, October 23, 2015 4:05 PM
To: Dmitry Kasatkin
Cc: zo...@linux.vnet.ibm.com; linux-ima-de...@lists.sourceforge.net; 
linux-security-module@vger.kernel.org; linux-ker...@vger.kernel.org; Dmitry 
Kasatkin
Subject: Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' 
keyring

On 15-10-22 21:49:25, Dmitry Kasatkin wrote:
> Require all keys added to the EVM keyring be signed by an
> existing trusted key on the system trusted keyring.
>
> This patch also switches IMA to use integrity_init_keyring().
>
> Changes in v3:
> * Added 'init_keyring' config based variable to skip initializing
>   keyring instead of using  __integrity_init_keyring() wrapper.
> * Added dependency back to CONFIG_IMA_TRUSTED_KEYRING
>
> Changes in v2:
> * Replace CONFIG_EVM_TRUSTED_KEYRING with IMA and EVM common
>   CONFIG_INTEGRITY_TRUSTED_KEYRING configuration option
> * Deprecate CONFIG_IMA_TRUSTED_KEYRING but keep it for config
>   file compatibility. (Mimi Zohar)
>
> Signed-off-by: Dmitry Kasatkin <dmitry.kasat...@huawei.com>
> ---
>  security/integrity/Kconfig| 11 +++
>  security/integrity/digsig.c   | 14 --
>  security/integrity/evm/evm_main.c |  8 +---
>  security/integrity/ima/Kconfig|  5 -
>  security/integrity/ima/ima.h  | 12 
>  security/integrity/ima/ima_init.c |  2 +-
>  security/integrity/integrity.h|  5 ++---
>  7 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
> index 73c457b..21d7568 100644
> --- a/security/integrity/Kconfig
> +++ b/security/integrity/Kconfig
> @@ -41,6 +41,17 @@ config INTEGRITY_ASYMMETRIC_KEYS
> This option enables digital signature verification using
> asymmetric keys.
>
> +config INTEGRITY_TRUSTED_KEYRING
> + bool "Require all keys on the integrity keyrings be signed"
> + depends on SYSTEM_TRUSTED_KEYRING
> + depends on INTEGRITY_ASYMMETRIC_KEYS
> + select KEYS_DEBUG_PROC_KEYS
> + default y
> + help
> +This option requires that all keys added to the .ima and
> +.evm keyrings be signed by a key on the system trusted
> +keyring.
> +
>  config INTEGRITY_AUDIT
>   bool "Enables integrity auditing support "
>   depends on AUDIT
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 5be9ffb..8ef1511 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -24,15 +24,22 @@
>  static struct key *keyring[INTEGRITY_KEYRING_MAX];
>
>  static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
> +#ifndef CONFIG_INTEGRITY_TRUSTED_KEYRING
>   "_evm",
> - "_module",
> -#ifndef CONFIG_IMA_TRUSTED_KEYRING
>   "_ima",
>  #else
> + ".evm",
>   ".ima",
>  #endif
> + "_module",
>  };
>
> +#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
> +static bool init_keyring __initdata = true;
> +#else
> +static bool init_keyring __initdata;
> +#endif
> +
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int 
> siglen,
>   const char *digest, int digestlen)
>  {
> @@ -68,6 +75,9 @@ int __init integrity_init_keyring(const unsigned int id)
>   const struct cred *cred = current_cred();
>   int err = 0;
>
> + if (!init_keyring)
> + return 0;
> +
>   keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
>   KGIDT_INIT(0), cred,
>   ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> diff --git a/security/integrity/evm/evm_main.c 
> b/security/integrity/evm/evm_main.c
> index 1334e02..75b7e30 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -478,15 +478,17 @@ static int __init init_evm(void)
>
>   evm_init_config();
>
> + error = integrity_init_keyring(INTEGRITY_KEYRING_EVM);
> + if (error)
> + return error;
> +
>   error = evm_init_secfs();
>   if (error < 0) {
>   pr_info("Error registering secfs\n");
> - goto err;
> + return error;
>   }
>
>   return 0;
> -err:
> - return error;
>  }
>
>  /*
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index df30334..a292b88 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -123,14 +123,17 @@ config IMA_APPRAISE
> If unsure,

[PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

2015-10-22 Thread Dmitry Kasatkin 
Require all keys added to the EVM keyring be signed by an
existing trusted key on the system trusted keyring.

This patch also switches IMA to use integrity_init_keyring().

Changes in v3:
* Added 'init_keyring' config based variable to skip initializing
  keyring instead of using  __integrity_init_keyring() wrapper.
* Added dependency back to CONFIG_IMA_TRUSTED_KEYRING

Changes in v2:
* Replace CONFIG_EVM_TRUSTED_KEYRING with IMA and EVM common
  CONFIG_INTEGRITY_TRUSTED_KEYRING configuration option
* Deprecate CONFIG_IMA_TRUSTED_KEYRING but keep it for config
  file compatibility. (Mimi Zohar)

Signed-off-by: Dmitry Kasatkin 
---
 security/integrity/Kconfig| 11 +++
 security/integrity/digsig.c   | 14 --
 security/integrity/evm/evm_main.c |  8 +---
 security/integrity/ima/Kconfig|  5 -
 security/integrity/ima/ima.h  | 12 
 security/integrity/ima/ima_init.c |  2 +-
 security/integrity/integrity.h|  5 ++---
 7 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 73c457b..21d7568 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -41,6 +41,17 @@ config INTEGRITY_ASYMMETRIC_KEYS
  This option enables digital signature verification using
  asymmetric keys.
 
+config INTEGRITY_TRUSTED_KEYRING
+   bool "Require all keys on the integrity keyrings be signed"
+   depends on SYSTEM_TRUSTED_KEYRING
+   depends on INTEGRITY_ASYMMETRIC_KEYS
+   select KEYS_DEBUG_PROC_KEYS
+   default y
+   help
+  This option requires that all keys added to the .ima and
+  .evm keyrings be signed by a key on the system trusted
+  keyring.
+
 config INTEGRITY_AUDIT
bool "Enables integrity auditing support "
depends on AUDIT
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 5be9ffb..8ef1511 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -24,15 +24,22 @@
 static struct key *keyring[INTEGRITY_KEYRING_MAX];
 
 static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
+#ifndef CONFIG_INTEGRITY_TRUSTED_KEYRING
"_evm",
-   "_module",
-#ifndef CONFIG_IMA_TRUSTED_KEYRING
"_ima",
 #else
+   ".evm",
".ima",
 #endif
+   "_module",
 };
 
+#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
+static bool init_keyring __initdata = true;
+#else
+static bool init_keyring __initdata;
+#endif
+
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
const char *digest, int digestlen)
 {
@@ -68,6 +75,9 @@ int __init integrity_init_keyring(const unsigned int id)
const struct cred *cred = current_cred();
int err = 0;
 
+   if (!init_keyring)
+   return 0;
+
keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
KGIDT_INIT(0), cred,
((KEY_POS_ALL & ~KEY_POS_SETATTR) |
diff --git a/security/integrity/evm/evm_main.c 
b/security/integrity/evm/evm_main.c
index 1334e02..75b7e30 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -478,15 +478,17 @@ static int __init init_evm(void)
 
evm_init_config();
 
+   error = integrity_init_keyring(INTEGRITY_KEYRING_EVM);
+   if (error)
+   return error;
+
error = evm_init_secfs();
if (error < 0) {
pr_info("Error registering secfs\n");
-   goto err;
+   return error;
}
 
return 0;
-err:
-   return error;
 }
 
 /*
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index df30334..a292b88 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -123,14 +123,17 @@ config IMA_APPRAISE
  If unsure, say N.
 
 config IMA_TRUSTED_KEYRING
-   bool "Require all keys on the .ima keyring be signed"
+   bool "Require all keys on the .ima keyring be signed (deprecated)"
depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
depends on INTEGRITY_ASYMMETRIC_KEYS
+   select INTEGRITY_TRUSTED_KEYRING
default y
help
   This option requires that all keys added to the .ima
   keyring be signed by a key on the system trusted keyring.
 
+  This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
+
 config IMA_LOAD_X509
bool "Load X509 certificate onto the '.ima' trusted keyring"
depends on IMA_TRUSTED_KEYRING
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e2a60c3..9e82367 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -251,16 +251,4 @@ static inline int security_filter_rule_match(u32 secid, 
u32 field, u32 op,
return -EINVAL;
 }
 #endif /* CONFIG_IMA_LSM_RULES */
-
-#ifdef