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 > 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 > Signed-off-by: Konrad Rzeszutek Wilk > --- > 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 -- Doug Goldstein