Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH] efi: Suppress spurious "Couldn't get size" error
From: Takashi Iwai @ 2020-02-17 16:10 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Javier Martinez Canillas, linux-efi, Joey Lee,
	linux-security-module
In-Reply-To: <CAKv+Gu_9AqVnhwUUOGyL1HMUYgemp6SD2O5CJjy2vKpeuP4eOA@mail.gmail.com>

On Mon, 17 Feb 2020 17:06:57 +0100,
Ard Biesheuvel wrote:
> 
> On Mon, 17 Feb 2020 at 16:45, Takashi Iwai <tiwai@suse.de> wrote:
> >
> > The current efi code emits the error message like
> >    Couldn't get size: 0x800000000000000e
> > on various Dell and other machines.  Although the whole problem is the
> > buggy firmware, showing this as an error level is rather annoying, as
> > the error message appears over the boot splash.  Basically this is the
> > result of missing entry and we have no explicit way to fix it for such
> > a firmware problem, the error message may be suppressed.
> >
> > This patch changes the error print condition and suppresses the error
> > message if status is EFI_NOT_FOUND.  It's a partial patch from the
> > more comprehensive one Joey Lee submitted in the past.
> >
> > Link: https://lore.kernel.org/linux-efi/20190322103350.27764-2-jlee@suse.com/
> > Cc: Joey Lee <jlee@suse.com>
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> Hello Takashi,
> 
> Javier sent a more comprehensive fix for this today. The problem is
> not buggy firmware, but buggy kernel :-)
> (the code assumes that all systems boot via shim, and that certain EFI
> variables are therefore guaranteed to exist, which is not the case)
> 
> https://lore.kernel.org/linux-efi/CAKv+Gu-a5Bo9i=K55pa3jEXRq-u5JYVGp1jFEE=UY5B=6eUkRQ@mail.gmail.com

Awesome, then please scratch mine.
Thanks!


Takashi

^ permalink raw reply

* Re: [PATCH] efi: Suppress spurious "Couldn't get size" error
From: Ard Biesheuvel @ 2020-02-17 16:06 UTC (permalink / raw)
  To: Takashi Iwai, Javier Martinez Canillas
  Cc: linux-efi, Joey Lee, linux-security-module
In-Reply-To: <20200217154549.20895-1-tiwai@suse.de>

On Mon, 17 Feb 2020 at 16:45, Takashi Iwai <tiwai@suse.de> wrote:
>
> The current efi code emits the error message like
>    Couldn't get size: 0x800000000000000e
> on various Dell and other machines.  Although the whole problem is the
> buggy firmware, showing this as an error level is rather annoying, as
> the error message appears over the boot splash.  Basically this is the
> result of missing entry and we have no explicit way to fix it for such
> a firmware problem, the error message may be suppressed.
>
> This patch changes the error print condition and suppresses the error
> message if status is EFI_NOT_FOUND.  It's a partial patch from the
> more comprehensive one Joey Lee submitted in the past.
>
> Link: https://lore.kernel.org/linux-efi/20190322103350.27764-2-jlee@suse.com/
> Cc: Joey Lee <jlee@suse.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Hello Takashi,

Javier sent a more comprehensive fix for this today. The problem is
not buggy firmware, but buggy kernel :-)
(the code assumes that all systems boot via shim, and that certain EFI
variables are therefore guaranteed to exist, which is not the case)

https://lore.kernel.org/linux-efi/CAKv+Gu-a5Bo9i=K55pa3jEXRq-u5JYVGp1jFEE=UY5B=6eUkRQ@mail.gmail.com

-- 
Ard.


> ---
>  security/integrity/platform_certs/load_uefi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index 111898aad56e..8501ea62cb3e 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -44,7 +44,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>
>         status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
>         if (status != EFI_BUFFER_TOO_SMALL) {
> -               pr_err("Couldn't get size: 0x%lx\n", status);
> +               if (status != EFI_NOT_FOUND)
> +                       pr_err("Couldn't get size: 0x%lx\n", status);
>                 return NULL;
>         }
>
> --
> 2.16.4
>

^ permalink raw reply

* [PATCH] efi: Suppress spurious "Couldn't get size" error
From: Takashi Iwai @ 2020-02-17 15:45 UTC (permalink / raw)
  To: linux-efi; +Cc: Joey Lee, Ard Biesheuvel, linux-security-module, inux-kernel

The current efi code emits the error message like
   Couldn't get size: 0x800000000000000e
on various Dell and other machines.  Although the whole problem is the
buggy firmware, showing this as an error level is rather annoying, as
the error message appears over the boot splash.  Basically this is the
result of missing entry and we have no explicit way to fix it for such
a firmware problem, the error message may be suppressed.

This patch changes the error print condition and suppresses the error
message if status is EFI_NOT_FOUND.  It's a partial patch from the
more comprehensive one Joey Lee submitted in the past.

Link: https://lore.kernel.org/linux-efi/20190322103350.27764-2-jlee@suse.com/
Cc: Joey Lee <jlee@suse.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 security/integrity/platform_certs/load_uefi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 111898aad56e..8501ea62cb3e 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -44,7 +44,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 
 	status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
 	if (status != EFI_BUFFER_TOO_SMALL) {
-		pr_err("Couldn't get size: 0x%lx\n", status);
+		if (status != EFI_NOT_FOUND)
+			pr_err("Couldn't get size: 0x%lx\n", status);
 		return NULL;
 	}
 
-- 
2.16.4


^ permalink raw reply related

* Re: [RESEND PATCH v2] efi: Only print errors about failing to get certs if EFI vars are found
From: Ard Biesheuvel @ 2020-02-17 13:37 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Linux Kernel Mailing List, linux-efi, Hans de Goede, Eric Richter,
	James Morris, Michael Ellerman, Mimi Zohar, Nayna Jain,
	Serge E. Hallyn, YueHaibing, linux-security-module
In-Reply-To: <20200217113947.2070436-1-javierm@redhat.com>

On Mon, 17 Feb 2020 at 12:40, Javier Martinez Canillas
<javierm@redhat.com> wrote:
>
> If CONFIG_LOAD_UEFI_KEYS is enabled, the kernel attempts to load the certs
> from the db, dbx and MokListRT EFI variables into the appropriate keyrings.
>
> But it just assumes that the variables will be present and prints an error
> if the certs can't be loaded, even when is possible that the variables may
> not exist. For example the MokListRT variable will only be present if shim
> is used.
>
> So only print an error message about failing to get the certs list from an
> EFI variable if this is found. Otherwise these printed errors just pollute
> the kernel log ring buffer with confusing messages like the following:
>
> [    5.427251] Couldn't get size: 0x800000000000000e
> [    5.427261] MODSIGN: Couldn't get UEFI db list
> [    5.428012] Couldn't get size: 0x800000000000000e
> [    5.428023] Couldn't get UEFI MokListRT
>
> Reported-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Tested-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

>
> ---
>
> Changes in v2:
> - Fix flaws in the logic, that caused the signature list was parsed if
>   the return code was EFI_NOT_FOUND that pointed out Hans de Goede.
> - Print debug messages if the variables are not found.
>
>  security/integrity/platform_certs/load_uefi.c | 40 ++++++++++++-------
>  1 file changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index 111898aad56..f0c90824196 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -35,16 +35,18 @@ static __init bool uefi_check_ignore_db(void)
>   * Get a certificate list blob from the named EFI variable.
>   */
>  static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
> -                                 unsigned long *size)
> +                                 unsigned long *size, efi_status_t *status)
>  {
> -       efi_status_t status;
>         unsigned long lsize = 4;
>         unsigned long tmpdb[4];
>         void *db;
>
> -       status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
> -       if (status != EFI_BUFFER_TOO_SMALL) {
> -               pr_err("Couldn't get size: 0x%lx\n", status);
> +       *status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
> +       if (*status == EFI_NOT_FOUND)
> +               return NULL;
> +
> +       if (*status != EFI_BUFFER_TOO_SMALL) {
> +               pr_err("Couldn't get size: 0x%lx\n", *status);
>                 return NULL;
>         }
>
> @@ -52,10 +54,10 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>         if (!db)
>                 return NULL;
>
> -       status = efi.get_variable(name, guid, NULL, &lsize, db);
> -       if (status != EFI_SUCCESS) {
> +       *status = efi.get_variable(name, guid, NULL, &lsize, db);
> +       if (*status != EFI_SUCCESS) {
>                 kfree(db);
> -               pr_err("Error reading db var: 0x%lx\n", status);
> +               pr_err("Error reading db var: 0x%lx\n", *status);
>                 return NULL;
>         }
>
> @@ -74,6 +76,7 @@ static int __init load_uefi_certs(void)
>         efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
>         void *db = NULL, *dbx = NULL, *mok = NULL;
>         unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
> +       efi_status_t status;
>         int rc = 0;
>
>         if (!efi.get_variable)
> @@ -83,9 +86,12 @@ static int __init load_uefi_certs(void)
>          * an error if we can't get them.
>          */
>         if (!uefi_check_ignore_db()) {
> -               db = get_cert_list(L"db", &secure_var, &dbsize);
> +               db = get_cert_list(L"db", &secure_var, &dbsize, &status);
>                 if (!db) {
> -                       pr_err("MODSIGN: Couldn't get UEFI db list\n");
> +                       if (status == EFI_NOT_FOUND)
> +                               pr_debug("MODSIGN: db variable wasn't found\n");
> +                       else
> +                               pr_err("MODSIGN: Couldn't get UEFI db list\n");
>                 } else {
>                         rc = parse_efi_signature_list("UEFI:db",
>                                         db, dbsize, get_handler_for_db);
> @@ -96,9 +102,12 @@ static int __init load_uefi_certs(void)
>                 }
>         }
>
> -       mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
> +       mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
>         if (!mok) {
> -               pr_info("Couldn't get UEFI MokListRT\n");
> +               if (status == EFI_NOT_FOUND)
> +                       pr_debug("MokListRT variable wasn't found\n");
> +               else
> +                       pr_info("Couldn't get UEFI MokListRT\n");
>         } else {
>                 rc = parse_efi_signature_list("UEFI:MokListRT",
>                                               mok, moksize, get_handler_for_db);
> @@ -107,9 +116,12 @@ static int __init load_uefi_certs(void)
>                 kfree(mok);
>         }
>
> -       dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
> +       dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, &status);
>         if (!dbx) {
> -               pr_info("Couldn't get UEFI dbx list\n");
> +               if (status == EFI_NOT_FOUND)
> +                       pr_debug("dbx variable wasn't found\n");
> +               else
> +                       pr_info("Couldn't get UEFI dbx list\n");
>         } else {
>                 rc = parse_efi_signature_list("UEFI:dbx",
>                                               dbx, dbxsize,
> --
> 2.24.1
>

^ permalink raw reply

* [RESEND PATCH v2] efi: Only print errors about failing to get certs if EFI vars are found
From: Javier Martinez Canillas @ 2020-02-17 11:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-efi, Hans de Goede, Javier Martinez Canillas, Eric Richter,
	James Morris, Michael Ellerman, Mimi Zohar, Nayna Jain,
	Serge E. Hallyn, YueHaibing, linux-security-module

If CONFIG_LOAD_UEFI_KEYS is enabled, the kernel attempts to load the certs
from the db, dbx and MokListRT EFI variables into the appropriate keyrings.

But it just assumes that the variables will be present and prints an error
if the certs can't be loaded, even when is possible that the variables may
not exist. For example the MokListRT variable will only be present if shim
is used.

So only print an error message about failing to get the certs list from an
EFI variable if this is found. Otherwise these printed errors just pollute
the kernel log ring buffer with confusing messages like the following:

[    5.427251] Couldn't get size: 0x800000000000000e
[    5.427261] MODSIGN: Couldn't get UEFI db list
[    5.428012] Couldn't get size: 0x800000000000000e
[    5.428023] Couldn't get UEFI MokListRT

Reported-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>

---

Changes in v2:
- Fix flaws in the logic, that caused the signature list was parsed if
  the return code was EFI_NOT_FOUND that pointed out Hans de Goede.
- Print debug messages if the variables are not found.

 security/integrity/platform_certs/load_uefi.c | 40 ++++++++++++-------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 111898aad56..f0c90824196 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -35,16 +35,18 @@ static __init bool uefi_check_ignore_db(void)
  * Get a certificate list blob from the named EFI variable.
  */
 static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
-				  unsigned long *size)
+				  unsigned long *size, efi_status_t *status)
 {
-	efi_status_t status;
 	unsigned long lsize = 4;
 	unsigned long tmpdb[4];
 	void *db;
 
-	status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
-	if (status != EFI_BUFFER_TOO_SMALL) {
-		pr_err("Couldn't get size: 0x%lx\n", status);
+	*status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
+	if (*status == EFI_NOT_FOUND)
+		return NULL;
+
+	if (*status != EFI_BUFFER_TOO_SMALL) {
+		pr_err("Couldn't get size: 0x%lx\n", *status);
 		return NULL;
 	}
 
@@ -52,10 +54,10 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 	if (!db)
 		return NULL;
 
-	status = efi.get_variable(name, guid, NULL, &lsize, db);
-	if (status != EFI_SUCCESS) {
+	*status = efi.get_variable(name, guid, NULL, &lsize, db);
+	if (*status != EFI_SUCCESS) {
 		kfree(db);
-		pr_err("Error reading db var: 0x%lx\n", status);
+		pr_err("Error reading db var: 0x%lx\n", *status);
 		return NULL;
 	}
 
@@ -74,6 +76,7 @@ static int __init load_uefi_certs(void)
 	efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
 	void *db = NULL, *dbx = NULL, *mok = NULL;
 	unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
+	efi_status_t status;
 	int rc = 0;
 
 	if (!efi.get_variable)
@@ -83,9 +86,12 @@ static int __init load_uefi_certs(void)
 	 * an error if we can't get them.
 	 */
 	if (!uefi_check_ignore_db()) {
-		db = get_cert_list(L"db", &secure_var, &dbsize);
+		db = get_cert_list(L"db", &secure_var, &dbsize, &status);
 		if (!db) {
-			pr_err("MODSIGN: Couldn't get UEFI db list\n");
+			if (status == EFI_NOT_FOUND)
+				pr_debug("MODSIGN: db variable wasn't found\n");
+			else
+				pr_err("MODSIGN: Couldn't get UEFI db list\n");
 		} else {
 			rc = parse_efi_signature_list("UEFI:db",
 					db, dbsize, get_handler_for_db);
@@ -96,9 +102,12 @@ static int __init load_uefi_certs(void)
 		}
 	}
 
-	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
+	mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
 	if (!mok) {
-		pr_info("Couldn't get UEFI MokListRT\n");
+		if (status == EFI_NOT_FOUND)
+			pr_debug("MokListRT variable wasn't found\n");
+		else
+			pr_info("Couldn't get UEFI MokListRT\n");
 	} else {
 		rc = parse_efi_signature_list("UEFI:MokListRT",
 					      mok, moksize, get_handler_for_db);
@@ -107,9 +116,12 @@ static int __init load_uefi_certs(void)
 		kfree(mok);
 	}
 
-	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
+	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, &status);
 	if (!dbx) {
-		pr_info("Couldn't get UEFI dbx list\n");
+		if (status == EFI_NOT_FOUND)
+			pr_debug("dbx variable wasn't found\n");
+		else
+			pr_info("Couldn't get UEFI dbx list\n");
 	} else {
 		rc = parse_efi_signature_list("UEFI:dbx",
 					      dbx, dbxsize,
-- 
2.24.1


^ permalink raw reply related

* [PATCH v2 2/2] ima: add sm3 algorithm to hash algorithm configuration list
From: Tianjia Zhang @ 2020-02-17  9:36 UTC (permalink / raw)
  To: herbert, davem, jarkko.sakkinen, zohar, ebiggers, dmitry.kasatkin,
	jmorris, serge
  Cc: linux-crypto, linux-integrity, linux-security-module,
	linux-kernel
In-Reply-To: <20200217093649.97938-1-tianjia.zhang@linux.alibaba.com>

sm3 has been supported by the ima hash algorithm, but it is not
yet in the Kconfig configuration list. After adding, both ima and tpm2
can support sm3 well.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 security/integrity/ima/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 711ff10fa36e..3f3ee4e2eb0d 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -112,6 +112,10 @@ choice
 	config IMA_DEFAULT_HASH_WP512
 		bool "WP512"
 		depends on CRYPTO_WP512=y && !IMA_TEMPLATE
+
+	config IMA_DEFAULT_HASH_SM3
+		bool "SM3"
+		depends on CRYPTO_SM3=y && !IMA_TEMPLATE
 endchoice
 
 config IMA_DEFAULT_HASH
@@ -121,6 +125,7 @@ config IMA_DEFAULT_HASH
 	default "sha256" if IMA_DEFAULT_HASH_SHA256
 	default "sha512" if IMA_DEFAULT_HASH_SHA512
 	default "wp512" if IMA_DEFAULT_HASH_WP512
+	default "sm3" if IMA_DEFAULT_HASH_SM3
 
 config IMA_WRITE_POLICY
 	bool "Enable multiple writes to the IMA policy"
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 1/2] crypto: fix mismatched hash algorithm name sm3-256 to sm3
From: Tianjia Zhang @ 2020-02-17  9:36 UTC (permalink / raw)
  To: herbert, davem, jarkko.sakkinen, zohar, ebiggers, dmitry.kasatkin,
	jmorris, serge
  Cc: linux-crypto, linux-integrity, linux-security-module,
	linux-kernel
In-Reply-To: <20200217093649.97938-1-tianjia.zhang@linux.alibaba.com>

The name sm3-256 is defined in hash_algo_name in hash_info, but the
algorithm name implemented in sm3_generic.c is sm3, which will cause
the sm3-256 algorithm to be not found in some application scenarios of
the hash algorithm, and an ENOENT error will occur. For example,
IMA, keys, and other subsystems that reference hash_algo_name all use
the hash algorithm of sm3.

According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value and there are no plans for
other length development, so there is no ambiguity in the name of sm3.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 Documentation/security/keys/trusted-encrypted.rst | 2 +-
 crypto/hash_info.c                                | 4 ++--
 drivers/char/tpm/tpm2-cmd.c                       | 2 +-
 include/crypto/hash_info.h                        | 2 +-
 include/linux/tpm.h                               | 2 +-
 include/uapi/linux/hash_info.h                    | 2 +-
 security/keys/trusted-keys/trusted_tpm2.c         | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 50ac8bcd6970..5b6fea88f335 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -66,7 +66,7 @@ Usage::
                      default 1 (resealing allowed)
        hash=         hash algorithm name as a string. For TPM 1.x the only
                      allowed value is sha1. For TPM 2.x the allowed values
-                     are sha1, sha256, sha384, sha512 and sm3-256.
+                     are sha1, sha256, sha384, sha512 and sm3.
        policydigest= digest for the authorization policy. must be calculated
                      with the same hash algorithm as specified by the 'hash='
                      option.
diff --git a/crypto/hash_info.c b/crypto/hash_info.c
index c754cb75dd1a..fe0119407219 100644
--- a/crypto/hash_info.c
+++ b/crypto/hash_info.c
@@ -26,7 +26,7 @@ const char *const hash_algo_name[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= "tgr128",
 	[HASH_ALGO_TGR_160]	= "tgr160",
 	[HASH_ALGO_TGR_192]	= "tgr192",
-	[HASH_ALGO_SM3_256]	= "sm3-256",
+	[HASH_ALGO_SM3]		= "sm3",
 	[HASH_ALGO_STREEBOG_256] = "streebog256",
 	[HASH_ALGO_STREEBOG_512] = "streebog512",
 };
@@ -50,7 +50,7 @@ const int hash_digest_size[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= TGR128_DIGEST_SIZE,
 	[HASH_ALGO_TGR_160]	= TGR160_DIGEST_SIZE,
 	[HASH_ALGO_TGR_192]	= TGR192_DIGEST_SIZE,
-	[HASH_ALGO_SM3_256]	= SM3256_DIGEST_SIZE,
+	[HASH_ALGO_SM3]		= SM3_DIGEST_SIZE,
 	[HASH_ALGO_STREEBOG_256] = STREEBOG256_DIGEST_SIZE,
 	[HASH_ALGO_STREEBOG_512] = STREEBOG512_DIGEST_SIZE,
 };
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 13696deceae8..44412538e47c 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -19,7 +19,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
 	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
 	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
-	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+	{HASH_ALGO_SM3, TPM_ALG_SM3},
 };
 
 int tpm2_get_timeouts(struct tpm_chip *chip)
diff --git a/include/crypto/hash_info.h b/include/crypto/hash_info.h
index eb9d2e368969..07cd6e0b0fee 100644
--- a/include/crypto/hash_info.h
+++ b/include/crypto/hash_info.h
@@ -31,7 +31,7 @@
 #define TGR192_DIGEST_SIZE 24
 
 /* not defined in include/crypto/ */
-#define SM3256_DIGEST_SIZE 32
+#define SM3_DIGEST_SIZE 32
 
 extern const char *const hash_algo_name[HASH_ALGO__LAST];
 extern const int hash_digest_size[HASH_ALGO__LAST];
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 03e9b184411b..042c3eb5f051 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -39,7 +39,7 @@ enum tpm_algorithms {
 	TPM_ALG_SHA384		= 0x000C,
 	TPM_ALG_SHA512		= 0x000D,
 	TPM_ALG_NULL		= 0x0010,
-	TPM_ALG_SM3_256		= 0x0012,
+	TPM_ALG_SM3		= 0x0012,
 };
 
 struct tpm_digest {
diff --git a/include/uapi/linux/hash_info.h b/include/uapi/linux/hash_info.h
index 74a8609fcb4d..1355525dd4aa 100644
--- a/include/uapi/linux/hash_info.h
+++ b/include/uapi/linux/hash_info.h
@@ -32,7 +32,7 @@ enum hash_algo {
 	HASH_ALGO_TGR_128,
 	HASH_ALGO_TGR_160,
 	HASH_ALGO_TGR_192,
-	HASH_ALGO_SM3_256,
+	HASH_ALGO_SM3,
 	HASH_ALGO_STREEBOG_256,
 	HASH_ALGO_STREEBOG_512,
 	HASH_ALGO__LAST
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..cbd5574a88c8 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -17,7 +17,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
 	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
 	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
-	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+	{HASH_ALGO_SM3, TPM_ALG_SM3},
 };
 
 /**
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2] IMA hash algorithm supports sm3
From: Tianjia Zhang @ 2020-02-17  9:36 UTC (permalink / raw)
  To: herbert, davem, jarkko.sakkinen, zohar, ebiggers, dmitry.kasatkin,
	jmorris, serge
  Cc: linux-crypto, linux-integrity, linux-security-module,
	linux-kernel

Fixed an issue where the sm3 algorithm name mismatch in the crypto subsystem hash_algo_name.
Make IMA support sm3 hash algorithm, added support for sm3 in IMA algorithm Kconfig configuration.



^ permalink raw reply

* [PATCH v7 12/12] doc/admin-guide: update kernel.rst with CAP_PERFMON information
From: Alexey Budankov @ 2020-02-17  8:13 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Update kernel.rst documentation file with the information
related to usage of CAP_PERFMON capability to secure performance
monitoring and observability operations in system.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 Documentation/admin-guide/sysctl/kernel.rst | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index def074807cee..b06ae9389809 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -720,20 +720,26 @@ perf_event_paranoid:
 ====================
 
 Controls use of the performance events system by unprivileged
-users (without CAP_SYS_ADMIN).  The default value is 2.
+users (without CAP_PERFMON). The default value is 2.
+
+For backward compatibility reasons access to system performance
+monitoring and observability remains open for CAP_SYS_ADMIN
+privileged processes but CAP_SYS_ADMIN usage for secure system
+performance monitoring and observability operations is discouraged
+with respect to CAP_PERFMON use cases.
 
 ===  ==================================================================
  -1  Allow use of (almost) all events by all users
 
      Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK
 
->=0  Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN
+>=0  Disallow ftrace function tracepoint by users without CAP_PERFMON
 
-     Disallow raw tracepoint access by users without CAP_SYS_ADMIN
+     Disallow raw tracepoint access by users without CAP_PERFMON
 
->=1  Disallow CPU event access by users without CAP_SYS_ADMIN
+>=1  Disallow CPU event access by users without CAP_PERFMON
 
->=2  Disallow kernel profiling by users without CAP_SYS_ADMIN
+>=2  Disallow kernel profiling by users without CAP_PERFMON
 ===  ==================================================================
 
 
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 11/12] doc/admin-guide: update perf-security.rst with CAP_PERFMON information
From: Alexey Budankov @ 2020-02-17  8:12 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Update perf-security.rst documentation file with the information
related to usage of CAP_PERFMON capability to secure performance
monitoring and observability operations in system.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 Documentation/admin-guide/perf-security.rst | 65 +++++++++++++--------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/Documentation/admin-guide/perf-security.rst b/Documentation/admin-guide/perf-security.rst
index 72effa7c23b9..81202d46a1ae 100644
--- a/Documentation/admin-guide/perf-security.rst
+++ b/Documentation/admin-guide/perf-security.rst
@@ -1,6 +1,6 @@
 .. _perf_security:
 
-Perf Events and tool security
+Perf events and tool security
 =============================
 
 Overview
@@ -42,11 +42,11 @@ categories:
 Data that belong to the fourth category can potentially contain
 sensitive process data. If PMUs in some monitoring modes capture values
 of execution context registers or data from process memory then access
-to such monitoring capabilities requires to be ordered and secured
-properly. So, perf_events/Perf performance monitoring is the subject for
-security access control management [5]_ .
+to such monitoring modes requires to be ordered and secured properly.
+So, perf_events performance monitoring and observability operations is
+the subject for security access control management [5]_ .
 
-perf_events/Perf access control
+perf_events access control
 -------------------------------
 
 To perform security checks, the Linux implementation splits processes
@@ -66,11 +66,25 @@ into distinct units, known as capabilities [6]_ , which can be
 independently enabled and disabled on per-thread basis for processes and
 files of unprivileged users.
 
-Unprivileged processes with enabled CAP_SYS_ADMIN capability are treated
+Unprivileged processes with enabled CAP_PERFMON capability are treated
 as privileged processes with respect to perf_events performance
-monitoring and bypass *scope* permissions checks in the kernel.
-
-Unprivileged processes using perf_events system call API is also subject
+monitoring and observability operations, thus, bypass *scope* permissions
+checks in the kernel. CAP_PERFMON implements the principal of least
+privilege [13]_ (POSIX 1003.1e: 2.2.2.39) for performance monitoring and
+observability operations in the kernel and provides secure approach to
+perfomance monitoring and observability in the system.
+
+For backward compatibility reasons access to perf_events monitoring and
+observability operations is also open for CAP_SYS_ADMIN privileged
+processes but CAP_SYS_ADMIN usage for secure monitoring and observability
+use cases is discouraged with respect to CAP_PERFMON capability.
+If system audit records [14]_ for a process using perf_events system call
+API contain denial records of acquiring both CAP_PERFMON and CAP_SYS_ADMIN
+capabilities then providing the process with CAP_PERFMON capability singly
+is recommended as the preferred secure approach to resolve double access
+denial logging related to usage of performance monitoring and observability.
+
+Unprivileged processes using perf_events system call are also subject
 for PTRACE_MODE_READ_REALCREDS ptrace access mode check [7]_ , whose
 outcome determines whether monitoring is permitted. So unprivileged
 processes provided with CAP_SYS_PTRACE capability are effectively
@@ -82,14 +96,14 @@ performance analysis of monitored processes or a system. For example,
 CAP_SYSLOG capability permits reading kernel space memory addresses from
 /proc/kallsyms file.
 
-perf_events/Perf privileged users
+Privileged Perf users groups
 ---------------------------------
 
 Mechanisms of capabilities, privileged capability-dumb files [6]_ and
-file system ACLs [10]_ can be used to create a dedicated group of
-perf_events/Perf privileged users who are permitted to execute
-performance monitoring without scope limits. The following steps can be
-taken to create such a group of privileged Perf users.
+file system ACLs [10]_ can be used to create dedicated groups of
+privileged Perf users who are permitted to execute performance monitoring
+and observability without scope limits. The following steps can be
+taken to create such groups of privileged Perf users.
 
 1. Create perf_users group of privileged Perf users, assign perf_users
    group to Perf tool executable and limit access to the executable for
@@ -108,30 +122,30 @@ taken to create such a group of privileged Perf users.
    -rwxr-x---  2 root perf_users  11M Oct 19 15:12 perf
 
 2. Assign the required capabilities to the Perf tool executable file and
-   enable members of perf_users group with performance monitoring
+   enable members of perf_users group with monitoring and observability
    privileges [6]_ :
 
 ::
 
-   # setcap "cap_sys_admin,cap_sys_ptrace,cap_syslog=ep" perf
-   # setcap -v "cap_sys_admin,cap_sys_ptrace,cap_syslog=ep" perf
+   # setcap "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" perf
+   # setcap -v "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" perf
    perf: OK
    # getcap perf
-   perf = cap_sys_ptrace,cap_sys_admin,cap_syslog+ep
+   perf = cap_sys_ptrace,cap_syslog,cap_perfmon+ep
 
 As a result, members of perf_users group are capable of conducting
-performance monitoring by using functionality of the configured Perf
-tool executable that, when executes, passes perf_events subsystem scope
-checks.
+performance monitoring and observability by using functionality of the
+configured Perf tool executable that, when executes, passes perf_events
+subsystem scope checks.
 
 This specific access control management is only available to superuser
 or root running processes with CAP_SETPCAP, CAP_SETFCAP [6]_
 capabilities.
 
-perf_events/Perf unprivileged users
+Unprivileged users
 -----------------------------------
 
-perf_events/Perf *scope* and *access* control for unprivileged processes
+perf_events *scope* and *access* control for unprivileged processes
 is governed by perf_event_paranoid [2]_ setting:
 
 -1:
@@ -166,7 +180,7 @@ is governed by perf_event_paranoid [2]_ setting:
      perf_event_mlock_kb locking limit is imposed but ignored for
      unprivileged processes with CAP_IPC_LOCK capability.
 
-perf_events/Perf resource control
+Resource control
 ---------------------------------
 
 Open file descriptors
@@ -227,4 +241,5 @@ Bibliography
 .. [10] `<http://man7.org/linux/man-pages/man5/acl.5.html>`_
 .. [11] `<http://man7.org/linux/man-pages/man2/getrlimit.2.html>`_
 .. [12] `<http://man7.org/linux/man-pages/man5/limits.conf.5.html>`_
-
+.. [13] `<https://sites.google.com/site/fullycapable>`_
+.. [14] `<http://man7.org/linux/man-pages/man8/auditd.8.html>`_
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 10/12] drivers/oprofile: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:12 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without
the rest of CAP_SYS_ADMIN credentials, excludes chances to misuse
the credentials and makes operation more secure.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and only
for the time that such privileges are actually required)

For backward compatibility reasons access to the monitoring remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
for secure monitoring is discouraged with respect to CAP_PERFMON
capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 drivers/oprofile/event_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/oprofile/event_buffer.c b/drivers/oprofile/event_buffer.c
index 12ea4a4ad607..6c9edc8bbc95 100644
--- a/drivers/oprofile/event_buffer.c
+++ b/drivers/oprofile/event_buffer.c
@@ -113,7 +113,7 @@ static int event_buffer_open(struct inode *inode, struct file *file)
 {
 	int err = -EPERM;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!perfmon_capable())
 		return -EPERM;
 
 	if (test_and_set_bit_lock(0, &buffer_opened))
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 09/12] drivers/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:11 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without
the rest of CAP_SYS_ADMIN credentials, excludes chances to misuse
the credentials and makes operation more secure.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and
only for the time that such privileges are actually required)

For backward compatibility reasons access to the monitoring remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
for secure monitoring is discouraged with respect to CAP_PERFMON
capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 drivers/perf/arm_spe_pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 4e4984a55cd1..5dff81bc3324 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -274,7 +274,7 @@ static u64 arm_spe_event_to_pmscr(struct perf_event *event)
 	if (!attr->exclude_kernel)
 		reg |= BIT(SYS_PMSCR_EL1_E1SPE_SHIFT);
 
-	if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && capable(CAP_SYS_ADMIN))
+	if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && perfmon_capable())
 		reg |= BIT(SYS_PMSCR_EL1_CX_SHIFT);
 
 	return reg;
@@ -700,7 +700,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
 		return -EOPNOTSUPP;
 
 	reg = arm_spe_event_to_pmscr(event);
-	if (!capable(CAP_SYS_ADMIN) &&
+	if (!perfmon_capable() &&
 	    (reg & (BIT(SYS_PMSCR_EL1_PA_SHIFT) |
 		    BIT(SYS_PMSCR_EL1_CX_SHIFT) |
 		    BIT(SYS_PMSCR_EL1_PCT_SHIFT))))
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 08/12] parisc/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:10 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without
the rest of CAP_SYS_ADMIN credentials, excludes chances to misuse
the credentials and makes operation more secure.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and only
for the time that such privileges are actually required)

For backward compatibility reasons access to the monitoring remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
for secure monitoring is discouraged with respect to CAP_PERFMON
capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 arch/parisc/kernel/perf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/perf.c b/arch/parisc/kernel/perf.c
index e1a8fee3ad49..d46b6709ec56 100644
--- a/arch/parisc/kernel/perf.c
+++ b/arch/parisc/kernel/perf.c
@@ -300,7 +300,7 @@ static ssize_t perf_write(struct file *file, const char __user *buf,
 	else
 		return -EFAULT;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!perfmon_capable())
 		return -EACCES;
 
 	if (count != sizeof(uint32_t))
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 07/12] powerpc/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:10 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without
the rest of CAP_SYS_ADMIN credentials, excludes chances to misuse
the credentials and makes operation more secure.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and
only for the time that such privileges are actually required)

For backward compatibility reasons access to the monitoring remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage
for secure monitoring is discouraged with respect to CAP_PERFMON
capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 arch/powerpc/perf/imc-pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e1fd2d..e837717492e4 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -898,7 +898,7 @@ static int thread_imc_event_init(struct perf_event *event)
 	if (event->attr.type != event->pmu->type)
 		return -ENOENT;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!perfmon_capable())
 		return -EACCES;
 
 	/* Sampling not supported */
@@ -1307,7 +1307,7 @@ static int trace_imc_event_init(struct perf_event *event)
 	if (event->attr.type != event->pmu->type)
 		return -ENOENT;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!perfmon_capable())
 		return -EACCES;
 
 	/* Return if this is a couting event */
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 06/12] trace/bpf_trace: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:09 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to bpf_trace monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and only
for the time that such privileges are actually required)

For backward compatibility reasons access to bpf_trace monitoring
remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
usage for secure bpf_trace monitoring is discouraged with respect to
CAP_PERFMON capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 kernel/trace/bpf_trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 19e793aa441a..70e8249eebe5 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1416,7 +1416,7 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
 	u32 *ids, prog_cnt, ids_len;
 	int ret;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!perfmon_capable())
 		return -EPERM;
 	if (event->attr.type != PERF_TYPE_TRACEPOINT)
 		return -EINVAL;
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 05/12] drm/i915/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:08 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to i915_perf monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and only
for the time that such privileges are actually required)

For backward compatibility reasons access to i915_events subsystem
remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
usage for secure i915_events monitoring is discouraged with respect to
CAP_PERFMON capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 0f556d80ba36..a3f32bd0aa47 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3378,10 +3378,10 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf,
 	/* Similar to perf's kernel.perf_paranoid_cpu sysctl option
 	 * we check a dev.i915.perf_stream_paranoid sysctl option
 	 * to determine if it's ok to access system wide OA counters
-	 * without CAP_SYS_ADMIN privileges.
+	 * without CAP_PERFMON or CAP_SYS_ADMIN privileges.
 	 */
 	if (privileged_op &&
-	    i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+	    i915_perf_stream_paranoid && !perfmon_capable()) {
 		DRM_DEBUG("Insufficient privileges to open i915 perf stream\n");
 		ret = -EACCES;
 		goto err_ctx;
@@ -3574,9 +3574,8 @@ static int read_properties_unlocked(struct i915_perf *perf,
 			} else
 				oa_freq_hz = 0;
 
-			if (oa_freq_hz > i915_oa_max_sample_rate &&
-			    !capable(CAP_SYS_ADMIN)) {
-				DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n",
+			if (oa_freq_hz > i915_oa_max_sample_rate && !perfmon_capable()) {
+				DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without CAP_PERFMON or CAP_SYS_ADMIN privileges\n",
 					  i915_oa_max_sample_rate);
 				return -EACCES;
 			}
@@ -3997,7 +3996,7 @@ int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
-	if (i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+	if (i915_perf_stream_paranoid && !perfmon_capable()) {
 		DRM_DEBUG("Insufficient privileges to add i915 OA config\n");
 		return -EACCES;
 	}
@@ -4144,7 +4143,7 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data,
 		return -ENOTSUPP;
 	}
 
-	if (i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+	if (i915_perf_stream_paranoid && !perfmon_capable()) {
 		DRM_DEBUG("Insufficient privileges to remove i915 OA config\n");
 		return -EACCES;
 	}
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 04/12] perf tool: extend Perf tool with CAP_PERFMON capability support
From: Alexey Budankov @ 2020-02-17  8:08 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Extend error messages to mention CAP_PERFMON capability as an option
to substitute CAP_SYS_ADMIN capability for secure system performance
monitoring and observability. Make perf_event_paranoid_check() and
__cmd_ftrace() to be aware of CAP_PERFMON capability.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and only
for the time that such privileges are actually required)

For backward compatibility reasons access to perf_events subsystem
remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
usage for secure perf_events monitoring is discouraged with respect to
CAP_PERFMON capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 tools/perf/builtin-ftrace.c |  5 +++--
 tools/perf/design.txt       |  3 ++-
 tools/perf/util/cap.h       |  4 ++++
 tools/perf/util/evsel.c     | 10 +++++-----
 tools/perf/util/util.c      |  1 +
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index d5adc417a4ca..55eda54240fb 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -284,10 +284,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		.events = POLLIN,
 	};
 
-	if (!perf_cap__capable(CAP_SYS_ADMIN)) {
+	if (!(perf_cap__capable(CAP_PERFMON) ||
+	      perf_cap__capable(CAP_SYS_ADMIN))) {
 		pr_err("ftrace only works for %s!\n",
 #ifdef HAVE_LIBCAP_SUPPORT
-		"users with the SYS_ADMIN capability"
+		"users with the CAP_PERFMON or CAP_SYS_ADMIN capability"
 #else
 		"root"
 #endif
diff --git a/tools/perf/design.txt b/tools/perf/design.txt
index 0453ba26cdbd..a42fab308ff6 100644
--- a/tools/perf/design.txt
+++ b/tools/perf/design.txt
@@ -258,7 +258,8 @@ gets schedule to. Per task counters can be created by any user, for
 their own tasks.
 
 A 'pid == -1' and 'cpu == x' counter is a per CPU counter that counts
-all events on CPU-x. Per CPU counters need CAP_SYS_ADMIN privilege.
+all events on CPU-x. Per CPU counters need CAP_PERFMON or CAP_SYS_ADMIN
+privilege.
 
 The 'flags' parameter is currently unused and must be zero.
 
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
index 051dc590ceee..ae52878c0b2e 100644
--- a/tools/perf/util/cap.h
+++ b/tools/perf/util/cap.h
@@ -29,4 +29,8 @@ static inline bool perf_cap__capable(int cap __maybe_unused)
 #define CAP_SYSLOG	34
 #endif
 
+#ifndef CAP_PERFMON
+#define CAP_PERFMON	38
+#endif
+
 #endif /* __PERF_CAP_H */
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c8dc4450884c..da57d1d4c601 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2493,14 +2493,14 @@ int perf_evsel__open_strerror(struct evsel *evsel, struct target *target,
 		 "You may not have permission to collect %sstats.\n\n"
 		 "Consider tweaking /proc/sys/kernel/perf_event_paranoid,\n"
 		 "which controls use of the performance events system by\n"
-		 "unprivileged users (without CAP_SYS_ADMIN).\n\n"
+		 "unprivileged users (without CAP_PERFMON or CAP_SYS_ADMIN).\n\n"
 		 "The current value is %d:\n\n"
 		 "  -1: Allow use of (almost) all events by all users\n"
 		 "      Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK\n"
-		 ">= 0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN\n"
-		 "      Disallow raw tracepoint access by users without CAP_SYS_ADMIN\n"
-		 ">= 1: Disallow CPU event access by users without CAP_SYS_ADMIN\n"
-		 ">= 2: Disallow kernel profiling by users without CAP_SYS_ADMIN\n\n"
+		 ">= 0: Disallow ftrace function tracepoint by users without CAP_PERFMON or CAP_SYS_ADMIN\n"
+		 "      Disallow raw tracepoint access by users without CAP_SYS_PERFMON or CAP_SYS_ADMIN\n"
+		 ">= 1: Disallow CPU event access by users without CAP_PERFMON or CAP_SYS_ADMIN\n"
+		 ">= 2: Disallow kernel profiling by users without CAP_PERFMON or CAP_SYS_ADMIN\n\n"
 		 "To make this setting permanent, edit /etc/sysctl.conf too, e.g.:\n\n"
 		 "	kernel.perf_event_paranoid = -1\n" ,
 				 target->system_wide ? "system-wide " : "",
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 969ae560dad9..51cf3071db74 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -272,6 +272,7 @@ int perf_event_paranoid(void)
 bool perf_event_paranoid_check(int max_level)
 {
 	return perf_cap__capable(CAP_SYS_ADMIN) ||
+			perf_cap__capable(CAP_PERFMON) ||
 			perf_event_paranoid() <= max_level;
 }
 
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 03/12] perf/core: open access to probes for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:07 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to monitoring via kprobes and uprobes and eBPF tracing for
CAP_PERFMON privileged process. Providing the access under CAP_PERFMON
capability singly, without the rest of CAP_SYS_ADMIN credentials,
excludes chances to misuse the credentials and makes operation more
secure.

perf kprobes and uprobes are used by ftrace and eBPF. perf probe uses
ftrace to define new kprobe events, and those events are treated as
tracepoint events. eBPF defines new probes via perf_event_open interface
and then the probes are used in eBPF tracing.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and only
for the time that such privileges are actually required)

For backward compatibility reasons access to perf_events subsystem
remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
usage for secure perf_events monitoring is discouraged with respect to
CAP_PERFMON capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 kernel/events/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 46464367c47a..4564caa2c527 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9107,7 +9107,7 @@ static int perf_kprobe_event_init(struct perf_event *event)
 	if (event->attr.type != perf_kprobe.type)
 		return -ENOENT;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!perfmon_capable())
 		return -EACCES;
 
 	/*
@@ -9167,7 +9167,7 @@ static int perf_uprobe_event_init(struct perf_event *event)
 	if (event->attr.type != perf_uprobe.type)
 		return -ENOENT;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!perfmon_capable())
 		return -EACCES;
 
 	/*
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 02/12] perf/core: open access to the core for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-02-17  8:06 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Open access to monitoring of kernel code, cpus, tracepoints and
namespaces data for a CAP_PERFMON privileged process. Providing the
access under CAP_PERFMON capability singly, without the rest of
CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials
and makes operation more secure.

CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39
principle of least privilege: A security design principle that states
that a process or program be granted only those privileges (e.g.,
capabilities) necessary to accomplish its legitimate function, and only
for the time that such privileges are actually required)

For backward compatibility reasons access to perf_events subsystem
remains open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN
usage for secure perf_events monitoring is discouraged with respect to
CAP_PERFMON capability.

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 include/linux/perf_event.h | 6 +++---
 kernel/events/core.c       | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 68e21e828893..5cbfc06c56b3 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1297,7 +1297,7 @@ static inline int perf_is_paranoid(void)
 
 static inline int perf_allow_kernel(struct perf_event_attr *attr)
 {
-	if (sysctl_perf_event_paranoid > 1 && !capable(CAP_SYS_ADMIN))
+	if (sysctl_perf_event_paranoid > 1 && !perfmon_capable())
 		return -EACCES;
 
 	return security_perf_event_open(attr, PERF_SECURITY_KERNEL);
@@ -1305,7 +1305,7 @@ static inline int perf_allow_kernel(struct perf_event_attr *attr)
 
 static inline int perf_allow_cpu(struct perf_event_attr *attr)
 {
-	if (sysctl_perf_event_paranoid > 0 && !capable(CAP_SYS_ADMIN))
+	if (sysctl_perf_event_paranoid > 0 && !perfmon_capable())
 		return -EACCES;
 
 	return security_perf_event_open(attr, PERF_SECURITY_CPU);
@@ -1313,7 +1313,7 @@ static inline int perf_allow_cpu(struct perf_event_attr *attr)
 
 static inline int perf_allow_tracepoint(struct perf_event_attr *attr)
 {
-	if (sysctl_perf_event_paranoid > -1 && !capable(CAP_SYS_ADMIN))
+	if (sysctl_perf_event_paranoid > -1 && !perfmon_capable())
 		return -EPERM;
 
 	return security_perf_event_open(attr, PERF_SECURITY_TRACEPOINT);
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3f1f77de7247..46464367c47a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11205,7 +11205,7 @@ SYSCALL_DEFINE5(perf_event_open,
 	}
 
 	if (attr.namespaces) {
-		if (!capable(CAP_SYS_ADMIN))
+		if (!perfmon_capable())
 			return -EACCES;
 	}
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH v7 01/12] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-02-17  8:06 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man
In-Reply-To: <c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com>


Introduce CAP_PERFMON capability designed to secure system performance
monitoring and observability operations so that CAP_PERFMON would assist
CAP_SYS_ADMIN capability in its governing role for performance
monitoring and observability subsystems.

CAP_PERFMON hardens system security and integrity during performance
monitoring and observability operations by decreasing attack surface
that is available to a CAP_SYS_ADMIN privileged process [2]. Providing
the access to system performance monitoring and observability operations
under CAP_PERFMON capability singly, without the rest of CAP_SYS_ADMIN
credentials, excludes chances to misuse the credentials and makes the
operation more secure. Thus, CAP_PERFMON implements the principal of
least privilege for performance monitoring and observability operations
(POSIX IEEE 1003.1e: 2.2.2.39 principle of least privilege: A security
design principle that states that a process or program be granted only
those privileges (e.g., capabilities) necessary to accomplish its
legitimate function, and only for the time that such privileges are
actually required)

CAP_PERFMON meets the demand to secure system performance monitoring and
observability operations for adoption in security sensitive, restricted,
multiuser production environments (e.g. HPC clusters, cloud and virtual
compute environments), where root or CAP_SYS_ADMIN credentials are not
available to mass users of a system, and securely unblocks accessibility
of system performance monitoring and observability operations beyond
the root and CAP_SYS_ADMIN use cases.

CAP_PERFMON takes over CAP_SYS_ADMIN credentials related to system
performance monitoring and observability operations and balances amount
of CAP_SYS_ADMIN credentials following the recommendations in the
capabilities man page [1] for CAP_SYS_ADMIN: "Note: this capability is
overloaded; see Notes to kernel developers, below." For backward
compatibility reasons access to system performance monitoring and
observability subsystems of the kernel remains open for CAP_SYS_ADMIN
privileged processes but CAP_SYS_ADMIN usage for secure system
performance monitoring and observability operations is discouraged with
respect to the designed CAP_PERFMON capability.

Although the software running under CAP_PERFMON can not ensure avoidance
of related hardware issues, the software can still mitigate these issues
following the official hardware issues mitigation procedure [2].
The bugs in the software itself can be fixed following the standard
kernel development process [3] to maintain and harden security of system
performance monitoring and observability operations.

[1] http://man7.org/linux/man-pages/man7/capabilities.7.html
[2] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
[3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
 include/linux/capability.h          | 4 ++++
 include/uapi/linux/capability.h     | 8 +++++++-
 security/selinux/include/classmap.h | 4 ++--
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index ecce0f43c73a..027d7e4a853b 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -251,6 +251,10 @@ extern bool privileged_wrt_inode_uidgid(struct user_namespace *ns, const struct
 extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
 extern bool file_ns_capable(const struct file *file, struct user_namespace *ns, int cap);
 extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns);
+static inline bool perfmon_capable(void)
+{
+	return capable(CAP_PERFMON) || capable(CAP_SYS_ADMIN);
+}
 
 /* audit system wants to get cap info from files as well */
 extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data *cpu_caps);
diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
index 272dc69fa080..e58c9636741b 100644
--- a/include/uapi/linux/capability.h
+++ b/include/uapi/linux/capability.h
@@ -367,8 +367,14 @@ struct vfs_ns_cap_data {
 
 #define CAP_AUDIT_READ		37
 
+/*
+ * Allow system performance and observability privileged operations
+ * using perf_events, i915_perf and other kernel subsystems
+ */
+
+#define CAP_PERFMON		38
 
-#define CAP_LAST_CAP         CAP_AUDIT_READ
+#define CAP_LAST_CAP         CAP_PERFMON
 
 #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
 
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 986f3ac14282..d233ab3f1533 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -27,9 +27,9 @@
 	    "audit_control", "setfcap"
 
 #define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
-		"wake_alarm", "block_suspend", "audit_read"
+		"wake_alarm", "block_suspend", "audit_read", "perfmon"
 
-#if CAP_LAST_CAP > CAP_AUDIT_READ
+#if CAP_LAST_CAP > CAP_PERFMON
 #error New capability defined, please update COMMON_CAP2_PERMS.
 #endif
 
-- 
2.20.1



^ permalink raw reply related

* [PATCH v7 00/12] Introduce CAP_PERFMON to secure system performance monitoring and observability
From: Alexey Budankov @ 2020-02-17  8:02 UTC (permalink / raw)
  To: James Morris, Serge Hallyn, Stephen Smalley, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Ingo Molnar,
	joonas.lahtinen@linux.intel.com, Alexei Starovoitov, Will Deacon,
	Paul Mackerras, Helge Deller, Thomas Gleixner
  Cc: Andi Kleen, Stephane Eranian, Igor Lubashev, Jiri Olsa,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
	linux-parisc@vger.kernel.org, oprofile-list,
	linux-doc@vger.kernel.org, linux-man


Currently access to perf_events, i915_perf and other performance
monitoring and observability subsystems of the kernel is open only for
a privileged process [1] with CAP_SYS_ADMIN capability enabled in the
process effective set [2].

This patch set introduces CAP_PERFMON capability designed to secure
system performance monitoring and observability operations so that
CAP_PERFMON would assist CAP_SYS_ADMIN capability in its governing role
for performance monitoring and observability subsystems of the kernel.

CAP_PERFMON intends to harden system security and integrity during
performance monitoring and observability operations by decreasing attack
surface that is available to a CAP_SYS_ADMIN privileged process [2].
Providing the access to performance monitoring and observability
operations under CAP_PERFMON capability singly, without the rest of
CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials
and makes the operation more secure. Thus, CAP_PERFMON implements the
principal of least privilege for performance monitoring and
observability operations (POSIX IEEE 1003.1e: 2.2.2.39 principle of
least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities)
necessary to accomplish its legitimate function, and only for the time
that such privileges are actually required)

CAP_PERFMON intends to meet the demand to secure system performance
monitoring and observability operations for adoption in security
sensitive, restricted, multiuser production environments (e.g. HPC
clusters, cloud and virtual compute environments), where root or
CAP_SYS_ADMIN credentials are not available to mass users of a system,
and securely unblock accessibility of system performance monitoring and
observability operations beyond root and CAP_SYS_ADMIN use cases.

CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to
system performance monitoring and observability operations and balance
amount of CAP_SYS_ADMIN credentials following the recommendations in
the capabilities man page [2] for CAP_SYS_ADMIN: "Note: this capability
is overloaded; see Notes to kernel developers, below." For backward
compatibility reasons access to system performance monitoring and
observability subsystems of the kernel remains open for CAP_SYS_ADMIN
privileged processes but CAP_SYS_ADMIN capability usage for secure
system performance monitoring and observability operations is
discouraged with respect to the designed CAP_PERFMON capability.

Possible alternative solution to this system security hardening,
capabilities balancing task of making performance monitoring and
observability operations more secure and accessible could be to use
the existing CAP_SYS_PTRACE capability to govern system performance
monitoring and observability subsystems. However CAP_SYS_PTRACE
capability still provides users with more credentials than are
required for secure performance monitoring and observability
operations and this excess is avoided by the designed CAP_PERFMON.

Although software running under CAP_PERFMON can not ensure avoidance of
related hardware issues, the software can still mitigate those issues
following the official hardware issues mitigation procedure [3]. The
bugs in the software itself can be fixed following the standard kernel
development process [4] to maintain and harden security of system
performance monitoring and observability operations. Finally, the patch
set is shaped in the way that simplifies backtracking procedure of
possible induced issues [5] as much as possible.

The patch set is for tip perf/core repository:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip perf/core
sha1: fdb64822443ec9fb8c3a74b598a74790ae8d2e22

---
Changes in v7:
- updated and extended kernel.rst and perf-security.rst documentation 
  files with the information about CAP_PERFMON capability and its use cases
- documented the case of double audit logging of CAP_PERFMON and CAP_SYS_ADMIN
  capabilities on a SELinux enabled system
Changes in v6:
- avoided noaudit checks in perfmon_capable() to explicitly advertise
  CAP_PERFMON usage thru audit logs to secure system performance
  monitoring and observability
Changes in v5:
- renamed CAP_SYS_PERFMON to CAP_PERFMON
- extended perfmon_capable() with noaudit checks
Changes in v4:
- converted perfmon_capable() into an inline function
- made perf_events kprobes, uprobes, hw breakpoints and namespaces data
  available to CAP_SYS_PERFMON privileged processes
- applied perfmon_capable() to drivers/perf and drivers/oprofile
- extended __cmd_ftrace() with support of CAP_SYS_PERFMON
Changes in v3:
- implemented perfmon_capable() macros aggregating required capabilities
  checks
Changes in v2:
- made perf_events trace points available to CAP_SYS_PERFMON privileged
  processes
- made perf_event_paranoid_check() treat CAP_SYS_PERFMON equally to
  CAP_SYS_ADMIN
- applied CAP_SYS_PERFMON to i915_perf, bpf_trace, powerpc and parisc
  system performance monitoring and observability related subsystems

---
Alexey Budankov (12):
  capabilities: introduce CAP_PERFMON to kernel and user space
  perf/core: open access to the core for CAP_PERFMON privileged process
  perf/core: open access to probes for CAP_PERFMON privileged process
  perf tool: extend Perf tool with CAP_PERFMON capability support
  drm/i915/perf: open access for CAP_PERFMON privileged process
  trace/bpf_trace: open access for CAP_PERFMON privileged process
  powerpc/perf: open access for CAP_PERFMON privileged process
  parisc/perf: open access for CAP_PERFMON privileged process
  drivers/perf: open access for CAP_PERFMON privileged process
  drivers/oprofile: open access for CAP_PERFMON privileged process
  doc/admin-guide: update perf-security.rst with CAP_PERFMON information
  doc/admin-guide: update kernel.rst with CAP_PERFMON information

 Documentation/admin-guide/perf-security.rst | 65 +++++++++++++--------
 Documentation/admin-guide/sysctl/kernel.rst | 16 +++--
 arch/parisc/kernel/perf.c                   |  2 +-
 arch/powerpc/perf/imc-pmu.c                 |  4 +-
 drivers/gpu/drm/i915/i915_perf.c            | 13 ++---
 drivers/oprofile/event_buffer.c             |  2 +-
 drivers/perf/arm_spe_pmu.c                  |  4 +-
 include/linux/capability.h                  |  4 ++
 include/linux/perf_event.h                  |  6 +-
 include/uapi/linux/capability.h             |  8 ++-
 kernel/events/core.c                        |  6 +-
 kernel/trace/bpf_trace.c                    |  2 +-
 security/selinux/include/classmap.h         |  4 +-
 tools/perf/builtin-ftrace.c                 |  5 +-
 tools/perf/design.txt                       |  3 +-
 tools/perf/util/cap.h                       |  4 ++
 tools/perf/util/evsel.c                     | 10 ++--
 tools/perf/util/util.c                      |  1 +
 18 files changed, 98 insertions(+), 61 deletions(-)

---
Validation (Intel Skylake, 8 cores, Fedora 29, 5.5.0-rc3+, x86_64):

libcap library [6], [7], [8] and Perf tool can be used to apply
CAP_PERFMON capability for secure system performance monitoring and
observability beyond the scope permitted by the system wide
perf_event_paranoid kernel setting [9] and below are the steps for
evaluation:

  - patch, build and boot the kernel
  - patch, build Perf tool e.g. to /home/user/perf
  ...
  # git clone git://git.kernel.org/pub/scm/libs/libcap/libcap.git libcap
  # pushd libcap
  # patch libcap/include/uapi/linux/capabilities.h with [PATCH 1]
  # make
  # pushd progs
  # ./setcap "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" /home/user/perf
  # ./setcap -v "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" /home/user/perf
  /home/user/perf: OK
  # ./getcap /home/user/perf
  /home/user/perf = cap_sys_ptrace,cap_syslog,cap_perfmon+ep
  # echo 2 > /proc/sys/kernel/perf_event_paranoid
  # cat /proc/sys/kernel/perf_event_paranoid 
  2
  ...
  $ /home/user/perf top
    ... works as expected ...
  $ cat /proc/`pidof perf`/status
  Name:	perf
  Umask:	0002
  State:	S (sleeping)
  Tgid:	2958
  Ngid:	0
  Pid:	2958
  PPid:	9847
  TracerPid:	0
  Uid:	500	500	500	500
  Gid:	500	500	500	500
  FDSize:	256
  ...
  CapInh:	0000000000000000
  CapPrm:	0000004400080000
  CapEff:	0000004400080000 => 01000100 00000000 00001000 00000000 00000000
                                     cap_perfmon,cap_sys_ptrace,cap_syslog
  CapBnd:	0000007fffffffff
  CapAmb:	0000000000000000
  NoNewPrivs:	0
  Seccomp:	0
  Speculation_Store_Bypass:	thread vulnerable
  Cpus_allowed:	ff
  Cpus_allowed_list:	0-7
  ...

Usage of cap_perfmon effectively avoids unused credentials excess:

- with cap_sys_admin:
  CapEff:	0000007fffffffff => 01111111 11111111 11111111 11111111 11111111

- with cap_perfmon:
  CapEff:	0000004400080000 => 01000100 00000000 00001000 00000000 00000000
                                    38   34               19
                               perfmon   syslog           sys_ptrace

---
[1] https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html
[2] http://man7.org/linux/man-pages/man7/capabilities.7.html
[3] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
[4] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
[5] https://www.kernel.org/doc/html/latest/process/management-style.html#decisions
[6] http://man7.org/linux/man-pages/man8/setcap.8.html
[7] https://git.kernel.org/pub/scm/libs/libcap/libcap.git
[8] https://sites.google.com/site/fullycapable/, posix_1003.1e-990310.pdf
[9] http://man7.org/linux/man-pages/man2/perf_event_open.2.html

^ permalink raw reply

* Re: [RFC PATCH] security: <linux/lsm_hooks.h>: fix all kernel-doc warnings
From: Casey Schaufler @ 2020-02-16 19:27 UTC (permalink / raw)
  To: Randy Dunlap, LKML, linux-security-module
  Cc: John Johansen, Kees Cook, Micah Morton, James Morris,
	Serge E. Hallyn, Paul Moore, Stephen Smalley, Eric Paris,
	Kentaro Takeda, Tetsuo Handa
In-Reply-To: <fb2c98bd-b579-6ad0-721a-56a4f81f0d6e@infradead.org>

On 2/15/2020 11:08 PM, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix all kernel-doc warnings in <linux/lsm_hooks.h>.
> Fixes the following warnings:
>
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'quotactl' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'quota_on' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_free_mnt_opts' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_eat_lsm_opts' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_kern_mount' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_show_options' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_add_mnt_opt' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'd_instantiate' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'getprocattr' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'setprocattr' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'locked_down' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_open' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_alloc' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_free' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_read' not described in 'security_list_options'
> ../include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_write' not described in 'security_list_options'
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

Thank you very much.

> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Micah Morton <mortonm@chromium.org>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: linux-security-module@vger.kernel.org
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Eric Paris <eparis@parisplace.org>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> Notes:
> a. The location for some of these might need to be modified.
> b. 'locked_down' was just missing a final ':'.
> c. Added a new section: Security hooks for perf events.
>
>  include/linux/lsm_hooks.h |   36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
>
> --- lnx-56-rc1.orig/include/linux/lsm_hooks.h
> +++ lnx-56-rc1/include/linux/lsm_hooks.h
> @@ -103,6 +103,10 @@
>   * @sb_free_security:
>   *	Deallocate and clear the sb->s_security field.
>   *	@sb contains the super_block structure to be modified.
> + * @sb_free_mnt_opts:
> + * 	Free memory associated with @mnt_ops.
> + * @sb_eat_lsm_opts:
> + * 	Eat (scan @orig options) and save them in @mnt_opts.
>   * @sb_statfs:
>   *	Check permission before obtaining filesystem statistics for the @mnt
>   *	mountpoint.
> @@ -136,6 +140,10 @@
>   *	@sb superblock being remounted
>   *	@data contains the filesystem-specific data.
>   *	Return 0 if permission is granted.
> + * @sb_kern_mount:
> + * 	Mount this @sb if allowed by permissions.
> + * @sb_show_options:
> + * 	Show (print on @m) mount options for this @sb.
>   * @sb_umount:
>   *	Check permission before the @mnt file system is unmounted.
>   *	@mnt contains the mounted file system.
> @@ -155,6 +163,8 @@
>   *	Copy all security options from a given superblock to another
>   *	@oldsb old superblock which contain information to clone
>   *	@newsb new superblock which needs filled in
> + * @sb_add_mnt_opt:
> + * 	Add one mount @option to @mnt_opts.
>   * @sb_parse_opts_str:
>   *	Parse a string of security data filling in the opts structure
>   *	@options string containing all mount options known by the LSM
> @@ -451,6 +461,12 @@
>   *	security module does not know about attribute or a negative error code
>   *	to abort the copy up. Note that the caller is responsible for reading
>   *	and writing the xattrs as this hook is merely a filter.
> + * @d_instantiate:
> + * 	Fill in @inode security information for a @dentry if allowed.
> + * @getprocattr:
> + * 	Read attribute @name for process @p and store it into @value if allowed.
> + * @setprocattr:
> + * 	Write (set) attribute @name to @value, size @size if allowed.
>   *
>   * Security hooks for kernfs node operations
>   *
> @@ -1113,6 +1129,7 @@
>   *	In case of failure, @secid will be set to zero.
>   *
>   * Security hooks for individual messages held in System V IPC message queues
> + *
>   * @msg_msg_alloc_security:
>   *	Allocate and attach a security structure to the msg->security field.
>   *	The security field is initialized to NULL when the structure is first
> @@ -1302,6 +1319,10 @@
>   *	@cap contains the capability <include/linux/capability.h>.
>   *	@opts contains options for the capable check <include/linux/security.h>
>   *	Return 0 if the capability is granted for @tsk.
> + * @quotactl:
> + * 	Check whether the quotactl syscall is allowed for this @sb.
> + * @quota_on:
> + * 	Check whether QUOTAON is allowed for this @dentry.
>   * @syslog:
>   *	Check permission before accessing the kernel message ring or changing
>   *	logging to the console.
> @@ -1449,11 +1470,24 @@
>   * @bpf_prog_free_security:
>   *	Clean up the security information stored inside bpf prog.
>   *
> - * @locked_down
> + * @locked_down:
>   *     Determine whether a kernel feature that potentially enables arbitrary
>   *     code execution in kernel space should be permitted.
>   *
>   *     @what: kernel feature being accessed
> + *
> + * Security hooks for perf events
> + *
> + * @perf_event_open:
> + * 	Check whether the @type of perf_event_open syscall is allowed.
> + * @perf_event_alloc:
> + * 	Allocate and save perf_event security info.
> + * @perf_event_free:
> + * 	Release (free) perf_event security info.
> + * @perf_event_read:
> + * 	Read perf_event security info if allowed.
> + * @perf_event_write:
> + * 	Write perf_event security info if allowed.
>   */
>  union security_list_options {
>  	int (*binder_set_context_mgr)(struct task_struct *mgr);
>

^ permalink raw reply

* Re: [PATCH v2 04/28] fsuidgid: add fsid mapping helpers
From: Christian Brauner @ 2020-02-16 16:55 UTC (permalink / raw)
  To: Jann Horn
  Cc: Stéphane Graber, Eric W. Biederman, Aleksa Sarai,
	Stephen Barber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
	Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
	Phil Estes, kernel list, linux-fsdevel, Linux Containers,
	linux-security-module, Linux API
In-Reply-To: <CAG48ez2o81ZwwL9muYyheN9vY69vJR5sB9LsLh=nk6wB4iuUgw@mail.gmail.com>

On Fri, Feb 14, 2020 at 08:11:36PM +0100, Jann Horn wrote:
> On Fri, Feb 14, 2020 at 7:37 PM Christian Brauner
> <christian.brauner@ubuntu.com> wrote:
> > This adds a set of helpers to translate between kfsuid/kfsgid and their
> > userspace fsuid/fsgid counter parts relative to a given user namespace.
> >
> > - kuid_t make_kfsuid(struct user_namespace *from, uid_t fsuid)
> >   Maps a user-namespace fsuid pair into a kfsuid.
> >   If no fsuid mappings have been written it behaves identical to calling
> >   make_kuid(). This ensures backwards compatibility for workloads unaware
> >   or not in need of fsid mappings.
> [...]
> > +#ifdef CONFIG_USER_NS_FSID
> > +/**
> > + *     make_kfsuid - Map a user-namespace fsuid pair into a kuid.
> > + *     @ns:  User namespace that the fsuid is in
> > + *     @fsuid: User identifier
> > + *
> > + *     Maps a user-namespace fsuid pair into a kernel internal kfsuid,
> > + *     and returns that kfsuid.
> > + *
> > + *     When there is no mapping defined for the user-namespace kfsuid
> > + *     pair INVALID_UID is returned.  Callers are expected to test
> > + *     for and handle INVALID_UID being returned.  INVALID_UID
> > + *     may be tested for using uid_valid().
> > + */
> > +kuid_t make_kfsuid(struct user_namespace *ns, uid_t fsuid)
> > +{
> > +       unsigned extents = ns->fsuid_map.nr_extents;
> > +       smp_rmb();
> > +
> > +       /* Map the fsuid to a global kernel fsuid */
> > +       if (extents == 0)
> > +               return KUIDT_INIT(map_id_down(&ns->uid_map, fsuid));
> > +
> > +       return KUIDT_INIT(map_id_down(&ns->fsuid_map, fsuid));
> > +}
> > +EXPORT_SYMBOL(make_kfsuid);
> 
> What effect is this fallback going to have for nested namespaces?
> 
> Let's say we have an outer namespace N1 with this uid_map:
> 
>     0 100000 65535
> 
> and with this fsuid_map:
> 
>     0 300000 65535
> 
> Now from in there, a process that is not aware of the existence of
> fsuid mappings creates a new user namespace N2 with the following
> uid_map:
> 
>     0 1000 1
> 
> At this point, if a process in N2 does chown("foo", 0, 0), is that
> going to make "foo" owned by kuid 101000, which isn't even mapped in
> N1?

So Jann just made a clever suggestion that would solve this problem fsid
maps can only be written if the corresponding id mapping has been
written and fsid mappings will only have an effect once the
corresponding id mapping has been written. That sounds rather sane to
me.

Christian

^ permalink raw reply

* Re: [PATCH v2 00/28] user_namespace: introduce fsid mappings
From: Christian Brauner @ 2020-02-16 16:40 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn,
	smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
	Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
	Phil Estes, linux-kernel, linux-fsdevel, containers,
	linux-security-module, linux-api
In-Reply-To: <87pneesf0a.fsf@mid.deneb.enyo.de>

On Sun, Feb 16, 2020 at 04:55:49PM +0100, Florian Weimer wrote:
> * Christian Brauner:
> 
> > With fsid mappings we can solve this by writing an id mapping of 0
> > 100000 100000 and an fsid mapping of 0 300000 100000. On filesystem
> > access the kernel will now lookup the mapping for 300000 in the fsid
> > mapping tables of the user namespace. And since such a mapping exists,
> > the corresponding files will have correct ownership.
> 
> I'm worried that this is a bit of a management nightmare because the
> data about the mapping does not live within the file system (it's
> externally determined, static, but crucial to the interpretation of
> file system content).  I expect that many organizations have

Iiuc, that's already the case with user namespaces right now e.g. when
you have an on-disk mapping that doesn't match your user namespace
mapping.

> centralized allocation of user IDs, but centralized allocation of the
> static mapping does not appear feasible.

I thought we're working on this right now with the new nss
infrastructure to register id mappings aka the shadow discussion we've
been having.

> 
> Have you considered a more complex design, where untranslated nested
> user IDs are store in a file attribute (or something like that)?  This

That doesn't sound like it would be feasible especially in the nesting
case wrt. to performance.

Christian

^ permalink raw reply

* Re: [PATCH v2 00/28] user_namespace: introduce fsid mappings
From: Florian Weimer @ 2020-02-16 15:55 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Stéphane Graber, Eric W. Biederman, Aleksa Sarai, Jann Horn,
	smbarber, Seth Forshee, Alexander Viro, Alexey Dobriyan,
	Serge Hallyn, James Morris, Kees Cook, Jonathan Corbet,
	Phil Estes, linux-kernel, linux-fsdevel, containers,
	linux-security-module, linux-api
In-Reply-To: <20200214183554.1133805-1-christian.brauner@ubuntu.com>

* Christian Brauner:

> With fsid mappings we can solve this by writing an id mapping of 0
> 100000 100000 and an fsid mapping of 0 300000 100000. On filesystem
> access the kernel will now lookup the mapping for 300000 in the fsid
> mapping tables of the user namespace. And since such a mapping exists,
> the corresponding files will have correct ownership.

I'm worried that this is a bit of a management nightmare because the
data about the mapping does not live within the file system (it's
externally determined, static, but crucial to the interpretation of
file system content).  I expect that many organizations have
centralized allocation of user IDs, but centralized allocation of the
static mapping does not appear feasible.

Have you considered a more complex design, where untranslated nested
user IDs are store in a file attribute (or something like that)?  This
way, any existing user ID infrastructure can be carried over largely
unchanged.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox