From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754276AbbEZQ16 (ORCPT ); Tue, 26 May 2015 12:27:58 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:45326 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753240AbbEZQ1l (ORCPT ); Tue, 26 May 2015 12:27:41 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-2e-55649ef9e451 Message-id: <1432657655.1974.21.camel@samsung.com> Subject: Re: [PATCH v2 0/7] Smack namespace From: Lukasz Pawelczyk To: Stephen Smalley Cc: "David S. Miller" , "Eric W. Biederman" , "Kirill A. Shutemov" , "Serge E. Hallyn" , Al Viro , Alexey Dobriyan , Andrew Morton , Andy Lutomirski , Casey Schaufler , David Howells , Fabian Frederick , Greg KH , James Morris , Jeff Layton , Jingoo Han , Joe Perches , John Johansen , Jonathan Corbet , Kees Cook , Mauro Carvalho Chehab , Miklos Szeredi , Oleg Nesterov , Paul Moore , Tetsuo Handa , Zefan Li , Rafal Krypa , linux-doc@vger.kernel.org, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, containers@lists.linux-foundation.org, Lukasz Pawelczyk Date: Tue, 26 May 2015 18:27:35 +0200 In-reply-to: <556484BD.2060004@tycho.nsa.gov> References: <1432209222-8479-1-git-send-email-l.pawelczyk@samsung.com> <1432557162-19123-1-git-send-email-l.pawelczyk@samsung.com> <556484BD.2060004@tycho.nsa.gov> Content-type: text/plain; charset=UTF-8 X-Mailer: Evolution 3.12.11 (3.12.11-1.fc21) MIME-version: 1.0 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Sa0hTcRjG++9cdrYanZbZnyKkQZRlK0PixS70pTgZXYgkrA92dIcVObWd NBWslbNilZkjtHkpm7d0Q0rTTKTSmIsib1Fp6MBbhpe0Rmqm5lyB356X5/fj+fIyhDKdWsWc iT4v6KP5KBUtJ9/ONH7YPJmnCd1qTpVB5uUQyCm30dBV9ZsGW9E3GnpfXkOQ02QkYeTKFAmz VUYpmKYmaUixltMwOxgAaT1HoS2/lQLbhAtBtquHBKc7lYZ3N3Rgu5NPQMX4dRryrxaQ0PY8 h4bRm900ZFy/JYUyu4GCsv4E6K99TMDTF6kIvmRkkpBtHKFgOKOEgIZaKwFNze+l0DTdSO1Z y7mGpknOYrhFc9mGFpKrfNQu4WosnVLuY20YZ3w9THE19lIJ58yaIrkXuTYpZ/5cjDhrmpni xvo65oTbc6y9bgBx7tKPxJHVJ+Q7NULUmXhBv2X3Kfnpauc1SWz5poTGjlapAY37mZCMwWwQ HjbmSrzZFzd3ldMmJGeUbCHCLfc6/h0/Ef40dI/wUAp2G3Z+qJw3lrOb8OeGOsqTaXYrHm+u m2d82PW46GsW8sgE65Lh3jTLXMEwJLsO/6re5WFk7GbsqKqnvAPFCBfU9Es9BTEnZ+Q9nOcx G4Db2zXe3WV4wtxFehE/XGEbJtIRa1lgWBZglgXYA0SUohVCXGSsGKHVBapFXifGRWvVkTG6 J8j7J+5nyOoIrkcsg1RLFNAQGaqk+HgxUVePMEOofBTJuZpQpULDJyYJ+phwfVyUINaj1Qyp WqnIff79mJLV8ueFs4IQK+j/txJGtsqAeF/7mOmgI6Dw/mjJ0HYqS7H4rco/RHNgpndbp9pu i7h7aNHeN5nvdPsOz1Lnchy+g0EnL1nSdgfvT6m4GBzalud/+JW+M3DLhuoecc1kd9BUTZ/7 z/vKJO1Sl6NwxYWQ4y3r3OyRflCn+/X+qGwiSp/tKEuOGQinCyZSAmP9wngVKZ7mAzcSepH/ C8RIsUQjAwAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Thanks for taking the interest and commenting on this. Replies below. On wto, 2015-05-26 at 10:35 -0400, Stephen Smalley wrote: > On 05/25/2015 08:32 AM, Lukasz Pawelczyk wrote: > > --- Design ideas --- > > > > "Smack namespace" is rather "Smack labels namespace" as not the whole > > MAC is namespaced, only the labels. There is a great analogy between > > Smack labels namespace and the user namespace part that remaps UIDs. > > > > The idea is to create a map of labels for a namespace so the namespace > > is only allowed to use those labels. Smack rules are always the same > > as in the init namespace (limited only by what labels are mapped) and > > cannot be manipulated from the child namespace. The map is actually > > only for labels' names. The underlying structures for labels remain > > the same. The filesystem also stores the "unmapped" labels from the > > init namespace. > > How do you achieve that without introducing additional hooks or > reworking the current hooks in the setxattr code path? At present, the > security module is allowed to rewrite getxattr requests on the > security.* namespace but it isn't allowed to do that for setxattr, so if > the process invokes setxattr with a mapped label, then it will be the > mapped label that gets passed to the filesystem implementation, not the > unmapped label. The security module may internally store it in unmapped > form and may even return that upon getxattr() calls, but if you then > reboot the system and later fetch from the filesystem, it will get the > mapped label value. I call the inode operation by hand in the post_setxattr. The label will effectively be set twice, which is not ideal, but there is no other option right now without reworking the hooks as you said. This shouldn't really be a problem because the Smack operations will not use the filesystem label (even when it's set incorrectly for a moment) but an already initialized smack_known structure for this inode that has all the values filled in properly. The only attack vector I can think of is hard rebooting the machine in a way that mapped label is really saved in the filesystem before the unmapped will have a chance. Should I be worried about that? This sounds a little unreal. @@ -1163,12 +1214,24 @@ static void smack_inode_post_setxattr(struct dentry *dentry, const char *name, skpp = &isp->smk_mmap; if (skpp) { - skp = smk_get_label(value, size, true); + skp = smk_get_label(value, size, true, ns); if (!IS_ERR(skp)) *skpp = skp; else *skpp = &smack_known_invalid; + + /* + * The label we get above might be a different than the one + * kernel has already set before calling this function. + * Be consistent and set the final value in the filesystem. + * The cases for this are errors and labels being used + * in a namespace where we want to store an unmapped + * value in the filesystem. + */ + dentry->d_inode->i_op->setxattr(dentry, name, + (*skpp)->smk_known, + size, flags); } } > > > --- Usage --- > > > > Smack namespace is written using LSM hooks inside user namespace. That > > means it's connected to it. > > > > To create a new Smack namespace you need to unshare() user namespace > > as usual. If that is all you do though, than there is no difference to > > what is now. To activate the Smack namespace you need to fill the > > labels' map. It is in a file /proc/$PID/smack_map. > > This should be /proc/$PID/attr/label_map or similar, modeled after the > existing /proc/$PID/attr/current and similar nodes. Then it isn't > module-specific and can be reused for other modules. Sure. I had some thoughts about that, but couldn't really figure out which option would be best. I'll change it as suggested. > > Writing to the map file is not disabled after the first write as it is > > in uid_map. For Smack we have no means to map ranges of labels, hence > > it can really be advantageous to be able to expand the map later > > on. But you can only add to the map. You cannot remove already mapped > > labels. You cannot change the already existing mappings. Also mappings > > has to be 1-1. All requests to create a map where either the unmapped > > or the mapped label already exists in the map will be denied. > > Isn't it a concern that I can then add additional labels to the mapping > for which I am not authorized? Or is this mitigated by the fact that I > cannot alter the rules? You can't add mappings for which you are not authorized. This is mitigated by both, by the fact that adding a mapping is a privileged operation in the parent (only init for now) namespace and that modifying the rules is a privileged operation allowed only in the init namespace (as it is now). > What about the situation for the predefined > labels in Smack - are you assuming that they will always be mapped up > front in the mapping file? If somebody wants to have a full Smack aware container with full Smack functionality that's what I'd expect, but it's not mandatory. -- Lukasz Pawelczyk Samsung R&D Institute Poland Samsung Electronics