Re: [PATCH] Exporting capability code/name pairs

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

There is also the issue of compiled code which explicitly raises and
lowers capabilities around critical code sections (ie., as they were
intended to be used) is also not well served by this change.

That is, unless the code was compiled with things like CAP_MAC_ADMIN
being #define'd then it won't be able to do things like cap_set_flag()
appropriately.

Cheers

Andrew

KaiGai Kohei wrote:
> Andrew Morgan wrote:
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> KaiGai Kohei wrote:
>>> Remaining issues:
>>> - We have to mount securityfs explicitly, or use /etc/fstab.
>>>   It can cause a matter when we want to use this feature on
>>>   very early phase on boot. (like /sbin/init)
>> I'm not altogether clear how you intend this to work.
>>
>> Are you saying that some future version of libcap will require that
>> securityfs be mounted before it (libcap) will work?
> 
> Yes, but implementing this feature on securityfs might be not good
> idea as as James said. If this feature is on procfs or sysfs, it is
> not necessary to mount securityfs explicitly.
> 
> Thanks,
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHfD7n+bHCR3gb8jsRAsgcAKDY6qXBR3JV2addHUg5sxyZHJEkDQCfdLOL
zJlf3T4SQsUNENr3kwR5pr8=
=v8C5
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Exporting capability code/name pairs

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

KaiGai Kohei wrote:
> Remaining issues:
> - We have to mount securityfs explicitly, or use /etc/fstab.
>   It can cause a matter when we want to use this feature on
>   very early phase on boot. (like /sbin/init)

I'm not altogether clear how you intend this to work.

Are you saying that some future version of libcap will require that
securityfs be mounted before it (libcap) will work?

Thanks

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHd8dE+bHCR3gb8jsRAurnAJ4t5GKckQVj4Xr1JeTjoowUniU2jACgr5V8
3D3zbx5PkrVTRaYay6ZwJSc=
=D2ZM
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] [PATCH -mm] oom_kill: remove uid==0 checks

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> Andrew, I've cc:d you here bc in doing this patch I noticed that your
> 64-bit capabilities patch switched this code from an explicit check
> of cap_t(p->cap_effective) to using __capable().  That means that
> now being glossed over by the oom killer means PF_SUPERPRIV will
> be set.  Is that intentional?

Yes, I switched the check because the old one didn't work with the new
capability representation.

However, I had not thought this aspect of this replacement through. At
the time, it seemed obvious but in this case it actually depends on
whether you think using privilege (PF_SUPERPRIV) means "benefited from
privilege", or "successfully completed a privileged operation".

I suspect, in this case, the correct thing to do is add the equivalent of:

#define CAPABLE_PROBE_ONLY(a,b)   (!security_capable(a,b))

and use that in the code in question. That is, return to the old
behavior in a way that will not break if we ever need to add more bits.

Thanks for finding this.

Cheers

Andrew

> 
> Signed-off-by: Serge Hallyn <[EMAIL PROTECTED]>
> ---
>  mm/oom_kill.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 016127e..9fd8d5d 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -128,7 +128,7 @@ unsigned long badness(struct task_struct *p, unsigned 
> long uptime,
>* Superuser processes are usually more important, so we make it
>* less likely that we kill those.
>*/
> - if (__capable(p, CAP_SYS_ADMIN) || p->uid == 0 || p->euid == 0)
> + if (__capable(p, CAP_SYS_ADMIN) || __capable(p, CAP_SYS_RESOURCE))
>   points /= 4;
>  
>   /*

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHYGln+bHCR3gb8jsRAgNwAKDQED4YNy479LKfDL1fhVGWMK22eACgjPMh
JcFgzPsvIQkoatjvJ1vtHQ8=
=50l1
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] capabilities: introduce per-process capability bounding set (v10)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

I've pushed it to a pamcap-enhancements branch and I'll will try to
review it quickly.

Thanks

Andrew

KaiGai Kohei wrote:
> Sorry, any TABs are replaced by MUA.
> I'll send the patch again.
> 
>> The attached patch provides several improvement for pam_cap module.
>> 1. It enables pam_cap to drop capabilities from process'es capability
>>bounding set.
>> 2. It enables to specify allowing inheritable capability set or dropping
>>bounding capability set for groups, not only users.
>> 3. It provide pam_sm_session() method, not only pam_sm_authenticate()
>>and pam_sm_setcred(). A system administrator can select more
>>appropriate mode for his purpose.
>> 4. In the auth/cred mode, it enables to cache the configuration file,
>>to avoid read and analyze it twice.
>> (Therefore, most of the part in the original one got replaced)
>>
>> The default configuration file is "/etc/security/capability.conf".
>> You can describe as follows:
>> 
>> # kaigai get cap_net_raw and cap_kill, tak get cap_sys_pacct pI.
>> # We can omit "i:" in the head of each line.
>> i:cap_net_raw,cap_kill kaigai
>> cap_sys_pacct  tak
>>
>> # ymj and tak lost cap_sys_chroot from cap_bset
>> b:cap_sys_chroot   ymj  tak
>>
>> # Any user within webadm group get cap_net_bind_service pI.
>> i:cap_net_bind_service @webadm
>>
>> # Any user within users group lost cap_sys_module from cap_bset
>> b:cap_sys_module   @users
>> 
>>
>> When a user or groups he belongs is on several lines, all configurations
>> are simplly compounded.
>>
>> In the above example, if tak belongs to webadm and users group,
>> he will get cap_sys_pacct and cap_net_bind_service pI, and lost
>> cap_sys_chroot and cap_sys_module from his cap_bset.
>>
>> Thanks,
> 
> Signed-off-by: KaiGai Kohei <[EMAIL PROTECTED]>
> --
>  pam_cap/capability.conf |6 +
>  pam_cap/pam_cap.c   |  495 
> ---
>  2 files changed, 305 insertions(+), 196 deletions(-)
> 
> diff --git a/pam_cap/capability.conf b/pam_cap/capability.conf
> index b543142..707cdc3 100644
> --- a/pam_cap/capability.conf
> +++ b/pam_cap/capability.conf
> @@ -24,6 +24,12 @@ cap_setfcapmorgan
>  ## 'everyone else' gets no inheritable capabilities
>  none  *
> 
> +# user 'kaigai' lost CAP_NET_RAW capability from bounding set
> +b:cap_net_raw   kaigai
> +
> +# group 'acctadm' get CAP_SYS_PACCT inheritable capability
> +i:cap_sys_pacct @acctadm
> +
>  ## if there is no '*' entry, all users not explicitly mentioned will
>  ## get all available capabilities. This is a permissive default, and
>  ## probably not what you want...
> diff --git a/pam_cap/pam_cap.c b/pam_cap/pam_cap.c
> index 94c5ebc..a917d5c 100644
> --- a/pam_cap/pam_cap.c
> +++ b/pam_cap/pam_cap.c
> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) 1999,2007 Andrew G. Morgan <[EMAIL PROTECTED]>
> + * Copyright (c) 2007  KaiGai Kohei <[EMAIL PROTECTED]>
>   *
>   * The purpose of this module is to enforce inheritable capability sets
>   * for a specified user.
> @@ -13,298 +14,400 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> 
>  #include 
> +#include 
> 
>  #include 
>  #include 
> 
> +#define MODULE_NAME  "pam_cap"
>  #define USER_CAP_FILE   "/etc/security/capability.conf"
>  #define CAP_FILE_BUFFER_SIZE4096
>  #define CAP_FILE_DELIMITERS " \t\n"
> -#define CAP_COMBINED_FORMAT "%s all-i %s+i"
> -#define CAP_DROP_ALL"%s all-i"
> +
> +#ifndef PR_CAPBSET_DROP
> +#define PR_CAPBSET_DROP  24
> +#endif
> +
> +extern char const *_cap_names[];
> 
>  struct pam_cap_s {
>  int debug;
>  const char *user;
>  const char *conf_filename;
> +/* set in read_capabilities_for_user() */
> +cap_t result;
> +int do_set_inh : 1;
> +int do_set_bset : 1;
>  };
> 
> -/* obtain the inheritable capabilities for the current user */
> -
> -static char *read_capabilities_for_user(const char *user, const char *source)
> +/* obtain the inheritable/bounding capabilities for the current user */
> +static int read_capabilities_for_user(struct pam_cap_s *pcs)
>  {
> -char *cap_string = NULL;
> -char buffer[CAP_FILE_BUFFER_SIZE], *line;
> +char buffer[CAP_FILE_BUFFER_SIZE];
>  FILE *cap_file;
> +struct passwd *pwd;
> +int line_num = 0;
> +int rc = -1; /* PAM_(AUTH|CRED|SESSION)_ERR */
> +
> +pwd = getpwnam(pcs->user);
> +if (!pwd) {
> + syslog(LOG_ERR, "user %s not in passwd entries", pcs->user);
> + return PAM_AUTH_ERR;
> +}
> 
> -cap_file = fopen(source, "r");
> -if (cap_file == NULL) {
> - D(("failed to open capability file"));
> - return NULL;
> +cap_file = fopen(pcs->conf_filename, "r");
> +if (!cap_file) {
> + if (errno == ENOENT) {
> + syslog(LOG_NOTICE, "%s is not found",
> + 

Re: [PATCH] capabilities: introduce per-process capability bounding set (v10)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

KaiGai Kohei wrote:
> BTW, could you tell me your intention about pam_cap.c is implemented
> with pam_sm_authenticate() and pam_sm_setcred()?
> I think it can be done with pam_sm_open_session(), and this approach
> enables to reduce the iteration of reading /etc/security/capability.conf.
> 
> How do you think the idea?

Good question! If you want to add session support you can. I'd prefer it
if you retained support for the auth/cred API too: admin choice and all
that. To remove the second read of the file, you can use a PAM data item
to cache the desired capability info after the first read of the file.

I implemented it as a credential module (which has to get the
authentication return code right to make the credential stack execute
correctly) because I think of capabilities as credentials.

That being said, the credentials vs. session thing is not well
delineated by many applications, so it is arguably useful to provide
both interfaces for the admin to make use of on a per application basis.

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHV4r8mwytjiwfWMwRAlOsAJ9MQQN0cLhH2lhx9gwvwHsMhQ72ggCfcKWt
/krnNdiAisfcbcXDfssdbLE=
=+0r1
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: 2.6.24-rc3-mm2 - add-64-bit-capability-support-to-the-kernel

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> Quoting [EMAIL PROTECTED] ([EMAIL PROTECTED]):
>> Question:
>>
>> The patch does the semantic equivalent of:
>>
>> -#define cap_clear(c) do { cap_t(c) =  0; } while(0)
>> -#define cap_set_full(c)  do { cap_t(c) = ~0; } while(0)
>>
>> +# define cap_clear(c) do { (c) = __cap_empty_set; } while (0)
>> +# define cap_set_full(c)  do { (c) = __cap_full_set; } while (0)
>> +# define cap_set_init_eff(c)  do { (c) = __cap_init_eff_set; } while (0)
>>
>> Was it intentional, or an oversight, that this blows chunks in modules
>> that try to use cap_clear() or cap_set_full() because the __cap_*
>> symbols don't get an EXPORT_SYMBOL() attached to them?

Definitely an oversight. Thanks Serge for the fix!

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHV1SP+bHCR3gb8jsRAtikAJ9FEm6ETGa9qrr82kLI4ZtsACZ43gCgqIcY
GbuHBsxpaZtQDEgU/3OCgA0=
=A9+O
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] capabilities: introduce per-process capability bounding set (v10)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

KaiGai Kohei wrote:
> Andrew Morgan wrote:
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> KaiGai Kohei wrote:
>>>> +if (!!cap_issubset(*inheritable,
>>>> +   cap_combine(target->cap_inheritable,
>>>> +   current->cap_bset))) {
>>>> +/* no new pI capabilities outside bounding set */
>>>> +return -EPERM;
>>>> +}
>>>>  

>> Yes, the !! was a bug. The correct check is a single !.
> 
> I was in trouble with getting -EPERM at pam_cap.so :-)
> 
>> (Thus, the correct check says no 'new' pI bits can be outside cap_bset.)
> 
> If this condition intends to dominate 'new' pI bits by 'old' pI bits masked
> with bounding set, we should not apply cap_combine() here.
> I think applying cap_intersect() is correct for the purpose.

The check is not meant to limit existing pI bits.

The check is meant to limit what new bits can be 'added' to pI (in the
case that pE & CAP_SETPCAP is true).

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHVsQ2mwytjiwfWMwRAs9RAKCUyjsjONVhRXooG5I2b+1zU/HLGwCfQIyh
tjdDI9QxJ1DWLCm2Ee29qYA=
=Gwwt
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] capabilities: introduce per-process capability bounding set (v10)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

KaiGai Kohei wrote:
> Serge,
> 
> Please tell me the meanings of the following condition.
> 
>> diff --git a/security/commoncap.c b/security/commoncap.c
>> index 3a95990..cb71bb0 100644
>> --- a/security/commoncap.c
>> +++ b/security/commoncap.c
>> @@ -133,6 +119,12 @@ int cap_capset_check (struct task_struct *target,
>> kernel_cap_t *effective,
>>  /* incapable of using this inheritable set */
>>  return -EPERM;
>>  }
>> +if (!!cap_issubset(*inheritable,
>> +   cap_combine(target->cap_inheritable,
>> +   current->cap_bset))) {
>> +/* no new pI capabilities outside bounding set */
>> +return -EPERM;
>> +}
>>  
>>  /* verify restrictions on target's new Permitted set */
>>  if (!cap_issubset (*permitted,
> 
> It seems to me this condition requires the new inheritable capability
> set must have a capability more than bounding set, at least.
> What is the purpose of this checking?

Yes, the !! was a bug. The correct check is a single !.

(Thus, the correct check says no 'new' pI bits can be outside cap_bset.)

Cheers

Andrew

> 
> In the initial state, any process have no inheritable capability set
> and full bounding set. Thus, we cannot do capset() always.
> 
> Thanks,

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHVPBS+bHCR3gb8jsRAnxQAJ0Vna82bl9M11OL/uuEe21nF5+9TACfSzGi
aY0SUvMmLZCIF0KovBTpihE=
=wT9N
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] capabilities: introduce per-process capability bounding set (v10)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

KaiGai Kohei wrote:
>> There is already a pam_cap module in the libcap2 package. Can we merge
>> this functionality?
> 
> I think it is a good idea.
> 
> However, this module already have a feature to modify inheritable
> capability set.
> How does it to be described in the "/etc/security/capability.conf"?
> 
> One idea is like a following convention:
> 
> # compatible configuration. We can omit "i:" at the head of line
> cap_setfcap tak
> # It drops any capabilities from b-set except for cap_net_raw and
> cap_fowner
> b:cap_net_raw,cap_fownerymj
> # It drops only cap_dac_override from b-set.
> b:-cap_dac_override kaigai
> # It drops only cap_sys_admin from b-set of any user within users group.
> b:-cap_sys_admingroup:users

I like the idea of a separate line for bounds.

For ease of parsing, perhaps '!' or some other symbol prefix to the line
could be used to identify lines that refer to cap_bound?

In other modules, @groupname is used to capture a group association.

Lines like this should be supported:

!cap_net_raw @regularusers# suppress from cap_bset
cap_net_raw  @pingers morgan  # add to pI

where morgan is not in group @pingers but is in group @regularusers.

Cheers

Andrew

> 
> Thanks,
> 
>> Cheers
>>
>> Andrew
>>
>> [EMAIL PROTECTED] wrote:
>>> Quoting KaiGai Kohei ([EMAIL PROTECTED]):
 Serge E. Hallyn wrote:
> The capability bounding set is a set beyond which capabilities
> cannot grow.  Currently cap_bset is per-system.  It can be
> manipulated through sysctl, but only init can add capabilities.
> Root can remove capabilities.  By default it includes all caps
> except CAP_SETPCAP.
 Serge,

 This feature makes me being interested in.
 I think you intend to apply this feature for the primary process
 of security container.
 However, it is also worthwhile to apply when a session is starting up.

 The following PAM module enables to drop capability bounding bit
 specified by the fifth field in /etc/passwd entry.
 This code is just an example now, but considerable feature.

 build and install:
 # gcc -Wall -c pam_cap_drop.c
 # gcc -Wall -shared -Xlinker -x -o pam_cap_drop.so pam_cap_drop.o -lpam
 # cp pam_cap_drop.so /lib/security

 modify /etc/passwd as follows:

 tak:x:1004:100:cap_drop=cap_net_raw,cap_chown:/home/tak:/bin/bash
^^
 example:
 [EMAIL PROTECTED] ~]$ ping 192.168.1.1
 PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
 64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=1.23 ms
 64 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=1.02 ms

 --- 192.168.1.1 ping statistics ---
 2 packets transmitted, 2 received, 0% packet loss, time 999ms
 rtt min/avg/max/mdev = 1.023/1.130/1.237/0.107 ms

 [EMAIL PROTECTED] ~]$ ssh [EMAIL PROTECTED]
 [EMAIL PROTECTED]'s password:
 Last login: Sat Dec  1 10:09:29 2007 from masu.myhome.cx
 [EMAIL PROTECTED] ~]$ export LANG=C
 [EMAIL PROTECTED] ~]$ ping 192.168.1.1
 ping: icmp open socket: Operation not permitted

 [EMAIL PROTECTED] ~]$ su
 Password:
 pam_cap_bset[6921]: user root does not have 'cap_drop=' property
 [EMAIL PROTECTED] tak]# cat /proc/self/status | grep ^Cap
 CapInh: 
 CapPrm: dffe
 CapEff: dffe
 [EMAIL PROTECTED] tak]#
>>> Neat.  A bigger-stick version of not adding the account to
>>> group wheel.  I'll use that.
>>>
>>> Is there any reason not to have a separate /etc/login.capbounds
>>> config file, though, so the account can still have a full name?
>>> Did you only use that for convenience of proof of concept, or
>>> is there another reason?
>>>
 # BTW, I replaced the James's address in the Cc: list,
 # because MTA does not accept it.
>>> Thanks!  I don't know what happened to my alias for him...
>>>
>>> thanks,
>>> -serge
>>>
 -- 
 KaiGai Kohei <[EMAIL PROTECTED]>

 
 pam_cap_drop.c
 

 /*
  * pam_cap_drop.c module -- drop capabilities bounding set
  *
  * Copyright: 2007 KaiGai Kohei <[EMAIL PROTECTED]>
  */

 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 

 #include 

 #ifndef PR_CAPBSET_DROP
 #define PR_CAPBSET_DROP 24
 #endif

 static char *captable[] = {
 "cap_chown",
 "cap_dac_override",
 "cap_dac_read_search",
 "cap_fowner",
 "cap_fsetid",
 "cap_kill",
 "cap_setgid",
 "cap_setuid",
 "cap_setpcap",
 "cap_linux_immutable",
 "cap_net_bind_service",
 "cap_net_broadcast",

Re: [PATCH] capabilities: introduce per-process capability bounding set (v10)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

There is already a pam_cap module in the libcap2 package. Can we merge
this functionality?

Cheers

Andrew

[EMAIL PROTECTED] wrote:
> Quoting KaiGai Kohei ([EMAIL PROTECTED]):
>> Serge E. Hallyn wrote:
>>> The capability bounding set is a set beyond which capabilities
>>> cannot grow.  Currently cap_bset is per-system.  It can be
>>> manipulated through sysctl, but only init can add capabilities.
>>> Root can remove capabilities.  By default it includes all caps
>>> except CAP_SETPCAP.
>> Serge,
>>
>> This feature makes me being interested in.
>> I think you intend to apply this feature for the primary process
>> of security container.
>> However, it is also worthwhile to apply when a session is starting up.
>>
>> The following PAM module enables to drop capability bounding bit
>> specified by the fifth field in /etc/passwd entry.
>> This code is just an example now, but considerable feature.
>>
>> build and install:
>> # gcc -Wall -c pam_cap_drop.c
>> # gcc -Wall -shared -Xlinker -x -o pam_cap_drop.so pam_cap_drop.o -lpam
>> # cp pam_cap_drop.so /lib/security
>>
>> modify /etc/passwd as follows:
>>
>> tak:x:1004:100:cap_drop=cap_net_raw,cap_chown:/home/tak:/bin/bash
>>^^
>> example:
>> [EMAIL PROTECTED] ~]$ ping 192.168.1.1
>> PING 192.168.1.1 (192.168.1.1) 56(84) bytes of data.
>> 64 bytes from 192.168.1.1: icmp_seq=1 ttl=64 time=1.23 ms
>> 64 bytes from 192.168.1.1: icmp_seq=2 ttl=64 time=1.02 ms
>>
>> --- 192.168.1.1 ping statistics ---
>> 2 packets transmitted, 2 received, 0% packet loss, time 999ms
>> rtt min/avg/max/mdev = 1.023/1.130/1.237/0.107 ms
>>
>> [EMAIL PROTECTED] ~]$ ssh [EMAIL PROTECTED]
>> [EMAIL PROTECTED]'s password:
>> Last login: Sat Dec  1 10:09:29 2007 from masu.myhome.cx
>> [EMAIL PROTECTED] ~]$ export LANG=C
>> [EMAIL PROTECTED] ~]$ ping 192.168.1.1
>> ping: icmp open socket: Operation not permitted
>>
>> [EMAIL PROTECTED] ~]$ su
>> Password:
>> pam_cap_bset[6921]: user root does not have 'cap_drop=' property
>> [EMAIL PROTECTED] tak]# cat /proc/self/status | grep ^Cap
>> CapInh: 
>> CapPrm: dffe
>> CapEff: dffe
>> [EMAIL PROTECTED] tak]#
> 
> Neat.  A bigger-stick version of not adding the account to
> group wheel.  I'll use that.
> 
> Is there any reason not to have a separate /etc/login.capbounds
> config file, though, so the account can still have a full name?
> Did you only use that for convenience of proof of concept, or
> is there another reason?
> 
>> # BTW, I replaced the James's address in the Cc: list,
>> # because MTA does not accept it.
> 
> Thanks!  I don't know what happened to my alias for him...
> 
> thanks,
> -serge
> 
>> -- 
>> KaiGai Kohei <[EMAIL PROTECTED]>
>>
>> 
>> pam_cap_drop.c
>> 
>>
>> /*
>>  * pam_cap_drop.c module -- drop capabilities bounding set
>>  *
>>  * Copyright: 2007 KaiGai Kohei <[EMAIL PROTECTED]>
>>  */
>>
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>>
>> #include 
>>
>> #ifndef PR_CAPBSET_DROP
>> #define PR_CAPBSET_DROP 24
>> #endif
>>
>> static char *captable[] = {
>>  "cap_chown",
>>  "cap_dac_override",
>>  "cap_dac_read_search",
>>  "cap_fowner",
>>  "cap_fsetid",
>>  "cap_kill",
>>  "cap_setgid",
>>  "cap_setuid",
>>  "cap_setpcap",
>>  "cap_linux_immutable",
>>  "cap_net_bind_service",
>>  "cap_net_broadcast",
>>  "cap_net_admin",
>>  "cap_net_raw",
>>  "cap_ipc_lock",
>>  "cap_ipc_owner",
>>  "cap_sys_module",
>>  "cap_sys_rawio",
>>  "cap_sys_chroot",
>>  "cap_sys_ptrace",
>>  "cap_sys_pacct",
>>  "cap_sys_admin",
>>  "cap_sys_boot",
>>  "cap_sys_nice",
>>  "cap_sys_resource",
>>  "cap_sys_time",
>>  "cap_sys_tty_config",
>>  "cap_mknod",
>>  "cap_lease",
>>  "cap_audit_write",
>>  "cap_audit_control",
>>  "cap_setfcap",
>>  NULL,
>> };
>>
>>
>> PAM_EXTERN int
>> pam_sm_open_session(pam_handle_t *pamh, int flags,
>> int argc, const char **argv)
>> {
>>  struct passwd *pwd;
>>  char *pos, *buf;
>>  char *username = NULL;
>>
>>  /* open system logger */
>>  openlog("pam_cap_bset", LOG_PERROR | LOG_PID, LOG_AUTHPRIV);
>>
>>  /* get the unix username */
>>  if (pam_get_item(pamh, PAM_USER, (void *) &username) != PAM_SUCCESS || 
>> !username)
>>  return PAM_USER_UNKNOWN;
>>
>>  /* get the passwd entry */
>>  pwd = getpwnam(username);
>>  if (!pwd)
>>  return PAM_USER_UNKNOWN;
>>
>>  /* Is there "cap_drop=" ? */
>>  pos = strstr(pwd->pw_gecos, "cap_drop=");
>>  if (pos) {
>>  buf = strdup(pos + sizeof("cap_drop=") - 1);
>>  if (!buf)
>>  return PAM_SESSION_ERR

Re: [PATCH] file capabilities: don't prevent signaling setuid root programs.

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge,

I still feel a bit uneasy about this. Looking ahead, with filesystem
capabilities, one can simulate this same situation with a setuid
'non-root' program as follows:

[EMAIL PROTECTED] ~]$ cat > test.c
main()
{
printf("sleeping (%u)\n", getpid());
sleep(100);
printf("woke up\n");
}
[EMAIL PROTECTED] ~]$ cc -o test test.c
[EMAIL PROTECTED] ~]$ chmod u+s ./test
[EMAIL PROTECTED] ~]$ ls -ltr test
- -rwsrwxr-x  1 morgan morgan 7090 Nov 26 20:01 test
[EMAIL PROTECTED] ~]$ setcap cap_net_raw+ep ~/test
[EMAIL PROTECTED] ~]$ getcap ~/test
/home/morgan/test = cap_net_raw+ep
[EMAIL PROTECTED] ~]$ su luser
Password:
[EMAIL PROTECTED] morgan]$ ./test
sleeping (5935)


[EMAIL PROTECTED] morgan]$ kill 5935
bash: kill: (5935) - Operation not permitted

Because of the euid=0 test, the piece of code you are adding will behave
differently in this situation. Is the root-behavior deserving of less
protection than this one? To my eye they seem equivalent.

Is there a compelling reason to include the euid==0 check?

Thanks

Andrew

Serge E. Hallyn wrote:
> This patch is needed to preserve legacy behavior when
> CONFIG_SECURITY_FILE_CAPABILITIES=y.  Without this patch, xinit can't
> kill X, so manually starting X in runlevel 3 then exiting your window
> manager will not cause X to exit. 
> 
> thanks,
> -serge
> 
>>From 81a6d780ad570f9a326fc27912ec0e373f5fa14f Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Tue, 20 Nov 2007 08:47:35 +
> Subject: [PATCH] file capabilities: don't prevent signaling setuid root 
> programs.
> 
> An unprivileged process must be able to kill a setuid root
> program started by the same user.  This is legacy behavior
> needed for instance for xinit to kill X when the window manager
> exits.
> 
> When an unprivileged user runs a setuid root program in !SECURE_NOROOT
> mode, fP, fI, and fE are set full on, so pP' and pE' are full on.
> Then cap_task_kill() prevents the user from signaling the setuid root
> task.  This is a change in behavior compared to when
> !CONFIG_SECURITY_FILE_CAPABILITIES.
> 
> This patch introduces a special check into cap_task_kill() just
> to check whether a non-root user is signaling a setuid root
> program started by the same user.  If so, then signal is allowed.
> 
> Changelog:
>   Nov 26: move test up above CAP_KILL test as per Andrew
>   Morgan's suggestion.
> 
> Signed-off-by: Serge E. Hallyn <[EMAIL PROTECTED]>
> ---
>  security/commoncap.c |9 +
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 302e8d0..5bc1895 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -526,6 +526,15 @@ int cap_task_kill(struct task_struct *p, struct siginfo 
> *info,
>   if (info != SEND_SIG_NOINFO && (is_si_special(info) || 
> SI_FROMKERNEL(info)))
>   return 0;
>  
> + /*
> +  * Running a setuid root program raises your capabilities.
> +  * Killing your own setuid root processes was previously
> +  * allowed.
> +  * We must preserve legacy signal behavior in this case.
> +  */
> + if (p->euid == 0 && p->uid == current->uid)
> + return 0;
> +
>   /* sigcont is permitted within same session */
>   if (sig == SIGCONT && (task_session_nr(current) == task_session_nr(p)))
>   return 0;
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHS5m/QheEq9QabfIRAmouAJkBBB0kXH57s9mvlgdG3XZhC0pZMwCfZUW3
L4vJUkR4tgAh33GTqEquIqw=
=sKCy
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] capabilities: introduce per-process capability bounding set (v10)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

This looks good to me.

[As you anticipated, there is a potential merge issue with Casey's
recent addition of MAC capabilities - which will make CAP_MAC_ADMIN the
highest allocated capability: ie.,

#define CAP_LAST_CAP CAP_MAC_ADMIN

].

Signed-off-by: Andrew G. Morgan <[EMAIL PROTECTED]>

Cheers

Andrew

Serge E. Hallyn wrote:
>>From 22da6ccb1a24d1b6fa481d990a26197c6bfdfa77 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Mon, 19 Nov 2007 13:54:05 -0500
> Subject: [PATCH 1/1] capabilities: introduce per-process capability bounding 
> set (v10)
> 
> The capability bounding set is a set beyond which capabilities
> cannot grow.  Currently cap_bset is per-system.  It can be
> manipulated through sysctl, but only init can add capabilities.
> Root can remove capabilities.  By default it includes all caps
> except CAP_SETPCAP.
> 
> This patch makes the bounding set per-process when file
> capabilities are enabled.  It is inherited at fork from parent.
> Noone can add elements, CAP_SETPCAP is required to remove them.
> 
> One example use of this is to start a safer container.  For
> instance, until device namespaces or per-container device
> whitelists are introduced, it is best to take CAP_MKNOD away
> from a container.
> 
> The bounding set will not affect pP and pE immediately.  It will
> only affect pP' and pE' after subsequent exec()s.  It also does
> not affect pI, and exec() does not constrain pI'.  So to really
> start a shell with no way of regain CAP_MKNOD, you would do
> 
>   prctl(PR_CAPBSET_DROP, CAP_MKNOD);
>   cap_t cap = cap_get_proc();
>   cap_value_t caparray[1];
>   caparray[0] = CAP_MKNOD;
>   cap_set_flag(cap, CAP_INHERITABLE, 1, caparray, CAP_DROP);
>   cap_set_proc(cap);
>   cap_free(cap);
> 
> The following test program will get and set the bounding
> set (but not pI).  For instance
> 
>   ./bset get
>   (lists capabilities in bset)
>   ./bset drop cap_net_raw
>   (starts shell with new bset)
>   (use capset, setuid binary, or binary with
>   file capabilities to try to increase caps)
> 
> 
> cap_bound.c
> 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> 
>  #ifndef PR_CAPBSET_READ
>  #define PR_CAPBSET_READ 23
>  #endif
> 
>  #ifndef PR_CAPBSET_DROP
>  #define PR_CAPBSET_DROP 24
>  #endif
> 
> int usage(char *me)
> {
>   printf("Usage: %s get\n", me);
>   printf("   %s drop \n", me);
>   return 1;
> }
> 
>  #define numcaps 32
> char *captable[numcaps] = {
>   "cap_chown",
>   "cap_dac_override",
>   "cap_dac_read_search",
>   "cap_fowner",
>   "cap_fsetid",
>   "cap_kill",
>   "cap_setgid",
>   "cap_setuid",
>   "cap_setpcap",
>   "cap_linux_immutable",
>   "cap_net_bind_service",
>   "cap_net_broadcast",
>   "cap_net_admin",
>   "cap_net_raw",
>   "cap_ipc_lock",
>   "cap_ipc_owner",
>   "cap_sys_module",
>   "cap_sys_rawio",
>   "cap_sys_chroot",
>   "cap_sys_ptrace",
>   "cap_sys_pacct",
>   "cap_sys_admin",
>   "cap_sys_boot",
>   "cap_sys_nice",
>   "cap_sys_resource",
>   "cap_sys_time",
>   "cap_sys_tty_config",
>   "cap_mknod",
>   "cap_lease",
>   "cap_audit_write",
>   "cap_audit_control",
>   "cap_setfcap"
> };
> 
> int getbcap(void)
> {
>   int comma=0;
>   unsigned long i;
>   int ret;
> 
>   printf("i know of %d capabilities\n", numcaps);
>   printf("capability bounding set:");
>   for (i=0; i   ret = prctl(PR_CAPBSET_READ, i);
>   if (ret < 0)
>   perror("prctl");
>   else if (ret==1)
>   printf("%s%s", (comma++) ? ", " : " ", captable[i]);
>   }
>   printf("\n");
>   return 0;
> }
> 
> int capdrop(char *str)
> {
>   unsigned long i;
> 
>   int found=0;
>   for (i=0; i   if (strcmp(captable[i], str) == 0) {
>   found=1;
>   break;
>   }
>   }
>   if (!found)
>   return 1;
>   if (prctl(PR_CAPBSET_DROP, i)) {
>   perror("prctl");
>   return 1;
>   }
>   return 0;
> }
> 
> int main(int argc, char *argv[])
> {
>   if (argc<2)
>   return usage(argv[0]);
>   if (strcmp(argv[1], "get")==0)
>   return getbcap();
>   if (strcmp(argv[1], "drop")!=0 || argc<3)
>   return usage(argv[0]);
>   if (capdrop(argv[2])) {
>   printf("unknown capability\n");
>   return 1;
>   }
>   return execl("/bin/bash", "/bin/bash", NULL);
> }
> 
> 
> Signe

Re: [PATCH] -mm (2.4.26-rc3-mm1) v2 Smack using capabilities 32 and 33

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Signed-off-by: Andrew G. Morgan <[EMAIL PROTECTED]>

Cheers

Andrew

Casey Schaufler wrote:
> From: Casey Schaufler <[EMAIL PROTECTED]>
> 
> This patch takes advantage of the increase in capability bits
> to allocate capabilities for Mandatory Access Control. Whereas
> Smack was overloading a previously allocated capability it is
> now using a pair, one for overriding access control checks and
> the other for changes to the MAC configuration.
> 
> The two capabilities allocated should be obvious in their intent.
> The comments in capability.h are intended to make it clear that
> there is no intention that implementations of MAC LSM modules
> be any more constrained by the presence of these capabilities
> than an implementation of DAC LSM modules are by the analogous
> DAC capabilities.
> 
> 
> Signed-off-by: Casey Schaufler <[EMAIL PROTECTED]>
> 
> ---
> 
> The companion patch for libcap-2.02 is provided as an attachment.
> The attachment is not a kernel patch, although it would be easy to
> mistake it for one.
> 
> Introduces CAP_FS_MASK_B1 and uses it as appropriate. I think that
> I found all the places it needs to be used, but don't hesitate to
> let me know if I missed something.
> 
> Thank you.
> 
>  include/linux/capability.h |   24 ++--
>  security/smack/smack.h |8 
>  security/smack/smack_lsm.c |8 
>  security/smack/smackfs.c   |   12 ++--
>  4 files changed, 32 insertions(+), 20 deletions(-)
> 
> diff -uprN -X linux-2.6.24-rc3-mm1-base/Documentation/dontdiff 
> linux-2.6.24-rc3-mm1-base/include/linux/capability.h 
> linux-2.6.24-rc3-mm1-smack/include/linux/capability.h
> --- linux-2.6.24-rc3-mm1-base/include/linux/capability.h  2007-11-22 
> 01:51:36.0 -0800
> +++ linux-2.6.24-rc3-mm1-smack/include/linux/capability.h 2007-11-25 
> 21:38:34.0 -0800
> @@ -314,6 +314,23 @@ typedef struct kernel_cap_struct {
>  
>  #define CAP_SETFCAP   31
>  
> +/* Override MAC access.
> +   The base kernel enforces no MAC policy.
> +   An LSM may enforce a MAC policy, and if it does and it chooses
> +   to implement capability based overrides of that policy, this is
> +   the capability it should use to do so. */
> +
> +#define CAP_MAC_OVERRIDE 32
> +
> +/* Allow MAC configuration or state changes.
> +   The base kernel requires no MAC configuration.
> +   An LSM may enforce a MAC policy, and if it does and it chooses
> +   to implement capability based checks on modifications to that
> +   policy or the data required to maintain it, this is the
> +   capability it should use to do so. */
> +
> +#define CAP_MAC_ADMIN33
> +
>  /*
>   * Bit location of each capability (used by user-space library and kernel)
>   */
> @@ -336,6 +353,8 @@ typedef struct kernel_cap_struct {
>   | CAP_TO_MASK(CAP_FOWNER)   \
>   | CAP_TO_MASK(CAP_FSETID))
>  
> +# define CAP_FS_MASK_B1 (CAP_TO_MASK(CAP_MAC_OVERRIDE))
> +
>  #if _LINUX_CAPABILITY_U32S != 2
>  # error Fix up hand-coded capability macro initializers
>  #else /* HAND-CODED capability initializers */
> @@ -343,8 +362,9 @@ typedef struct kernel_cap_struct {
>  # define CAP_EMPTY_SET{{ 0, 0 }}
>  # define CAP_FULL_SET {{ ~0, ~0 }}
>  # define CAP_INIT_EFF_SET {{ ~CAP_TO_MASK(CAP_SETPCAP), ~0 }}
> -# define CAP_FS_SET   {{ CAP_FS_MASK_B0, 0 }}
> -# define CAP_NFSD_SET {{ CAP_FS_MASK_B0|CAP_TO_MASK(CAP_SYS_RESOURCE), 0 
> }}
> +# define CAP_FS_SET   {{ CAP_FS_MASK_B0, CAP_FS_MASK_B1 } }
> +# define CAP_NFSD_SET {{ CAP_FS_MASK_B0|CAP_TO_MASK(CAP_SYS_RESOURCE), \
> +  CAP_FS_MASK_B1 } }
>  
>  #endif /* _LINUX_CAPABILITY_U32S != 2 */
>  
> diff -uprN -X linux-2.6.24-rc3-mm1-base/Documentation/dontdiff 
> linux-2.6.24-rc3-mm1-base/security/smack/smackfs.c 
> linux-2.6.24-rc3-mm1-smack/security/smack/smackfs.c
> --- linux-2.6.24-rc3-mm1-base/security/smack/smackfs.c2007-11-22 
> 01:51:43.0 -0800
> +++ linux-2.6.24-rc3-mm1-smack/security/smack/smackfs.c   2007-11-24 
> 11:29:29.0 -0800
> @@ -241,7 +241,7 @@ static ssize_t smk_write_load(struct fil
>* No partial writes.
>* Enough data must be present.
>*/
> - if (!capable(CAP_MAC_OVERRIDE))
> + if (!capable(CAP_MAC_ADMIN))
>   return -EPERM;
>   if (*ppos != 0)
>   return -EINVAL;
> @@ -474,7 +474,7 @@ static ssize_t smk_write_cipso(struct fi
>* No partial writes.
>* Enough data must be present.
>*/
> - if (!capable(CAP_MAC_OVERRIDE))
> + if (!capable(CAP_MAC_ADMIN))
>   return -EPERM;
>   if (*ppos != 0)
>   return -EINVAL;
> @@ -601,7 +601,7 @@ static ssize_t smk_write_doi(struct file
>   char temp[80];
>   int i;
>  
> - if (!capable(CAP_MAC_OVERRIDE))
> + if (!capable(CAP_MAC_ADMIN))
>   return -EPERM;
>  
>  

Re: [PATCH] -mm (2.6.24-rc3-mm1) Smack using capabilities 32 and 33

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1



Casey Schaufler wrote:
> diff -uprN -X linux-2.6.24-rc3-mm1-base/Documentation/dontdiff 
> linux-2.6.24-rc3-mm1-base/include/linux/capability.h 
> linux-2.6.24-rc3-mm1-smack/include/linux/capability.h
> --- linux-2.6.24-rc3-mm1-base/include/linux/capability.h  2007-11-22 
> 01:51:36.0 -0800
> +++ linux-2.6.24-rc3-mm1-smack/include/linux/capability.h 2007-11-24 
> 11:26:51.0 -0800
> @@ -314,6 +314,23 @@ typedef struct kernel_cap_struct {
>  
>  #define CAP_SETFCAP   31
>  
> +/* Override MAC access.
> +   The base kernel enforces no MAC policy.
> +   An LSM may enforce a MAC policy, and if it does and it chooses
> +   to implement capability based overrides of that policy, this is
> +   the capability it should use to do so. */
> +
> +#define CAP_MAC_OVERRIDE 32
> +
> +/* Allow MAC configuration or state changes.
> +   The base kernel requires no MAC configuration.
> +   An LSM may enforce a MAC policy, and if it does and it chooses
> +   to implement capability based checks on modifications to that
> +   policy or the data required to maintain it, this is the
> +   capability it should use to do so. */
> +
> +#define CAP_MAC_ADMIN33
> +
>  /*
>   * Bit location of each capability (used by user-space library and kernel)
>   */
> @@ -334,7 +351,8 @@ typedef struct kernel_cap_struct {
>   | CAP_TO_MASK(CAP_DAC_OVERRIDE) \
>   | CAP_TO_MASK(CAP_DAC_READ_SEARCH)  \
>   | CAP_TO_MASK(CAP_FOWNER)   \
> - | CAP_TO_MASK(CAP_FSETID))
> + | CAP_TO_MASK(CAP_FSETID) \

The following looks a bit fishy:
> + | CAP_TO_MASK(CAP_MAC_OVERRIDE))

  (1<<32) & 0x == 0

I think you need to define CAP_FS_MASK_B1.

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHSaPvmwytjiwfWMwRAjC8AJ9V4pntMzw4ayK1lb5mgUdjKYlCPQCeLkkz
jas90SJYDo58pQ8Skw1IK60=
=5PUQ
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: + smack-version-11c-simplified-mandatory-access-control-kernel.patch added to -mm tree

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

I believe it was you who once eloquently observed that, at its heart,
the POSIX (sic) capabilities model was all about providing a mechanism
for overriding the prevailing security policy (be it MAC or DAC or
whatever) in a defined way.

Casey Schaufler wrote:
> Now I know that there are lots of people who don't share my
> views on granularity, but I have lots of experiance with this
> and the cases where it actually makes sense to break the MAC
> capabilities up are rare.
> 
> That's my going in position, at any rate. I'm always open to
> finding out why I'm wrong.

Its not so much why you are wrong, as being clear that we're not using a
generic name and inadvertently limiting ourselves to a SMACK-like model...

It feels to me as if a MAC "override capability" is, if true to its
name, extra to the MAC model; any MAC model that needs an 'override' to
function seems under-specified... SELinux clearly feels no need for one,
and browsing through your SMACK patch, there are many instances where
this capability is used as an convenience privileged override. However,
in other situations, it appears as if the capability is required for
basic SMACK operations to succeed.

My sense is that there is a case to be made for: CAP_MAC_ADMIN and
CAP_MAC_OVERRIDE here. The former being for cases where SMACK (or
whatever MAC supports it) requires privilege to perform a privileged MAC
operation, and the latter for saying "OK, I'm without a paddle but need
one" (or words to that effect).

Cheers

Andrew

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHR8AA+bHCR3gb8jsRAqY/AJsGI56TDQyBD42LCovpJTYHkaL0pQCdHM5S
kk5v2O4ohY2O0z93JNdKVBY=
=dbQn
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: + smack-version-11c-simplified-mandatory-access-control-kernel.patch added to -mm tree

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Casey Schaufler wrote:
> In the end we can call it CAP_LATE_FOR_DINNER if that's the only way
> I can move forward. CAP_MAC_OVERRIDE is the obvious partner to
> CAP_DAC_OVERRIDE, so that's still my preference. CAP_SMACK_OVERRIDE
> unnecessarily ties it to one LSM, and in spite of what some people
> still seem to think, I see more LSMs in the pipeline.

I'd personally not like to see SMACK appear in a capability name. No
offense Casey, but SMACK may be displaced with YAMAC (*) someday, and
I'd hate to have wasted a capability on it. Using CAP_MAC_OVERRIDE makes
sense to me - even if its not (yet/ever) honored by all MAC LSMs.

I do have a question about whether one capability is sufficient in
general for MAC. Looking at the:

  http://wt.xpilot.org/publications/posix.1e/download.html

last draft, there are no less than 5 capabilities (p173) allocated for
MAC. Presumably there was a good reason for 5 and not 1 back then -
could you summarize what is different now?

Thanks

Andrew

(*) yet-another example of yet-another

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHR5mc+bHCR3gb8jsRAlB9AJsHPi1+fFp1ONKJCMFDpLS1lYG4AwCfYxMX
8aaU+sOBNHU01uldtrJ8cEI=
=/USy
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Posix file capabilities in 2.6.24rc2; now 2.6.24-rc3

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> The problem is that when you run a setuid binary, its pP and pE are
> fully raised.  The following patch fixes it for me.  Chris, does it fix
> your problem?  Andrew, am I again confusing myself and doing something
> unsafe?

I think this is yet another example of the fragile mess that is UID
emulation with capabilities. Your patch is an example of privilege
escalation - luser can kill a more-capable process. In the kill CONT
case we reached the opposite conclusion to this one. As was the case
then, I didn't disagree then :*). If it meets folk's expectations, then
this is probably a good patch...

> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -543,6 +543,9 @@ int cap_task_kill(struct task_struct *p, struct siginfo 
> *info,
>   if (capable(CAP_KILL))
>   return 0;
>  
> + if (p->euid==0 && p->uid==current->uid)
> + return 0;
> +

Its late and I'm obviously tired, but is there any reason not to simply use:

 if (p->uid == current->uid)
 return 0;

Whatever the case, could you put the new code closer to the sig ==
SIGCONT test? The capability tests are at the end of cap_task_kill() and
this new check breaks that pattern.

Cheers

Andrew

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHRTLlQheEq9QabfIRAt/hAKCJgj2kbuyAWI486LOwwDLdkbcpoQCfQdrQ
J+bcvi+9pGTodFn42PsHJHA=
=cXaG
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: 2.6.24-rc2-mm1 -- strange apparent network failures

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Kevin Winchester wrote:
> However, I got around the problem by making the code change manually -
> and my network connection is now working.  Looking at the code being
> bypassed:
> 
> if (pE.cap[i] || pP.cap[i] || pP.cap[i])
> 
> looks somewhat weird as it is testing the same condition twice.  Should
> it have been:
> 
> if (pE.cap[i] || pP.cap[i] || pI.cap[i])

Yes, that was also a bug. However, upon reflection (and as per my "0 &&"
hack), I now believe these few lines of code are problematic in general.

Thanks for reporting this bug. I'll post a more clear patch (that isn't
GPG'd).

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHP5vy+bHCR3gb8jsRAliTAKCvCsfZuNN7Og57S0s8O4SZNveSUwCgq4VP
vHUE/S+x09l5I24E2/rmLj4=
=JaWT
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: 2.6.24-rc2-mm1 -- strange apparent network failures

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Kevin,

Can you try this quick hack?

diff --git a/kernel/capability.c b/kernel/capability.c
index e57d1aa..4088610 100644
- --- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -109,7 +109,7 @@ out:
kdata[i].permitted = pP.cap[i];
kdata[i].inheritable = pI.cap[i];
}
- -   while (i < _LINUX_CAPABILITY_U32S) {
+   while (0 && (i < _LINUX_CAPABILITY_U32S)) {
if (pE.cap[i] || pP.cap[i] || pP.cap[i]) {
/* Cannot represent w/ legacy structure */
return -ERANGE;

Thanks

Andrew

Kevin Winchester wrote:
> On November 17, 2007 01:16:58 am Andrew Morgan wrote:
>> Hi,
>>
>> This warning is just saying that you might want to reconsider
>> recompiling your dhclient with a newer libcap - which has native support
>> for 64-bit capabilities. This is supposed to be informative, and not be
>> associated with any particular error.
>>
>> From your comments, you believe that this patch causes something in your
>> boot process to fail. Can you supply some detail about the version of
>> dhclient you are using? I'd like to understand exactly what it is doing
>> (via libcap).
>>
>> Thanks
>>
> 
> The boot succeeds (and appears to bring initialize the network adapter 
> properly - it autonegotiates a 100Mbps link speed), but the dhcp client is 
> never able to get an address.  However, applying the rc2-mm1 patch series up 
> to just before:
> 
>   add-64-bit-capability-support-to-the-kernel.patch
> 
> results in a working kernel.  Applying just this patch causes the failure.  
> To 
> be sure, I also tried applying the above patch plus the following ones:
> 
>   add-64-bit-capability-support-to-the-kernel-checkpatch-fixes.patch
>   add-64-bit-capability-support-to-the-kernel-fix.patch
>   add-64-bit-capability-support-to-the-kernel-fix-fix.patch
>   remove-unnecessary-include-from-include-linux-capabilityh.patch
> 
> but the problem still occurs even with all of these.
> 
> As to versions, I'm running Kubuntu gutsy, so I have the default:
> 
> dhcp3-client   3.0.5-3ubuntu4
> libcap11:1.10-14build1
> 
> packages installed.
> 
> Let me know if you need any other information, or if you have a patch you 
> would like tested.
> 
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHP37LQheEq9QabfIRAst5AJ9Nsw0RtF2NDuUAMvQZh5OFWEB4ugCeIxMH
lp5/Ka7SJZLIrQpZDijrd1E=
=GN18
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: 2.6.24-rc2-mm1 -- strange apparent network failures

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hi,

This warning is just saying that you might want to reconsider
recompiling your dhclient with a newer libcap - which has native support
for 64-bit capabilities. This is supposed to be informative, and not be
associated with any particular error.

- From your comments, you believe that this patch causes something in your
boot process to fail. Can you supply some detail about the version of
dhclient you are using? I'd like to understand exactly what it is doing
(via libcap).

Thanks

Andrew

Andrew Morton wrote:
> On Thu, 15 Nov 2007 20:28:29 -0400
> Kevin Winchester <[EMAIL PROTECTED]> wrote:
> 
>> On November 15, 2007 06:02:09 am Andy Whitcroft wrote:
>>> When testing some of the later 2.6.24-rc2-mm1+hotfix combinations on three
>>> of our test systems one job from each batch (1/4) failed.  In each case the
>>> machine appears to have booted normally all the way to a login: prompt.
>>> However in the failed boots the networking though apparently initialised
>>> completely and correctly (as far as I can tell from the console output), is
>>> reported as not responding to ssh connections.  The network interface seems
>>> to have been initialised on the right port, and the ssh daemons started.
>>>
>>> Two of the machines are powerpc boxes, the other an older x86_64.
>>> One machine is 4/4 in testing, just one.  Most of the other machines are
>>> still not able to compile this stack so do not contribute to our knowledge.
>>>
>>> Any ideas?
>>>
>> I see this as well - the computer boots fine but no network.  The only clues 
>> in the dmesg are:
>>
>> [  294.097876] warning: process `dhclient' gets w/ old libcap
>> [  294.097893] warning: process `dhclient' sets w/ old libcap
>>
>> So I'll try backing up the patch series to before:
>>
>> add-64-bit-capability-support-to-the-kernel.patch
> 
> Yes, that's a good one to suspect.
> 
> What a peculiar error message.
> 
>> or so, and see if that's the problem.  If anyone has any other ideas, let me 
>> know.
> 
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHPnlIQheEq9QabfIRAlglAKCG2NG1xnwMT8G/Lk8GoEPwtBzq9QCdFLYi
k+pt5Sd2AdtOJ+TjMIt1y6g=
=5wpX
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Linux Security *Module* Framework (Was: LSM conversion to static interface)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Peter Dolding wrote:
> On 11/1/07, Casey Schaufler <[EMAIL PROTECTED]> wrote:
>> --- Peter Dolding <[EMAIL PROTECTED]> wrote:
>> Posix capabilities predate SELinux. SELinux is not interested in
>> Posix capabilities.
>>
>>> But no IBM had to do it.
>> Err, no. It was done by Andrew Morgan back in the dark ages.
>> Why on earth do you think IBM did it?
> 
> Posix file capabilities the option to replace SUID bit with something
> more security safe only handing out segments of root power instead of
> the complete box and dice like SUID.  Even different on a user by user
> base.
> 
> Posix capabilites is what Posix file capabilities is based on.  Yes I
> know the words appear close.  The word file is important.  Please read
> Website.  http://www.ibm.com/developerworks/linux/library/l-posixcap.html

For the record, I think you are both right. I took a stab at it back
when Casey and I first met:

ftp://ftp.kernel.org/pub/linux/libs/security/linux-privs/old/kernel-2.4-fcap/README

all that stuff worked fine it was just a bit ahead of its time...

- From memory, at that point in time "extended attributes" were an
external patch, and having some trouble getting merged. My sense was
that EA was a pre-requisite and I was happy to wait for that support to
become integrated before pushing my file capability support.

In the midst of all this LSM emerged as a reaction to Linus' clear
unhappiness about all extensions security. I didn't have the time to
participate in the LSM, and my work sat in the form of these patches.

SELinux at that time existed as a separate infrastructure, and evidently
did have the time to embrace LSM.

> IBM coders worked and got it into the main line really recently to
> provide at least some way to avoid fault of SUID of course it could

[...]

So, yes, IBM (Serge) deserve full credit for starting over, and getting
it merged...

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHLr6EQheEq9QabfIRAsOrAJ9XzTL0Lqm5jaxwO6UoPB9Pwh3SzQCfVWFd
cPyjsGp/s6D6HuBE6M4NJH0=
=G/ah
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] file capabilities: allow sigcont within session (v2)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> Quoting Stephen Smalley ([EMAIL PROTECTED]):
>> On Wed, 2007-10-31 at 18:49 -0500, Serge E. Hallyn wrote:
[..]
>>> Also don't do file-capabilities signaling checks when uids for
>>> the processes don't match, since the standard check_kill_permission
>>> will have done those checks.
>> Description doesn't match the code.
> 
> Egads.  I knew I should've just kept that part out of it for the first
> patch...
> 
> New patch on top of previous one is appended.

Dang! I stared at the code a long time to see what you were doing...

And concluded that you had coded what you intended; allow processes that
share UIDs to kill one another - independent of capabilities. The fact
that this is the reverse of the words you used to introduce your patch,
I didn't notice.

I totally missed the fact that this was (unwanted) new functionality!!
Mea culpa for the bad review.

I certainly Sign off the revised patch.

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHLOicmwytjiwfWMwRAq5HAJ49eajMT4myf1oKfrab2oCw/o9HnwCgkYt2
RyIsmHVWmClsrxCz5s1HRJY=
=hGLO
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] file capabilities: allow sigcont within session (v2)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

[kernel/signal.c:check_kill_permission() could probably benefit from
getting more consistently indented!]

I'm not sure I can grok your comment. Did you mean:

  /* as per, check_kill_permission(), permit if tasks have same uid */

As to content:

Signed-off-by: Andrew G. Morgan <[EMAIL PROTECTED]>

Cheers

Andrew

Serge E. Hallyn wrote:
>>From 5bff8967f45a35f858b96ca673d9bf98eac53d49 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Wed, 31 Oct 2007 11:22:04 -0500
> Subject: [PATCH 1/1] file capabilities: allow sigcont within session (v2)
> 
> (This is a proposed fix to http://bugzilla.kernel.org/show_bug.cgi?id=9247)
> 
> Allow sigcont to be sent to a process with greater capabilities
> if it is in the same session.  Otherwise, a shell from which
> I've started a root shell and done 'suspend' can't be restarted
> by the parent shell.
> 
> Also don't do file-capabilities signaling checks when uids for
> the processes don't match, since the standard check_kill_permission
> will have done those checks.
> 
> Signed-off-by: Serge E. Hallyn <[EMAIL PROTECTED]>
> ---
>  security/commoncap.c |9 +
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/security/commoncap.c b/security/commoncap.c
> index bf67871..4de6857 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -526,6 +526,15 @@ int cap_task_kill(struct task_struct *p, struct siginfo 
> *info,
>   if (info != SEND_SIG_NOINFO && (is_si_special(info) || 
> SI_FROMKERNEL(info)))
>   return 0;
>  
> + /* if tasks have same uid, then check_kill_permission did check */
> + if (current->uid == p->uid || current->euid == p->uid ||
> + current->uid == p->suid || current->euid == p->suid)
> + return 0;
> +
> + /* sigcont is permitted within same session */
> + if (sig == SIGCONT && (task_session_nr(current)==task_session_nr(p)))
> + return 0;
> +
>   if (secid)
>   /*
>* Signal sent as a particular user.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHKVpRQheEq9QabfIRAnp9AKCZHb526eioQWKycH7V7LfcHP7VvQCdG0AJ
QTVOLvQ2hip+j2qZ1mb2Y6w=
=45et
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] file capabilities: allow sigcont within session (v2)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Ackd.

Cheers

Andrew

Serge E. Hallyn wrote:
>>From 5bff8967f45a35f858b96ca673d9bf98eac53d49 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Wed, 31 Oct 2007 11:22:04 -0500
> Subject: [PATCH 1/1] file capabilities: allow sigcont within session (v2)
> 
> (This is a proposed fix to http://bugzilla.kernel.org/show_bug.cgi?id=9247)
> 
> Allow sigcont to be sent to a process with greater capabilities
> if it is in the same session.  Otherwise, a shell from which
> I've started a root shell and done 'suspend' can't be restarted
> by the parent shell.
> 
> Also don't do file-capabilities signaling checks when uids for
> the processes don't match, since the standard check_kill_permission
> will have done those checks.
> 
> Signed-off-by: Serge E. Hallyn <[EMAIL PROTECTED]>
> ---
>  security/commoncap.c |9 +
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/security/commoncap.c b/security/commoncap.c
> index bf67871..4de6857 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -526,6 +526,15 @@ int cap_task_kill(struct task_struct *p, struct siginfo 
> *info,
>   if (info != SEND_SIG_NOINFO && (is_si_special(info) || 
> SI_FROMKERNEL(info)))
>   return 0;
>  
> + /* if tasks have same uid, then check_kill_permission did check */
> + if (current->uid == p->uid || current->euid == p->uid ||
> + current->uid == p->suid || current->euid == p->suid)
> + return 0;
> +
> + /* sigcont is permitted within same session */
> + if (sig == SIGCONT && (task_session_nr(current)==task_session_nr(p)))
> + return 0;
> +
>   if (secid)
>   /*
>* Signal sent as a particular user.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHKSuSmwytjiwfWMwRAkHXAJ4lr07yW936ychUGNxqQSOanZTecwCePYVc
d8uP0I3AEDjTpG8s7Nojo7Y=
=8777
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] [PATCH 2/2] capabilities: implement 64-bit capabilities

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> Quoting Chris Wright ([EMAIL PROTECTED]):
>> * Serge E. Hallyn ([EMAIL PROTECTED]) wrote:
>>> I guess now that I've written this out, it seems pretty clear
>>> that capget64() and capget64() are the way to go.  Any objections?
>> How is capget64() different from capget() that supports 2 different
>> header->versions (I thought that was the whole point of the versioned,
>> rather opaque interface)?  I don't object to a new syscall, but I don't
>> see why it's required to avoid breaking libcap.
> 
> Hmm, I guess it *works*, it's just harder to explain the "inconsistent"
> behavior.  Now instead of saying "capget() will fail under certain
> conditions while capget64() will always succeed", capget() will actually
> fail under certain conditions only if you send in a certain header.
> 
> Again, once I've written it out, I guess it isn't *so* bad.

[I'm just wading back into a mass of neglected email. Long story.]

Chris is right, this is precisely why the interface is versioned, and
there is at least one version of libcap that was written to support this
versioning scheme

cvs -z3
- -d:pserver:[EMAIL PROTECTED]:/cvsroot/linux-privs
co -r libcap-pre2 libcap

I'll try and unwind all the threads of email I've been neglecting and
have something useful to say over the next few days.

Cheers

Andrew

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHF3tj+bHCR3gb8jsRAhF1AJ9gfmUnO+O0YyzPLaqGVv++pZjvdgCgzz3J
+yF6CRASj8QVYArDydc84k8=
=K/Wb
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3] CRED: Move the effective capabilities into the cred struct

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

David Howells wrote:
> Move the effective capabilities mask from the task struct into the credentials
> record.
> 
> Note that the effective capabilities mask in the cred struct shadows that in
> the task_struct because a thread can have its capabilities masks changed by
> another thread.  The shadowing is performed by update_current_cred() which is
> invoked on entry to any system call that might need it.

OOC If we were to simply drop support for one process changing the
capabilities of another, would we need this patch?

Thanks

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFG8fLrQheEq9QabfIRApPOAKCHAoazhTTpY/qSjdmRZxDptqeqiACfd4Q7
mdIPx+xpG19ih9uiVv1NSBU=
=TfZd
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [2.6 patch] remove securebits

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> To summarize more clearly, I think that so long as we support
> process trees with a sort of !SECURE_NOROOT support, that
> support should include the ability to use prctl(KEEP_CAPS) the
> way one uses it now.

> When a process tree is in strict capability mode,
> prctl(PR_{G,S}ET_KEEP_CAPS) should return -EINVAL.

I agree. I'll try to code it up in a way that its clear how to delete
this functionality when folk realize they no longer need it...

- -static inline int get_file_caps(struct linux_binprm *bprm)
+int cap_bprm_set_security(struct linux_binprm *bprm)
 {
bprm_clear_caps(bprm);
+   bprm_force_uid0_caps(bprm);
+   current->keep_capabilities = 0;

> This is being moved from bprm_apply to bprm_set, which moves it
> earlier.  If exec fails later on, keep_capabilities might be set
> to 0 even though exec failed.

I'll look at it again, but I had thought I had preserved the previous
behavior with this condensed version of the code. Are you suggesting an
improvement to what was there, or pointing out I'm inadvertently
breaking the old behavior?

Thanks

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFG1hSu+bHCR3gb8jsRAhHJAJ9Pn8w2InrhbNjBjpqT9NEE0HX61QCgkBR8
Bo1xJcZGqbsr+IhQ+DDyENA=
=PKx4
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [2.6 patch] remove securebits

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Attached is what I consider only an RFC patch.

I've not really thought through (to my satisfaction) the re-purposing of
current->keep_capabilities in the non-filesystem-supporting-capability
configuration, but this is basically the code I'm thinking about. (I'm
typing this email from a system running this patch over 2.6.23-rc3-mm1
so its not 'obviously' broken.)

Adrian Bunk wrote:
>> The user would be userspace...
>>
>> Unless by 'the user' you actually mean the patch itself which will allow
>> the setting of secure_noroot per-process.  I don't know for sure, but
>> suspect Andrew might like to wait until file capabilities make it into
>> and stabilize in Linus' tree before going on with that.
> 
> That's what I am talking about.
> 
> This patch should be submitted and discussed together with the changes 
> Andrew has for securebits.

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFG08y0QheEq9QabfIRAnUhAKCEHyUko292kULNTkRqQOGki2NohgCdGXvV
bc+bHzBbI6sPimdf4UTAzGY=
=vB0u
-----END PGP SIGNATURE-
>From a8366ab7a12ed4283e24096e891ac13c1d471756 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <[EMAIL PROTECTED]>
Date: Mon, 27 Aug 2007 23:09:20 -0700
Subject: [PATCH] Remove global securebits from the kernel.

In the absence of filesystem capability support, there is no
longer any internal support for disabling root. The plain fact is
that while there was internal support for it, the userspace API for
enabling it was not implemented, and having a global personality
of this sort was almost impossible to use.

With filesystem capability support, a new prctl(PR_[GS]ET_CAPONLY)
per-process flag is available. Once set (to 1) a process and all
of its children will irrevocably lose root-is-all-capable privilege.

NOTE: just because root within a given process tree is not capable,
it does not mean root is impotent. Most Linux systems are riddled
with critical system files that are owned by root (uid=0), so much
care should be used relying on the security provided by this support.
It is likely that the best way to use this feature is within some
sort of restricted (chroot etc.) environment. Be especially careful
of where you mount /proc/
---
 include/linux/capability.h |   10 +++-
 include/linux/prctl.h  |4 +
 include/linux/sched.h  |1 -
 include/linux/securebits.h |   30 --
 include/linux/security.h   |   17 --
 kernel/sys.c   |   14 +
 security/capability.c  |1 +
 security/commoncap.c   |  132 +---
 security/dummy.c   |5 +-
 security/security.c|5 +-
 security/selinux/hooks.c   |6 +-
 11 files changed, 124 insertions(+), 101 deletions(-)
 delete mode 100644 include/linux/securebits.h

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 7a8d7ad..0fcef09 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -147,8 +147,14 @@ typedef __u32 kernel_cap_t;
  ** Linux-specific capabilities
  **/
 
-/* Transfer any capability in your permitted set to any pid,
-   remove any capability in your permitted set from any pid */
+/* With filesystem support for capabilities configured:
+ *   Permit the current process to raise any capability in its inheritable set.
+ *   Permit the current process to lock the process into capability-only mode
+ * - prctl(PR_SET_CAPONLY, 1, ...)
+ * Otherwise:
+ *   Transfer any capability in your permitted set to any pid,
+ *   remove any capability in your permitted set from any pid
+ */
 
 #define CAP_SETPCAP  8
 
diff --git a/include/linux/prctl.h b/include/linux/prctl.h
index e2eff90..e3f49a7 100644
--- a/include/linux/prctl.h
+++ b/include/linux/prctl.h
@@ -63,4 +63,8 @@
 #define PR_GET_SECCOMP	21
 #define PR_SET_SECCOMP	22
 
+/* Get/Set process personality to secure-by-capabilities alone */
+#define PR_GET_CAPONLY  23
+#define PR_SET_CAPONLY  24
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2b3c936..be2e9c4 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -67,7 +67,6 @@ struct sched_param {
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/include/linux/securebits.h b/include/linux/securebits.h
deleted file mode 100644
index 5b06178..000
--- a/include/linux/securebits.h
+++ /dev/null
@@ -1,30 +0,0 @@
-#ifndef _LINUX_SECUREBITS_H
-#define _LINUX_SECUREBITS_H 1
-
-#define SECUREBITS_DEFAULT 0x
-
-extern unsigned securebits;
-
-/* When set UID 0 has no special privileges. When unset, we support
-   inheritance of root-permissions and suid-root executable under
-   compatibility mode. We raise the effective and inheritable bitmasks
-   *of the executable file* if the effective uid of the new process is
- 

Re: [2.6 patch] remove securebits

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

FWIW, in the mm kernel, I've actually already removed them when one
configures without capabilities.

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc3/2.6.23-rc3-mm1/broken-out/v3-file-capabilities-alter-behavior-of-cap_setpcap.patch

Other than writing a custom module, so far as I can tell, there is/was
no way to set them anyway.

I'd obviously prefer to wait for the mm-merge process to complete and
minimize the churn in this area, but I basically agree that the bits as
implemented are pretty useless in their current form. In a per-process
mode (with filesystem capability support) they are much more useful...

Cheers

Andrew

Serge E. Hallyn wrote:
> Quoting Adrian Bunk ([EMAIL PROTECTED]):
>> It seems that since it was added in kernel 2.2.0 (sic) securebits 
>> was never used.
>>
>> This patch therefore removes it.
> 
> Actually IIUC Andrew Morgan had plans of making securebits per-process,
> which would make them far more usable.
> 
> Now maybe he'd just as soon start with a clean slate...  Andrew?
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGz6bwQheEq9QabfIRAjx5AKCZSFZ0dv4HTUtUYtm6OdVlOUi3ewCdGHzE
TnoeF19cOljfiyntgkcSCbE=
=lW84
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] file capabilities: clear fcaps on inode change (v2)

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
>>From 1376764cbb54243f088cf00c39000c4f4418f461 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Mon, 6 Aug 2007 14:20:06 -0400
> Subject: [PATCH 1/1] file capabilities: clear fcaps on inode change (v2)

Signed-off-by: Andrew Morgan <[EMAIL PROTECTED]>

Cheers

Andrew


-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGuBZDQheEq9QabfIRAtoVAJ9uOG2q9Za0CJqmH0ueLgUHmRIABACdHW3c
SykZm/puZe5JPpiVPAF2rS8=
=Smnk
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] file capabilities: clear caps cleanup

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Fine with me.

Cheers

Andrew

Serge E. Hallyn wrote:
>>From 88115394044e697ac852f7fb9f30483e87f4f598 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Wed, 11 Jul 2007 10:01:01 -0400
> Subject: [PATCH 1/1] file capabilities: clear caps cleanup
> 
> As suggested by Steve Beattie, rather than jump into a
> conditional block in certain cases, define and use a
> static inline bprm_clear_caps().

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGlQF6+bHCR3gb8jsRArmZAJ9Je0Da0SDtg78tx5ks7gvY/E5x/wCfYikC
bkD1uf7L3cdKzBMEvdjqB/c=
=/z3n
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> 1. Exactly Andrew describes.  Once userspace switches to a new cap
> format, an older kernel simply won't support them

Mmm. Let me see. I think I prefer this one! :-)

> 2. As Andrew describes, but also encode the version number into the
> capability name, i.e. security.capability.v3.  Now userspace can
> optionally tack on more than one capability version to be backward
> compatible.

If you have a significant legacy of use of earlier versions, I guess
this makes sense. However, given the experimental nature of this support
(it will be a while before the user space support for this is
secure/robust), I'm not all that concerned about legacy support.

> 3. Somewhat different than Andrew describes.  We mandate that any
> capability version N+1 consist of
> 
> struct vfs_cap_data {
>   __u32 magic;
>   capability_version_1;
>   capability_version_2;
>   ...
>   capability_version_N;
>   capability_version_N+1;
> };

Ugh. I don't like this. It presumes that the kernel will get more and
more complicated over time. Please don't do this one.

> Or, for brevity,
> 
> struct vfs_cap_data {
>   __u32 first_magic;
>   __u32 last_magic;
>   capability_version_first;
>   ...
>   capability_version_last;
> };
> 
> 4. Stick to the current plan, where switching to 64-bit caps will be
> done as
> 
> struct vfs_cap_data_disk {
>   __le32 version;
>   __le32 data[]; /* eff[0], perm[0], inh[0], eff[1], ... */
> };

While asserting that it is more flexible etc., no one has yet actually
given an example of where fE being richer than a simple binary helps
anything. Until I see an example, I'm going to hold the position that
this is needless "complexity".

Cheers

Andrew

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGjBFXmwytjiwfWMwRAofJAKCXX2GkN39o45fCQmxpNpZIEVH8EgCeLaDy
AoWZNj/1MqT7oayabxUhIn8=
=OSBu
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Casey Schaufler wrote:
>> Would there be a difference between that and setting either fI or fP
>> (depending on your intent) to those caps, and setting fE=1 in Andrew's
>> scheme?
> 
> Arg, you're making me think. The POSIX group went through this,
> let me see if I can reconstruct the logic.
> 
> The main issue is one if there being a possible case where you
> have a capability ignorant program that you want to exec with
> a different fP and fE. On first glance it seems that since the
> program is capability ignorant it can't matter. But what if your
> capability ignorant program exec's a capability aware program
> to perform a helper function? You may well want the first program
> to have a capability that it does not use in fP (but not fE)
> to pass along to the helper program. True, you could probably

I'm not sure I've quite flogged this horse to death yet.. :-)

In my other reply, I quoted the rules. Here they are again:

pI' = pI
pP' = (X & fP) | (pI & fI)
pE' = pP' & fE

If program A exec()utes helper program B, then the only capabilities
(p*') that B can get from A are a subset of A's pI set.

If A doesn't know about capabilities, then nothing about the fE value
associated with the A program file can alter A's pI set and thus affect
B. That is, nothing about the fE or fP value used to exec()ute A gets
propagated through a subsequent exec() to B.

So far as I can see, to achieve the helper program support you are
describing, the value of pI that program A (and thus program B) inherits
will have to contain the relevant capabilities, and B will have to have
a sufficient fI value to pick them up...

Incidentally, this is also where my request that we require (pP' >= fP)
be true comes in. If a helper program (which may also be a legacy
program) is used in a way that it is configured (via fP) to have powers
that are denied to it (via X=cap_bset etc.,) then it should simply not
be permitted to run (-EPERM). It should not have the opportunity to
silently confuse itself (as was the case with sendmail when we tried to
emulate setuid-0 behavior with capabilities a few years back).

> come up with a way to set the capabilities on the helper program
> to account for this use, but there may be design and security
> constraints that make doing so complicated. 

I've not seen anything yet to make be believe there is a case for a
non-single bit fE value... Its a little ironic that I read all of the
rationale I've been espousing in POSIX drafts - so far as I'm aware the
only detail I'm mixing in there is the (pP' >= fP), -EPERM, thing.

If you or anyone can cite some counter examples, please do!

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGhJjxQheEq9QabfIRAmyLAKCUxirmAuS4VM0U+9HloeOF6cKt2gCgi/fh
ElhM1CISM4a+e0umBjK9GV0=
=Vrqj
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] file capabilities: get_file_caps cleanups

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
kfree(dcaps);
> Move this two lines down (rc defaults to 0 in goto above):
> from here-->
 +clear_caps:
 +  if (rc) {
> to here-->
> 
>> Hmm?  But if we succeeded we still want to free dcaps if we
>> kmalloc()'d it.

I wasn't clear enough... Let me try again with different words.

If you look at your patch, you will see that the only use of the label
'clear_caps:' is as a jump target from a location in which rc=0. As
such, you will *not* clear the bprm->cap_* sets... This is the reverse
of what you intended to do.

You need to put the jump target 'inside' the 'if (rc) { <-here ... }'.

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGg+F6+bHCR3gb8jsRAm2AAJ9v0fz/S/+HvGWOhU6UBPbB4iY+MACggZqA
+yCrxg2UVKdDKOh6kPfh0QI=
=Qksf
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Casey Schaufler wrote:
>> The only reason for having an fE bitmap is to allow a capability-aware
>> program (you really trust to do its privileged operations carefully) to
>> be lazy and get some of its capabilities raised for free. Perhaps you
>> can clarify why this is a desirable thing? :-)
> 
> No, it's to allow you to grant a subset of the available capabilities
> to a program that is not aware of capabilities. You can give "date"
> the capability to reset the clock without giving it the capability
> to remove other people's files without changing the code or running
> it setuid.

This is precisely what fE = 0 or ~0 provides.

Recall, an exec()'d program gets its p*' capabilities from a convolution
of its exec()er's inheritable set (pI) *and* the file's capabilities
(fI,fP,fE):

pI' = pI
pP' = (X & fP) | (pI & fI)
pE' = pP' & fE

[Linux essentially has cap_bset for X.]

The fine-grain ability for ping to do its thing without becoming
powerful enough to load a kernel module, for example, is facilitated by
the "pP' &" part of the derivation of pE' (and not simply the unfiltered
value of fE!).

As I said before, either the program knows how to raise and lower bits
in its pE set, or it doesn't. In the former case, fE=0. In the latter
case, fE=~0 will ensure that it gets all of the capabilities it is
permitted to exercise at time of execution.

Could you cite some examples of where this position is unreasonable?

Thanks

Andrew

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGg9i1+bHCR3gb8jsRAvViAJ9T5x1fHrLGF4niRq7VhRqg4sej3wCgxkom
oAFQEQwLkd/D6J5gi7Fb3Ww=
=+ZLb
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
>> Does that explain it?
> 
> Yes, thanks, but then it still could come in handy to have fE be a full
> bitset, so the application gets some eff caps automatically, while
> others it has to manually set...

[We touched on this a number of emails back.]

If an application is capability aware, it can manipulate its own
capabilities and should have fE=0.

If an application is not capability aware, it needs to have *all* of its
capabilities enabled at exec() time. Otherwise, it won't work.

The only reason for having an fE bitmap is to allow a capability-aware
program (you really trust to do its privileged operations carefully) to
be lazy and get some of its capabilities raised for free. Perhaps you
can clarify why this is a desirable thing? :-)

Cheers

Andrew
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGg1LqQheEq9QabfIRAo3BAKCO8QrfcKBNqhfnn2BHp8O/qDkgXgCgleEl
xP7LZPU9Qn6AjqI3ZM3FZ+4=
=urmz
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] file capabilities: get_file_caps cleanups

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

This contains a typo:

Serge E. Hallyn wrote:
>>From 588755d9498c87c4e963527ba0f49c11107de354 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Wed, 27 Jun 2007 19:55:27 -0400
> Subject: [PATCH 1/1] file capabilities: get_file_caps cleanups
> 
> Two cleanups relating to set_file caps.
[...]
> @@ -187,12 +186,21 @@ out:
>   dput(dentry);
>   if ((void *)dcaps != (void *)&v1caps)
>   kfree(dcaps);
Move this two lines down (rc defaults to 0 in goto above):
from here-->
> +clear_caps:
> + if (rc) {
to here-->
> + cap_clear (bprm->cap_inheritable);
> + cap_clear (bprm->cap_permitted);
> + cap_clear (bprm->cap_effective);
> + }
>   return rc;

Cheers

Andrew

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGg1DNQheEq9QabfIRApNqAJ99rNMF4VU/FPOolDLHlXbbQ2MRMwCeO3Fy
8ze7XhlBuBKdHElbZmWFJ5Y=
=gAEK
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
> 
>> I don't particularly mind, but can you point out any case where
>> it is an advantage to have the one bit for f'E rather than just
>> drop f'E altogether?  Instead of having
> 
>>  f'I=something
>>  f'P=something
>>  f'E=off
> 
>> we can always just remove the security.capability xattr.  Right?

No. Bear in mind that capabilities are all about trusting specific
applications with privilege as opposed to trusting the superuser to not
run dangerous applications.

There are three situations, we'll take them in turn:

  - no capabilities (fP=fI=fE=0): this is for applications that are not
intended to operate with privilege. Because of the way the capability
convolution rules work, such a program can't execute with privilege. Period.

  - with capabilities (fP and/or fI != 0), but fE=0 (off): this is for
applications that are intended to operate with privilege, but they were
designed to know about capabilities and they manipulate (raise and
lower) capabilities as needed to do the things they do.

  - with capabilities, but fE=1 (on): this is a class of applications
loosely called 'legacy'. They can use privilege to operate, but don't
strictly need to know about it. For example, /bin/chown . Such a program
will have fP=0,fI=CAP_CHOWN. Since the administrator sets
fP=0,fI=CAP_CHOWN,fE=1 on the /bin/chown file, any process with
CAP_CHOWN in its inheritable set (pI) can "exec /bin/chown" and have it
do the thing historically reserved for the superuser...

In some future world, the legacy fE bit may become unnecessary because
every application will be rewritten to be careful about exercising
privilege explicitly. In the meantime, the fE bit can be used to drop
the setuid-0 bits from things like ping and traceroute.

>> If there's a case where that does not suffice, then I have no objection
>> to doing it this way.

Does that explain it?

> 2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
> whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
> way too overloaded and this functionality is special.
> 
>> The functionality is special, but someone with CAP_SYS_ADMIN can always
>> unload the capability module and create the security.capability xattr
>> using the dummy module.

This argument leads down a rat hole. (As appears to have happened with
the non-modularization LSM thread elsewhere...)

The simple fact is that CAP_SYS_ADMIN is equivalent to every other
capability in the system if it can be used to load any flavor of kernel
module. Arguing that you don't need a capability for something because
you can do it with CAP_SYS_ADMIN is very close to admitting that you
prefer the superuser (uid=0) model.

>> If we do add this cap, do we want to make it apply to all security.*
>> xattrs?

I recommend limiting it to just capabilities.

For now, you can leave the other security attributes with the
CAP_SYS_ADMIN ("misc") capability. In the original 'POSIX' drafts, there
is a separate notion of advisory and mandatory access control;
leveraging different capabilities to override.

> 3) The cap_from_disk() interface checking needs some work Most
> notably, size must be greater than sizeof(u32) or the very first line
> will do something nasty... I'd recommend you use code like this:
> 
> [...] cap_from_disk(...)
> {
>if (size != sizeof(struct vfs_cap_data)) {
[...]
> mistake at least once... The future is uncertain, so don't trust it will
> look the way you want it to. ;-)
> 
>> Ok, so you're saying that when we do switch to 64-bit caps or some other
>> evolution, we switch to completely separate logic based on the
>> VFS_CAP_REVISION?

Yes.

>> That seems sane to me.

You might also want to verify that unallocated bits hold the value
'zero'. Its funny what people do when they realize they can silently
store bits in obscure places like this. That really messes up allocating
bits in the future. Add a check that is something like:

  if (version & ~(VFS_CAP_REVISION_MASK|VFS_CAP_FLAGS_EFFECTIVE)) {
  return -EINVAL;
  }

> 7) This one is subtle, and to my mind not well appreciated. In
> cap_bprm_apply_creds(), the wart of the global 'cap_bset' masking
> permitted bits can lead to problems like the one we saw a few years back
> with sendmail and capabilities. There is an assumption in setting
> permitted (they are called 'forced' in some documents) capabilities on a
> file that the file will execute with at least these. The inheritable
> ones are optional.
> 
>> Hmm, changing the behavior of the cap_bset is something that seems to
>> belong in 8), though I see what you're saying, it does affect the
>> behavior of vfs caps.

I'm not really changing the behavior of cap_bset. I'm specifying the
behavior of fP. ;-)

[Your comments on cap_bset and CAP_SETPCAP are exactly where my
ax^H^H^Hscalpel will fall after all this VFS support is stable. But
*that* is a subject for a different thread... aka item 8.]

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge,

[time passes]

I'm a little better up to speed on all the kernel now. I don't feel that
I conceptually object so much to this patch-series any more :-)

I do, however, think the patch needs some work:

1) As previously discussed, fE should be an all or nothing single bit:

How about?:

#define VFS_CAP_REVISION_MASK 0xFF00
#define VFS_CAP_REVISION  0x0100

#define VFS_CAP_FLAGS_MASK~VFS_CAP_REVISION_MASK
#define VFS_CAP_FLAGS_EFFECTIVE 0x01

struct vfs_cap_data {
__u32  magic_etc;
struct {
__u32 permitted; /* Little endian */
__u32 inheritable;   /* Little endian */
} data[1];
};

2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
way too overloaded and this functionality is special.

3) The cap_from_disk() interface checking needs some work Most
notably, size must be greater than sizeof(u32) or the very first line
will do something nasty... I'd recommend you use code like this:

[...] cap_from_disk(...)
{
   if (size != sizeof(struct vfs_cap_data)) {
printk(KERN_WARNING "%s: invalid cap size %d for file %s\n",
 __FUNCTION__, size, bprm->filename);
return -EINVAL;
   }

   switch ((version & VFS_CAP_REVISION_MASK)) {
   case VFS_CAP_REVISION:
bprm->cap_effective = (version & VFS_CAP_FLAGS_EFFECTIVE)
? CAP_FULL_SET : CAP_EMPTY_SET;
bprm->cap_permitted =
to_cap_t( le32_to_cpu(dcap->data[0].permitted) );
bprm->cap_inheritable =
to_cap_t( le32_to_cpu(dcap->data[0].inheritable) );
return 0;
   default:
return -EINVAL;
   }
}

Basically, I don't believe in designing a secure interface to be forward
compatible - things never work out that way and the legacy you are
implicitly committing to will haunt you in the future... FWIW I've known
a few x86 MSR designers over the years and each one has made this
mistake at least once... The future is uncertain, so don't trust it will
look the way you want it to. ;-)

5) I would rename 'set_file_caps' to 'get_file_caps' since this is what
the function actually does. If you must use 'set' then call the function
'set_bprm_caps'.

6) I also don't see the value of explicitly zero'ing the capabilities
(in cap_bprm_set_security()) only to override them elsewhere.

I'd move the 'cap_clear (bprm->cap_...)' code from
cap_bprm_set_security() into the 'out:' code at the end of
'get_file_caps()' (sic). Put rc=0 at the top of the function, and
replace the return 0; at the top of that function with a 'goto
clear_out;' then replace the out: code as follows:

   out:
   dput(dentry);
   if ((void *)dcaps != (void *)&v1caps)
kfree(dcaps);
   if (rc) {
   clear_out:
cap_clear (bprm->cap_inheritable);
cap_clear (bprm->cap_permitted);
cap_clear (bprm->cap_effective);
   }
   return rc;

7) This one is subtle, and to my mind not well appreciated. In
cap_bprm_apply_creds(), the wart of the global 'cap_bset' masking
permitted bits can lead to problems like the one we saw a few years back
with sendmail and capabilities. There is an assumption in setting
permitted (they are called 'forced' in some documents) capabilities on a
file that the file will execute with at least these. The inheritable
ones are optional.

The long and the short of it is there needs to be a check somewhere that:

   current->cap_permitted is a superset of file->cap_permitted

That is, what cap_bset takes away, current->cap_inheritable gives back.
If the above is not true, then the executable should fail to execute;
- -EPERM. On the surface I don't see how to do this with the LSM framework
because the relevant function is a 'void' one and can't return an error.

8) There are a number of (massive) cleanups that I would like to see
done, but they are more related to the non-file capabilities support in
the kernel and I won't pollute this present discussion any more with those.

I hope that was helpful. FWIW I did set up a git repostitory on
kernel.org to port my old patches, but in the process of porting them
better understood what you had done. If you do the above I think I'd be
happy to work from that...

Cheers

Andrew

PS. If anyone is touching file with my transmeta email in them, feel
free to replace them with the @kernel.org address.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGfNYwQheEq9QabfIRAgQ2AJ9q3+BgOPlZvTboqEyM3O845xKZOQCcCLQm
zKVfemAw2F5h43rApDXuJ4o=
=OJWn
-END PGP SIGNATURE-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/