| Message ID | 20160310171059.GA32334@char.us.oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 3/10/16 11:10 AM, Konrad Rzeszutek Wilk wrote: > On Wed, Mar 09, 2016 at 08:40:05PM -0600, Doug Goldstein wrote: >> On 3/9/16 4:09 PM, Daniel De Graaf wrote: >>> On 03/09/2016 04:17 PM, Konrad Rzeszutek Wilk wrote: >>>> On Wed, Mar 09, 2016 at 01:24:15PM +0000, Andrew Cooper wrote: >>>>> On 09/03/16 01:51, Konrad Rzeszutek Wilk wrote: >>>>>> Hey, >>>>>> >>>>>> I was wondering if it we should change the default flask_bootparam >>>>>> option from permissive to disabled? >> >>>>> >>>>> By the looks of it, "permissive" shouldn't be an available option at >>>>> all. >>> >>> Permissive is meant for developing (or debugging) a disaggregated system, >>> where the restrictions on non-dom0 would also break the system. However, >>> I agree that it needs to be harder to end up in this mode by accident. >>> >>> The simplest solution in my opinion is to change the boot parameter to >>> default to "flask=enforcing", which will fail the boot if a policy is >>> not available prior to dom0 creation. This would require any setup >>> where the policy is loaded from userspace to explicitly specify >>> "flask=late", whereas they can currently get away with no parameter. >>> >>> Another solution would be to default to "flask=late" and either deny the >>> creation of domains if a policy is not present, or automatically revert >>> to the dummy module on domain creation with no loaded policy. The latter >>> probably deserves a different name ("flask=auto"?). >>> >> >> Honestly I'm in favor of secure by default approach. Since Xen is not >> built with flask by default to me the sane approach would be to default >> the system to "flask=enforcing". >> >> "flask=late" not allowing the creation of domains sounds good but what >> if you're using a disaggregated dom0 with some domDs and one of them >> needs to be up to fetch your policy? Just a hypothetical. >> >> XSMs like LSMs just aren't meant to be swapped around at runtime and >> like Daniel points out if go down the road of swapping to the dummy >> module there could be further dragons and whose to say someone won't >> look at that and put something in that allows you to switch to another >> later on (yes I know there's only really 1 but I'm speaking of the >> hypothetical). > > > I presume this patch would be to folks +1: > > From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001 > From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Date: Thu, 10 Mar 2016 12:05:29 -0500 > Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode. > > By default the mode was 'permissive' which is "meant for > developing (or debugging) a disaggregated system, > where the restrictions on non-dom0 would also break the system." > > However this default mode made it possible to boot an machine > in this state if a policy file during bootup was not provided. > > The end was less secure than with XSM-enabled - any guest > could do any operation (including rebooting the machine). > > Alternative solutions such as switching from flask to dummy. > However "The main issue with starting with dummy and then > switching to FLASK is that any domains created while using > the dummy policy won't have flask_domain_alloc_security called > to populate domain->ssid, and the rest of the flask code relies > on this being non-NULL. The same would be true for event channels, > but inlining the field to save space makes that a non-issue." > > (both excerpts are from Daniel De Graaf emails). > > This is a much easier fix. > > Suggested-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > docs/misc/xen-command-line.markdown | 2 +- > xen/xsm/flask/flask_op.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown > index ca77e3b..9e77f8a 100644 > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -662,7 +662,7 @@ to use the default. > ### flask > > `= permissive | enforcing | late | disabled` > > -> Default: `permissive` > +> Default: `enforcing` > > Specify how the FLASK security server should be configured. This option is only > available if the hypervisor was compiled with XSM support (which can be enabled > diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c > index f4f5dd1..aaed75d 100644 > --- a/xen/xsm/flask/flask_op.c > +++ b/xen/xsm/flask/flask_op.c > @@ -25,7 +25,7 @@ > #define _copy_to_guest copy_to_guest > #define _copy_from_guest copy_from_guest > > -enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE; > +enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING; > static void parse_flask_param(char *s); > custom_param("flask", parse_flask_param); > > +1 You also found a spot that I didn't find when doing the Kconfig conversion of CONFIG_XSM / CONFIG_FLASK. My search didn't catch XSM\_ENABLE so follow on patch from me for that will be coming. Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
On 10/03/16 17:10, Konrad Rzeszutek Wilk wrote: > I presume this patch would be to folks +1: > > From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001 > From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Date: Thu, 10 Mar 2016 12:05:29 -0500 > Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode. > > By default the mode was 'permissive' which is "meant for > developing (or debugging) a disaggregated system, > where the restrictions on non-dom0 would also break the system." > > However this default mode made it possible to boot an machine > in this state if a policy file during bootup was not provided. > > The end was less secure than with XSM-enabled - any guest > could do any operation (including rebooting the machine). > > Alternative solutions such as switching from flask to dummy. > However "The main issue with starting with dummy and then > switching to FLASK is that any domains created while using > the dummy policy won't have flask_domain_alloc_security called > to populate domain->ssid, and the rest of the flask code relies > on this being non-NULL. The same would be true for event channels, > but inlining the field to save space makes that a non-issue." > > (both excerpts are from Daniel De Graaf emails). > > This is a much easier fix. > > Suggested-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > docs/misc/xen-command-line.markdown | 2 +- > xen/xsm/flask/flask_op.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown > index ca77e3b..9e77f8a 100644 > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -662,7 +662,7 @@ to use the default. > ### flask > > `= permissive | enforcing | late | disabled` > > -> Default: `permissive` > +> Default: `enforcing` > > Specify how the FLASK security server should be configured. This option is only > available if the hypervisor was compiled with XSM support (which can be enabled > diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c > index f4f5dd1..aaed75d 100644 > --- a/xen/xsm/flask/flask_op.c > +++ b/xen/xsm/flask/flask_op.c > @@ -25,7 +25,7 @@ > #define _copy_to_guest copy_to_guest > #define _copy_from_guest copy_from_guest > > -enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE; > +enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING; > static void parse_flask_param(char *s); > custom_param("flask", parse_flask_param); >
Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] XSM permissive by default."):
> I presume this patch would be to folks +1:
>
> From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Thu, 10 Mar 2016 12:05:29 -0500
> Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode.
This has a Reviewed-by from Doug Goldstein and Andrew Cooper. And
then it was revised by Daniel. But AFAICT it has not yet been
committed. I think this is an important change that is in 4.7.
Is there some reason why this patch hasn't been committed ?
Thanks,
Ian.
>>> On 04.04.16 at 19:12, <Ian.Jackson@eu.citrix.com> wrote: > Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] XSM permissive by default."): >> I presume this patch would be to folks +1: >> >> From 3373a50f386b41eea6ecede4b430e4fa09b2fe7e Mon Sep 17 00:00:00 2001 >> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >> Date: Thu, 10 Mar 2016 12:05:29 -0500 >> Subject: [PATCH] flask: By default be in FLASK_BOOTPARAM_ENFORCING mode. > > This has a Reviewed-by from Doug Goldstein and Andrew Cooper. And > then it was revised by Daniel. But AFAICT it has not yet been > committed. I think this is an important change that is in 4.7. > > Is there some reason why this patch hasn't been committed ? There are two reasons: The thread here isn't rooting at a patch submission, i.e. to me looks more like a discussion (and as such isn't even a possible subject for queuing). And then I can't find any ack from Daniel. Jan
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index ca77e3b..9e77f8a 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -662,7 +662,7 @@ to use the default. ### flask > `= permissive | enforcing | late | disabled` -> Default: `permissive` +> Default: `enforcing` Specify how the FLASK security server should be configured. This option is only available if the hypervisor was compiled with XSM support (which can be enabled diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c index f4f5dd1..aaed75d 100644 --- a/xen/xsm/flask/flask_op.c +++ b/xen/xsm/flask/flask_op.c @@ -25,7 +25,7 @@ #define _copy_to_guest copy_to_guest #define _copy_from_guest copy_from_guest -enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE; +enum flask_bootparam_t __read_mostly flask_bootparam = FLASK_BOOTPARAM_ENFORCING; static void parse_flask_param(char *s); custom_param("flask", parse_flask_param);