diff mbox

Is:livepatch-build-tools.git declare it supported? Was:Re: [PATCH for-4.9] livepatch: Declare live patching as a supported feature

Message ID 20170829144414.GA5224@x230.dumpdata.com (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk Aug. 29, 2017, 2:44 p.m. UTC 
.giant snip..
> Indeed; and as I think I said before, I think we need to move forward
> with getting a statement on livepatching in, and since most of the
> voices involved in this conversation seem to be in favor of saying
> livepatch-tools are *not* supported, I won't object. I'm only still

Thank you.

As such, here is the patch. Would folks like me to repost it, or
OK with Acking/Reviewing it as such?

I think the point 3) succinctly explains the position that has been so hotly
debated. I can of course expand it, but not sure if it makes sense?



From f968f003d36ac2b9d1b670b34bbe2a1222830138 Mon Sep 17 00:00:00 2001
From: Ross Lagerwall <ross.lagerwall@citrix.com>
Date: Wed, 28 Jun 2017 17:13:44 +0100
Subject: [PATCH v3] livepatch: Declare live patching as a supported feature

See docs/features/livepatch.pandoc for the details.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

--
v2:
 - Moved it into a feature document.
 - Clarified a few bits and pieces based on feedback.
v3:
 - default X86
---
 docs/features/livepatch.pandoc | 103 +++++++++++++++++++++++++++++++++++++++++
 xen/common/Kconfig             |   4 +-
 2 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 docs/features/livepatch.pandoc

Comments

Andrew Cooper Aug. 29, 2017, 2:46 p.m. UTC | #1 
On 29/08/17 15:44, Konrad Rzeszutek Wilk wrote:
> .giant snip..
>> Indeed; and as I think I said before, I think we need to move forward
>> with getting a statement on livepatching in, and since most of the
>> voices involved in this conversation seem to be in favor of saying
>> livepatch-tools are *not* supported, I won't object. I'm only still
> Thank you.
>
> As such, here is the patch. Would folks like me to repost it, or
> OK with Acking/Reviewing it as such?
>
> I think the point 3) succinctly explains the position that has been so hotly
> debated. I can of course expand it, but not sure if it makes sense?
>
>
>
> From f968f003d36ac2b9d1b670b34bbe2a1222830138 Mon Sep 17 00:00:00 2001
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
> Date: Wed, 28 Jun 2017 17:13:44 +0100
> Subject: [PATCH v3] livepatch: Declare live patching as a supported feature
>
> See docs/features/livepatch.pandoc for the details.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> --
> v2:
>  - Moved it into a feature document.
>  - Clarified a few bits and pieces based on feedback.
> v3:
>  - default X86
> ---
>  docs/features/livepatch.pandoc | 103 +++++++++++++++++++++++++++++++++++++++++
>  xen/common/Kconfig             |   4 +-
>  2 files changed, 105 insertions(+), 2 deletions(-)
>  create mode 100644 docs/features/livepatch.pandoc
>
> diff --git a/docs/features/livepatch.pandoc b/docs/features/livepatch.pandoc
> new file mode 100644
> index 0000000000..faaf2d1e77
> --- /dev/null
> +++ b/docs/features/livepatch.pandoc
> @@ -0,0 +1,103 @@
> +% Live Patching
> +% Revision 1
> +
> +\clearpage
> +
> +# Basics
> +
> +---------------- ----------------------------------------------------
> +         Status: **Supported**
> +
> +   Architecture: x86

Sorry to only be noticing this now.

This should read Supported for x86, Tech Preview/Experimental (as
appropriate) for ARM.

~Andrew

> +
> +      Component: Hypervisor, toolstack
> +---------------- ----------------------------------------------------
George Dunlap Aug. 29, 2017, 2:48 p.m. UTC | #2 
On 08/29/2017 03:44 PM, Konrad Rzeszutek Wilk wrote:
> .giant snip..
>> Indeed; and as I think I said before, I think we need to move forward
>> with getting a statement on livepatching in, and since most of the
>> voices involved in this conversation seem to be in favor of saying
>> livepatch-tools are *not* supported, I won't object. I'm only still
> 
> Thank you.
> 
> As such, here is the patch. Would folks like me to repost it, or
> OK with Acking/Reviewing it as such?
> 
> I think the point 3) succinctly explains the position that has been so hotly
> debated. I can of course expand it, but not sure if it makes sense?

I'd prefer to remove a justification I believe to be unreasonable, and
just say what the support status is:

> +3) Bugs in livepatch-build-tools creating an incorrect live patch that
> +   results in an insecure host:
> +   If livepatch-build-tools creates an incorrect live patch that
> +   results in an insecure host, this shall not be considered a security
> +   issue. A live patch should be checked to verify that it is valid
> +   before loading.

Is that OK with everyone?

 -George
diff mbox

Patch

diff --git a/docs/features/livepatch.pandoc b/docs/features/livepatch.pandoc
new file mode 100644
index 0000000000..faaf2d1e77
--- /dev/null
+++ b/docs/features/livepatch.pandoc
@@ -0,0 +1,103 @@ 
+% Live Patching
+% Revision 1
+
+\clearpage
+
+# Basics
+
+---------------- ----------------------------------------------------
+         Status: **Supported**
+
+   Architecture: x86
+
+      Component: Hypervisor, toolstack
+---------------- ----------------------------------------------------
+
+
+# Details
+
+Xen Live Patching has been available as tech preview feature since Xen
+4.7 and has now had a couple of releases to stabilize. Xen Live patching
+has been used by multiple vendors to fix several real-world security
+issues without any severe bugs encountered. Additionally, there are now
+tests in OSSTest that test live patching to ensure that no regressions
+are introduced.
+
+Based on the amount of testing and usage it has had, we are ready to
+declare live patching as a 'Supported' feature on x86.
+
+Live patching is slightly peculiar when it comes to support because it
+allows the host administrator to break their system rather easily
+depending on the content of the live patch. Because of this, it is
+worth detailing the scope of security support:
+
+1) Unprivileged access to live patching operations:
+   Live patching operations should only be accessible to privileged
+   guests and it shall be treated as a security issue if this is not
+   the case.
+
+2) Bugs in the patch-application code such that vulnerabilities exist
+   after application:
+   If a correct live patch is loaded but it is not applied correctly
+   such that it might result in an insecure system (e.g. not all
+   functions are patched), it shall be treated as a security issue.
+
+3) Bugs in livepatch-build-tools creating an incorrect live patch that
+   results in an insecure host:
+   If livepatch-build-tools creates an incorrect live patch that
+   results in an insecure host, this shall not be considered a security
+   issue. There are too many OSes and toolchains to consider supporting
+   this. A live patch should be checked to verify that it is valid
+   before loading.
+
+4) Loading an incorrect live patch that results in an insecure host or
+   host crash:
+   If a live patch (whether created using livepatch-build-tools or some
+   alternative) is loaded and it results in an insecure host or host
+   crash due to the content of the live patch being incorrect or the
+   issue being inappropriate to live patch, this is not considered as a
+   security issue.
+
+5) Bugs in the live patch parsing code (the ELF loader):
+   Bugs in the live patch parsing code such as out-of-bounds reads
+   caused by invalid ELF files are not considered to be security issues
+   because the it can only be triggered by a privileged domain.
+
+6) Bugs which allow a guest to prevent the application of a livepatch:
+   A guest should not be able to prevent the application of a live
+   patch. If an unprivileged guest can somehow prevent the application
+   of a live patch despite pausing it (xl pause ...), it shall be
+   treated as a security issue.
+
+Note: It is expected that live patches are tested in a test environment
+before being used in production to avoid unexpected issues. In
+particular, to avoid the issues described by (3), (4), & (5).
+
+There are also some generic security questions which are worth asking:
+
+1) Is guest->host privilege escalation possible?
+
+The new live patching sysctl subops are only accessible to privileged
+domains and this is tested by OSSTest with an XTF test.
+There is a caveat -- an incorrect live patch can introduce a guest->host
+privilege escalation.
+
+2) Is guest user->guest kernel escalation possible?
+
+No, although an incorrect live patch can introduce a guest user->guest
+kernel privilege escalation.
+
+3) Is there any information leakage?
+
+The new live patching sysctl subops are only accessible to privileged
+domains so it is not possible for an unprivileged guest to access the
+list of loaded live patches. This is tested by OSSTest with an XTF test.
+There is a caveat -- an incorrect live patch can introduce an
+information leakage.
+
+4) Can a Denial-of-Service be triggered?
+
+There are no known ways that an unprivileged guest can prevent a live
+patch from being loaded.
+Once again, there is a caveat that an incorrect live patch can introduce
+an arbitrary denial of service.
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index dc8e876439..e9bb849298 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -226,8 +226,8 @@  config CRYPTO
 	bool
 
 config LIVEPATCH
-	bool "Live patching support (TECH PREVIEW)"
-	default n
+	bool "Live patching support"
+	default X86
 	depends on HAS_BUILD_ID = "y"
 	---help---
 	  Allows a running Xen hypervisor to be dynamically patched using