All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Babis Chalios <bchalios@amazon.es>
Cc: virtio-comment@lists.oasis-open.org,
	virtio-dev@lists.oasis-open.org, "Cali,
	Marco" <xmarcalx@amazon.co.uk>, "Graf (AWS),
	Alexander" <graf@amazon.de>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	aams@amazon.de
Subject: Re: [virtio-dev] [PATCH RFC 3/3] rng: leak detection support
Date: Tue, 12 Sep 2023 17:05:27 -0400	[thread overview]
Message-ID: <20230912170032-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <0abb7518-16e0-4227-bfe1-a29bd27124e8@amazon.es>

On Thu, Aug 31, 2023 at 12:16:22PM +0200, Babis Chalios wrote:
> Hi Michael,
> 
> On 21/11/22 17:30, Michael S. Tsirkin wrote:
> > Add virtqueues to support reporting entropy leaks (similar to virtio based vmgenid).
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >   virtio-rng.tex | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 60 insertions(+), 1 deletion(-)
> > 
> > diff --git a/virtio-rng.tex b/virtio-rng.tex
> > index 1ec7164..4760dfa 100644
> > --- a/virtio-rng.tex
> > +++ b/virtio-rng.tex
> > @@ -9,10 +9,14 @@ \subsection{Device ID}\label{sec:Device Types / Entropy Device / Device ID}
> >   \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
> >   \begin{description}
> >   \item[0] requestq
> > +\item[1] leakq1 (only if VIRTIO_RNG_F_LEAK is offered)
> > +\item[2] leakq2 (only if VIRTIO_RNG_F_LEAK is offered)
> >   \end{description}
> >   \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits}
> > -  None currently defined
> > +\begin{description}
> > +\item[VIRTIO_RNG_F_LEAK (0)] Device can report and handle information leaks.
> > +\end{description}
> >   \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout}
> >     None currently defined.
> > @@ -21,6 +25,7 @@ \subsection{Device Initialization}\label{sec:Device Types / Entropy Device / Dev
> >   \begin{enumerate}
> >   \item The \field{requestq} virtqueue is initialized
> > +\item If VIRTIO_RNG_F_LEAK has been negotiated, \field{leakq1} and \field{leakq2} are initialized
> >   \end{enumerate}
> >   \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}
> > @@ -41,3 +46,57 @@ \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O
> >   The device MUST place one or more random bytes into the buffer
> >   made available to it through \field{requestq}, but it
> >   MAY use less than the entire buffer length.
> > +
> > +\subsubsection{Reporting Information Leaks}{Device Types / Entropy Device / Device Operation / Reporting Information Leaks}
> > +
> > +The device might, after the fact, detect that some of the entropy
> > +it supplied to the driver has after the fact degraded in quality
> > +or leaked to the outside world.  One example is when the device
> > +is part of the virtual machine undergoing a restore from snapshot
> > +operation. Another example is when the information leaks from the
> > +host system through a side-channel.
> > +
> > +The driver would typically react by causing regeneration of any
> > +information that might have leaked and that has to be secret or
> > +unique.  It is understood that when an information leak has been
> > +detected it is likely not limited to the entropy received through
> > +the specific device. In particular, this is the case for
> > +snapshoting It is thus suggested that the system fully
> > +regenerate any unique/secret information in this scenario.
> > +
> > +If VIRTIO_RNG_F_LEAK has been negotiated the device can report
> > +such leaks to the driver through a set of dedicated leak
> > +queues: \field{leakq1} and \field{leakq2}.
> > +
> > +Buffers added to the leak queues can have one of two forms:
> > +\begin{enumerate}
> > +\item A write-only buffer. It will be completely filled by random data by the device.
> > +\item A buffer consisting of read-only section followed by a
> > +write-only section, both of identical size. The
> > +device will copy data from the read-only section to the write-only
> > +section.
> > +\end{enumerate}
> > +
> > +The steps for operating the virtqueue are:
> > +
> > +\begin{enumerate}
> > +\item At each time, only one of \field{leakq1}, \field{leakq2} is active
> > +      (has buffers added/used).
> > +\item After initialization, \field{leakq1} is active.
> > +\item Driver adds multiple buffers to the active leak queue.
> > +\item The buffers are not used until an information leak is
> > +      detected, as long as that is the case driver can
> > +      add more buffers to the active queue.
> > +\item Upon detecting an information leak, device starts
> > +      using buffers in the active leak queue.
> > +\item Upon detecting that buffers have been used, driver
> > +      switches to another leak queue making it active
> > +      (e.g. from \field{leakq1} to \field{leakq2} or vice versa).
> > +      It then starts adding buffers to the new leak queue.\
> 
> I have been discussing with Alex and we think there's a potential race here,
> between the time the driver
> sees the used buffers in the active leak queue until the time it adds new
> buffers to the next leak queue.
> If a new entropy leak event arrives the VMM won't find any buffers in the
> queue.

I do not understand why this matters though. we know there was a leak,
why does it matter whether there was one or two leaks?

> In the last RFC implementing this in Linux we sent to LKML [1] we avoid the
> issue by pre-populating both
> queues, but that does not solve the problem if a third entropy leak event
> arrives. The probability of this
> happening is indeed small, but we thought of a potential solution to this.
> 
> What if we modify the spec here to instruct the VMM to deny taking a
> snapshot if there are not any buffers
> in the active leak queue? If we did this, we could even simplify the spec to
> just introduce a single entropy
> leak queue, so we could avoid the complexity of switching between active
> leak queues in the driver and
> the device. WDYT?

here's the problem: 

- driver adds batch 1 of buffers
- leak
- device starts using buffers from batch 1
- driver sees some buffers and starts adding batch 2
- device sees batch 2 and thinks this is part of batch 1
  consumes them all




> Cheers,
> Babis
> 
> [1]
> https://lore.kernel.org/lkml/20230823090107.65749-3-bchalios@amazon.es/T/
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Babis Chalios <bchalios@amazon.es>
Cc: virtio-comment@lists.oasis-open.org,
	virtio-dev@lists.oasis-open.org, "Cali,
	Marco" <xmarcalx@amazon.co.uk>, "Graf (AWS),
	Alexander" <graf@amazon.de>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	aams@amazon.de
Subject: [virtio-comment] Re: [virtio-dev] [PATCH RFC 3/3] rng: leak detection support
Date: Tue, 12 Sep 2023 17:05:27 -0400	[thread overview]
Message-ID: <20230912170032-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <0abb7518-16e0-4227-bfe1-a29bd27124e8@amazon.es>

On Thu, Aug 31, 2023 at 12:16:22PM +0200, Babis Chalios wrote:
> Hi Michael,
> 
> On 21/11/22 17:30, Michael S. Tsirkin wrote:
> > Add virtqueues to support reporting entropy leaks (similar to virtio based vmgenid).
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >   virtio-rng.tex | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 60 insertions(+), 1 deletion(-)
> > 
> > diff --git a/virtio-rng.tex b/virtio-rng.tex
> > index 1ec7164..4760dfa 100644
> > --- a/virtio-rng.tex
> > +++ b/virtio-rng.tex
> > @@ -9,10 +9,14 @@ \subsection{Device ID}\label{sec:Device Types / Entropy Device / Device ID}
> >   \subsection{Virtqueues}\label{sec:Device Types / Entropy Device / Virtqueues}
> >   \begin{description}
> >   \item[0] requestq
> > +\item[1] leakq1 (only if VIRTIO_RNG_F_LEAK is offered)
> > +\item[2] leakq2 (only if VIRTIO_RNG_F_LEAK is offered)
> >   \end{description}
> >   \subsection{Feature bits}\label{sec:Device Types / Entropy Device / Feature bits}
> > -  None currently defined
> > +\begin{description}
> > +\item[VIRTIO_RNG_F_LEAK (0)] Device can report and handle information leaks.
> > +\end{description}
> >   \subsection{Device configuration layout}\label{sec:Device Types / Entropy Device / Device configuration layout}
> >     None currently defined.
> > @@ -21,6 +25,7 @@ \subsection{Device Initialization}\label{sec:Device Types / Entropy Device / Dev
> >   \begin{enumerate}
> >   \item The \field{requestq} virtqueue is initialized
> > +\item If VIRTIO_RNG_F_LEAK has been negotiated, \field{leakq1} and \field{leakq2} are initialized
> >   \end{enumerate}
> >   \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device Operation}
> > @@ -41,3 +46,57 @@ \subsection{Device Operation}\label{sec:Device Types / Entropy Device / Device O
> >   The device MUST place one or more random bytes into the buffer
> >   made available to it through \field{requestq}, but it
> >   MAY use less than the entire buffer length.
> > +
> > +\subsubsection{Reporting Information Leaks}{Device Types / Entropy Device / Device Operation / Reporting Information Leaks}
> > +
> > +The device might, after the fact, detect that some of the entropy
> > +it supplied to the driver has after the fact degraded in quality
> > +or leaked to the outside world.  One example is when the device
> > +is part of the virtual machine undergoing a restore from snapshot
> > +operation. Another example is when the information leaks from the
> > +host system through a side-channel.
> > +
> > +The driver would typically react by causing regeneration of any
> > +information that might have leaked and that has to be secret or
> > +unique.  It is understood that when an information leak has been
> > +detected it is likely not limited to the entropy received through
> > +the specific device. In particular, this is the case for
> > +snapshoting It is thus suggested that the system fully
> > +regenerate any unique/secret information in this scenario.
> > +
> > +If VIRTIO_RNG_F_LEAK has been negotiated the device can report
> > +such leaks to the driver through a set of dedicated leak
> > +queues: \field{leakq1} and \field{leakq2}.
> > +
> > +Buffers added to the leak queues can have one of two forms:
> > +\begin{enumerate}
> > +\item A write-only buffer. It will be completely filled by random data by the device.
> > +\item A buffer consisting of read-only section followed by a
> > +write-only section, both of identical size. The
> > +device will copy data from the read-only section to the write-only
> > +section.
> > +\end{enumerate}
> > +
> > +The steps for operating the virtqueue are:
> > +
> > +\begin{enumerate}
> > +\item At each time, only one of \field{leakq1}, \field{leakq2} is active
> > +      (has buffers added/used).
> > +\item After initialization, \field{leakq1} is active.
> > +\item Driver adds multiple buffers to the active leak queue.
> > +\item The buffers are not used until an information leak is
> > +      detected, as long as that is the case driver can
> > +      add more buffers to the active queue.
> > +\item Upon detecting an information leak, device starts
> > +      using buffers in the active leak queue.
> > +\item Upon detecting that buffers have been used, driver
> > +      switches to another leak queue making it active
> > +      (e.g. from \field{leakq1} to \field{leakq2} or vice versa).
> > +      It then starts adding buffers to the new leak queue.\
> 
> I have been discussing with Alex and we think there's a potential race here,
> between the time the driver
> sees the used buffers in the active leak queue until the time it adds new
> buffers to the next leak queue.
> If a new entropy leak event arrives the VMM won't find any buffers in the
> queue.

I do not understand why this matters though. we know there was a leak,
why does it matter whether there was one or two leaks?

> In the last RFC implementing this in Linux we sent to LKML [1] we avoid the
> issue by pre-populating both
> queues, but that does not solve the problem if a third entropy leak event
> arrives. The probability of this
> happening is indeed small, but we thought of a potential solution to this.
> 
> What if we modify the spec here to instruct the VMM to deny taking a
> snapshot if there are not any buffers
> in the active leak queue? If we did this, we could even simplify the spec to
> just introduce a single entropy
> leak queue, so we could avoid the complexity of switching between active
> leak queues in the driver and
> the device. WDYT?

here's the problem: 

- driver adds batch 1 of buffers
- leak
- device starts using buffers from batch 1
- driver sees some buffers and starts adding batch 2
- device sees batch 2 and thinks this is part of batch 1
  consumes them all




> Cheers,
> Babis
> 
> [1]
> https://lore.kernel.org/lkml/20230823090107.65749-3-bchalios@amazon.es/T/
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


  reply	other threads:[~2023-09-12 21:05 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-21 16:30 [virtio-comment] [PATCH RFC 0/3] virtio-rng based entropy leak reporting Michael S. Tsirkin
2022-11-21 16:30 ` [virtio-comment] [PATCH RFC 1/3] rng: move to a file of its own Michael S. Tsirkin
2022-11-21 16:30 ` [virtio-comment] [PATCH RFC 2/3] rng: be specific about the virtqueue Michael S. Tsirkin
2022-11-21 16:30 ` [virtio-dev] [PATCH RFC 3/3] rng: leak detection support Michael S. Tsirkin
2022-11-25 12:41   ` [virtio-dev] " Babis Chalios
2022-12-12 10:10     ` Babis Chalios
2023-01-11 13:57   ` Babis Chalios
2023-08-31 10:16   ` [virtio-dev] " Babis Chalios
2023-09-12 21:05     ` Michael S. Tsirkin [this message]
2023-09-12 21:05       ` [virtio-comment] " Michael S. Tsirkin
2023-09-13  9:32       ` Babis Chalios
2023-09-13  9:37         ` Michael S. Tsirkin
2023-09-13  9:37           ` [virtio-comment] " Michael S. Tsirkin
2023-09-13 11:19           ` Babis Chalios
2023-09-18 11:14             ` Babis Chalios
2023-09-18 12:41             ` Michael S. Tsirkin
2023-09-18 12:41               ` [virtio-comment] " Michael S. Tsirkin
2023-09-18 13:00               ` Babis Chalios
2023-09-18 13:58                 ` Michael S. Tsirkin
2023-09-18 13:58                   ` [virtio-comment] " Michael S. Tsirkin
2023-09-18 14:02                   ` Babis Chalios
2023-09-18 14:05                     ` Michael S. Tsirkin
2023-09-18 14:05                       ` [virtio-comment] " Michael S. Tsirkin
2023-09-18 16:30                       ` Babis Chalios
2023-09-19  7:32                         ` Babis Chalios
2023-09-19 10:01                           ` Michael S. Tsirkin
2023-09-19 10:01                             ` [virtio-comment] " Michael S. Tsirkin
2023-09-19 10:11                             ` Babis Chalios
2023-09-22 12:30                               ` Babis Chalios
2023-09-22 15:06                               ` Michael S. Tsirkin
2023-09-22 15:06                                 ` [virtio-comment] " Michael S. Tsirkin
2023-09-22 15:40                                 ` Babis Chalios
2023-09-22 16:01                                   ` Michael S. Tsirkin
2023-09-22 16:01                                     ` [virtio-comment] " Michael S. Tsirkin
2023-09-27 10:43                                     ` Babis Chalios
2023-09-27 21:47                                       ` Michael S. Tsirkin
2023-09-27 21:47                                         ` [virtio-comment] " Michael S. Tsirkin
2023-09-28 18:16                                         ` Babis Chalios
2023-10-13  7:49                                           ` Babis Chalios
2023-10-13 13:38                                             ` Michael S. Tsirkin
2023-10-13 13:38                                               ` [virtio-comment] " Michael S. Tsirkin
2023-11-02 11:20                                           ` Michael S. Tsirkin
2023-11-02 11:20                                             ` [virtio-comment] " Michael S. Tsirkin
2023-11-02 11:38                                             ` Babis Chalios
2023-11-02 11:51                                               ` Michael S. Tsirkin
2023-11-02 11:51                                                 ` [virtio-comment] " Michael S. Tsirkin
2023-11-02 13:42                                                 ` Babis Chalios
2023-11-02 11:25                                   ` Michael S. Tsirkin
2023-11-02 11:25                                     ` [virtio-comment] " Michael S. Tsirkin
2023-11-02 11:51                                     ` Babis Chalios
2023-01-12  7:02 ` [virtio-dev] Re: [PATCH RFC 0/3] virtio-rng based entropy leak reporting Michael S. Tsirkin
2023-01-16 11:39   ` Babis Chalios
     [not found]     ` <CAHmME9ry2fss2gsbPs2zVJkY=8Cdeae0XFD9FzCVnW67Xy3thA@mail.gmail.com>
2023-01-16 18:11       ` [virtio-comment] " Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230912170032-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=Jason@zx2c4.com \
    --cc=aams@amazon.de \
    --cc=bchalios@amazon.es \
    --cc=graf@amazon.de \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=xmarcalx@amazon.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.