Re: [PATCHv3 3/6] evm: enable EVM when X509 certificate is loaded

2015-10-26 Thread Dmitry Kasatkin
Hi,

I added error printing to the patch

http://git.kernel.org/cgit/linux/kernel/git/kasatkin/linux-digsig.git/log/?h=ima-next

Dmitry


On Fri, Oct 23, 2015 at 9:31 PM, Mimi Zohar  wrote:
> On Thu, 2015-10-22 at 21:49 +0300, Dmitry Kasatkin wrote:
>> In order to enable EVM before starting 'init' process,
>> evm_initialized needs to be non-zero. Before it was
>> indicating that HMAC key is loaded. When EVM loads
>> X509 before calling 'init', it is possible to enable
>> EVM to start signature based verification.
>>
>> This patch defines bits to enable EVM if key of any type
>> is loaded.
>
> Thanks, Dmitry.  There's one comment inline.
>
>> Changes in v2:
>> * EVM_STATE_KEY_SET replaced by EVM_INIT_HMAC
>> * EVM_STATE_X509_SET replaced by EVM_INIT_X509
>>
>> Signed-off-by: Dmitry Kasatkin 
>> ---
>>  security/integrity/evm/evm.h| 3 +++
>>  security/integrity/evm/evm_crypto.c | 2 ++
>>  security/integrity/evm/evm_main.c   | 6 +-
>>  security/integrity/evm/evm_secfs.c  | 4 ++--
>>  4 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
>> index 88bfe77..f5f1272 100644
>> --- a/security/integrity/evm/evm.h
>> +++ b/security/integrity/evm/evm.h
>> @@ -21,6 +21,9 @@
>>
>>  #include "../integrity.h"
>>
>> +#define EVM_INIT_HMAC0x0001
>> +#define EVM_INIT_X5090x0002
>> +
>>  extern int evm_initialized;
>>  extern char *evm_hmac;
>>  extern char *evm_hash;
>> diff --git a/security/integrity/evm/evm_crypto.c 
>> b/security/integrity/evm/evm_crypto.c
>> index 159ef3e..34e1a6f 100644
>> --- a/security/integrity/evm/evm_crypto.c
>> +++ b/security/integrity/evm/evm_crypto.c
>> @@ -40,6 +40,8 @@ static struct shash_desc *init_desc(char type)
>>   struct shash_desc *desc;
>>
>>   if (type == EVM_XATTR_HMAC) {
>> + if (!(evm_initialized & EVM_INIT_HMAC))
>> + return ERR_PTR(-ENOKEY);
>
> init_desc() is called from a couple of different places.  In some
> instances, like when converting from a signature to an hmac, if
> init_desc() fails, the xattr isn't converted to an HMAC.  No big deal.
> But there are other cases, like when a protected xattr is modified,
> failing the write will make the file inaccessible.  Does there need to
> be an error msg of some sort or an audit msg?
>
> Mimi
>
>>   tfm = _tfm;
>>   algo = evm_hmac;
>>   } else {
>> diff --git a/security/integrity/evm/evm_main.c 
>> b/security/integrity/evm/evm_main.c
>> index 519de0a..420d94d 100644
>> --- a/security/integrity/evm/evm_main.c
>> +++ b/security/integrity/evm/evm_main.c
>> @@ -475,7 +475,11 @@ EXPORT_SYMBOL_GPL(evm_inode_init_security);
>>  #ifdef CONFIG_EVM_LOAD_X509
>>  void __init evm_load_x509(void)
>>  {
>> - integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
>> + int rc;
>> +
>> + rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
>> + if (!rc)
>> + evm_initialized |= EVM_INIT_X509;
>>  }
>>  #endif
>>
>> diff --git a/security/integrity/evm/evm_secfs.c 
>> b/security/integrity/evm/evm_secfs.c
>> index cf12a04..3f775df 100644
>> --- a/security/integrity/evm/evm_secfs.c
>> +++ b/security/integrity/evm/evm_secfs.c
>> @@ -64,7 +64,7 @@ static ssize_t evm_write_key(struct file *file, const char 
>> __user *buf,
>>   char temp[80];
>>   int i, error;
>>
>> - if (!capable(CAP_SYS_ADMIN) || evm_initialized)
>> + if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_INIT_HMAC))
>>   return -EPERM;
>>
>>   if (count >= sizeof(temp) || count == 0)
>> @@ -80,7 +80,7 @@ static ssize_t evm_write_key(struct file *file, const char 
>> __user *buf,
>>
>>   error = evm_init_key();
>>   if (!error) {
>> - evm_initialized = 1;
>> + evm_initialized |= EVM_INIT_HMAC;
>>   pr_info("initialized\n");
>>   } else
>>   pr_err("initialization failed\n");
>
>



-- 
Thanks,
Dmitry
--
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


[PATCHv3 3/6] evm: enable EVM when X509 certificate is loaded

2015-10-22 Thread Dmitry Kasatkin
In order to enable EVM before starting 'init' process,
evm_initialized needs to be non-zero. Before it was
indicating that HMAC key is loaded. When EVM loads
X509 before calling 'init', it is possible to enable
EVM to start signature based verification.

This patch defines bits to enable EVM if key of any type
is loaded.

Changes in v2:
* EVM_STATE_KEY_SET replaced by EVM_INIT_HMAC
* EVM_STATE_X509_SET replaced by EVM_INIT_X509

Signed-off-by: Dmitry Kasatkin 
---
 security/integrity/evm/evm.h| 3 +++
 security/integrity/evm/evm_crypto.c | 2 ++
 security/integrity/evm/evm_main.c   | 6 +-
 security/integrity/evm/evm_secfs.c  | 4 ++--
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 88bfe77..f5f1272 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -21,6 +21,9 @@
 
 #include "../integrity.h"
 
+#define EVM_INIT_HMAC  0x0001
+#define EVM_INIT_X509  0x0002
+
 extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index 159ef3e..34e1a6f 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -40,6 +40,8 @@ static struct shash_desc *init_desc(char type)
struct shash_desc *desc;
 
if (type == EVM_XATTR_HMAC) {
+   if (!(evm_initialized & EVM_INIT_HMAC))
+   return ERR_PTR(-ENOKEY);
tfm = _tfm;
algo = evm_hmac;
} else {
diff --git a/security/integrity/evm/evm_main.c 
b/security/integrity/evm/evm_main.c
index 519de0a..420d94d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -475,7 +475,11 @@ EXPORT_SYMBOL_GPL(evm_inode_init_security);
 #ifdef CONFIG_EVM_LOAD_X509
 void __init evm_load_x509(void)
 {
-   integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
+   int rc;
+
+   rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
+   if (!rc)
+   evm_initialized |= EVM_INIT_X509;
 }
 #endif
 
diff --git a/security/integrity/evm/evm_secfs.c 
b/security/integrity/evm/evm_secfs.c
index cf12a04..3f775df 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -64,7 +64,7 @@ static ssize_t evm_write_key(struct file *file, const char 
__user *buf,
char temp[80];
int i, error;
 
-   if (!capable(CAP_SYS_ADMIN) || evm_initialized)
+   if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_INIT_HMAC))
return -EPERM;
 
if (count >= sizeof(temp) || count == 0)
@@ -80,7 +80,7 @@ static ssize_t evm_write_key(struct file *file, const char 
__user *buf,
 
error = evm_init_key();
if (!error) {
-   evm_initialized = 1;
+   evm_initialized |= EVM_INIT_HMAC;
pr_info("initialized\n");
} else
pr_err("initialization failed\n");
-- 
2.1.4

--
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