[apparmor] [PATCH 23/32] apparmor: provide the ability to boot with a default profile set on init

[Seth Arnold]

On Wed, Jan 16, 2013 at 01:28:52PM -0800, John Johansen wrote:
> add the ability to boot with a basic default profile instead of the
> unconfined state.  This provides a way to provide total system confinement
> without having to load policy in the init ramfs.
> 
> The basic default profile can be replaced during early boot to achieve
> system confinement.
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> ---
>  security/apparmor/Kconfig            |   11 +++++++++++
>  security/apparmor/apparmorfs.c       |    4 ++++
>  security/apparmor/include/apparmor.h |    1 +
>  security/apparmor/include/policy.h   |    1 +
>  security/apparmor/lsm.c              |   17 ++++++++++++++---
>  security/apparmor/policy.c           |   21 +++++++++++++++++++++
>  security/apparmor/procattr.c         |    8 +++++---
>  7 files changed, 57 insertions(+), 6 deletions(-)

This was a lot simpler than I expected. Nice.

> diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig
> index 9b9013b..1108be7 100644
> --- 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 T.

"answer Y" :)

>  /**
>   * set_init_cxt - set a task context and profile on the first task.
> - *
> - * TODO: allow setting an alternate profile than unconfined
>   */
>  static int __init set_init_cxt(void)
>  {
> @@ -881,7 +884,15 @@ static int __init set_init_cxt(void)
>  	if (!cxt)
>  		return -ENOMEM;
>  
> -	cxt->profile = aa_get_profile(root_ns->unconfined);
> +	if (!aa_g_unconfined_init) {
> +		cxt->profile = aa_setup_default_profile();
> +		if (!cxt->profile) {
> +			aa_free_task_context(cxt);
> +			return -ENOMEM;
> +		}
> +		/* fs setup of default is done in aa_create_aafs() */
> +	} else
> +		cxt->profile = aa_get_profile(root_ns->unconfined);
>  	cred_cxt(cred) = cxt;

What's the consequences of returning -ENOMEM here? I traced it back up
through security_initcall() and gave up when it looked like magic kernel
magic invoked it through magic. :)

Does it fail to boot? (Secure.) Does it boot anyhow?

> +/**
> + * aa_setup_default_profile - create the initial default profile
> + */
> +struct aa_profile *aa_setup_default_profile(void)
> +{
> +	struct aa_profile *profile = aa_alloc_profile("default");
> +	if (!profile)
> +		return NULL;
> +
> +	/* the default profile pretends to be unconfined until it is replaced */
> +	profile->flags = PFLAG_UNCONFINED | PFLAG_IX_ON_NAME_ERROR;
> +
> +	profile->ns = aa_get_namespace(root_ns);

Something seems a bit funny about incrementing the unconfined reference
count to hold the namespace for the default profile which replaces the
unconfined profile. :) (Don't mind me, I just think it's funny.)

> diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
> index 6c93901..39ce95a 100644
> --- a/security/apparmor/procattr.c
> +++ b/security/apparmor/procattr.c
> @@ -41,6 +41,7 @@ int aa_getprocattr(struct aa_profile *profile, char **string)
>  	const char *ns_name = NULL;
>  	struct aa_namespace *ns = profile->ns;
>  	struct aa_namespace *current_ns = __aa_current_profile()->ns;
> +	bool unconfined;
>  	char *s;
>  
>  	if (!aa_ns_visible(current_ns, ns))
> @@ -53,8 +54,9 @@ int aa_getprocattr(struct aa_profile *profile, char **string)
>  	if (ns_len)
>  		ns_len += 4;
>  
> -	/* unconfined profiles don't have a mode string appended */
> -	if (!unconfined(profile))
> +	/* 'unconfined' profile don't have a mode string appended */
> +	unconfined = unconfined(profile) && profile == profile->ns->unconfined;
> +	if (!unconfined)
>  		mode_len = strlen(mode_str) + 3;	/* + 3 for _() */

Oh, this is a bit unfortunate. (Also, && feels wrong, should it be
||?) Can unconfined() be extended to handle this case? Would it be
wrong elsewhere?

>  	name_len = strlen(profile->base.hname);
> @@ -68,7 +70,7 @@ int aa_getprocattr(struct aa_profile *profile, char **string)
>  		sprintf(s, ":%s://", ns_name);
>  		s += ns_len;
>  	}
> -	if (unconfined(profile))
> +	if (unconfined)
>  		/* mode string not being appended */
>  		sprintf(s, "%s\n", profile->base.hname);
>  	else

Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130130/9380054f/attachment.pgp>