* [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM
2018-04-17 22:56 [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures Matthew Garrett
@ 2018-04-17 22:56 ` Matthew Garrett
2018-06-12 23:42 ` Mimi Zohar
2018-04-18 12:13 ` [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures Mimi Zohar
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Matthew Garrett @ 2018-04-17 22:56 UTC (permalink / raw)
To: linux-integrity; +Cc: zohar, Matthew Garrett
The kernel is taking security.apparmor into account when validating EVM,
so evmctl should be doing the same.
Signed-off-by: Matthew Garrett <mjg59@google.com>
---
src/evmctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/evmctl.c b/src/evmctl.c
index 43d261f..e350f69 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -69,6 +69,7 @@
static char *evm_default_xattrs[] = {
XATTR_NAME_SELINUX,
XATTR_NAME_SMACK,
+ XATTR_NAME_APPARMOR,
XATTR_NAME_IMA,
XATTR_NAME_CAPS,
NULL
@@ -80,6 +81,7 @@ static char *evm_extra_smack_xattrs[] = {
XATTR_NAME_SMACKEXEC,
XATTR_NAME_SMACKTRANSMUTE,
XATTR_NAME_SMACKMMAP,
+ XATTR_NAME_APPARMOR,
XATTR_NAME_IMA,
XATTR_NAME_CAPS,
NULL
--
2.17.0.484.g0c8726318c-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM
2018-04-17 22:56 ` [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM Matthew Garrett
@ 2018-06-12 23:42 ` Mimi Zohar
2018-06-14 19:43 ` Matthew Garrett
0 siblings, 1 reply; 10+ messages in thread
From: Mimi Zohar @ 2018-06-12 23:42 UTC (permalink / raw)
To: Matthew Garrett, linux-integrity
On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> The kernel is taking security.apparmor into account when validating EVM,
> so evmctl should be doing the same.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
The XATTR_NAME_APPARMOR is dependent on the version of
"/usr/include/linux/xattr.h". Without it defined, evmctl fails to
build.
Mimi
> ---
> src/evmctl.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 43d261f..e350f69 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -69,6 +69,7 @@
> static char *evm_default_xattrs[] = {
> XATTR_NAME_SELINUX,
> XATTR_NAME_SMACK,
> + XATTR_NAME_APPARMOR,
> XATTR_NAME_IMA,
> XATTR_NAME_CAPS,
> NULL
> @@ -80,6 +81,7 @@ static char *evm_extra_smack_xattrs[] = {
> XATTR_NAME_SMACKEXEC,
> XATTR_NAME_SMACKTRANSMUTE,
> XATTR_NAME_SMACKMMAP,
> + XATTR_NAME_APPARMOR,
> XATTR_NAME_IMA,
> XATTR_NAME_CAPS,
> NULL
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM
2018-06-12 23:42 ` Mimi Zohar
@ 2018-06-14 19:43 ` Matthew Garrett
2018-06-14 20:41 ` Mimi Zohar
0 siblings, 1 reply; 10+ messages in thread
From: Matthew Garrett @ 2018-06-14 19:43 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity
On Tue, Jun 12, 2018 at 4:42 PM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>
> On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> > The kernel is taking security.apparmor into account when validating EVM,
> > so evmctl should be doing the same.
> >
> > Signed-off-by: Matthew Garrett <mjg59@google.com>
>
> The XATTR_NAME_APPARMOR is dependent on the version of
> "/usr/include/linux/xattr.h". Without it defined, evmctl fails to
> build.
Hmm, true. Is it reasonable to just hardcode it rather than using the define?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM
2018-06-14 19:43 ` Matthew Garrett
@ 2018-06-14 20:41 ` Mimi Zohar
2018-07-01 20:28 ` Mimi Zohar
0 siblings, 1 reply; 10+ messages in thread
From: Mimi Zohar @ 2018-06-14 20:41 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-integrity
On Thu, 2018-06-14 at 12:43 -0700, Matthew Garrett wrote:
> On Tue, Jun 12, 2018 at 4:42 PM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >
> > On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> > > The kernel is taking security.apparmor into account when validating EVM,
> > > so evmctl should be doing the same.
> > >
> > > Signed-off-by: Matthew Garrett <mjg59@google.com>
> >
> > The XATTR_NAME_APPARMOR is dependent on the version of
> > "/usr/include/linux/xattr.h". Without it defined, evmctl fails to
> > build.
>
> Hmm, true. Is it reasonable to just hardcode it rather than using the define?
I'm not sure how difficult it would be to tie the package name/version
to a specific kernel release. Commit 096b85464832 ("EVM: Include
security.apparmor in EVM measurements") was upstreamed in linux-4.15.
Mimi
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM
2018-06-14 20:41 ` Mimi Zohar
@ 2018-07-01 20:28 ` Mimi Zohar
0 siblings, 0 replies; 10+ messages in thread
From: Mimi Zohar @ 2018-07-01 20:28 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-integrity
On Thu, 2018-06-14 at 16:41 -0400, Mimi Zohar wrote:
> On Thu, 2018-06-14 at 12:43 -0700, Matthew Garrett wrote:
> > On Tue, Jun 12, 2018 at 4:42 PM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > >
> > > On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> > > > The kernel is taking security.apparmor into account when validating EVM,
> > > > so evmctl should be doing the same.
> > > >
> > > > Signed-off-by: Matthew Garrett <mjg59@google.com>
> > >
> > > The XATTR_NAME_APPARMOR is dependent on the version of
> > > "/usr/include/linux/xattr.h". Without it defined, evmctl fails to
> > > build.
> >
> > Hmm, true. Is it reasonable to just hardcode it rather than using the define?
>
> I'm not sure how difficult it would be to tie the package name/version
> to a specific kernel release. Commit 096b85464832 ("EVM: Include
> security.apparmor in EVM measurements") was upstreamed in linux-4.15.
Instead, I've made the following change and pushed it out to master.
+#ifndef XATTR_APPAARMOR_SUFFIX
+#define XATTR_APPARMOR_SUFFIX "apparmor"
+#define XATTR_NAME_APPARMOR XATTR_SECURITY_PREFIX XATTR_APPARMOR_SUFFIX
+#endif
+
thanks,
Mimi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures
2018-04-17 22:56 [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures Matthew Garrett
2018-04-17 22:56 ` [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM Matthew Garrett
@ 2018-04-18 12:13 ` Mimi Zohar
2018-06-13 13:41 ` Mimi Zohar
2018-06-22 20:24 ` Mimi Zohar
3 siblings, 0 replies; 10+ messages in thread
From: Mimi Zohar @ 2018-04-18 12:13 UTC (permalink / raw)
To: Matthew Garrett, linux-integrity
On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> EVM signatures are always being generated with SHA1 even if the -a
> argument has been provided to evmctl. Fix this so the provided hash
> algorithm is used instead.
The kernel assumes both the HMAC and signature are sha1 as well.
static char * const evm_hmac = "hmac(sha1)";
static char * const evm_hash = "sha1";
Mimi
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> src/evmctl.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 2ffee78..43d261f 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -313,6 +313,7 @@ err:
>
> static int calc_evm_hash(const char *file, unsigned char *hash)
> {
> + const EVP_MD *md;
> struct stat st;
> int err;
> uint32_t generation = 0;
> @@ -374,7 +375,13 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
> return -1;
> }
>
> - err = EVP_DigestInit(pctx, EVP_sha1());
> + md = EVP_get_digestbyname(params.hash_algo);
> + if (!md) {
> + log_err("EVP_get_digestbyname() failed\n");
> + return 1;
> + }
> +
> + err = EVP_DigestInit(pctx, md);
> if (!err) {
> log_err("EVP_DigestInit() failed\n");
> return 1;
> @@ -498,7 +505,7 @@ static int sign_evm(const char *file, const char *key)
> if (len <= 1)
> return len;
>
> - len = sign_hash("sha1", hash, len, key, NULL, sig + 1);
> + len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
> if (len <= 1)
> return len;
>
> @@ -967,6 +974,7 @@ static int cmd_setxattr_ima(struct command *cmd)
>
> static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *hash)
> {
> + const EVP_MD *md;
> struct stat st;
> int err = -1;
> uint32_t generation = 0;
> @@ -1033,7 +1041,13 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
> goto out;
> }
>
> - err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), EVP_sha1(), NULL);
> + md = EVP_get_digestbyname(params.hash_algo);
> + if (!md) {
> + log_err("EVP_get_digestbyname() failed\n");
> + goto out;
> + }
> +
> + err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), md, NULL);
> if (err) {
> log_err("HMAC_Init() failed\n");
> goto out;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures
2018-04-17 22:56 [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures Matthew Garrett
2018-04-17 22:56 ` [USER] [PATCH 2/2] Add security.apparmor to the set of extended attributes used by EVM Matthew Garrett
2018-04-18 12:13 ` [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures Mimi Zohar
@ 2018-06-13 13:41 ` Mimi Zohar
2018-06-14 19:42 ` Matthew Garrett
2018-06-22 20:24 ` Mimi Zohar
3 siblings, 1 reply; 10+ messages in thread
From: Mimi Zohar @ 2018-06-13 13:41 UTC (permalink / raw)
To: Matthew Garrett, linux-integrity
On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> @@ -1033,7 +1041,13 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
> goto out;
> }
>
> - err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), EVP_sha1(), NULL);
> + md = EVP_get_digestbyname(params.hash_algo);
HMAC is still limited to sha1.
> + if (!md) {
> + log_err("EVP_get_digestbyname() failed\n");
> + goto out;
> + }
> +
> + err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), md, NULL);
> if (err) {
> log_err("HMAC_Init() failed\n");
> goto out;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures
2018-06-13 13:41 ` Mimi Zohar
@ 2018-06-14 19:42 ` Matthew Garrett
0 siblings, 0 replies; 10+ messages in thread
From: Matthew Garrett @ 2018-06-14 19:42 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity
On Wed, Jun 13, 2018 at 6:42 AM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>
> On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> > - err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), EVP_sha1(), NULL);
> > + md = EVP_get_digestbyname(params.hash_algo);
>
> HMAC is still limited to sha1.
Yes, let me revert that hunk and I'll resend.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures
2018-04-17 22:56 [USER] [PATCH 1/2] Remove hardcoding of SHA1 in EVM signatures Matthew Garrett
` (2 preceding siblings ...)
2018-06-13 13:41 ` Mimi Zohar
@ 2018-06-22 20:24 ` Mimi Zohar
3 siblings, 0 replies; 10+ messages in thread
From: Mimi Zohar @ 2018-06-22 20:24 UTC (permalink / raw)
To: Matthew Garrett, linux-integrity
Hi Matthew,
On Tue, 2018-04-17 at 15:56 -0700, Matthew Garrett wrote:
> EVM signatures are always being generated with SHA1 even if the -a
> argument has been provided to evmctl. Fix this so the provided hash
> algorithm is used instead.
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> src/evmctl.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 2ffee78..43d261f 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -313,6 +313,7 @@ err:
>
> static int calc_evm_hash(const char *file, unsigned char *hash)
Please make sure the caller provides a large enough buffer for the
hash.
Mimi
> {
> + const EVP_MD *md;
> struct stat st;
> int err;
> uint32_t generation = 0;
> @@ -374,7 +375,13 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
> return -1;
> }
>
> - err = EVP_DigestInit(pctx, EVP_sha1());
> + md = EVP_get_digestbyname(params.hash_algo);
> + if (!md) {
> + log_err("EVP_get_digestbyname() failed\n");
> + return 1;
> + }
> +
> + err = EVP_DigestInit(pctx, md);
> if (!err) {
> log_err("EVP_DigestInit() failed\n");
> return 1;
> @@ -498,7 +505,7 @@ static int sign_evm(const char *file, const char *key)
> if (len <= 1)
> return len;
>
> - len = sign_hash("sha1", hash, len, key, NULL, sig + 1);
> + len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
> if (len <= 1)
> return len;
>
> @@ -967,6 +974,7 @@ static int cmd_setxattr_ima(struct command *cmd)
>
> static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *hash)
> {
> + const EVP_MD *md;
> struct stat st;
> int err = -1;
> uint32_t generation = 0;
> @@ -1033,7 +1041,13 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
> goto out;
> }
>
> - err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), EVP_sha1(), NULL);
> + md = EVP_get_digestbyname(params.hash_algo);
> + if (!md) {
> + log_err("EVP_get_digestbyname() failed\n");
> + goto out;
> + }
> +
> + err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), md, NULL);
> if (err) {
> log_err("HMAC_Init() failed\n");
> goto out;
^ permalink raw reply [flat|nested] 10+ messages in thread