linux-kernel - Re: [PATCH] capabilities: add a option PR_SET_CAPS for sys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHC9VhT0WtK_QnO14SGHL3ZQ+v1D57jX4OEfEqXcsWo=PtbcoA@mail.gmail.com>
Date:   Thu, 19 Oct 2023 14:57:37 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Yunhui Cui <cuiyunhui@...edance.com>
Cc:     serge@...lyn.com, jmorris@...ei.org, peterz@...radead.org,
        chris.hyser@...cle.com, linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] capabilities: add a option PR_SET_CAPS for sys_prctl

On Wed, Oct 18, 2023 at 8:20 AM Yunhui Cui <cuiyunhui@...edance.com> wrote:
>
> By infecting the container process, the already running container is
> cloned, which means that each process of the container forks
> independently. But the process in the container lacks some permissions
> that cannot be completed.
>
> For a container that is already running, we cannot modify the
> configuration and restart it to complete the permission elevation.
> Since capset() can only complete the setting of a subset of the
> capabilities of the process, it cannot meet the requirements for
> raising permissions. So an option is added to prctl() to complete it.

I'm having a difficult time understanding the description above, would
it be possible for you to explain the problem differently?

> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index 82cb4210ba50..9a8dae2be801 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -246,6 +246,7 @@ struct prctl_mm_map {
>  # define PR_SCHED_CORE_SHARE_FROM      3 /* pull core_sched cookie to pid */
>  # define PR_SCHED_CORE_MAX             4
>
> +#define PR_SET_CAPS                    63
>  /* Clone and personalize thread */
>  #define PR_PERSONALIZED_CLONE          1000
>  /* Isolation eventfd & epollfd during fork */
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 1444f3954d75..968edd8b3564 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -266,11 +270,17 @@ SYSCALL_DEFINE2(capset, cap_user_header_t, header, const cap_user_data_t, data)
>         if (!new)
>                 return -ENOMEM;
>
> -       ret = security_capset(new, current_cred(),
> -                             &effective, &inheritable, &permitted);
> -       if (ret < 0)
> -               goto error;
> -
> +       if (!prctl) {
> +               ret = security_capset(new, current_cred(),
> +                               &effective, &inheritable, &permitted);
> +               if (ret < 0)
> +                       goto error;
> +       } else {
> +               ret = __capset(new, current_cred(),
> +                                &effective, &inheritable, &permitted);
> +               if (ret < 0)
> +                       goto error;
> +       }

It isn't clear to me why we would want to avoid the security_capset()
hook in the prctl case; the change to the task's capabilities is the
same, yes?  If the goal of prctl(PR_SET_CAPS, ...) is to bypass the
security_capset() controls, you're going to need to do a much better
job of explaining why this is necessary.

>         audit_log_capset(new, current_cred());
>
>         return commit_creds(new);

-- 
paul-moore.com