In order to make ssh disks usable, we need to be able to validate a
remote host. To do this, add a <knownHosts> xml element for ssh disks to
allow the user to specify a location for a file that contains known host
keys. Implementation to follow.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 docs/formatdomain.rst             |  6 ++++++
 src/conf/schemas/domaincommon.rng | 11 +++++++++++
 2 files changed, 17 insertions(+)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index bf071255c5..d5ad5d80b0 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2953,6 +2953,12 @@ paravirtualized driver is specified via the ``disk`` element.
          If the reconnect feature is enabled, accepts ``yes`` and ``no``
       ``timeout``
          The amount of seconds after which hypervisor tries to reconnect.
+   ``knownHosts``
+      For storage accessed via the ``ssh`` protocol, this element configures a
+      path to a file containing a list of known ssh hosts to be used to verify
+      the remote host. The location of the file is specified via the ``path``
+      attribute.
+      :since:`Since 9.1.0`
 
 
    For a "file" or "volume" disk type which represents a cdrom or floppy (the
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index bc582e450e..f38f1f3ff1 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2160,6 +2160,14 @@
     </element>
   </define>
 
+  <define name="diskSourceNetworkProtocolSSHHostVerify">
+    <element name="knownHosts">
+      <attribute name="path">
+        <ref name="absFilePath"/>
+      </attribute>
+    </element>
+  </define>
+
   <define name="diskSourceNetworkProtocolSSH">
     <element name="source">
       <interleave>
@@ -2175,6 +2183,9 @@
           <ref name="encryption"/>
         </optional>
         <ref name="diskSourceNetworkProtocolPropsCommon"/>
+        <optional>
+          <ref name="diskSourceNetworkProtocolSSHHostVerify"/>
+        </optional>
         <optional>
           <ref name="diskAuth"/>
         </optional>
-- 
2.39.1

On Tue, Feb 14, 2023 at 11:08:16 -0600, Jonathon Jongsma wrote:
> In order to make ssh disks usable, we need to be able to validate a
> remote host. To do this, add a <knownHosts> xml element for ssh disks to
> allow the user to specify a location for a file that contains known host
> keys. Implementation to follow.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
>  docs/formatdomain.rst             |  6 ++++++
>  src/conf/schemas/domaincommon.rng | 11 +++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index bf071255c5..d5ad5d80b0 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2953,6 +2953,12 @@ paravirtualized driver is specified via the ``disk`` element.
>           If the reconnect feature is enabled, accepts ``yes`` and ``no``
>        ``timeout``
>           The amount of seconds after which hypervisor tries to reconnect.
> +   ``knownHosts``
> +      For storage accessed via the ``ssh`` protocol, this element configures a
> +      path to a file containing a list of known ssh hosts to be used to verify
> +      the remote host. The location of the file is specified via the ``path``
> +      attribute.
> +      :since:`Since 9.1.0`

How does nbdkit do enrollment here? Does it expect a pre-filled set of
known hosts? Or does it allow new host on first use?

On 2/16/23 10:45 AM, Peter Krempa wrote:
> On Tue, Feb 14, 2023 at 11:08:16 -0600, Jonathon Jongsma wrote:
>> In order to make ssh disks usable, we need to be able to validate a
>> remote host. To do this, add a <knownHosts> xml element for ssh disks to
>> allow the user to specify a location for a file that contains known host
>> keys. Implementation to follow.
>>
>> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
>> ---
>>   docs/formatdomain.rst             |  6 ++++++
>>   src/conf/schemas/domaincommon.rng | 11 +++++++++++
>>   2 files changed, 17 insertions(+)
>>
>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>> index bf071255c5..d5ad5d80b0 100644
>> --- a/docs/formatdomain.rst
>> +++ b/docs/formatdomain.rst
>> @@ -2953,6 +2953,12 @@ paravirtualized driver is specified via the ``disk`` element.
>>            If the reconnect feature is enabled, accepts ``yes`` and ``no``
>>         ``timeout``
>>            The amount of seconds after which hypervisor tries to reconnect.
>> +   ``knownHosts``
>> +      For storage accessed via the ``ssh`` protocol, this element configures a
>> +      path to a file containing a list of known ssh hosts to be used to verify
>> +      the remote host. The location of the file is specified via the ``path``
>> +      attribute.
>> +      :since:`Since 9.1.0`
> 
> How does nbdkit do enrollment here? Does it expect a pre-filled set of
> known hosts? Or does it allow new host on first use?
> 

It expects a prefilled known hosts file. Here's what it says in the 
manpage for nbdkit-ssh-plugin:

   Known hosts
        The SSH server’s host key is checked at connection time, and 
must be present and correct in the local "known hosts" file.

        If you have never connected to the SSH server before then the 
connection will usually fail.  You can:

        •   connect to the server first using ssh(1) so you can manually 
accept the host key, or

        •   provide the host key in an alternate file which you specify 
using the "known-hosts" option, or

        •   set verify-remote-host=false on the command line.  This 
latter option is dangerous because it allows a MITM attack to be 
conducted against you.

On Thu, Feb 16, 2023 at 16:59:33 -0600, Jonathon Jongsma wrote:
> On 2/16/23 10:45 AM, Peter Krempa wrote:
> > On Tue, Feb 14, 2023 at 11:08:16 -0600, Jonathon Jongsma wrote:
> > > In order to make ssh disks usable, we need to be able to validate a
> > > remote host. To do this, add a <knownHosts> xml element for ssh disks to
> > > allow the user to specify a location for a file that contains known host
> > > keys. Implementation to follow.
> > > 
> > > Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> > > ---
> > >   docs/formatdomain.rst             |  6 ++++++
> > >   src/conf/schemas/domaincommon.rng | 11 +++++++++++
> > >   2 files changed, 17 insertions(+)
> > > 
> > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > > index bf071255c5..d5ad5d80b0 100644
> > > --- a/docs/formatdomain.rst
> > > +++ b/docs/formatdomain.rst
> > > @@ -2953,6 +2953,12 @@ paravirtualized driver is specified via the ``disk`` element.
> > >            If the reconnect feature is enabled, accepts ``yes`` and ``no``
> > >         ``timeout``
> > >            The amount of seconds after which hypervisor tries to reconnect.
> > > +   ``knownHosts``
> > > +      For storage accessed via the ``ssh`` protocol, this element configures a
> > > +      path to a file containing a list of known ssh hosts to be used to verify
> > > +      the remote host. The location of the file is specified via the ``path``
> > > +      attribute.
> > > +      :since:`Since 9.1.0`
> > 
> > How does nbdkit do enrollment here? Does it expect a pre-filled set of
> > known hosts? Or does it allow new host on first use?
> > 
> 
> It expects a prefilled known hosts file. Here's what it says in the manpage
> for nbdkit-ssh-plugin:
> 
>   Known hosts
>        The SSH server’s host key is checked at connection time, and must be
> present and correct in the local "known hosts" file.
> 
>        If you have never connected to the SSH server before then the
> connection will usually fail.  You can:
> 
>        •   connect to the server first using ssh(1) so you can manually
> accept the host key, or
> 
>        •   provide the host key in an alternate file which you specify using
> the "known-hosts" option, or
> 
>        •   set verify-remote-host=false on the command line.  This latter
> option is dangerous because it allows a MITM attack to be conducted against
> you.

Okay. The fact that it expects a pre-filled knownHosts should be
mentioned in the docs. I think it's a reasonable limitation. I'd not
bother with allowing to disable verification at all.

With docs updated:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>