[prev in list] [next in list] [prev in thread] [next in thread]
List: apparmor-dev
Subject: Re: [apparmor] =?utf-8?b?W3BhdGNoIDMvM10gcHJvZmlsZXM6IGFwYWNoZTIg4oCU?=
From: Steve Beattie <steve () nxnw ! org>
Date: 2014-06-23 22:52:53
Message-ID: 20140623225253.GJ3900 () nxnw ! org
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
[Attachment #4 (multipart/mixed)]
On Fri, Jun 20, 2014 at 09:31:36AM -0700, Kees Cook wrote:
> On Wed, Jun 18, 2014 at 11:29:31PM -0700, Seth Arnold wrote:
> > On Wed, Jun 18, 2014 at 05:44:05PM -0700, Steve Beattie wrote:
> > > This patch adds the abstractions/base abstraction to the
> > > HANDLING_UNTRUSTED_INPUT apache2 hat.
> > >
> > > [I dislike this because the idea for the HANDLING_UNTRUSTED_INPUT is
> > > that it is to be as minimal as possible, as sort of a poor man's
> > > privilege separation for when apache is parsing a request and
> > > determining what to do with it. The abstractions/base abstraction allows
> > > too much for such a hat IMO. (Honestly, I'd like cut down the existing
> > > allowed accesses in it.)]
> >
> > HANDLING_UNTRUSTED_INPUT has always had some unexpected consequences; I
> > love the idea but it just might not work with Apache's reality. Since we
> > don't have much chance of fixing reality and changing the module to do
> > something else is probably not going to happen soon, we might as well make
> > this as painless as possible.
> >
> > Also, agreed on cutting down on abstractions/base, but I'm so reluctant to
> > tighten shipped profiles.
>
> This part is a little weird, actually. So, my bug report didn't entirely
> match my patch for HANDLING_UNTRUSTED_INPUT. (In the report I say to add
> the signal handling only... but in the patch I end up adding base and
> apache2-common.) However, this probably doesn't matter much because the
> current default for HANDLING_UNTRUSTED_INPUT is:
>
> / rw,
> /** mrwlkix,
>
> Which is totally crazy if the goal is tight isolation (which I totally
> agree with and mentioned in the bug).
I realize I was unclear in my initial posting, but I meant cutting down
the accesses in HANDLING_UNTRUSTED_INPUT, not abstractions/base, though
in truth it could use a good review, too.
> I ran out of time trying to further analyze the needs of
> HANDLING_UNTRUSTED_INPUT, but it seems that any access needed during
> location resolution is needed (e.g. authentication modules), so it may make
> sense to extend this with a "local" include. In my case I had to explicitly
> allow "/run/saslauthd/mux rw," in apache2-common.
Looking over the 2.2->2,4 api changes for http_request [1] it seems
I missed the note about ap_hook_access_checker() being deprecated
and that ap_hook_check_access_ex()[2] and ap_hook_check_access()[3]
are the new hooks to use.
This can be seen by enabling mod_info and examining where mod_apparmor
ends up:
Check Access:
20 mod_authz_core.c
Check Access (legacy):
00 mod_apparmor.c
10 mod_access_compat.c
If we register with ap_hook_check_access_ex(), the ordering looks like:
Check Access:
00 mod_apparmor.c
20 mod_authz_core.c
Check Access (legacy):
10 mod_access_compat.c
(Registering with ap_hook_check_access() leaves us with the same
sequence as registering with ap_hook_access_checker().)
Kees, Christian, can you test one of the attached patches and
see if this reduces the amount of stuff that needs to show up in
HANDLING_UNTRUSTED_INPUT? The patches are on top of the previous
patches I've sent to the list, reordering the hat sequence, and for
the patch against trunk, after the cleanup patches.
Also, if you could enable mod_info[4] and report to me (privately,
if preferred) the output for your configurations so that I can have an
idea what the ordering looks like, that might help me diagnose things.
[1] http://httpd.apache.org/docs/2.4/developer/new_api_2_4.html#http_request
[2] http://ci.apache.org/projects/httpd/trunk/doxygen/group__APACHE__CORE__REQ.html#ga2a9dc3af1fbed52e36fc9e9386e4f919
[3] http://ci.apache.org/projects/httpd/trunk/doxygen/group__APACHE__CORE__REQ.html#ga342d354cf3541a6251d12eee6e9fe0c8
[4] http://httpd.apache.org/docs/2.4/mod/mod_info.html
--
Steve Beattie
<sbeattie@ubuntu.com>
http://NxNW.org/~steve/
["mod_apparmor-new-2.4-access-check.patch" (text/x-diff)]
Subject: For apache 2.4, use new access control hook
Use ap_hook_check_access_ex() instead of
ap_hook_access_checker() for apache 2.4; see
http://httpd.apache.org/docs/2.4/developer/new_api_2_4.html#http_request
Signed-off-by: Steve Beattie <steve@nxnw.org>
---
changehat/mod_apparmor/mod_apparmor.c | 8 ++++++++
1 file changed, 8 insertions(+)
Index: b/changehat/mod_apparmor/mod_apparmor.c
===================================================================
--- a/changehat/mod_apparmor/mod_apparmor.c
+++ b/changehat/mod_apparmor/mod_apparmor.c
@@ -404,7 +404,15 @@ register_hooks(apr_pool_t *p)
{
ap_hook_post_config(aa_init, NULL, NULL, APR_HOOK_MIDDLE);
ap_hook_child_init(aa_child_init, NULL, NULL, APR_HOOK_MIDDLE);
+
+#if AP_SERVER_MAJORVERSION_NUMBER == 2 && AP_SERVER_MINORVERSION_NUMBER < 3
+ /* Compatibility with apache 2.2 */
ap_hook_access_checker(aa_enter_hat, NULL, NULL, APR_HOOK_FIRST);
+#else
+ /* apache 2.4 mod_authz hook */
+ ap_hook_check_access_ex(aa_enter_hat, NULL, NULL, APR_HOOK_FIRST, AP_AUTH_INTERNAL_PER_CONF);
+#endif
+
/* ap_hook_post_read_request(aa_enter_hat, NULL, NULL, APR_HOOK_FIRST); */
ap_hook_log_transaction(aa_exit_hat, NULL, NULL, APR_HOOK_LAST);
}
["mod_apparmor-2.8-new-2.4-access-check.patch" (text/x-diff)]
Subject: [apparmor 2.8] For apache 2.4, use new access control hook
Use ap_hook_check_access_ex() instead of
ap_hook_access_checker() for apache 2.4; see
http://httpd.apache.org/docs/2.4/developer/new_api_2_4.html#http_request
Signed-off-by: Steve Beattie <steve@nxnw.org>
---
changehat/mod_apparmor/mod_apparmor.c | 8 ++++++++
1 file changed, 8 insertions(+)
Index: b/changehat/mod_apparmor/mod_apparmor.c
===================================================================
--- a/changehat/mod_apparmor/mod_apparmor.c
+++ b/changehat/mod_apparmor/mod_apparmor.c
@@ -355,7 +355,15 @@ register_hooks (apr_pool_t *p)
{
ap_hook_post_config (immunix_init, NULL, NULL, APR_HOOK_MIDDLE);
ap_hook_child_init (immunix_child_init, NULL, NULL, APR_HOOK_MIDDLE);
+
+#if AP_SERVER_MAJORVERSION_NUMBER == 2 && AP_SERVER_MINORVERSION_NUMBER < 3
+ /* Compatibility with apache 2.2 */
ap_hook_access_checker(immunix_enter_hat, NULL, NULL, APR_HOOK_FIRST);
+#else
+ /* apache 2.4 mod_authz hook */
+ ap_hook_check_access_ex(immunix_enter_hat, NULL, NULL, APR_HOOK_FIRST, AP_AUTH_INTERNAL_PER_CONF);
+#endif
+
/* ap_hook_post_read_request(immunix_enter_hat, NULL, NULL, APR_HOOK_FIRST); */
ap_hook_log_transaction(immunix_exit_hat, NULL, NULL, APR_HOOK_LAST);
}
["signature.asc" (application/pgp-signature)]
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic