Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755720AbdGLAPz (ORCPT <rfc822;w@1wt.eu>);
        Tue, 11 Jul 2017 20:15:55 -0400
Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:53246 "EHLO
        mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL)
        by vger.kernel.org with ESMTP id S1754769AbdGLAPx (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 11 Jul 2017 20:15:53 -0400
Subject: Re: [PATCH v2] xattr: Enable security.capability in user namespaces
To: "Serge E. Hallyn" <serge@hallyn.com>,
        Stefan Berger <StefanBergerstefanb@linux.vnet.ibm.com>
References: <1499785511-17192-1-git-send-email-stefanb@linux.vnet.ibm.com>
 <1499785511-17192-2-git-send-email-stefanb@linux.vnet.ibm.com>
 <20170711171222.GB31603@mail.hallyn.com>
Cc: ebiederm@xmission.com, containers@lists.linux-foundation.org,
        lkp@01.org, linux-kernel@vger.kernel.org, zohar@linux.vnet.ibm.com,
        tycho@docker.com, James.Bottomley@HansenPartnership.com,
        vgoyal@redhat.com, christian.brauner@mailbox.org, amir73il@gmail.com,
        linux-security-module@vger.kernel.org, casey@schaufler-ca.com
From: Stefan Berger <stefanb@linux.vnet.ibm.com>
Date: Tue, 11 Jul 2017 20:15:46 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <20170711171222.GB31603@mail.hallyn.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
X-TM-AS-GCONF: 00
x-cbid: 17071200-0008-0000-0000-000008359CB0
X-IBM-SpamModules-Scores: 
X-IBM-SpamModules-Versions: BY=3.00007351; HX=3.00000241; KW=3.00000007;
 PH=3.00000004; SC=3.00000214; SDB=6.00886244; UDB=6.00442365; IPR=6.00666394;
 BA=6.00005468; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000;
 ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00016186; XFM=3.00000015;
 UTC=2017-07-12 00:15:51
X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused
x-cbparentid: 17071200-0009-0000-0000-00004309FEEA
Message-Id: <ca6e0001-6aeb-74dc-ab91-44aed3b7d128@linux.vnet.ibm.com>
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-07-11_13:,,
 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0
 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam
 adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000
 definitions=main-1707120001
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 5306
Lines: 163

On 07/11/2017 01:12 PM, Serge E. Hallyn wrote:
> Quoting Stefan Berger (Stefan Bergerstefanb@linux.vnet.ibm.com):
>> er.kernel.org>
>> X-Mailing-List: linux-kernel@vger.kernel.org
>> Content-Length: 19839
>> Lines: 700
>> X-UID: 24770
>> Status: RO
>>
>> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>
>> This patch enables security.capability in user namespaces but also
>> takes a more general approach to enabling extended attributes in user
>> namespaces.
>>
>> The following rules describe the approach using security.foo as a
>> 'user namespace enabled' extended attribute:
>>
>> Reading of extended attributes:
>>
>> 1a) Reading security.foo from a user namespace will read
>>      security.foo@uid=<uid> of the parent user namespace instead with uid
>>      being the mapping of root in that parent user namespace. An
>>      exception is if root is mapped to uid 0 on the host, and in this case
>>      we will read security.foo directly.
>>      --> reading security.foo will read security.foo@uid=1000 for uid
>>          mapping of root to 1000.
>>
>> 1b) If security.foo@uid=<uid> is not available, the security.foo of the
>>      parent namespace is tried to be read. This procedure is repeated up to
>>      the init user namespace. This step only applies for reading of extended
>>      attributes and provides the same behavior as older system where the
>>      host's extended attributes applied to user namespaces.
>>
>> 2) All security.foo@uid=<uid> with valid uid mapping in the user namespace
>>     can be read. The uid within the user namespace will be mapped to the
>>     corresponding uid on the host and that uid will be used in the name of
>>     the extended attribute.
>>     -> reading security.foo@uid=1 will read security.foo@uid=1001 for uid
>>        mapping of root to 1000, size of at least 2.
>>
>>     All security.foo@uid=<uid> can be read (by root) on the host with values
>>     of <uid> also being subject to checking for valid mappings.
>>
>> 3) No other security.foo* can be read.
>>
>> The same rules for reading apply to writing and removing of user
>> namespace enabled extended attributes.
>>
>> When listing extended attributes of a file, only those are presented
>> to the user namespace that have a valid mapping. Besides that, names
>> of the extended attributes are adjusted to represent the mapping.
>> This means that if root is mapped to uid 1000 on the host, the
>> security.foo@uid=1000 will be listed as security.foo in the user
>> namespace, security.foo@uid=1001 becomes security.foo@uid=1 and so on.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Signed-off-by: Serge Hallyn <serge@hallyn.com>
>> Reviewed-by: Serge Hallyn <serge@hallyn.com>
>> ---
>>   fs/xattr.c               | 509 +++++++++++++++++++++++++++++++++++++++++++++--
>>   security/commoncap.c     |  36 +++-
>>   security/selinux/hooks.c |   9 +-
>>   3 files changed, 523 insertions(+), 31 deletions(-)
>>
>> diff --git a/fs/xattr.c b/fs/xattr.c
>> index 464c94b..eacad9e 100644
>> --- a/fs/xattr.c
>> +++ b/fs/xattr.c
>> @@ -133,20 +133,440 @@ xattr_permission(struct inode *inode, const char *name, int mask)
>>   	return inode_permission(inode, mask);
>>   }
>>   
>> +/*
>> + * A list of extended attributes that are supported in user namespaces
>> + */
>> +static const char *const userns_xattrs[] = {
>> +	XATTR_NAME_CAPS,
>> +	NULL
>> +};
>> +
>> +/*
>> + * xattrs_is_userns_supported - Check whether an xattr is supported in userns
>> + *
>> + * @name:   full name of the extended attribute
>> + * @prefix: do a prefix match (true) or a full match (false)
>> + *
>> + * This function returns < 0 if not supported, an index into userns_xattrs[]
>> + * otherwise.
>> + */
>> +static int
>> +xattr_is_userns_supported(const char *name, int prefix)
>> +{
>> +	int i;
>> +
>> +	if (!name)
>> +		return -1;
>> +
>> +	for (i = 0; userns_xattrs[i]; i++) {
>> +		if (prefix) {
>> +			if (!strncmp(userns_xattrs[i], name,
>> +				     strlen(userns_xattrs[i])))
>> +				return i;
> I think you here need to also check that the next char is either
> '\0' or '.' (or maybe '@')

I have the checks for '@' and '\0' done by the caller. With the current 
support of only security.capability I don't think we need to check for '.'.

>
>> +		} else {
>> +			if (!strcmp(userns_xattrs[i], name))
>> +				return i;
>> +		}
>> +	}
>> +	return -1;
>> +}
>> +
>> +/*
>> + * xattr_write_uid - print a string in the format of "%s@uid=%u", which
>> + *                   includes a prefix string
>> + *
>> + * @uid:     the uid
>> + * @prefix:  prefix string; may be NULL
>> + *
>> + * This function returns a buffer with the string, or a NULL pointer in
>> + * case of out-of-memory error.
>> + */
>> +static char *
>> +xattr_write_uid(uid_t uid, const char *prefix)
>> +{
>> +	size_t buflen;
>> +	char *buffer;
>> +
>> +	buflen = sizeof("@uid=") - 1 + sizeof("4294967295") - 1 + 1;
>> +	if (prefix)
>> +		buflen += strlen(prefix);
>> +
>> +	buffer = kmalloc(buflen, GFP_KERNEL);
>> +	if (!buffer)
>> +		return NULL;
>> +
>> +	if (uid == 0)
>> +		*buffer = 0;
> Do you need to print out the prefix here?

Right. Fixed.


>
>> +	else
>> +		sprintf(buffer, "%s@uid=%u",
>> +			(prefix) ? prefix : "",
>> +			uid);
>> +
>> +	return buffer;
>> +}


Thanks.

    Stefan