Re: [PATCH] capabilities: implement per-process securebits

[Serge E. Hallyn]

Quoting Andrew G. Morgan ([EMAIL PROTECTED]):
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
>
> Andrew G. Morgan wrote:
> | Serge E. Hallyn wrote:
> | |> It all looks good to me.
> | |
> | |> Since we've confirmed that wireshark uses capabilities it must be 
> using
> | |> prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify
> that
> | |> your changes to that codepath (with
> CONFIG_SECURITY_FILE_CAPABILITIES=n)
> | |> are 100% correct, and might set minds at ease.  Is that something
> you're
> | |> set up to be able to do?
> |
> | I guess I need someone to offer an existence proof that this particular
> | wireshark code ever worked? (ldd dumpcap|grep libcap). For reference,
> | I'm looking at:
>
> Doh! :*)
>
> I had mistaken cap_init() for cap_get_proc() in the wireshark code...
>
> I now see what wireshark is trying to do, and can *confirm* that with
> the present patch I do maintain the legacy behavior. :-)

Cool, thanks!

> FWIW I've updated capsh in the libcap git tree to add a few more hooks
> and with the following sequence can now verify that the keep-caps works
> as before:
>
> As root:
> # rm -f tcapsh
> # cp capsh tcapsh
> # chown root.root tcapsh
> # chmod u+s tcapsh
> # ls -l tcapsh
> # ./capsh --uid=500 -- -c "./tcapsh --keep=1 \
> ~  --caps=\"cap_net_raw,cap_net_admin=ip\" --uid=500 \
> ~  --caps=\"cap_net_raw,cap_net_admin=pie\" --print"
> # echo $?
> 0
>
> The wireshark problem, that you have been discussing (in the other
> thread), can also be simulated as follows:
>
> # ./capsh --uid=500 -- -c "./tcapsh --keep=1 \
> ~  --caps=\"cap_net_raw,cap_net_admin=ip\" \
> ~  --uid=500 --forkfor=10 --caps= --print \
> ~  --killit=9 --print"
>
> You might like to re-post your fix for that problem as a stand alone
> patch; I suspect it may be lost in the noise at this point. (You might
> also like to update the comment in that fix since the old comment looks
> very stale if you delete the ->euid==0 check. I think it is safe to
> simply say /* legacy signal behavior requires that a user can kill any
> process running with their uid */)

I did resend it on Wednesday and add the comment that

'Without this patch wireshark is reported to fail.'

I'll resend explicitly to Linus and add your comment change on monday
if it's not upstream yet.  (Will be offline this weekend).

thanks,
-serge

>
> Cheers
>
> Andrew
>
> |
> | wireshark-0.99.7/dumpcap.c:302
> |
> | void
> | relinquish_privs_except_capture(void)
> | {
> | ~/* CAP_NET_ADMIN: Promiscuous mode and a truckload of other
> | ~ *stuff we don't need (and shouldn't have).
> | ~ * CAP_NET_RAW:   Packet capture (raw sockets).
> | ~ */
> | ~cap_value_t cap_list[2] = { CAP_NET_ADMIN, CAP_NET_RAW };
> | ~cap_t caps = cap_init();
> | ~int cl_len = sizeof(cap_list) / sizeof(cap_value_t);
> |
> | ~if (started_with_special_privs()) {
> | ~print_caps("Pre drop, pre set");
> | ~if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1) {
> | ~perror("prctl()");
> | ~}
> |
> | ~cap_set_flag(caps, CAP_PERMITTED,   cl_len, cap_list, CAP_SET);
> | ~cap_set_flag(caps, CAP_INHERITABLE, cl_len, cap_list, CAP_SET);
> |
> | [ XXX:AGM since (caps.pE > caps.pP) this next line should fail ]
> | ~if (cap_set_proc(caps)) {
> | ~perror("capset()");
> | ~}
> | ~print_caps("Pre drop, post set");
> | ~}
> |
> | ~relinquish_special_privs_perm();
> |
> | ~print_caps("Post drop, pre set");
> | ~cap_set_flag(caps, CAP_EFFECTIVE,   cl_len, cap_list, CAP_SET);
> | ~if (cap_set_proc(caps)) {
> | ~perror("capset()");
> | ~}
> | ~print_caps("Post drop, post set");
> | ~cap_free(caps);
> | }
> | #endif /* HAVE_LIBCAP */
> |
> | My reading of the above code suggests that the application believes that
> | it can raise/retain effective capabilities that are not in its permitted
> | set.
> |
> | Browsing back in my git tree all the way back to 'v2.6.12-rc2', the
> | following code (cap_capset_check) correctly requires:
> |
> | ~   96  /* verify the _new_Effective_ is a subset of the
> _new_Permitted_ */
> | ~   97  if (!cap_issubset (*effective, *permitted)) {
> | ~   98   return -EPERM;
> | ~   99  }
> |
> | so my question is, why should one expect this wireshark code to work? It
> | looks wrong to me.
> |
> | Thanks
> |
> | Andrew
> |
> | |
> | |> -serge
> | |
> | | Thanks
> | |
> | | Andrew
> |
> | ~From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
> | From: Andrew G. Morgan <[EMAIL PROTECTED]>
> | Date: Mon, 18 Feb 2008 15:23:28 -0800
> | Subject: [PATCH] Implement per-process securebits
> | |>
> | [This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
> | ~ is enabled at configure time.]
> | |>
> | Filesystem capability support makes it possible to do away with
> | (set)uid-0 based privilege and use capabilities instead. That is, with
> | filesystem support for 

Re: [PATCH] capabilities: implement per-process securebits

[Serge E. Hallyn]

Quoting Andrew G. Morgan ([EMAIL PROTECTED]):
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1


 Andrew G. Morgan wrote:
 | Serge E. Hallyn wrote:
 | | It all looks good to me.
 | |
 | | Since we've confirmed that wireshark uses capabilities it must be 
 using
 | | prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify
 that
 | | your changes to that codepath (with
 CONFIG_SECURITY_FILE_CAPABILITIES=n)
 | | are 100% correct, and might set minds at ease.  Is that something
 you're
 | | set up to be able to do?
 |
 | I guess I need someone to offer an existence proof that this particular
 | wireshark code ever worked? (ldd dumpcap|grep libcap). For reference,
 | I'm looking at:

 Doh! :*)

 I had mistaken cap_init() for cap_get_proc() in the wireshark code...

 I now see what wireshark is trying to do, and can *confirm* that with
 the present patch I do maintain the legacy behavior. :-)

Cool, thanks!

 FWIW I've updated capsh in the libcap git tree to add a few more hooks
 and with the following sequence can now verify that the keep-caps works
 as before:

 As root:
 # rm -f tcapsh
 # cp capsh tcapsh
 # chown root.root tcapsh
 # chmod u+s tcapsh
 # ls -l tcapsh
 # ./capsh --uid=500 -- -c ./tcapsh --keep=1 \
 ~  --caps=\cap_net_raw,cap_net_admin=ip\ --uid=500 \
 ~  --caps=\cap_net_raw,cap_net_admin=pie\ --print
 # echo $?
 0

 The wireshark problem, that you have been discussing (in the other
 thread), can also be simulated as follows:

 # ./capsh --uid=500 -- -c ./tcapsh --keep=1 \
 ~  --caps=\cap_net_raw,cap_net_admin=ip\ \
 ~  --uid=500 --forkfor=10 --caps= --print \
 ~  --killit=9 --print

 You might like to re-post your fix for that problem as a stand alone
 patch; I suspect it may be lost in the noise at this point. (You might
 also like to update the comment in that fix since the old comment looks
 very stale if you delete the -euid==0 check. I think it is safe to
 simply say /* legacy signal behavior requires that a user can kill any
 process running with their uid */)

I did resend it on Wednesday and add the comment that

'Without this patch wireshark is reported to fail.'

I'll resend explicitly to Linus and add your comment change on monday
if it's not upstream yet.  (Will be offline this weekend).

thanks,
-serge


 Cheers

 Andrew

 |
 | wireshark-0.99.7/dumpcap.c:302
 |
 | void
 | relinquish_privs_except_capture(void)
 | {
 | ~/* CAP_NET_ADMIN: Promiscuous mode and a truckload of other
 | ~ *stuff we don't need (and shouldn't have).
 | ~ * CAP_NET_RAW:   Packet capture (raw sockets).
 | ~ */
 | ~cap_value_t cap_list[2] = { CAP_NET_ADMIN, CAP_NET_RAW };
 | ~cap_t caps = cap_init();
 | ~int cl_len = sizeof(cap_list) / sizeof(cap_value_t);
 |
 | ~if (started_with_special_privs()) {
 | ~print_caps(Pre drop, pre set);
 | ~if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1) {
 | ~perror(prctl());
 | ~}
 |
 | ~cap_set_flag(caps, CAP_PERMITTED,   cl_len, cap_list, CAP_SET);
 | ~cap_set_flag(caps, CAP_INHERITABLE, cl_len, cap_list, CAP_SET);
 |
 | [ XXX:AGM since (caps.pE  caps.pP) this next line should fail ]
 | ~if (cap_set_proc(caps)) {
 | ~perror(capset());
 | ~}
 | ~print_caps(Pre drop, post set);
 | ~}
 |
 | ~relinquish_special_privs_perm();
 |
 | ~print_caps(Post drop, pre set);
 | ~cap_set_flag(caps, CAP_EFFECTIVE,   cl_len, cap_list, CAP_SET);
 | ~if (cap_set_proc(caps)) {
 | ~perror(capset());
 | ~}
 | ~print_caps(Post drop, post set);
 | ~cap_free(caps);
 | }
 | #endif /* HAVE_LIBCAP */
 |
 | My reading of the above code suggests that the application believes that
 | it can raise/retain effective capabilities that are not in its permitted
 | set.
 |
 | Browsing back in my git tree all the way back to 'v2.6.12-rc2', the
 | following code (cap_capset_check) correctly requires:
 |
 | ~   96  /* verify the _new_Effective_ is a subset of the
 _new_Permitted_ */
 | ~   97  if (!cap_issubset (*effective, *permitted)) {
 | ~   98   return -EPERM;
 | ~   99  }
 |
 | so my question is, why should one expect this wireshark code to work? It
 | looks wrong to me.
 |
 | Thanks
 |
 | Andrew
 |
 | |
 | | -serge
 | |
 | | Thanks
 | |
 | | Andrew
 |
 | ~From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
 | From: Andrew G. Morgan [EMAIL PROTECTED]
 | Date: Mon, 18 Feb 2008 15:23:28 -0800
 | Subject: [PATCH] Implement per-process securebits
 | |
 | [This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
 | ~ is enabled at configure time.]
 | |
 | Filesystem capability support makes it possible to do away with
 | (set)uid-0 based privilege and use capabilities instead. That is, with
 | filesystem support for capabilities but without this present patch,
 | it is (conceptually) possible to manage a system with capabilities
 | alone and never need to obtain privilege via (set)uid-0.
 -BEGIN PGP 

Re: [PATCH] capabilities: implement per-process securebits

[Andrew G. Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Andrew G. Morgan wrote:
| Serge E. Hallyn wrote:
| |> It all looks good to me.
| |
| |> Since we've confirmed that wireshark uses capabilities it must be using
| |> prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify
that
| |> your changes to that codepath (with
CONFIG_SECURITY_FILE_CAPABILITIES=n)
| |> are 100% correct, and might set minds at ease.  Is that something
you're
| |> set up to be able to do?
|
| I guess I need someone to offer an existence proof that this particular
| wireshark code ever worked? (ldd dumpcap|grep libcap). For reference,
| I'm looking at:

Doh! :*)

I had mistaken cap_init() for cap_get_proc() in the wireshark code...

I now see what wireshark is trying to do, and can *confirm* that with
the present patch I do maintain the legacy behavior. :-)

FWIW I've updated capsh in the libcap git tree to add a few more hooks
and with the following sequence can now verify that the keep-caps works
as before:

As root:
# rm -f tcapsh
# cp capsh tcapsh
# chown root.root tcapsh
# chmod u+s tcapsh
# ls -l tcapsh
# ./capsh --uid=500 -- -c "./tcapsh --keep=1 \
~  --caps=\"cap_net_raw,cap_net_admin=ip\" --uid=500 \
~  --caps=\"cap_net_raw,cap_net_admin=pie\" --print"
# echo $?
0

The wireshark problem, that you have been discussing (in the other
thread), can also be simulated as follows:

# ./capsh --uid=500 -- -c "./tcapsh --keep=1 \
~  --caps=\"cap_net_raw,cap_net_admin=ip\" \
~  --uid=500 --forkfor=10 --caps= --print \
~  --killit=9 --print"

You might like to re-post your fix for that problem as a stand alone
patch; I suspect it may be lost in the noise at this point. (You might
also like to update the comment in that fix since the old comment looks
very stale if you delete the ->euid==0 check. I think it is safe to
simply say /* legacy signal behavior requires that a user can kill any
process running with their uid */)

Cheers

Andrew

|
| wireshark-0.99.7/dumpcap.c:302
|
| void
| relinquish_privs_except_capture(void)
| {
| ~/* CAP_NET_ADMIN: Promiscuous mode and a truckload of other
| ~ *stuff we don't need (and shouldn't have).
| ~ * CAP_NET_RAW:   Packet capture (raw sockets).
| ~ */
| ~cap_value_t cap_list[2] = { CAP_NET_ADMIN, CAP_NET_RAW };
| ~cap_t caps = cap_init();
| ~int cl_len = sizeof(cap_list) / sizeof(cap_value_t);
|
| ~if (started_with_special_privs()) {
| ~print_caps("Pre drop, pre set");
| ~if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1) {
| ~perror("prctl()");
| ~}
|
| ~cap_set_flag(caps, CAP_PERMITTED,   cl_len, cap_list, CAP_SET);
| ~cap_set_flag(caps, CAP_INHERITABLE, cl_len, cap_list, CAP_SET);
|
| [ XXX:AGM since (caps.pE > caps.pP) this next line should fail ]
| ~if (cap_set_proc(caps)) {
| ~perror("capset()");
| ~}
| ~print_caps("Pre drop, post set");
| ~}
|
| ~relinquish_special_privs_perm();
|
| ~print_caps("Post drop, pre set");
| ~cap_set_flag(caps, CAP_EFFECTIVE,   cl_len, cap_list, CAP_SET);
| ~if (cap_set_proc(caps)) {
| ~perror("capset()");
| ~}
| ~print_caps("Post drop, post set");
| ~cap_free(caps);
| }
| #endif /* HAVE_LIBCAP */
|
| My reading of the above code suggests that the application believes that
| it can raise/retain effective capabilities that are not in its permitted
| set.
|
| Browsing back in my git tree all the way back to 'v2.6.12-rc2', the
| following code (cap_capset_check) correctly requires:
|
| ~   96  /* verify the _new_Effective_ is a subset of the
_new_Permitted_ */
| ~   97  if (!cap_issubset (*effective, *permitted)) {
| ~   98   return -EPERM;
| ~   99  }
|
| so my question is, why should one expect this wireshark code to work? It
| looks wrong to me.
|
| Thanks
|
| Andrew
|
| |
| |> -serge
| |
| | Thanks
| |
| | Andrew
|
| ~From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
| From: Andrew G. Morgan <[EMAIL PROTECTED]>
| Date: Mon, 18 Feb 2008 15:23:28 -0800
| Subject: [PATCH] Implement per-process securebits
| |>
| [This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
| ~ is enabled at configure time.]
| |>
| Filesystem capability support makes it possible to do away with
| (set)uid-0 based privilege and use capabilities instead. That is, with
| filesystem support for capabilities but without this present patch,
| it is (conceptually) possible to manage a system with capabilities
| alone and never need to obtain privilege via (set)uid-0.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHvmeB+bHCR3gb8jsRAknmAKCMw0Qe7uDwtuRE+f3YVmnlE5pK4wCgsv0f
5E6+K9Z0Xp1P74iOlnt221o=
=+sLL
-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: implement per-process securebits

[Andrew G. Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
|> It all looks good to me.
|
|> Since we've confirmed that wireshark uses capabilities it must be using
|> prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify that
|> your changes to that codepath (with CONFIG_SECURITY_FILE_CAPABILITIES=n)
|> are 100% correct, and might set minds at ease.  Is that something you're
|> set up to be able to do?

I guess I need someone to offer an existence proof that this particular
wireshark code ever worked? (ldd dumpcap|grep libcap). For reference,
I'm looking at:

wireshark-0.99.7/dumpcap.c:302

void
relinquish_privs_except_capture(void)
{
~/* CAP_NET_ADMIN: Promiscuous mode and a truckload of other
~ *stuff we don't need (and shouldn't have).
~ * CAP_NET_RAW:   Packet capture (raw sockets).
~ */
~cap_value_t cap_list[2] = { CAP_NET_ADMIN, CAP_NET_RAW };
~cap_t caps = cap_init();
~int cl_len = sizeof(cap_list) / sizeof(cap_value_t);

~if (started_with_special_privs()) {
~print_caps("Pre drop, pre set");
~if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1) {
~perror("prctl()");
~}

~cap_set_flag(caps, CAP_PERMITTED,   cl_len, cap_list, CAP_SET);
~cap_set_flag(caps, CAP_INHERITABLE, cl_len, cap_list, CAP_SET);

[ XXX:AGM since (caps.pE > caps.pP) this next line should fail ]
~if (cap_set_proc(caps)) {
~perror("capset()");
~}
~print_caps("Pre drop, post set");
~}

~relinquish_special_privs_perm();

~print_caps("Post drop, pre set");
~cap_set_flag(caps, CAP_EFFECTIVE,   cl_len, cap_list, CAP_SET);
~if (cap_set_proc(caps)) {
~perror("capset()");
~}
~print_caps("Post drop, post set");
~cap_free(caps);
}
#endif /* HAVE_LIBCAP */

My reading of the above code suggests that the application believes that
it can raise/retain effective capabilities that are not in its permitted
set.

Browsing back in my git tree all the way back to 'v2.6.12-rc2', the
following code (cap_capset_check) correctly requires:

~   96  /* verify the _new_Effective_ is a subset of the _new_Permitted_ */
~   97  if (!cap_issubset (*effective, *permitted)) {
~   98   return -EPERM;
~   99  }

so my question is, why should one expect this wireshark code to work? It
looks wrong to me.

Thanks

Andrew

|
|> -serge
|
| Thanks
|
| Andrew

~From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
From: Andrew G. Morgan <[EMAIL PROTECTED]>
Date: Mon, 18 Feb 2008 15:23:28 -0800
Subject: [PATCH] Implement per-process securebits
|>
[This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
~ is enabled at configure time.]
|>
Filesystem capability support makes it possible to do away with
(set)uid-0 based privilege and use capabilities instead. That is, with
filesystem support for capabilities but without this present patch,
it is (conceptually) possible to manage a system with capabilities
alone and never need to obtain privilege via (set)uid-0.
|>
Of course, conceptually isn't quite the same as currently possible
since few user applications, certainly not enough to run a viable
system, are currently prepared to leverage capabilities to exercise
privilege. Further, many applications exist that may never get
upgraded in this way, and the kernel will continue to want to support
their setuid-0 base privilege needs.
|>
Where pure-capability applications evolve and replace setuid-0
binaries, it is desirable that there be a mechanisms by which they
can contain their privilege. In addition to leveraging the per-process
bounding and inheritable sets, this should include suppressing the
privilege of the uid-0 superuser from the process' tree of children.
|>
The feature added by this patch can be leveraged to suppress the
privilege associated with (set)uid-0. This suppression requires
CAP_SETPCAP to initiate, and only immediately affects the 'current'
process (it is inherited through fork()/exec()). This
reimplementation differs significantly from the historical support for
securebits which was system-wide, unwieldy and which has ultimately
withered to a dead relic in the source of the modern kernel.
|>
With this patch applied a process, that is capable(CAP_SETPCAP), can
now drop all legacy privilege (through uid=0) for itself and all
subsequently fork()'d/exec()'d children with:
|>
~  prctl(PR_SET_SECUREBITS, 0x2f);
|>
[2008/02/18: This version includes an int -> long argument fix from Serge.]
|>
Signed-off-by: Andrew G. Morgan <[EMAIL PROTECTED]>
Acked-by: Serge Hallyn <[EMAIL PROTECTED]>
Reviewed-by: James Morris <[EMAIL PROTECTED]>
- ---
~ include/linux/capability.h |3 +-
~ include/linux/init_task.h  |3 +-
~ include/linux/prctl.h  |9 +++-
~ include/linux/sched.h  |3 +-
~ include/linux/securebits.h |   25 ---
~ include/linux/security.h   |   14 +++---
~ kernel/sys.c   |   25 +--
~ 

Re: [PATCH] capabilities: implement per-process securebits

[Andrew G. Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Serge E. Hallyn wrote:
| It all looks good to me.
|
| Since we've confirmed that wireshark uses capabilities it must be using
| prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify that
| your changes to that codepath (with CONFIG_SECURITY_FILE_CAPABILITIES=n)
| are 100% correct, and might set minds at ease.  Is that something you're
| set up to be able to do?

I guess I need someone to offer an existence proof that this particular
wireshark code ever worked? (ldd dumpcap|grep libcap). For reference,
I'm looking at:

wireshark-0.99.7/dumpcap.c:302

void
relinquish_privs_except_capture(void)
{
~/* CAP_NET_ADMIN: Promiscuous mode and a truckload of other
~ *stuff we don't need (and shouldn't have).
~ * CAP_NET_RAW:   Packet capture (raw sockets).
~ */
~cap_value_t cap_list[2] = { CAP_NET_ADMIN, CAP_NET_RAW };
~cap_t caps = cap_init();
~int cl_len = sizeof(cap_list) / sizeof(cap_value_t);

~if (started_with_special_privs()) {
~print_caps(Pre drop, pre set);
~if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1) {
~perror(prctl());
~}

~cap_set_flag(caps, CAP_PERMITTED,   cl_len, cap_list, CAP_SET);
~cap_set_flag(caps, CAP_INHERITABLE, cl_len, cap_list, CAP_SET);

[ XXX:AGM since (caps.pE  caps.pP) this next line should fail ]
~if (cap_set_proc(caps)) {
~perror(capset());
~}
~print_caps(Pre drop, post set);
~}

~relinquish_special_privs_perm();

~print_caps(Post drop, pre set);
~cap_set_flag(caps, CAP_EFFECTIVE,   cl_len, cap_list, CAP_SET);
~if (cap_set_proc(caps)) {
~perror(capset());
~}
~print_caps(Post drop, post set);
~cap_free(caps);
}
#endif /* HAVE_LIBCAP */

My reading of the above code suggests that the application believes that
it can raise/retain effective capabilities that are not in its permitted
set.

Browsing back in my git tree all the way back to 'v2.6.12-rc2', the
following code (cap_capset_check) correctly requires:

~   96  /* verify the _new_Effective_ is a subset of the _new_Permitted_ */
~   97  if (!cap_issubset (*effective, *permitted)) {
~   98   return -EPERM;
~   99  }

so my question is, why should one expect this wireshark code to work? It
looks wrong to me.

Thanks

Andrew

|
| -serge
|
| Thanks
|
| Andrew

~From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
From: Andrew G. Morgan [EMAIL PROTECTED]
Date: Mon, 18 Feb 2008 15:23:28 -0800
Subject: [PATCH] Implement per-process securebits
|
[This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
~ is enabled at configure time.]
|
Filesystem capability support makes it possible to do away with
(set)uid-0 based privilege and use capabilities instead. That is, with
filesystem support for capabilities but without this present patch,
it is (conceptually) possible to manage a system with capabilities
alone and never need to obtain privilege via (set)uid-0.
|
Of course, conceptually isn't quite the same as currently possible
since few user applications, certainly not enough to run a viable
system, are currently prepared to leverage capabilities to exercise
privilege. Further, many applications exist that may never get
upgraded in this way, and the kernel will continue to want to support
their setuid-0 base privilege needs.
|
Where pure-capability applications evolve and replace setuid-0
binaries, it is desirable that there be a mechanisms by which they
can contain their privilege. In addition to leveraging the per-process
bounding and inheritable sets, this should include suppressing the
privilege of the uid-0 superuser from the process' tree of children.
|
The feature added by this patch can be leveraged to suppress the
privilege associated with (set)uid-0. This suppression requires
CAP_SETPCAP to initiate, and only immediately affects the 'current'
process (it is inherited through fork()/exec()). This
reimplementation differs significantly from the historical support for
securebits which was system-wide, unwieldy and which has ultimately
withered to a dead relic in the source of the modern kernel.
|
With this patch applied a process, that is capable(CAP_SETPCAP), can
now drop all legacy privilege (through uid=0) for itself and all
subsequently fork()'d/exec()'d children with:
|
~  prctl(PR_SET_SECUREBITS, 0x2f);
|
[2008/02/18: This version includes an int - long argument fix from Serge.]
|
Signed-off-by: Andrew G. Morgan [EMAIL PROTECTED]
Acked-by: Serge Hallyn [EMAIL PROTECTED]
Reviewed-by: James Morris [EMAIL PROTECTED]
- ---
~ include/linux/capability.h |3 +-
~ include/linux/init_task.h  |3 +-
~ include/linux/prctl.h  |9 +++-
~ include/linux/sched.h  |3 +-
~ include/linux/securebits.h |   25 ---
~ include/linux/security.h   |   14 +++---
~ kernel/sys.c   |   25 +--
~ security/capability.c  |1 +
~ 

Re: [PATCH] capabilities: implement per-process securebits

[Andrew G. Morgan]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Andrew G. Morgan wrote:
| Serge E. Hallyn wrote:
| | It all looks good to me.
| |
| | Since we've confirmed that wireshark uses capabilities it must be using
| | prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify
that
| | your changes to that codepath (with
CONFIG_SECURITY_FILE_CAPABILITIES=n)
| | are 100% correct, and might set minds at ease.  Is that something
you're
| | set up to be able to do?
|
| I guess I need someone to offer an existence proof that this particular
| wireshark code ever worked? (ldd dumpcap|grep libcap). For reference,
| I'm looking at:

Doh! :*)

I had mistaken cap_init() for cap_get_proc() in the wireshark code...

I now see what wireshark is trying to do, and can *confirm* that with
the present patch I do maintain the legacy behavior. :-)

FWIW I've updated capsh in the libcap git tree to add a few more hooks
and with the following sequence can now verify that the keep-caps works
as before:

As root:
# rm -f tcapsh
# cp capsh tcapsh
# chown root.root tcapsh
# chmod u+s tcapsh
# ls -l tcapsh
# ./capsh --uid=500 -- -c ./tcapsh --keep=1 \
~  --caps=\cap_net_raw,cap_net_admin=ip\ --uid=500 \
~  --caps=\cap_net_raw,cap_net_admin=pie\ --print
# echo $?
0

The wireshark problem, that you have been discussing (in the other
thread), can also be simulated as follows:

# ./capsh --uid=500 -- -c ./tcapsh --keep=1 \
~  --caps=\cap_net_raw,cap_net_admin=ip\ \
~  --uid=500 --forkfor=10 --caps= --print \
~  --killit=9 --print

You might like to re-post your fix for that problem as a stand alone
patch; I suspect it may be lost in the noise at this point. (You might
also like to update the comment in that fix since the old comment looks
very stale if you delete the -euid==0 check. I think it is safe to
simply say /* legacy signal behavior requires that a user can kill any
process running with their uid */)

Cheers

Andrew

|
| wireshark-0.99.7/dumpcap.c:302
|
| void
| relinquish_privs_except_capture(void)
| {
| ~/* CAP_NET_ADMIN: Promiscuous mode and a truckload of other
| ~ *stuff we don't need (and shouldn't have).
| ~ * CAP_NET_RAW:   Packet capture (raw sockets).
| ~ */
| ~cap_value_t cap_list[2] = { CAP_NET_ADMIN, CAP_NET_RAW };
| ~cap_t caps = cap_init();
| ~int cl_len = sizeof(cap_list) / sizeof(cap_value_t);
|
| ~if (started_with_special_privs()) {
| ~print_caps(Pre drop, pre set);
| ~if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1) {
| ~perror(prctl());
| ~}
|
| ~cap_set_flag(caps, CAP_PERMITTED,   cl_len, cap_list, CAP_SET);
| ~cap_set_flag(caps, CAP_INHERITABLE, cl_len, cap_list, CAP_SET);
|
| [ XXX:AGM since (caps.pE  caps.pP) this next line should fail ]
| ~if (cap_set_proc(caps)) {
| ~perror(capset());
| ~}
| ~print_caps(Pre drop, post set);
| ~}
|
| ~relinquish_special_privs_perm();
|
| ~print_caps(Post drop, pre set);
| ~cap_set_flag(caps, CAP_EFFECTIVE,   cl_len, cap_list, CAP_SET);
| ~if (cap_set_proc(caps)) {
| ~perror(capset());
| ~}
| ~print_caps(Post drop, post set);
| ~cap_free(caps);
| }
| #endif /* HAVE_LIBCAP */
|
| My reading of the above code suggests that the application believes that
| it can raise/retain effective capabilities that are not in its permitted
| set.
|
| Browsing back in my git tree all the way back to 'v2.6.12-rc2', the
| following code (cap_capset_check) correctly requires:
|
| ~   96  /* verify the _new_Effective_ is a subset of the
_new_Permitted_ */
| ~   97  if (!cap_issubset (*effective, *permitted)) {
| ~   98   return -EPERM;
| ~   99  }
|
| so my question is, why should one expect this wireshark code to work? It
| looks wrong to me.
|
| Thanks
|
| Andrew
|
| |
| | -serge
| |
| | Thanks
| |
| | Andrew
|
| ~From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
| From: Andrew G. Morgan [EMAIL PROTECTED]
| Date: Mon, 18 Feb 2008 15:23:28 -0800
| Subject: [PATCH] Implement per-process securebits
| |
| [This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
| ~ is enabled at configure time.]
| |
| Filesystem capability support makes it possible to do away with
| (set)uid-0 based privilege and use capabilities instead. That is, with
| filesystem support for capabilities but without this present patch,
| it is (conceptually) possible to manage a system with capabilities
| alone and never need to obtain privilege via (set)uid-0.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHvmeB+bHCR3gb8jsRAknmAKCMw0Qe7uDwtuRE+f3YVmnlE5pK4wCgsv0f
5E6+K9Z0Xp1P74iOlnt221o=
=+sLL
-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: implement per-process securebits

[Serge E. Hallyn]

Quoting Andrew G. Morgan ([EMAIL PROTECTED]):
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> Andrew
>
> Here is the patch to add per-process securebits again. This version
> includes Serge's argument type fix (thanks), but is otherwise unchanged
> from the one posted a couple of weeks back. It is against Linus' tree as
> of a the 15th.
>
> This change is all code that lives inside the capability LSM and the new
> securebits implementation is only active if
> CONFIG_SECURITY_FILE_CAPABILITIES is enabled (it doesn't make much sense
> to support this feature without filesystem capabilities).
>
> As indicated in the last round, this patch has been Acked by Serge and
> Reviewed by James (Cc:d).

Thanks Andrew.

It all looks good to me.

Since we've confirmed that wireshark uses capabilities it must be using
prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify that
your changes to that codepath (with CONFIG_SECURITY_FILE_CAPABILITIES=n)
are 100% correct, and might set minds at ease.  Is that something you're
set up to be able to do?

-serge

>
> Thanks
>
> Andrew
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.2.6 (GNU/Linux)
>
> iD8DBQFHuj4g+bHCR3gb8jsRAjBqAKCuMrlQqIOTY+5Tba6aM5HHcy3cWQCgvA2p
> v+MAuce9OULRL9vOKdivq8Q=
> =L/XN
> -END PGP SIGNATURE-

> From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
> From: Andrew G. Morgan <[EMAIL PROTECTED]>
> Date: Mon, 18 Feb 2008 15:23:28 -0800
> Subject: [PATCH] Implement per-process securebits
> 
> [This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
>  is enabled at configure time.]
> 
> Filesystem capability support makes it possible to do away with
> (set)uid-0 based privilege and use capabilities instead. That is, with
> filesystem support for capabilities but without this present patch,
> it is (conceptually) possible to manage a system with capabilities
> alone and never need to obtain privilege via (set)uid-0.
> 
> Of course, conceptually isn't quite the same as currently possible
> since few user applications, certainly not enough to run a viable
> system, are currently prepared to leverage capabilities to exercise
> privilege. Further, many applications exist that may never get
> upgraded in this way, and the kernel will continue to want to support
> their setuid-0 base privilege needs.
> 
> Where pure-capability applications evolve and replace setuid-0
> binaries, it is desirable that there be a mechanisms by which they
> can contain their privilege. In addition to leveraging the per-process
> bounding and inheritable sets, this should include suppressing the
> privilege of the uid-0 superuser from the process' tree of children.
> 
> The feature added by this patch can be leveraged to suppress the
> privilege associated with (set)uid-0. This suppression requires
> CAP_SETPCAP to initiate, and only immediately affects the 'current'
> process (it is inherited through fork()/exec()). This
> reimplementation differs significantly from the historical support for
> securebits which was system-wide, unwieldy and which has ultimately
> withered to a dead relic in the source of the modern kernel.
> 
> With this patch applied a process, that is capable(CAP_SETPCAP), can
> now drop all legacy privilege (through uid=0) for itself and all
> subsequently fork()'d/exec()'d children with:
> 
>   prctl(PR_SET_SECUREBITS, 0x2f);
> 
> [2008/02/18: This version includes an int -> long argument fix from Serge.]
> 
> Signed-off-by: Andrew G. Morgan <[EMAIL PROTECTED]>
> Acked-by: Serge Hallyn <[EMAIL PROTECTED]>
> Reviewed-by: James Morris <[EMAIL PROTECTED]>
> ---
>  include/linux/capability.h |3 +-
>  include/linux/init_task.h  |3 +-
>  include/linux/prctl.h  |9 +++-
>  include/linux/sched.h  |3 +-
>  include/linux/securebits.h |   25 ---
>  include/linux/security.h   |   14 +++---
>  kernel/sys.c   |   25 +--
>  security/capability.c  |1 +
>  security/commoncap.c   |  103 
> 
>  security/dummy.c   |2 +-
>  security/security.c|4 +-
>  security/selinux/hooks.c   |5 +-
>  12 files changed, 139 insertions(+), 58 deletions(-)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 7d50ff6..eaab759 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -155,6 +155,7 @@ typedef struct kernel_cap_struct {
>   *   Add any capability from current's capability bounding set
>   *   to the current process' inheritable set
>   *   Allow taking bits out of capability bounding set
> + *   Allow modification of the securebits for a process
>   */
>  
>  #define CAP_SETPCAP  8
> @@ -490,8 +491,6 @@ extern const kernel_cap_t __cap_init_eff_set;
>  int capable(int cap);
>  int __capable(struct task_struct *t, int cap);
>  
> -extern long cap_prctl_drop(unsigned long cap);
> -
>  #endif /* __KERNEL__ */
>  
>  #endif /* 

Re: [PATCH] capabilities: implement per-process securebits

[Serge E. Hallyn]

Quoting Andrew G. Morgan ([EMAIL PROTECTED]):
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 Andrew

 Here is the patch to add per-process securebits again. This version
 includes Serge's argument type fix (thanks), but is otherwise unchanged
 from the one posted a couple of weeks back. It is against Linus' tree as
 of a the 15th.

 This change is all code that lives inside the capability LSM and the new
 securebits implementation is only active if
 CONFIG_SECURITY_FILE_CAPABILITIES is enabled (it doesn't make much sense
 to support this feature without filesystem capabilities).

 As indicated in the last round, this patch has been Acked by Serge and
 Reviewed by James (Cc:d).

Thanks Andrew.

It all looks good to me.

Since we've confirmed that wireshark uses capabilities it must be using
prctl(PR_SET_KEEPCAPS), so running it might be a good way to verify that
your changes to that codepath (with CONFIG_SECURITY_FILE_CAPABILITIES=n)
are 100% correct, and might set minds at ease.  Is that something you're
set up to be able to do?

-serge


 Thanks

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

 iD8DBQFHuj4g+bHCR3gb8jsRAjBqAKCuMrlQqIOTY+5Tba6aM5HHcy3cWQCgvA2p
 v+MAuce9OULRL9vOKdivq8Q=
 =L/XN
 -END PGP SIGNATURE-

 From 006ddf6903983dd596e360ab1ab8e537b29fab46 Mon Sep 17 00:00:00 2001
 From: Andrew G. Morgan [EMAIL PROTECTED]
 Date: Mon, 18 Feb 2008 15:23:28 -0800
 Subject: [PATCH] Implement per-process securebits
 
 [This patch represents a no-op unless CONFIG_SECURITY_FILE_CAPABILITIES
  is enabled at configure time.]
 
 Filesystem capability support makes it possible to do away with
 (set)uid-0 based privilege and use capabilities instead. That is, with
 filesystem support for capabilities but without this present patch,
 it is (conceptually) possible to manage a system with capabilities
 alone and never need to obtain privilege via (set)uid-0.
 
 Of course, conceptually isn't quite the same as currently possible
 since few user applications, certainly not enough to run a viable
 system, are currently prepared to leverage capabilities to exercise
 privilege. Further, many applications exist that may never get
 upgraded in this way, and the kernel will continue to want to support
 their setuid-0 base privilege needs.
 
 Where pure-capability applications evolve and replace setuid-0
 binaries, it is desirable that there be a mechanisms by which they
 can contain their privilege. In addition to leveraging the per-process
 bounding and inheritable sets, this should include suppressing the
 privilege of the uid-0 superuser from the process' tree of children.
 
 The feature added by this patch can be leveraged to suppress the
 privilege associated with (set)uid-0. This suppression requires
 CAP_SETPCAP to initiate, and only immediately affects the 'current'
 process (it is inherited through fork()/exec()). This
 reimplementation differs significantly from the historical support for
 securebits which was system-wide, unwieldy and which has ultimately
 withered to a dead relic in the source of the modern kernel.
 
 With this patch applied a process, that is capable(CAP_SETPCAP), can
 now drop all legacy privilege (through uid=0) for itself and all
 subsequently fork()'d/exec()'d children with:
 
   prctl(PR_SET_SECUREBITS, 0x2f);
 
 [2008/02/18: This version includes an int - long argument fix from Serge.]
 
 Signed-off-by: Andrew G. Morgan [EMAIL PROTECTED]
 Acked-by: Serge Hallyn [EMAIL PROTECTED]
 Reviewed-by: James Morris [EMAIL PROTECTED]
 ---
  include/linux/capability.h |3 +-
  include/linux/init_task.h  |3 +-
  include/linux/prctl.h  |9 +++-
  include/linux/sched.h  |3 +-
  include/linux/securebits.h |   25 ---
  include/linux/security.h   |   14 +++---
  kernel/sys.c   |   25 +--
  security/capability.c  |1 +
  security/commoncap.c   |  103 
 
  security/dummy.c   |2 +-
  security/security.c|4 +-
  security/selinux/hooks.c   |5 +-
  12 files changed, 139 insertions(+), 58 deletions(-)
 
 diff --git a/include/linux/capability.h b/include/linux/capability.h
 index 7d50ff6..eaab759 100644
 --- a/include/linux/capability.h
 +++ b/include/linux/capability.h
 @@ -155,6 +155,7 @@ typedef struct kernel_cap_struct {
   *   Add any capability from current's capability bounding set
   *   to the current process' inheritable set
   *   Allow taking bits out of capability bounding set
 + *   Allow modification of the securebits for a process
   */
  
  #define CAP_SETPCAP  8
 @@ -490,8 +491,6 @@ extern const kernel_cap_t __cap_init_eff_set;
  int capable(int cap);
  int __capable(struct task_struct *t, int cap);
  
 -extern long cap_prctl_drop(unsigned long cap);
 -
  #endif /* __KERNEL__ */
  
  #endif /* !_LINUX_CAPABILITY_H */
 diff --git a/include/linux/init_task.h b/include/linux/init_task.h
 index 1f74e1d..e8a894a 100644
 ---