Also affected: 7.2.z
qemu-kvm-rhev-2.3.0-31.el7_2.11
Depending on how likely we think it will be for an upstream NBD client contact a RHEV server, we may want this in z-stream.

Hi
 
   Could you share reproduced steps with QE?

Thanks
Jing Zhao

(In reply to jingzhao from comment #6)
> Hi
>  
>    Could you share reproduced steps with QE?

What have you tried?  It is possible to set up a qemu NBD server on the affected machine (the older package with a bug); something like:

$ truncate --size=10M file
$ /path/to/qemu-nbd -f raw -x foo file

then [attempt] to connect the newer client:

$ /path/to/newer/qemu/qemu-io -f raw -t none nbd://localhost:10809/foo

if both versions are installed on the same machine. With new enough server, the connection should succeed; with broken server, the client will get an error message and disconnect early rather than leaving you in qemu-io.

(In reply to Eric Blake from comment #7)
> (In reply to jingzhao from comment #6)
> > Hi
> >  
> >    Could you share reproduced steps with QE?
> 
> What have you tried?  It is possible to set up a qemu NBD server on the
> affected machine (the older package with a bug); something like:
> 
> $ truncate --size=10M file
> $ /path/to/qemu-nbd -f raw -x foo file
> 
> then [attempt] to connect the newer client:
> 
> $ /path/to/newer/qemu/qemu-io -f raw -t none nbd://localhost:10809/foo
> 
> if both versions are installed on the same machine. With new enough server,
> the connection should succeed; with broken server, the client will get an
> error message and disconnect early rather than leaving you in qemu-io.

Hi Eric

 Didn't reproduced it on qemu-kvm-rhev-2.5.0-4.el7.x86_64. Following is the reproduce steps

1.truncate --size=10M file
2.qemu-nbd -f raw -x foo /home/file

[root@localhost bin]# qemu-nbd -f raw -x foo /home/file
qemu-nbd: invalid option -- 'x'
qemu-nbd: Try `qemu-nbd --help' for more information.

So I tried with 
[root@localhost home]# qemu-nbd -f raw /home/file -p 10556

[root@localhost home]# qemu-io -f raw -t none nbd://10.66.6.246:10556
qemu-io> info 
format name: raw
qemu-io> quit

Could you help to check the steps? 


Thanks
Jing Zhao

(In reply to jingzhao from comment #8)

>  Didn't reproduced it on qemu-kvm-rhev-2.5.0-4.el7.x86_64. Following is the
> reproduce steps
> 
> 1.truncate --size=10M file
> 2.qemu-nbd -f raw -x foo /home/file
> 
> [root@localhost bin]# qemu-nbd -f raw -x foo /home/file
> qemu-nbd: invalid option -- 'x'
> qemu-nbd: Try `qemu-nbd --help' for more information.

Oh, right. Older qemu didn't support named exports.  But use of NBD_FLAG_FIXED_NEWSTYLE in the server is required to see the bug; so I'm checking if there is a way that older qemu advertises that flag even without named exports.

> 
> So I tried with 
> [root@localhost home]# qemu-nbd -f raw /home/file -p 10556
> 
> [root@localhost home]# qemu-io -f raw -t none nbd://10.66.6.246:10556

Which version of qemu-nbd and qemu-io are you using here?  If I'm right about provoking the bug, the problem will only manifest with old qemu-nbd and new qemu-io; if both programs are at the same version, then they won't have problems.

Meanwhile, let me investigate if older qemu could even serve up NBD_FLAG_FIXED_NEWSTYLE to a client...

> > So I tried with 
> > [root@localhost home]# qemu-nbd -f raw /home/file -p 10556
> > 
> > [root@localhost home]# qemu-io -f raw -t none nbd://10.66.6.246:10556
> 
> Which version of qemu-nbd and qemu-io are you using here?  If I'm right
> about provoking the bug, the problem will only manifest with old qemu-nbd
> and new qemu-io; if both programs are at the same version, then they won't
> have problems.
> 
> Meanwhile, let me investigate if older qemu could even serve up
> NBD_FLAG_FIXED_NEWSTYLE to a client...

Hi Eric

  qemu-nbd: I used the version
[root@localhost home]# rpm -qa |grep qemu
qemu-kvm-rhev-2.5.0-4.el7.x86_64
[root@localhost home]# which qemu-nbd
/usr/local/bin/qemu-nbd

For the qemu-io, I used two versions
qemu-kvm-rhev-2.5.0-4.el7.x86_64 and qemu-img-rhev-2.6.0-20.el7.x86_64 

For the above versions, I didn't reproduce the bz

Thanks
Jing Zhao

Okay, I've looked into the code in more depth.  The qemu-kvm-rhev-2.5.0-4.el7 build definitely has the bug that it aborts the connection after any unknown NBD_OPT options from the client; while 2.6.0-24.el7 is immune and keeps the connection alive.  So it seems that the problem you are having is figuring out how to get the newer qemu-img-rhev-2.6.0 to send an unknown NBD_OPT. It looks like the only candidate option when using 2.6.0 is NBD_OPT_STARTTLS, which requires that you request TLS encryption when starting the client.  However, I'm less familiar than Dan Berrange for the best command line for actually setting up an NBD client that requests encryption to test whether a server is broken or fixed; adding him on needinfo.

You can run QEMU NBD with TLS enabled using

  qemu-nbd --object tls-creds-x509,id=tls0,endpoint=server,\
                        dir=/home/berrange/security/qemutls \
               --tls-creds tls0 \
               --exportname default

Sorry, hit submit too soon.

And you can connect QEMU with a NBD backed disk to this server using

    
      $QEMU -object tls-creds-x509,id=tls0,endpoint=client,\
                    dir=/home/berrange/security/qemutls \
            -drive driver=nbd,host=localhost,port=9000,tls-creds=tls0

Hi Daniel

  Thanks for your comments. But I always hit issue when tested the bz, following are my test steps, could you help to check?

enviro:

[root@jinzhao home]# uname -r
3.10.0-503.el7.x86_64
[root@jinzhao home]# rpm -qa |grep qemu
qemu-kvm-rhev-debuginfo-2.6.0-23.el7.x86_64
ipxe-roms-qemu-20160127-4.git6366fa7a.el7.noarch
qemu-kvm-common-rhev-2.6.0-23.el7.x86_64
qemu-kvm-rhev-2.6.0-23.el7.x86_64
qemu-kvm-tools-rhev-2.6.0-23.el7.x86_64
qemu-img-rhev-2.6.0-23.el7.x86_64
[root@jinzhao home]# hostname
jinzhao

generate the tls file
[root@jinzhao spice_x509-lXf]# ls
ca-cert.pem  ca-key.pem       client-key.pem   server-key.pem
ca.info-ckR  client-cert.pem  server-cert.pem

scripts, please check the attachment

sever and client on the same machine
[root@jinzhao home]# qemu-nbd --object tls-creds-x509,id=tls0,endpoint=server,dir=/home/spice_x509-lXf --tls-creds tls0 --export-name jinzhao -f raw /home/file -p 9000
nbd/server.c:nbd_negotiate_options():L411: read failed
nbd/server.c:nbd_negotiate():L587: option negotiation failed

[root@jinzhao qemu]# /usr/libexec/qemu-kvm -object tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive driver=nbd,host=jinzhao,port=9000,tls-creds=tls0 
qemu-kvm: -drive driver=nbd,host=jinzhao,port=9000,tls-creds=tls0: No export with name '' available


PS: I tried to reproduce the issue with qemu-kvm-rhev-2.5.0-4.el7.x86_64, but the qemu-kvm-rhev-2.5.0-4.el7.x86_64 didn't support the parameter "--object",So I think QE can verify the issue according to comment11 and the new version, how about you, any idea for the bz?

Thanks
Jing Zhao

Created attachment 1200333 [details]
scripts for tls

Oh, sorry, the qemu command should have included the export name, eg

 /usr/libexec/qemu-kvm -object tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive driver=nbd,host=jinzhao,port=9000,tls-creds=tls0,export=default

alternatively, remove the --exportname arg from qemu-nbd

Tested it with following scenario:
1. tested it on qemu-kvm-rhev-2.5.0-4.el7.x86_64
[root@jinzhao ~]# qemu-nbd --object tls-creds-x509,id=tls0,endpoint=server,dir=/home/spice_x509-lXf --tls-creds tls0 --export-name jinzhao -f raw /home/file -p 9000
qemu-nbd: unrecognized option '--object'
qemu-nbd: Try `qemu-nbd --help' for more information.

2.Verified it on qemu-kvm-rhev-2.6.0-23.el7.x86_64
Server:
[root@jinzhao ~]# qemu-nbd --object tls-creds-x509,id=tls0,endpoint=server,dir=/home/spice_x509-lXf --tls-creds tls0 --export-name jinzhao -f raw /home/file -p 9000

client:
[root@jinzhao ~]# /usr/libexec/qemu-kvm -object tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive driver=nbd,host=jinzhao,port=9000,tls-creds=tls0,export=jinzhao --monitor stdio
QEMU 2.6.0 monitor - type 'help' for more information
(qemu) VNC server running on '127.0.0.1;5900'

(qemu) info status
VM status: running

Eric, according to Daniel's comments, can not reproduce the bz because qemu-kvm-rhev-2.5.0-4.el7.x86_64 didn't support --object parameter, but QE can test it on qemu-kvm-rhev-2.6.0-23.el7.x86_64.  So QE think according to comment 11 and this comment, verify the bz. any idea about you ?

Thanks
Jing Zhao

(In reply to jingzhao from comment #17)
> Tested it with following scenario:
> 1. tested it on qemu-kvm-rhev-2.5.0-4.el7.x86_64
> [root@jinzhao ~]# qemu-nbd --object
> tls-creds-x509,id=tls0,endpoint=server,dir=/home/spice_x509-lXf --tls-creds
> tls0 --export-name jinzhao -f raw /home/file -p 9000
> qemu-nbd: unrecognized option '--object'
> qemu-nbd: Try `qemu-nbd --help' for more information.

This is attempting to run qemu-nbd to serve up an encrypted export, and you proved that qemu-nbd was too old to serve it.  But the bug that we are TRYING to test is that qemu-nbd that is NOT serving an encrypted export incorrectly rejects clients that ARE trying to request encryption.  Testing the bug should be done by running qemu-nbd WITHOUT encryption, then run the newer client that does use encryption.  The newer client will reject servers that don't offer encryption, but the disconnect should be at the client's choice, and not hard rejection from the server.  So we still haven't even demonstrated whether the bug can be exercised.  :(

> 
> 2.Verified it on qemu-kvm-rhev-2.6.0-23.el7.x86_64
> Server:
> [root@jinzhao ~]# qemu-nbd --object
> tls-creds-x509,id=tls0,endpoint=server,dir=/home/spice_x509-lXf --tls-creds
> tls0 --export-name jinzhao -f raw /home/file -p 9000
> 
> client:
> [root@jinzhao ~]# /usr/libexec/qemu-kvm -object
> tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive
> driver=nbd,host=jinzhao,port=9000,tls-creds=tls0,export=jinzhao --monitor
> stdio
> QEMU 2.6.0 monitor - type 'help' for more information
> (qemu) VNC server running on '127.0.0.1;5900'
> 
> (qemu) info status
> VM status: running

This verifies that you were able to run an encrypted connection, but DID NOT verify that the newer server was still happy with unknown requests from the client, because this time the server understood the request.  We want a NEWER client that sends a request that the server does NOT understand, to prove that the server does not hang up on the client.  Upstream qemu 2.8 will be adding clients that attempt NBD_OPT_GO; such a client would qualify.  But the code for NBD_OPT_GO is still under review; it has not yet landed in upstream qemu.git, so testing with that option is more difficult than necessary at the moment.  (Also, using NBD_OPT_GO would be a great way to test that the 2.5 server is broken).

> 
> Eric, according to Daniel's comments, can not reproduce the bz because
> qemu-kvm-rhev-2.5.0-4.el7.x86_64 didn't support --object parameter, but QE
> can test it on qemu-kvm-rhev-2.6.0-23.el7.x86_64.  So QE think according to
> comment 11 and this comment, verify the bz. any idea about you ?

At this point, I don't know how else to get the situation tested.  I still think it should be possible to trigger the bug (which we have not done; although an encryption request from a 2.6 client to a 2.5 server should do it), as well as to trigger that we have fixed the bug (using encryption alone between a 2.6 server and a 2.6 client won't do it; the only other idea I can think of is to use a custom build or an upstream 2.8 client).  Since that is proving difficult to do, I'm okay if you want to call this verified by code inspection.

Here's the difference in code:

In 2.5, nbd.c:nbd_receive_options():

        switch (be32_to_cpu(tmp)) {
        case NBD_OPT_LIST:
            ret = nbd_handle_list(client, length);
            if (ret < 0) {
                return ret;
            }
            break;

        case NBD_OPT_ABORT:
            return -EINVAL;

        case NBD_OPT_EXPORT_NAME:
            return nbd_handle_export_name(client, length);

        default:
            tmp = be32_to_cpu(tmp);
            LOG("Unsupported option 0x%x", tmp);
            nbd_send_rep(client->sock, NBD_REP_ERR_UNSUP, tmp);
            return -EINVAL;

Notice that the default case (handling ANY unknown option) returns -EINVAL, which is the same result as the NBD_OPT_ABORT case; effectively, this means that a 2.5 server rejects ALL clients that send an unknown option, without giving the client a second chance to fall back to an older protocol.

In 2.6, nbd/server.c:nbd_negotiate_options():

            switch (clientflags) {
            case NBD_OPT_LIST:
                ret = nbd_negotiate_handle_list(client, length);
                if (ret < 0) {
                    return ret;
                }
                break;

            case NBD_OPT_ABORT:
                return -EINVAL;
...
            default:
                TRACE("Unsupported option 0x%x", clientflags);
                if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
                    return -EIO;
                }
                nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP,
                                       clientflags);
                break;
            }

Now the default case break's the switch statement, but continues the outermost loop to continue reading additional options from the client rather than immediately returning.

(In reply to Eric Blake from comment #18)
> (In reply to jingzhao from comment #17)
> > Tested it with following scenario:
> > 1. tested it on qemu-kvm-rhev-2.5.0-4.el7.x86_64
> > [root@jinzhao ~]# qemu-nbd --object
> > tls-creds-x509,id=tls0,endpoint=server,dir=/home/spice_x509-lXf --tls-creds
> > tls0 --export-name jinzhao -f raw /home/file -p 9000
> > qemu-nbd: unrecognized option '--object'
> > qemu-nbd: Try `qemu-nbd --help' for more information.
> 
> This is attempting to run qemu-nbd to serve up an encrypted export, and you
> proved that qemu-nbd was too old to serve it.  But the bug that we are
> TRYING to test is that qemu-nbd that is NOT serving an encrypted export
> incorrectly rejects clients that ARE trying to request encryption.  Testing
> the bug should be done by running qemu-nbd WITHOUT encryption, then run the
> newer client that does use encryption.  The newer client will reject servers
> that don't offer encryption, but the disconnect should be at the client's
> choice, and not hard rejection from the server.  So we still haven't even
> demonstrated whether the bug can be exercised.  :(

Eric, thanks for you comments.
According to above comments, I have tried it with following steps
1. Server:
qemu-kvm-rhev-2.5.0-4.el7.x86_64
[root@localhost qemu-kvm-rhev-2.5.0-4.el7]# truncate --size=10M file
[root@localhost qemu-kvm-rhev-2.5.0-4.el7]# qemu-nbd -f raw /home/file -p 9000

2.Client:
qemu-kvm-rhev-2.6.0-23.el7.x86_64

[root@jinzhao home]# /usr/libexec/qemu-kvm -object tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive driver=nbd,host=10.66.4.211,port=9000,tls-creds=tls0,export=10.66.4.211 --monitor stdio
qemu-kvm: -drive driver=nbd,host=10.66.4.211,port=9000,tls-creds=tls0,export=10.66.4.211: Server does not support export names

[root@jinzhao home]# /usr/libexec/qemu-kvm -object tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive driver=nbd,host=10.66.4.211,port=9000,tls-creds=tls0 --monitor stdio
qemu-kvm: -drive driver=nbd,host=10.66.4.211,port=9000,tls-creds=tls0: Server does not support STARTTLS

and the qemu-nbd of server side quit


> 
> > 
> > 2.Verified it on qemu-kvm-rhev-2.6.0-23.el7.x86_64
> > Server:
> > [root@jinzhao ~]# qemu-nbd --object
> > tls-creds-x509,id=tls0,endpoint=server,dir=/home/spice_x509-lXf --tls-creds
> > tls0 --export-name jinzhao -f raw /home/file -p 9000
> > 
> > client:
> > [root@jinzhao ~]# /usr/libexec/qemu-kvm -object
> > tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive
> > driver=nbd,host=jinzhao,port=9000,tls-creds=tls0,export=jinzhao --monitor
> > stdio
> > QEMU 2.6.0 monitor - type 'help' for more information
> > (qemu) VNC server running on '127.0.0.1;5900'
> > 
> > (qemu) info status
> > VM status: running
> 
> This verifies that you were able to run an encrypted connection, but DID NOT
> verify that the newer server was still happy with unknown requests from the
> client, because this time the server understood the request.  We want a
> NEWER client that sends a request that the server does NOT understand, to
> prove that the server does not hang up on the client.  Upstream qemu 2.8
> will be adding clients that attempt NBD_OPT_GO; such a client would qualify.
> But the code for NBD_OPT_GO is still under review; it has not yet landed in
> upstream qemu.git, so testing with that option is more difficult than
> necessary at the moment.  (Also, using NBD_OPT_GO would be a great way to
> test that the 2.5 server is broken).
> 
> > 
> > Eric, according to Daniel's comments, can not reproduce the bz because
> > qemu-kvm-rhev-2.5.0-4.el7.x86_64 didn't support --object parameter, but QE
> > can test it on qemu-kvm-rhev-2.6.0-23.el7.x86_64.  So QE think according to
> > comment 11 and this comment, verify the bz. any idea about you ?
> 
> At this point, I don't know how else to get the situation tested.  I still
> think it should be possible to trigger the bug (which we have not done;
> although an encryption request from a 2.6 client to a 2.5 server should do
> it), as well as to trigger that we have fixed the bug (using encryption
> alone between a 2.6 server and a 2.6 client won't do it; the only other idea
> I can think of is to use a custom build or an upstream 2.8 client).  Since
> that is proving difficult to do, I'm okay if you want to call this verified
> by code inspection.
> 
> Here's the difference in code:
> 
> In 2.5, nbd.c:nbd_receive_options():
> 
>         switch (be32_to_cpu(tmp)) {
>         case NBD_OPT_LIST:
>             ret = nbd_handle_list(client, length);
>             if (ret < 0) {
>                 return ret;
>             }
>             break;
> 
>         case NBD_OPT_ABORT:
>             return -EINVAL;
> 
>         case NBD_OPT_EXPORT_NAME:
>             return nbd_handle_export_name(client, length);
> 
>         default:
>             tmp = be32_to_cpu(tmp);
>             LOG("Unsupported option 0x%x", tmp);
>             nbd_send_rep(client->sock, NBD_REP_ERR_UNSUP, tmp);
>             return -EINVAL;
> 
> Notice that the default case (handling ANY unknown option) returns -EINVAL,
> which is the same result as the NBD_OPT_ABORT case; effectively, this means
> that a 2.5 server rejects ALL clients that send an unknown option, without
> giving the client a second chance to fall back to an older protocol.
> 
> In 2.6, nbd/server.c:nbd_negotiate_options():
> 
>             switch (clientflags) {
>             case NBD_OPT_LIST:
>                 ret = nbd_negotiate_handle_list(client, length);
>                 if (ret < 0) {
>                     return ret;
>                 }
>                 break;
> 
>             case NBD_OPT_ABORT:
>                 return -EINVAL;
> ...
>             default:
>                 TRACE("Unsupported option 0x%x", clientflags);
>                 if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
>                     return -EIO;
>                 }
>                 nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP,
>                                        clientflags);
>                 break;
>             }
> 
> Now the default case break's the switch statement, but continues the
> outermost loop to continue reading additional options from the client rather
> than immediately returning.

For verify the bz, check the code on 2.6 branch (path/qemu-kvm/nbd/server.c) and the code really in the 2.6 branch.

[guest@localhost nbd]$ git show-branch 
[rhel7/master-1.5.3] Update to qemu-kvm-1.5.3-125.el7
[guest@localhost nbd]$ git status
HEAD detached at origin/rhev7/master-2.6.0
nothing to commit, working directory clean

Eric, thanks again, could you double check the above reproduce info ?

Thanks
Jing Zhao

(In reply to jingzhao from comment #19)

> Eric, thanks for you comments.
> According to above comments, I have tried it with following steps
> 1. Server:
> qemu-kvm-rhev-2.5.0-4.el7.x86_64
> [root@localhost qemu-kvm-rhev-2.5.0-4.el7]# truncate --size=10M file
> [root@localhost qemu-kvm-rhev-2.5.0-4.el7]# qemu-nbd -f raw /home/file -p
> 9000
> 
> 2.Client:
> qemu-kvm-rhev-2.6.0-23.el7.x86_64
> 

> [root@jinzhao home]# /usr/libexec/qemu-kvm -object
> tls-creds-x509,id=tls0,endpoint=client,dir=/home/spice_x509-lXf -drive
> driver=nbd,host=10.66.4.211,port=9000,tls-creds=tls0 --monitor stdio
> qemu-kvm: -drive driver=nbd,host=10.66.4.211,port=9000,tls-creds=tls0:
> Server does not support STARTTLS
> 
> and the qemu-nbd of server side quit

Yay - we've reproduced that the 2.5.0-4 server has the bug.  It's a little bit hard to say that the server is buggy with STARTTLS (since the client hangs up if the server doesn't support TLS but the client wants to use it), and would be a bit more obvious if you have a newer client that attempts to do NBD_OPT_GO and stay connected with a fall back to NBD_OPT_EXPORTNAME, but that's obviously tougher.

> For verify the bz, check the code on 2.6 branch (path/qemu-kvm/nbd/server.c)
> and the code really in the 2.6 branch.
> 
> [guest@localhost nbd]$ git show-branch 
> [rhel7/master-1.5.3] Update to qemu-kvm-1.5.3-125.el7
> [guest@localhost nbd]$ git status
> HEAD detached at origin/rhev7/master-2.6.0
> nothing to commit, working directory clean
> 
> Eric, thanks again, could you double check the above reproduce info ?

So at this point, I think you have done diligence and can mark the BZ verified.  Getting a client that sends NBD_OPT_GO is an extra step, which would lend more strength to the argument, but may not be worth the effort since it is not even in upstream qemu.git yet.

According to comment 17-comment 20, changed to verified.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2016-2673.html