| Message ID | 1457634629-28324-1-git-send-email-dgdegra@tycho.nsa.gov (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 10, 2016 at 01:30:29PM -0500, Daniel De Graaf wrote: I've added Ian and Jan on the email as scripts/get_maintainer.pl spits out their names (Oddly not yours?) > The previous default of "permissive" is meant for developing or > debugging a disaggregated system. However, this default makes it too > easy to accidentally boot a machine in this state, which does not place > any restrictions on guests. This is not suitable for normal systems > because any guest can perform any operation (including operations like > rebooting the machine, kexec, and reading or writing another domain's > memory). > > This change will cause the boot to fail if you do not specify an XSM > policy during boot; if you need to load a policy from dom0, use the > "flask=late" boot parameter. > > Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified > to also change the default value of flask_enforcing so that the policy > is not still in permissive mode. This also removes the (no longer > documented) command line argument directly changing that variable since > it has been superseded by the flask= parameter. > Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> .. however: > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > --- > > docs/misc/xen-command-line.markdown | 2 +- > docs/misc/xsm-flask.txt | 12 ++++++------ > xen/xsm/flask/flask_op.c | 8 +++++--- > 3 files changed, 12 insertions(+), 10 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/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt > index fb2fe9f..00a2b13 100644 > --- a/docs/misc/xsm-flask.txt > +++ b/docs/misc/xsm-flask.txt > @@ -283,12 +283,12 @@ for passthrough, run: > > This command must be rerun on each boot or after any policy reload. > > -The example policy was only tested with simple domain creation and may be > -missing rules allowing accesses by dom0 or domU when a number of hypervisor > -features are used. When first loading or writing a policy, you should run FLASK > -in permissive mode (the default) and check the Xen logs (xl dmesg) for AVC > -denials before using it in enforcing mode (flask_enforcing=1 on the command > -line, or xl setenforce). > +When first loading or writing a policy, you should run FLASK in permissive mode > +(flask=permissive on the command line) and check the Xen logs (xl dmesg) for AVC > +denials before using it in enforcing mode (the default value of the boot > +parameter, which can also be changed using xl setenforce). When using the > +default types for domains (domU_t), the example policy shipped with Xen should > +allow the same operations on or between domains as when not using FLASK. > > > MLS/MCS policy > diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c > index f4f5dd1..cdb462c 100644 > --- a/xen/xsm/flask/flask_op.c > +++ b/xen/xsm/flask/flask_op.c > @@ -25,12 +25,11 @@ > #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); > > -bool_t __read_mostly flask_enforcing = 0; > -boolean_param("flask_enforcing", flask_enforcing); > +bool_t __read_mostly flask_enforcing = 1; Since you set that to the default value should the parse_flask_param 'flask_enforcing = 1' for the 'enforcing' and 'late' be removed? (If you agree, the committer could do it). > > #define MAX_POLICY_SIZE 0x4000000 > > @@ -76,7 +75,10 @@ static void __init parse_flask_param(char *s) > else if ( !strcmp(s, "disabled") ) > flask_bootparam = FLASK_BOOTPARAM_DISABLED; > else if ( !strcmp(s, "permissive") ) > + { > + flask_enforcing = 0; > flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE; > + } > else > flask_bootparam = FLASK_BOOTPARAM_INVALID; > } > -- > 2.5.0 >
On 03/10/2016 02:12 PM, Konrad Rzeszutek Wilk wrote: > On Thu, Mar 10, 2016 at 01:30:29PM -0500, Daniel De Graaf wrote: > > I've added Ian and Jan on the email as scripts/get_maintainer.pl spits out > their names (Oddly not yours?) >> The previous default of "permissive" is meant for developing or >> debugging a disaggregated system. However, this default makes it too >> easy to accidentally boot a machine in this state, which does not place >> any restrictions on guests. This is not suitable for normal systems >> because any guest can perform any operation (including operations like >> rebooting the machine, kexec, and reading or writing another domain's >> memory). >> >> This change will cause the boot to fail if you do not specify an XSM >> policy during boot; if you need to load a policy from dom0, use the >> "flask=late" boot parameter. >> >> Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified >> to also change the default value of flask_enforcing so that the policy >> is not still in permissive mode. This also removes the (no longer >> documented) command line argument directly changing that variable since >> it has been superseded by the flask= parameter. >> > > Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > .. however: > [...] > > Since you set that to the default value should the parse_flask_param > 'flask_enforcing = 1' for the 'enforcing' and 'late' be removed? > > (If you agree, the committer could do it). Sure. I left them in so that a command line such as "flask=permissive flask=enforcing" would do the right thing, but I haven't checked that that is even possible.
>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote: > This change will cause the boot to fail if you do not specify an XSM > policy during boot; if you need to load a policy from dom0, use the > "flask=late" boot parameter. And what mode is the system in until that happens? From the command line doc, I understand it would be in not-enforcing mode, but that seems contrary to the code (already before your change) setting flask_enforcing to 1 in that case. > Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified > to also change the default value of flask_enforcing so that the policy > is not still in permissive mode. This also removes the (no longer > documented) command line argument directly changing that variable since > it has been superseded by the flask= parameter. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Question now is - who is to be considered the author of this patch? Jan
On Fri, Mar 11, 2016 at 02:07:11AM -0700, Jan Beulich wrote: > >>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote: > > This change will cause the boot to fail if you do not specify an XSM > > policy during boot; if you need to load a policy from dom0, use the > > "flask=late" boot parameter. > > And what mode is the system in until that happens? From the > command line doc, I understand it would be in not-enforcing > mode, but that seems contrary to the code (already before > your change) setting flask_enforcing to 1 in that case. > > > Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified > > to also change the default value of flask_enforcing so that the policy > > is not still in permissive mode. This also removes the (no longer > > documented) command line argument directly changing that variable since > > it has been superseded by the flask= parameter. > > > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Question now is - who is to be considered the author of this patch? Daniel.
On 03/11/2016 04:07 AM, Jan Beulich wrote: >>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote: >> This change will cause the boot to fail if you do not specify an XSM >> policy during boot; if you need to load a policy from dom0, use the >> "flask=late" boot parameter. > > And what mode is the system in until that happens? From the > command line doc, I understand it would be in not-enforcing > mode, but that seems contrary to the code (already before > your change) setting flask_enforcing to 1 in that case. The FLASK code does not deny any actions until a policy has been loaded, so the flask_enforcing value only takes effect then. With flask=late, userspace code can also adjust the value (xl setenforce) before loading the policy.
>>> On 11.03.16 at 16:39, <dgdegra@tycho.nsa.gov> wrote: > On 03/11/2016 04:07 AM, Jan Beulich wrote: >>>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote: >>> This change will cause the boot to fail if you do not specify an XSM >>> policy during boot; if you need to load a policy from dom0, use the >>> "flask=late" boot parameter. >> >> And what mode is the system in until that happens? From the >> command line doc, I understand it would be in not-enforcing >> mode, but that seems contrary to the code (already before >> your change) setting flask_enforcing to 1 in that case. > > The FLASK code does not deny any actions until a policy has been loaded, > so the flask_enforcing value only takes effect then. With flask=late, > userspace code can also adjust the value (xl setenforce) before loading > the policy. So doesn't this leave the system again in an insecure state then? Jan
On 03/11/2016 10:43 AM, Jan Beulich wrote: >>>> On 11.03.16 at 16:39, <dgdegra@tycho.nsa.gov> wrote: >> On 03/11/2016 04:07 AM, Jan Beulich wrote: >>>>>> On 10.03.16 at 19:30, <dgdegra@tycho.nsa.gov> wrote: >>>> This change will cause the boot to fail if you do not specify an XSM >>>> policy during boot; if you need to load a policy from dom0, use the >>>> "flask=late" boot parameter. >>> >>> And what mode is the system in until that happens? From the >>> command line doc, I understand it would be in not-enforcing >>> mode, but that seems contrary to the code (already before >>> your change) setting flask_enforcing to 1 in that case. >> >> The FLASK code does not deny any actions until a policy has been loaded, >> so the flask_enforcing value only takes effect then. With flask=late, >> userspace code can also adjust the value (xl setenforce) before loading >> the policy. > > So doesn't this leave the system again in an insecure state then? > > Jan It does, at least until the policy is loaded. The point of "flask=late" is that dom0 loads the policy early in boot, preferably before creating any domains. Since all xen architectures now support loading the policy from the bootloader, this is never a required mode of operation. We did discuss preventing the creation of domains without a policy loaded to avoid making this mistake, but since this is no longer the default, I don't think that type of guard isnecessary.
-----Original Message----- From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Konrad Rzeszutek Wilk Sent: 10 March 2016 19:12 To: Daniel De Graaf <dgdegra@tycho.nsa.gov>; Ian Jackson <Ian.Jackson@citrix.com>; jbeulich@suse.com Cc: xen-devel@lists.xenproject.org; cardoe@cardoe.com; Andrew Cooper <Andrew.Cooper3@citrix.com> Subject: Re: [Xen-devel] [PATCH] flask: change default state to enforcing On Thu, Mar 10, 2016 at 01:30:29PM -0500, Daniel De Graaf wrote: I've added Ian and Jan on the email as scripts/get_maintainer.pl spits out their names (Oddly not yours?) > The previous default of "permissive" is meant for developing or > debugging a disaggregated system. However, this default makes it too > easy to accidentally boot a machine in this state, which does not > place any restrictions on guests. This is not suitable for normal > systems because any guest can perform any operation (including > operations like rebooting the machine, kexec, and reading or writing > another domain's memory). > > This change will cause the boot to fail if you do not specify an XSM > policy during boot; if you need to load a policy from dom0, use the > "flask=late" boot parameter. > > Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified > to also change the default value of flask_enforcing so that the policy > is not still in permissive mode. This also removes the (no longer > documented) command line argument directly changing that variable > since it has been superseded by the flask= parameter. > Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> .. however: > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > --- > > docs/misc/xen-command-line.markdown | 2 +- > docs/misc/xsm-flask.txt | 12 ++++++------ > xen/xsm/flask/flask_op.c | 8 +++++--- > 3 files changed, 12 insertions(+), 10 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/docs/misc/xsm-flask.txt > b/docs/misc/xsm-flask.txt index fb2fe9f..00a2b13 100644 > --- a/docs/misc/xsm-flask.txt > +++ b/docs/misc/xsm-flask.txt > @@ -283,12 +283,12 @@ for passthrough, run: > > This command must be rerun on each boot or after any policy reload. > > -The example policy was only tested with simple domain creation and > may be -missing rules allowing accesses by dom0 or domU when a number > of hypervisor -features are used. When first loading or writing a > policy, you should run FLASK -in permissive mode (the default) and > check the Xen logs (xl dmesg) for AVC -denials before using it in > enforcing mode (flask_enforcing=1 on the command -line, or xl setenforce). > +When first loading or writing a policy, you should run FLASK in > +permissive mode (flask=permissive on the command line) and check the > +Xen logs (xl dmesg) for AVC denials before using it in enforcing mode > +(the default value of the boot parameter, which can also be changed > +using xl setenforce). When using the default types for domains > +(domU_t), the example policy shipped with Xen should allow the same operations on or between domains as when not using FLASK. > > > MLS/MCS policy > diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c index > f4f5dd1..cdb462c 100644 > --- a/xen/xsm/flask/flask_op.c > +++ b/xen/xsm/flask/flask_op.c > @@ -25,12 +25,11 @@ > #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); > > -bool_t __read_mostly flask_enforcing = 0; > -boolean_param("flask_enforcing", flask_enforcing); > +bool_t __read_mostly flask_enforcing = 1; Since you set that to the default value should the parse_flask_param 'flask_enforcing = 1' for the 'enforcing' and 'late' be removed? (If you agree, the committer could do it). > > #define MAX_POLICY_SIZE 0x4000000 > > @@ -76,7 +75,10 @@ static void __init parse_flask_param(char *s) > else if ( !strcmp(s, "disabled") ) > flask_bootparam = FLASK_BOOTPARAM_DISABLED; > else if ( !strcmp(s, "permissive") ) > + { > + flask_enforcing = 0; > flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE; > + } > else > flask_bootparam = FLASK_BOOTPARAM_INVALID; } > -- > 2.5.0 > There is no need to explicitly set flask_enforcing=0. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
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/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt index fb2fe9f..00a2b13 100644 --- a/docs/misc/xsm-flask.txt +++ b/docs/misc/xsm-flask.txt @@ -283,12 +283,12 @@ for passthrough, run: This command must be rerun on each boot or after any policy reload. -The example policy was only tested with simple domain creation and may be -missing rules allowing accesses by dom0 or domU when a number of hypervisor -features are used. When first loading or writing a policy, you should run FLASK -in permissive mode (the default) and check the Xen logs (xl dmesg) for AVC -denials before using it in enforcing mode (flask_enforcing=1 on the command -line, or xl setenforce). +When first loading or writing a policy, you should run FLASK in permissive mode +(flask=permissive on the command line) and check the Xen logs (xl dmesg) for AVC +denials before using it in enforcing mode (the default value of the boot +parameter, which can also be changed using xl setenforce). When using the +default types for domains (domU_t), the example policy shipped with Xen should +allow the same operations on or between domains as when not using FLASK. MLS/MCS policy diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c index f4f5dd1..cdb462c 100644 --- a/xen/xsm/flask/flask_op.c +++ b/xen/xsm/flask/flask_op.c @@ -25,12 +25,11 @@ #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); -bool_t __read_mostly flask_enforcing = 0; -boolean_param("flask_enforcing", flask_enforcing); +bool_t __read_mostly flask_enforcing = 1; #define MAX_POLICY_SIZE 0x4000000 @@ -76,7 +75,10 @@ static void __init parse_flask_param(char *s) else if ( !strcmp(s, "disabled") ) flask_bootparam = FLASK_BOOTPARAM_DISABLED; else if ( !strcmp(s, "permissive") ) + { + flask_enforcing = 0; flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE; + } else flask_bootparam = FLASK_BOOTPARAM_INVALID; }
The previous default of "permissive" is meant for developing or debugging a disaggregated system. However, this default makes it too easy to accidentally boot a machine in this state, which does not place any restrictions on guests. This is not suitable for normal systems because any guest can perform any operation (including operations like rebooting the machine, kexec, and reading or writing another domain's memory). This change will cause the boot to fail if you do not specify an XSM policy during boot; if you need to load a policy from dom0, use the "flask=late" boot parameter. Originally by Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; modified to also change the default value of flask_enforcing so that the policy is not still in permissive mode. This also removes the (no longer documented) command line argument directly changing that variable since it has been superseded by the flask= parameter. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- docs/misc/xen-command-line.markdown | 2 +- docs/misc/xsm-flask.txt | 12 ++++++------ xen/xsm/flask/flask_op.c | 8 +++++--- 3 files changed, 12 insertions(+), 10 deletions(-)