[apparmor] dbus/pair address rule encoding
One of the decisions made last week while several us met at a sprint. Was to change the dbus prototype syntax slightly to make it follow the same general format of the proposed network/ipc rules. Currently this is just a syntactic change, no new functionality is being added at this time. The change is to separate the local and remote addresses of a connection, message. dbus name=foo.com acquire, will acquire/bind a name for a local address. The only change here is the use of the more appropriate word 'name' instead of 'dest' For sending messages the syntax becomes dbus - name=foo.com send, the change here is '-' to a remote/peer address is going to be specified and again 'name' instead of dest. This syntax allows for rules (though unsupported atm) that specify both, dbus name=foo.com - name=bar.com send, which allows tying a send to a specific source and to a specific destination. This doesn't make a lot of sense when just addresses are used but it may when labeling is supported in the future. Now while this syntactic change can be encoded using the current rule encoding, it is problematic for potential future expansion in that it overlays both the address being acquired and the address being sent to. So we need to change to a pair encoding which is similar to link, mount rules and what is planned for the extended networking. the general scheme is to encode the address that is to be looked up first/most common. I would assume this would be the local address for the acquire rule as this would mean that lookups for the acquire perm would not require both addresses, and having both addresses does not make sense. the second address encoding follows the first as a pairing. This allows us to restrict access to the second address based on the first if so desired. Eg. for link rules we have /path/to/file\0/path/to/link ^ ^ perm1 perm2 the permissions are encoded out of band (that is each state could have its own set of permissions. And we use one special perm to indicate there is more match data/perms to follow. When making a permission request you only need to proceed with matching until your requested permissions are matched (knowing the encoding you can short circuit). In the case of dbus I see local address set encodingremote address set encoding ^^ perm1perm2 the AA_ACQUIRE perm gets encoded into perm1. And if there is a remote address for the rule, the AA_CONT perm which means there is more matching that can happen. AA_CONT isn't actually required it just allows bailing out early if no more matches are possible (it would also be filtered out from the set of permissions returned). technically we could even encode send/receive in perm1 if it is followed by a match that allows all remote addresses. With the way the backend of the compiler is setup currently this means we need to generate 2 rules when a remote address needs to be specified local address perm1 local address separator remote address perm2 perm1 - may just be the AA_CONT perm. While the AA_CONT perm isn't required having it present makes sure short circuiting can be done in the future either from user space using a none naive query (see below) or if dbus moves into the kernel. the backend will take care of collapsing/recombining states where possible. I would like to get to a point where this isn't needed but, we aren't there yet. the separator is anything that separates the two addresses in the encoding. In link pairs, its a null transition (\0 character). We use this already in the address encoding to separate the various possible strings (path, name, iface, member). We can either put in a second \0, so at the separator point we have \0\0 where the first is the separator for the member encoding, or we can just rely on the fact that we know we have a fixed number of parts to the source address, and that no additional separator is needed. For the permissions I think we should encode them on the \0 separator, not the last character of the string. This will allow us to specify a don't care/any source for the query string of \0\0\0\0 which will find us the source permission if rules are encoded such that for the parts of the address that can match anything they are encoded using the regex [^0]*\0 To query this encoding, aquire: just needs to specify the local address and return the perms found there. send/receive: need to specify both the source and dest addresses in their query string. This allows us to allow these permissions conditionally based on what source they come from. I know that we aren't doing the currently but it leaves the possibility open in the future. I would also assume the source address would be the well known address as long as the
Re: [apparmor] [patch] backport python3 compability patch to 2.8 branch
On Tue, May 07, 2013 at 10:51:01PM +0200, Christian Boltz wrote: You probably also want to pull in trunk commit 2108 as well. openSUSE seems to be better than Ubuntu - at least my test builds worked without r2108 ;-) Oddly enough, the 2.8 backported patch kicked off jenkins builds that succeeded on 13.04 (as well as other releases). However, it makes sense to me to use python-config if available. Anyway, r2108 is a small patch and looks good to me - I'd say Skimmed-by: Christian Boltz appar...@cboltz.de ;-) (credits for this new patch review level go to Seth ;-) To be explicit, the patch looks like so: === modified file 'libraries/libapparmor/m4/ac_python_devel.m4' --- libraries/libapparmor/m4/ac_python_devel.m4 2012-06-12 12:56:57 + +++ libraries/libapparmor/m4/ac_python_devel.m4 2013-01-29 23:21:47 + @@ -79,6 +79,9 @@ # Check for Python include path # AC_MSG_CHECKING([for Python include path]) +if type $PYTHON-config; then +PYTHON_CPPFLAGS=`$PYTHON-config --includes` +fi if test -z $PYTHON_CPPFLAGS; then python_path=`$PYTHON -c import sys; import distutils.sysconfig;\ sys.stdout.write('%s\n' % distutils.sysconfig.get_python_inc());` @@ -94,6 +97,9 @@ # Check for Python library path # AC_MSG_CHECKING([for Python library path]) +if type $PYTHON-config; then +PYTHON_LDFLAGS=`$PYTHON-config --ldflags` +fi if test -z $PYTHON_LDFLAGS; then # (makes two attempts to ensure we've got a version number # from the interpreter) As nice as a skimmed-by review is, I admit to wanting a proper acked-by statement, if possible. :) -- Steve Beattie sbeat...@ubuntu.com http://NxNW.org/~steve/ signature.asc Description: Digital signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 08/36] apparmor: provide the ability to boot with a default profile set on init
On Wed, May 01, 2013 at 02:30:53PM -0700, John Johansen wrote: --- a/security/apparmor/Kconfig +++ b/security/apparmor/Kconfig @@ -29,3 +29,14 @@ config SECURITY_APPARMOR_BOOTPARAM_VALUE boot. If you are unsure how to answer this question, answer 1. + +config SECURITY_APPARMOR_UNCONFINED_INIT + bool Set init to unconfined on boot + depends on SECURITY_APPARMOR + default y + help + This option determines policy behavior during early boot by + placing the init process in the unconfined state, or the + 'default' profile. + + If you are unsure how to answer this question, answer Y. I think this description needs some enhancement; I thought the boolean was the other way around until I thought I spotted a bug with a ! in the conditionals. How about: + This option determines policy behavior during early boot by + placing the init process in the unconfined state, or the + 'default' profile. + + 'Y' means init and its children are not confined, and never + can be confined; loaded policy will only apply to processes + started afterwards. + + 'N' means init and its children are confined in a profile + named 'default', which can be replaced later and thus + provide for confining even processes started early at boot, + though not confined during early boot. This can provide for + complete system confinement. + + If you are unsure how to answer this question, answer Y. Thanks signature.asc Description: Digital signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 09/36] apparmor: fix fs extry display for default profile
On Wed, May 01, 2013 at 02:30:54PM -0700, John Johansen wrote: The default profile needs its replaced by information set as its on the profile list and will have an fs interface (and the fs interface files require a valid replacedby). Signed-off-by: John Johansen john.johan...@canonical.com --- security/apparmor/policy.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 333cbb7..a7e6bd9 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -731,6 +731,9 @@ struct aa_profile *aa_setup_default_profile(void) profile-ns = aa_get_namespace(root_ns); + /* replacedby being set needed by fs interface */ + rcu_assign_pointer(profile-replacedby-profile, +aa_get_profile(profile)); __list_add_profile(root_ns-base.profiles, profile); return profile; Will aa_get_profile(profile) here cause an inability to ever free the profile, say in case it is replaced? I know this whole area is drastically changed in a later patch, but if this is going to be part of a bisectable kernel tree, it'd be nice if this wasn't terribly leaky. :) This patch might profitably be merged with an earlier patch; it looks like a necessary bugfix for something earlier. Thanks signature.asc Description: Digital signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 10/36] apparmor: allow setting any profile into the unconfined state
On Wed, May 01, 2013 at 02:30:55PM -0700, John Johansen wrote: Allow emulating the default profile behavior from boot, by allowing loading of a profile in the unconfined state into a new NS. Signed-off-by: John Johansen john.johan...@canonical.com Acked-by: Seth Arnold seth.arn...@canonical.com ... with the caveat / note that the following hunk _may_ require userspace changes. (Those changes may already have been made.) index 69894ad..c69f7c4 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -510,12 +510,16 @@ static struct aa_profile *unpack_profile(struct aa_ext *e) goto fail; if (!unpack_u32(e, tmp, NULL)) goto fail; - if (tmp) + if (tmp PACKED_FLAG_HAT) profile-flags |= PFLAG_HAT; if (!unpack_u32(e, tmp, NULL)) goto fail; - if (tmp) + if (tmp == PACKED_MODE_COMPLAIN) profile-mode = APPARMOR_COMPLAIN; + else if (tmp == PACKED_MODE_KILL) + profile-mode = APPARMOR_KILL; + else if (tmp == PACKED_MODE_UNCONFINED) + profile-mode = APPARMOR_UNCONFINED; if (!unpack_u32(e, tmp, NULL)) goto fail; if (tmp) Thanks signature.asc Description: Digital signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 01/36] apparmor: provide base for multiple profiles to be replaced at once
On 05/01/2013 05:05 PM, Seth Arnold wrote: On Wed, May 01, 2013 at 02:30:46PM -0700, John Johansen wrote: previously profiles had to be loaded one at a time, which could result in cases where a replacement would partially succeed, and then fail resulting in inconsitent policy. Allow multiple profiles to replaced atomically so that the replacement either succeeeds or fails atomically for the set of profiles. Note: this does not provide multiple load of profiles when adding a parent and its children as an atomic profile set. Because of this limitation the atomic set load of profiles should not be exposed to userspace at this time. Signed-off-by: John Johansen john.johan...@canonical.com I'm having trouble reviewing this one, it all looks too familiar -- it all looks like 'normal', now. The only thing I spotted is that the description isn't true now, since you did get child profiles in the atomic load. thats interesting as this patch saw the largest overhaul of all the previous patches sent. Anyways thanks for catching the description problem, its fixed now. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor