POSIX capabilities support

[gj0aqzda]

BPO	1615158
Nosy	@loewis, @birkenfeld, @pitrou, @giampaolo, @nanjekyejoannah
PRs	WIP: bpo-1615158 : POSIX capabilities support (In a separate module) #15815
Files	patch-svn.diff: POSIX capabilities patch patch-svn-doc.diff: POSIX capabilities documentation patch patch-20080620-1232.diff: POSIX capabilities patch III patch-20080620-1314.diff: POSIX capabilities patch IV

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2006-12-13.18:10:43.000>
labels = ['extension-modules', 'type-feature']
title = 'POSIX capabilities support'
updated_at = <Date 2019-09-09.20:46:04.768>
user = 'https://bugs.python.org/gj0aqzda'

bugs.python.org fields:

activity = <Date 2019-09-09.20:46:04.768>
actor = 'nanjekyejoannah'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2006-12-13.18:10:43.000>
creator = 'gj0aqzda'
dependencies = []
files = ['7652', '7653', '10671', '10672']
hgrepos = []
issue_num = 1615158
keywords = ['patch', 'needs review']
message_count = 22.0
messages = ['51513', '51514', '51515', '51516', '51517', '51518', '51519', '51520', '51521', '68452', '68454', '68477', '89339', '110511', '136008', '136031', '136051', '136066', '136067', '136094', '136097', '136127']
nosy_count = 11.0
nosy_names = ['loewis', 'nnorwitz', 'georg.brandl', 'pitrou', 'giampaolo.rodola', 'gj0aqzda', 'Arfrever', 'neologix', 'rosslagerwall', 'Christian H', 'nanjekyejoannah']
pr_nums = ['15815']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue1615158'
versions = ['Python 3.5']

[gj0aqzda]

Attached is a patch which adds POSIX capabilities support. The following API functions are supported:

• cap_clear
• cap_copy_ext
• cap_dup
• cap_from_text
• cap_get_flag
• cap_get_proc
• cap_init
• cap_set_flag
• cap_set_proc
• cap_size
• cap_to_text

The following API function is supported, but is broken with certain versions of libcap (I am running debian testing's libcap1, version 1.10-14, which has an issue; I have reported this upstream):

• cap_copy_int

The following API functions are in there as stubs, but currently are not compiled. I need access to a machine to test these. I will probably add autoconf tests for availability of these functions in due course:

• cap_get_fd
• cap_get_file
• cap_set_fd
• cap_set_file

The patch includes diffs to configure. My autoconf is however at a different revision to that used on the python trunk. You may want to re-autoconf configure.in.

I've added a few API tests to test_posix.py.

[gj0aqzda]

I should further add that I have implemented the following API calls as methods of the new CapabilityState object in addition to the standard functions:

• cap_clear
• cap_copy_ext
• cap_dup
• cap_get_flag
• cap_set_flag
• cap_set_proc
• cap_size
• cap_to_text

[loewis]

Can you please provide documentation changes as well?

[birkenfeld]

(If you don't want to write LaTeX, it's enough to write the docs in plaintext, there are a few volunteers who will convert it appropriately.)

[gj0aqzda]

I've attached a documentation patch, which should be applied in addition to the base patch.
File Added: patch-svn-doc.diff

[gj0aqzda]

No news on these patches in a while.

To summarise, the patches are ready to go in. The issues surrounding cap_copy_int(), cap_get_() and cap_set_() are pretty minor. The vast majority of uses will be of the cap_get_proc(), cap_set_flag(), cap_set_proc() variety.

I am not trying to hassle you; I know you don't have enough time to get through everything. However, I'll hang fire on future development of stuff that I, personally, am not going to use, until I know when/if these patches are going to go in.

[loewis]

The patch cannot go in in its current form (I started applying it, but then found that I just can't do it). It contains conditional, commented out code. Either the code is correct, then it should be added, or it is incorrect, in which case it should be removed entirely. There shouldn't be any work-in-progress code in the Python repository whatsoever. This refers to both the if 0 blocks (which I thought I can safely delete), as well as commented-out entries in CapabilityStateMethods (for which I didn't know what to do).

So while you are revising it, I have a few remarks:

• you can safely omit the generated configure changes from the patch - I will regenerate them, anyway.
• please follow the alphabet in the header files in configure.in (bsdtty.h < capabilities.h)
• please don't expose method on objects on which they aren't methods. E.g. cap_clear is available both as a method and a module-level function; that can't be both right (there should be one way to do it)
  Following the socket API, I think offering these as methods is reasonable
• try avoiding the extra copy in copy_ext (copying directly into the string). If you keep malloc calls, don't return NULL without setting a Python exception.
• use the "s" format for copy_int and from_text
• consider using booleans for [gs]et_flags

[nnorwitz]

ISTM that this would be better as a separate module or an optional submodule to posix. The posix module is already 8720 lines. I really don't want it to get bigger, especially when you realize how much #ifdef'ery is in there.

Some other things I noticed:

You should use PyMem_Malloc instead of a raw malloc (same deal with free). Methods that take no arguments should use METH_NOARGS and then there's no need to call PyArgs_ParseTuple (e.g., posix_cap_get_proc).

There definitely shouldn't be any abort()s in there, even if #ifdef'ed out.

Is this 64-bit safe? My manpage (gentoo) says this: int cap_set_flag(cap_t cap_p, cap_flag_t flag, int ncap, cap_value_t *caps, cap_flag_value_t value);

I see that you are using ints. I don't know if that's correct on a 64-bit platform. If not, you will need to modify the places that ints are used to take longs.

[loewis]

I don't mind the POSIX module getting bigger. In C, these functions are all in a flat namespace, also. I like the view "if it's specified by POSIX, you find it in the POSIX module" (although this specific API was rejected for inclusion into POSIX). The functions are all very small, as the real functionality is in the C library, or even the OS kernel.

As for the ifdefery: most of it is straight-forward: functionality is either present or it isn't. It gets messy when it tries to use alternative underlying APIs, e.g. for Win32 or OS/2. If the code is to be refactored, this should be the way to go (i.e. move all Win32 and OS/2 implementations out of the module)

As for PyMem_Malloc: I see no need to use that API; it doesn't improve the code to do so, compared to malloc/free. All that matters it is symmetric.

[gj0aqzda]

Updated patch with numerous changes, which (hopefully) address the
issues you raised.

[gj0aqzda]

Updated patch with further documentation fixes.

[loewis]

Unfortunately, these changes missed the beta for 2.6, so it must be
delayed until 2.7.

[gj0aqzda]

Ping. Anything I can do?

[BreamoreBoy]

Matt Kern has put a lot of work into the attached patches from what I can see. Common courtesy suggests that someone make an effort to review his work which now can only go into 3.2. I would take it on myself but know nothing about POSIX and still find the Python C API intimidating.

[rosslagerwall]

Adding this to the posix module would enforce linking with lcap and lattr always. The development headers for these are not installed by default on some distributions.

I think it would be better if they are added to a separate module (especially since all the functions are prefixed with cap_, it is like they are in their own namespace) which means that the module is optional for people that don't have/want to build the functionality.

What are your thoughts?

[Arfrever]

posix module has many optional functions, which are available only on some systems.

[loewis]

The development headers for these are not installed by default on
some distributions.

This is not an issue at all - that's what autoconf is for.

Adding this to the posix module would enforce linking with lcap and
lattr always.

That's a more serious problem, IMO; I think some people won't like the
additional dependency.

I think it would be better if they are added to a separate module

Can you propose a name for the module?

[rosslagerwall]

> I think it would be better if they are added to a separate module

Can you propose a name for the module?

I would say either posixcap or capabitilies.

[rosslagerwall]

I would say either posixcap or capabitilies.

The problem with capabilities is that it's easy to misspell, as I did :-)

[Arfrever]

Another possibility is to make it a private module _posixcapabilities, which would be used in os module:

try:
from _posixcapabilities import *
except ImportError:
pass

[pitrou]

"posixcap" sounds ok to me.

[loewis]

"posixcap" sounds ok to me.

Bike-sheddingly, it bothers me that these functions are actually
*not* defined by POSIX, but have been withdrawn before becoming
standard. So I'd rather call it linuxcap. Using _linuxcap, and
exposing them from os sounds fine to me.