Libguestfs - Aug 2018 - Re: [PATCH nbdkit] protocol: Implement NBD_OPT_GO.

On 08/06/2018 02:54 AM, Richard W.M. Jones wrote:
> On Sun, Aug 05, 2018 at 12:11:04AM +0300, Nir Soffer wrote:
>> But we have a bug - server configure to allow access to
"export",
>> but allow access to the default empty "" export.
> 
> nbdkit ignores the export name you pass in and always has done:
> 
>
https://github.com/libguestfs/nbdkit/blob/a2c4e88af503310892ebce6da3ac478beb47888a/src/connections.c#L626
> 
> The -e option on the command line is only for advertizing an export
> name to clients.  It is otherwise unused & unenforced.
Agreed - it is not necessarily a bug for the server to permit the client 
to connect to different name(s) than what the server was told to advertise.
> 
> One day we'll do something useful with it but we're not sure what.
We might eventually reject clients that don't request the same export 
name as what was advertised, but that doesn't make the current behavior 
buggy (it only means that current clients shouldn't rely on a mismatched 
name always working).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Mon, Aug 6, 2018 at 5:06 PM Eric Blake <eblake@redhat.com> wrote:
> On 08/06/2018 02:54 AM, Richard W.M. Jones wrote:
> > On Sun, Aug 05, 2018 at 12:11:04AM +0300, Nir Soffer wrote:
> >> But we have a bug - server configure to allow access to
"export",
> >> but allow access to the default empty "" export.
> >
> > nbdkit ignores the export name you pass in and always has done:
> >
> >
>
https://github.com/libguestfs/nbdkit/blob/a2c4e88af503310892ebce6da3ac478beb47888a/src/connections.c#L626
> >
> > The -e option on the command line is only for advertizing an export
> > name to clients.  It is otherwise unused & unenforced.
>
> Agreed - it is not necessarily a bug for the server to permit the client
> to connect to different name(s) than what the server was told to advertise.
>
> >
> > One day we'll do something useful with it but we're not sure
what.
>
> We might eventually reject clients that don't request the same export
> name as what was advertised, but that doesn't make the current behavior
> buggy (it only means that current clients shouldn't rely on a
mismatched
> name always working).
>
I does not make sense to allow access if the server was configured
with one export, and the user ask for another export. This is likely a bug
in user code or the code running nbdkit.

If users do not want to specify the export, they run nbdkit with empty
export
name, and use empty export name in NBD_OPT_GO. If an export was
specified it should be handled in a strict way.

Here is one use case that will be easily avoided by being strict about
export name:

1. application starts nbdkit on a port
2. application reports port to user
3. application time out, terminating nbdkit
4. applcation starts nbdkit again on same port
   (maybe port range is cycled)
5. application reports port to another user
6. first user access nbdkit, accessing another user disk

If we use random export name, and the server is strict, this failure
is not possible.

Eric, can you point us to the part of the spec allowing ignoring the export
name sent by the client?

Regardless since this is the current behavior we can improve this later.

Nir

On 08/06/2018 09:31 AM, Nir Soffer wrote:
> Eric, can you point us to the part of the spec allowing ignoring the export
> name sent by the client?
Nothing in the NBD spec requires the server to reject unknown export 
names. So nbdkit never rejects export names (which means it behaves as 
if all names work, regardless of whether or not it was the name it was 
configured with, since it exports exactly one volume all the time).

An NBD server that exports more than one volume at a time (such as qemu 
via QMP [but oddly not qemu-nbd], or nbd-server) DOES have to pay 
attention to export names. And if nbdkit were ever taught to manage more 
than one export at once, then that would also imply that export names 
matter.  But until then, I don't see nbdkit's behavior as a spec
violation.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Mon, Aug 06, 2018 at 05:31:54PM +0300, Nir Soffer
wrote:
> On Mon, Aug 6, 2018 at 5:06 PM Eric Blake <eblake@redhat.com> wrote:
> 
> > On 08/06/2018 02:54 AM, Richard W.M. Jones wrote:
> > > On Sun, Aug 05, 2018 at 12:11:04AM +0300, Nir Soffer wrote:
> > >> But we have a bug - server configure to allow access to
"export",
> > >> but allow access to the default empty "" export.
> > >
> > > nbdkit ignores the export name you pass in and always has done:
> > >
> > >
> >
https://github.com/libguestfs/nbdkit/blob/a2c4e88af503310892ebce6da3ac478beb47888a/src/connections.c#L626
> > >
> > > The -e option on the command line is only for advertizing an
export
> > > name to clients.  It is otherwise unused & unenforced.
> >
> > Agreed - it is not necessarily a bug for the server to permit the
client
> > to connect to different name(s) than what the server was told to
advertise.
> >
> > >
> > > One day we'll do something useful with it but we're not
sure what.
> >
> > We might eventually reject clients that don't request the same
export
> > name as what was advertised, but that doesn't make the current
behavior
> > buggy (it only means that current clients shouldn't rely on a
mismatched
> > name always working).
> >
> 
> I does not make sense to allow access if the server was configured
> with one export, and the user ask for another export. This is likely a bug
> in user code or the code running nbdkit.
> 
> If users do not want to specify the export, they run nbdkit with empty
> export
> name, and use empty export name in NBD_OPT_GO. If an export was
> specified it should be handled in a strict way.
> 
> Here is one use case that will be easily avoided by being strict about
> export name:
> 
> 1. application starts nbdkit on a port
> 2. application reports port to user
> 3. application time out, terminating nbdkit
> 4. applcation starts nbdkit again on same port
>    (maybe port range is cycled)
> 5. application reports port to another user
> 6. first user access nbdkit, accessing another user disk
> 
> If we use random export name, and the server is strict, this failure
> is not possible.
Export name validation should not be considered a security feature, as
the export name is not designated or treated as sensitive data.

If you want to prevent one user access other users' disks, whether by
accidental or maliciously, you need to use TLS and validate clients
certificates in the server, or use the new PSK credentials.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|