[prev in list] [next in list] [prev in thread] [next in thread]
List: qubes-devel
Subject: [qubes-devel] Re: [Xen-devel] Critique of the Xen Security Process
From: chris <tknchris () gmail ! com>
Date: 2015-11-09 22:00:57
Message-ID: CAKnNFz_8HzM6ePPinOwQTHT9nx_fEvMOfP2qp1XhGYZ670FcNg () mail ! gmail ! com
[Download RAW message or body]
+1... so many great points here that ive thought many times its almost as
if i could have written it
great post!
chris
On Fri, Nov 6, 2015 at 12:22 PM, Joanna Rutkowska <
joanna@invisiblethingslab.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hello,
>
> Recently Xen has released the XSA-148 advisory [1] addressing a fatal bug
> in the
> hypervisor. The bug has been lurking there for the last 7 years! We, the
> Qubes
> OS Project, have commented on this in our Security Bulletin #22 [2]. And
> far
> from enthusiastic commentary that was (FWIW, it was me who wrote this QSB,
> as
> evidenced in the commits log, in case some from the Xen community would
> like to
> direct their rage towards a particular human being ;) Ian Jackson then
> wrote a
> response on the Xen blog [3]. I was then asked to share some more thoughts
> about
> how I thought Xen could actually improve its security process [4]. So, I
> share
> some these below:
>
> 1. First of all, I wish Xen was somehow more defensively coded. To provide
> some
> examples:
>
> a. In XSA-109 [5] there was a problem with the hypervisor dereferencing a
> NULL
> pointer. The problem was fixed by the Xen Security Team by applying a patch
> which (hopefully) made sure the execution path that lead to this NULL
> pointer
> dereferencing code was never taken. Back then I suggested (on the Xen
> pre-disclosure list) to make this patch more explicit though:
>
> > On Wed, Jan 21, 2015 at 02:31:51PM +0100, Joanna Rutkowska wrote:
> > (...)
> > >
> > > Wouldn't it be prudent to also check if:
> > >
> > > (v->arch.paging.mode>{write_guest_entry,cmpxchg_guest_entry} != NULL)
> > >
> > > ... in the two affected functions, just before derefing these function
> > > pointers?
> > >
> > > Going even a step further: how about replacing all
> > > function-pointer-based calls with macros that first validates the
> > > pointer before derefing it? At least when the system doesn't have SMEP?
> > >
>
> ...to which I got a reply from one of the Xen Security Team engineers that
> the
> above might perhaps be justified in debug builds only, followed by a
> standard:
> "feel free to contribute a patch".
>
> b. The XSA-123 [6] was another critical security bug in Xen, this time
> resulting
> from one of the hypervisor developer's fetish to use an absolutely
> confusing
> construct in order to save a few modest bytes in a structure which might
> have
> been allocated by the system maybe a few tens of times at best. Even more
> worrying was the way how Xen Security Team decided to fix the bug: again by
> modifying some condition in the code further up the execution path, with
> the
> hopes that this time they would ensure this puzzling construct would
> always be
> used properly. We wrote more about this in our QSB #18 [7].
>
> c. Finally, the way how Xen fixed the recent XSA-148 looks also very
> reactive,
> IMHO. With a bug of this calibre, I would expect Xen to carefully review
> and
> augment all its PV memory virtualization code with additional checks
> (ASSERTs),
> ensuring certain invariants are always satisfied. Such as e.g. that none
> of the
> pages containing PDEs or PTEs are becoming writeable by the VM.
>
> I can't help but have a feeling that some of the Xen developers seem to be
> overconfident in their belief they can fully understand all the possible
> execution paths in their code. Well, the XSAs quoted above are an
> indisputable
> prove that this is not quite always the case. Realizing that, each
> developer by
> themselves, might be a great step towards a more secure hypervisor...
>
> 2. Another security-related aspect of the Xen project is how it totally
> ignores
> problems related to the build process security. Those who don't believe me
> should grep the sources for wget, which is now disguised as "FETCHER" shell
> variable... (so grep for "FETCHER" string)
>
> I feel embarrassed that I need to explain, at the end of 2015, why the
> build
> process of any serious software project should not blindly download
> unsigned
> components (sources) from the Internet, especially if it is about to
> execute
> Makefiles from these components a moment later... Come on, guys!
>
> (Of course we have been forced to get around this gapping security whole in
> Qubes OS [8] ourselves, sadly with a method that is not well suited for
> upstreaming).
>
> 3. Another thing is, of course: stop adding features to the core
> hypervisor. We
> really need Xen to finally mature, stabilize, and for its development
> process to
> be slowing down over time (just the bug fixes). We need a
> long-term-supported
> hypervisor, which doesn't change with subsonic speed. This would allow
> this core
> code to be widely audited by many experts. If some users want features,
> these
> should perhaps be maintained as additional modules (no, I don't mean
> dynamically
> loaded modules, just compile-time included), preferably in separate repos.
>
> Perhaps also to move all the non-hypervisor code, such as all the
> toolstacks,
> stubdom, etc, into separate repos also. For hygiene, if for nothing else.
>
> Admittedly, some of the features are a result of hardware evolution, such
> as
> e.g. UEFI support. But many are not. Again, maintaining these as optional
> code
> (in separate repos) would be a great step into getting the hypervisor
> maturing,
> finally.
>
> I have already written about it years ago [9], as a matter of fact.
>
> 4. Finally, I've been really surprised by the line of reasoning Ian
> expressed in
> the above-mentioned blog post. TL;DR: "we're still doing pretty great,
> compared
> to other projects, because: 1) we have smaller number of publicly disclosed
> bugs, and 2) we actually publicly disclose these bugs which we are aware
> of".
>
> The attitude presented in the blog post is so wrong, that I'm not even sure
> where to start commenting on this...
>
> With a single bug like the XSA-148 which, let me repeat that one more
> time: had
> been present in the hypervisor for the last 7 years, so with a bug like
> this it
> really doesn't matter how many (i.e. what number) of critical bugs does the
> competition have. Because only one bug of this calibre is enough for the
> attacker to never really bother to find another one. The mere fact that
> competing hypervisors might got 12 bugs during the same period, really
> doesn't
> make Xen look any better, sorry.
>
> Also, there is really nothing to be proud that you disclose the bugs. It
> would
> be a problem if you didn't.
>
> Hope the above comments might help improve the Xen security. Perhaps some
> would
> perceive them as arrogant or rude. Too bad. Remember the actual attackers
> will
> not be arrogant or rude -- they will just come and exploit bugs, silently.
> Admittedly this might not hurt some of the developers ego, not in the
> short time
> at least.
>
> Can we, the Qubes OS project, or myself personally, help with implementing
> the
> above suggestions? Sadly, no. While some of us do contribute occasional
> patches
> to Xen (specifically Marek Marczykowski-Górecki), we really work for a
> different
> project and have different tasks and responsibilities.
>
> Regards,
> joanna.
>
> [1] http://xenbits.xen.org/xsa/advisory-148.html
> [2]
> https://github.com/QubesOS/qubes-secpack/blob/master/QSBs/qsb-022-2015.txt
> [3] https://blog.xenproject.org/2015/10/30/security-vs-features/
> [4] https://twitter.com/xen_org/status/660151720463482880
> [5] http://xenbits.xen.org/xsa/advisory-109.html
> [6] http://xenbits.xen.org/xsa/advisory-123.html
> [7]
> https://github.com/QubesOS/qubes-secpack/blob/master/QSBs/qsb-018-2015.txt
> [8]
> https://github.com/QubesOS/qubes-vmm-xen/commit/dcd6c0a4f2c6226a9b706e62469d420579c86975
> [9] http://lists.xen.org/archives/html/xen-devel/2013-09/msg01815.html
> -----BEGIN PGP SIGNATURE-----
>
> iQIcBAEBAgAGBQJWPOHUAAoJEDOT2L8N3GcYzpEP/An/PTnKDOaC4tPKw3+Y8VIL
> n/xIfRPRnPRy18Tbx6EnrKzgohtVvtigtmd/FIxjYVuZ3Luw2B4RFSqUENg758Aa
> ANLs4kUD+yaUO82Jfg1nq/6PXBNZlKFovQuYV20LEW9JV6DvMCbzYJ2evZ6t0XS/
> EAhUOP1OqY4vb0kah4dwhQKepqwPcD5Tm5LLZn/qbO30e2zN9MkKB851vguQtVIz
> k5I8pv+MSQp1efRG2eg470onGtU36IIYFsY1OLihJA9MYh+74FpIA1xoURenJg6+
> NJhXEDnxxlz78BJaGOiSwHwB59yd2DXDJKAaUNV/H1LqQu3o1ED+8IZWUARkc0Wl
> ckTfQz/++exDhyRcWVHF5GnxEHWdyu/gNZOCNjl4o4HiYS4SQrhTRn7rWwalbyBB
> /jG3bAnU8m/Gtp95FtuWXCwuKeeOeBSfnxKMrksxu3JFSNevsYPZu5lrdtUEGLZA
> 97SwLj70GLesvMqEV3k7XmrQyt8LwyBzLCm+cCocaPEmOQAymeuslrs/RehjGSCQ
> L34Ipjvg85GoND64N8X56NuvD+/LrteRhp8hS+aEWv2YpNVJB9tmcOTzLzf7faPK
> KPyqa2lW7XKwm7n0WCbtPnrVHRbFPbKvkJRDnfPDwEAEiZcj0SjJ8h7fIDSQ4qac
> vgYBTJr/2cRrtC4y1An5
> =zQmw
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
--
You received this message because you are subscribed to the Google Groups \
"qubes-devel" group. To unsubscribe from this group and stop receiving emails from \
it, send an email to qubes-devel+unsubscribe@googlegroups.com. To post to this group, \
send email to qubes-devel@googlegroups.com. To view this discussion on the web visit \
https://groups.google.com/d/msgid/qubes-devel/CAKnNFz_8HzM6ePPinOwQTHT9nx_fEvMOfP2qp1XhGYZ670FcNg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
[Attachment #3 (text/html)]
<div dir="ltr">+1... so many great points here that ive thought many times its almost \
as if i could have written it<div><br></div><div>great \
post!</div><div><br></div><div>chris</div></div><div class="gmail_extra"><br><div \
class="gmail_quote">On Fri, Nov 6, 2015 at 12:22 PM, Joanna Rutkowska <span \
dir="ltr"><<a href="mailto:joanna@invisiblethingslab.com" \
target="_blank">joanna@invisiblethingslab.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<br>
Hello,<br>
<br>
Recently Xen has released the XSA-148 advisory [1] addressing a fatal bug in the<br>
hypervisor. The bug has been lurking there for the last 7 years! We, the Qubes<br>
OS Project, have commented on this in our Security Bulletin #22 [2]. And far<br>
from enthusiastic commentary that was (FWIW, it was me who wrote this QSB, as<br>
evidenced in the commits log, in case some from the Xen community would like to<br>
direct their rage towards a particular human being ;) Ian Jackson then wrote a<br>
response on the Xen blog [3]. I was then asked to share some more thoughts about<br>
how I thought Xen could actually improve its security process [4]. So, I share<br>
some these below:<br>
<br>
1. First of all, I wish Xen was somehow more defensively coded. To provide some<br>
examples:<br>
<br>
a. In XSA-109 [5] there was a problem with the hypervisor dereferencing a NULL<br>
pointer. The problem was fixed by the Xen Security Team by applying a patch<br>
which (hopefully) made sure the execution path that lead to this NULL pointer<br>
dereferencing code was never taken. Back then I suggested (on the Xen<br>
pre-disclosure list) to make this patch more explicit though:<br>
<br>
> On Wed, Jan 21, 2015 at 02:31:51PM +0100, Joanna Rutkowska wrote:<br>
> (...)<br>
>><br>
>> Wouldn't it be prudent to also check if:<br>
>><br>
>> (v->arch.paging.mode>{write_guest_entry,cmpxchg_guest_entry} != \
NULL)<br> >><br>
>> ... in the two affected functions, just before derefing these function<br>
>> pointers?<br>
>><br>
>> Going even a step further: how about replacing all<br>
>> function-pointer-based calls with macros that first validates the<br>
>> pointer before derefing it? At least when the system doesn't have \
SMEP?<br> >><br>
<br>
...to which I got a reply from one of the Xen Security Team engineers that the<br>
above might perhaps be justified in debug builds only, followed by a standard:<br>
"feel free to contribute a patch".<br>
<br>
b. The XSA-123 [6] was another critical security bug in Xen, this time resulting<br>
from one of the hypervisor developer's fetish to use an absolutely confusing<br>
construct in order to save a few modest bytes in a structure which might have<br>
been allocated by the system maybe a few tens of times at best. Even more<br>
worrying was the way how Xen Security Team decided to fix the bug: again by<br>
modifying some condition in the code further up the execution path, with the<br>
hopes that this time they would ensure this puzzling construct would always be<br>
used properly. We wrote more about this in our QSB #18 [7].<br>
<br>
c. Finally, the way how Xen fixed the recent XSA-148 looks also very reactive,<br>
IMHO. With a bug of this calibre, I would expect Xen to carefully review and<br>
augment all its PV memory virtualization code with additional checks (ASSERTs),<br>
ensuring certain invariants are always satisfied. Such as e.g. that none of the<br>
pages containing PDEs or PTEs are becoming writeable by the VM.<br>
<br>
I can't help but have a feeling that some of the Xen developers seem to be<br>
overconfident in their belief they can fully understand all the possible<br>
execution paths in their code. Well, the XSAs quoted above are an indisputable<br>
prove that this is not quite always the case. Realizing that, each developer by<br>
themselves, might be a great step towards a more secure hypervisor...<br>
<br>
2. Another security-related aspect of the Xen project is how it totally ignores<br>
problems related to the build process security. Those who don't believe me<br>
should grep the sources for wget, which is now disguised as "FETCHER" \
shell<br> variable... (so grep for "FETCHER" string)<br>
<br>
I feel embarrassed that I need to explain, at the end of 2015, why the build<br>
process of any serious software project should not blindly download unsigned<br>
components (sources) from the Internet, especially if it is about to execute<br>
Makefiles from these components a moment later... Come on, guys!<br>
<br>
(Of course we have been forced to get around this gapping security whole in<br>
Qubes OS [8] ourselves, sadly with a method that is not well suited for<br>
upstreaming).<br>
<br>
3. Another thing is, of course: stop adding features to the core hypervisor. We<br>
really need Xen to finally mature, stabilize, and for its development process to<br>
be slowing down over time (just the bug fixes). We need a long-term-supported<br>
hypervisor, which doesn't change with subsonic speed. This would allow this \
core<br> code to be widely audited by many experts. If some users want features, \
these<br> should perhaps be maintained as additional modules (no, I don't mean \
dynamically<br> loaded modules, just compile-time included), preferably in separate \
repos.<br> <br>
Perhaps also to move all the non-hypervisor code, such as all the toolstacks,<br>
stubdom, etc, into separate repos also. For hygiene, if for nothing else.<br>
<br>
Admittedly, some of the features are a result of hardware evolution, such as<br>
e.g. UEFI support. But many are not. Again, maintaining these as optional code<br>
(in separate repos) would be a great step into getting the hypervisor maturing,<br>
finally.<br>
<br>
I have already written about it years ago [9], as a matter of fact.<br>
<br>
4. Finally, I've been really surprised by the line of reasoning Ian expressed \
in<br> the above-mentioned blog post. TL;DR: "we're still doing pretty \
great, compared<br> to other projects, because: 1) we have smaller number of publicly \
disclosed<br> bugs, and 2) we actually publicly disclose these bugs which we are \
aware of".<br> <br>
The attitude presented in the blog post is so wrong, that I'm not even sure<br>
where to start commenting on this...<br>
<br>
With a single bug like the XSA-148 which, let me repeat that one more time: had<br>
been present in the hypervisor for the last 7 years, so with a bug like this it<br>
really doesn't matter how many (i.e. what number) of critical bugs does the<br>
competition have. Because only one bug of this calibre is enough for the<br>
attacker to never really bother to find another one. The mere fact that<br>
competing hypervisors might got 12 bugs during the same period, really \
doesn't<br> make Xen look any better, sorry.<br>
<br>
Also, there is really nothing to be proud that you disclose the bugs. It would<br>
be a problem if you didn't.<br>
<br>
Hope the above comments might help improve the Xen security. Perhaps some would<br>
perceive them as arrogant or rude. Too bad. Remember the actual attackers will<br>
not be arrogant or rude -- they will just come and exploit bugs, silently.<br>
Admittedly this might not hurt some of the developers ego, not in the short time<br>
at least.<br>
<br>
Can we, the Qubes OS project, or myself personally, help with implementing the<br>
above suggestions? Sadly, no. While some of us do contribute occasional patches<br>
to Xen (specifically Marek Marczykowski-Górecki), we really work for a different<br>
project and have different tasks and responsibilities.<br>
<br>
Regards,<br>
joanna.<br>
<br>
[1] <a href="http://xenbits.xen.org/xsa/advisory-148.html" rel="noreferrer" \
target="_blank">http://xenbits.xen.org/xsa/advisory-148.html</a><br> [2] <a \
href="https://github.com/QubesOS/qubes-secpack/blob/master/QSBs/qsb-022-2015.txt" \
rel="noreferrer" target="_blank">https://github.com/QubesOS/qubes-secpack/blob/master/QSBs/qsb-022-2015.txt</a><br>
[3] <a href="https://blog.xenproject.org/2015/10/30/security-vs-features/" \
rel="noreferrer" target="_blank">https://blog.xenproject.org/2015/10/30/security-vs-features/</a><br>
[4] <a href="https://twitter.com/xen_org/status/660151720463482880" rel="noreferrer" \
target="_blank">https://twitter.com/xen_org/status/660151720463482880</a><br> [5] <a \
href="http://xenbits.xen.org/xsa/advisory-109.html" rel="noreferrer" \
target="_blank">http://xenbits.xen.org/xsa/advisory-109.html</a><br> [6] <a \
href="http://xenbits.xen.org/xsa/advisory-123.html" rel="noreferrer" \
target="_blank">http://xenbits.xen.org/xsa/advisory-123.html</a><br> [7] <a \
href="https://github.com/QubesOS/qubes-secpack/blob/master/QSBs/qsb-018-2015.txt" \
rel="noreferrer" target="_blank">https://github.com/QubesOS/qubes-secpack/blob/master/QSBs/qsb-018-2015.txt</a><br>
[8] <a href="https://github.com/QubesOS/qubes-vmm-xen/commit/dcd6c0a4f2c6226a9b706e62469d420579c86975" \
rel="noreferrer" target="_blank">https://github.com/QubesOS/qubes-vmm-xen/commit/dcd6c0a4f2c6226a9b706e62469d420579c86975</a><br>
[9] <a href="http://lists.xen.org/archives/html/xen-devel/2013-09/msg01815.html" \
rel="noreferrer" target="_blank">http://lists.xen.org/archives/html/xen-devel/2013-09/msg01815.html</a><br>
-----BEGIN PGP SIGNATURE-----<br>
<br>
iQIcBAEBAgAGBQJWPOHUAAoJEDOT2L8N3GcYzpEP/An/PTnKDOaC4tPKw3+Y8VIL<br>
n/xIfRPRnPRy18Tbx6EnrKzgohtVvtigtmd/FIxjYVuZ3Luw2B4RFSqUENg758Aa<br>
ANLs4kUD+yaUO82Jfg1nq/6PXBNZlKFovQuYV20LEW9JV6DvMCbzYJ2evZ6t0XS/<br>
EAhUOP1OqY4vb0kah4dwhQKepqwPcD5Tm5LLZn/qbO30e2zN9MkKB851vguQtVIz<br>
k5I8pv+MSQp1efRG2eg470onGtU36IIYFsY1OLihJA9MYh+74FpIA1xoURenJg6+<br>
NJhXEDnxxlz78BJaGOiSwHwB59yd2DXDJKAaUNV/H1LqQu3o1ED+8IZWUARkc0Wl<br>
ckTfQz/++exDhyRcWVHF5GnxEHWdyu/gNZOCNjl4o4HiYS4SQrhTRn7rWwalbyBB<br>
/jG3bAnU8m/Gtp95FtuWXCwuKeeOeBSfnxKMrksxu3JFSNevsYPZu5lrdtUEGLZA<br>
97SwLj70GLesvMqEV3k7XmrQyt8LwyBzLCm+cCocaPEmOQAymeuslrs/RehjGSCQ<br>
L34Ipjvg85GoND64N8X56NuvD+/LrteRhp8hS+aEWv2YpNVJB9tmcOTzLzf7faPK<br>
KPyqa2lW7XKwm7n0WCbtPnrVHRbFPbKvkJRDnfPDwEAEiZcj0SjJ8h7fIDSQ4qac<br>
vgYBTJr/2cRrtC4y1An5<br>
=zQmw<br>
-----END PGP SIGNATURE-----<br>
<br>
_______________________________________________<br>
Xen-devel mailing list<br>
<a href="mailto:Xen-devel@lists.xen.org">Xen-devel@lists.xen.org</a><br>
<a href="http://lists.xen.org/xen-devel" rel="noreferrer" \
target="_blank">http://lists.xen.org/xen-devel</a><br> </blockquote></div><br></div>
<p></p>
-- <br />
You received this message because you are subscribed to the Google Groups \
"qubes-devel" group.<br /> To unsubscribe from this group and stop \
receiving emails from it, send an email to <a \
href="mailto:qubes-devel+unsubscribe@googlegroups.com">qubes-devel+unsubscribe@googlegroups.com</a>.<br \
/> To post to this group, send email to <a \
href="mailto:qubes-devel@googlegroups.com">qubes-devel@googlegroups.com</a>.<br /> To \
view this discussion on the web visit <a \
href="https://groups.google.com/d/msgid/qubes-devel/CAKnNFz_8HzM6ePPinOwQTHT9nx_fEvMOf \
P2qp1XhGYZ670FcNg%40mail.gmail.com?utm_medium=email&utm_source=footer">https://groups. \
google.com/d/msgid/qubes-devel/CAKnNFz_8HzM6ePPinOwQTHT9nx_fEvMOfP2qp1XhGYZ670FcNg%40mail.gmail.com</a>.<br \
/> For more options, visit <a \
href="https://groups.google.com/d/optout">https://groups.google.com/d/optout</a>.<br \
/>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic