Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
server: Wait until handshake complete before calling .open callback
Currently we call the plugin .open callback as soon as we receive a TCP connection: $ nbdkit -fv --tls=require --tls-certificates=tests/pki null \ --run "telnet localhost 10809" [...] Trying ::1... Connected to localhost. Escape character is '^]'. nbdkit: debug: accepted connection nbdkit: debug: null: open readonly=0 ◀ NOTE nbdkit: null[1]: debug: newstyle negotiation: flags: global 0x3 NBDMAGICIHAVEOPT In plugins such as curl, guestfs, ssh, vddk and others we do a considerable amount of work in the .open callback (such as making a remote connection or launching an appliance). Therefore we are providing an easy Denial of Service / Amplification Attack for unauthorized clients to cause a lot of work to be done for only the cost of a simple TCP 3 way handshake. This commit moves the call to the .open callback after the NBD handshake has mostly completed. In particular TLS authentication must be complete before we will call into the plugin. It is unlikely that there are plugins which really depend on the current behaviour of .open (which I found surprising even though I guess I must have written it). If there are then we could add a new .connect callback or similar to allow plugins to get control at the earlier point in the connection. After this change you can see that the .open callback is not called from just a simple TCP connection: $ ./nbdkit -fv --tls=require --tls-certificates=tests/pki null \ --run "telnet localhost 10809" [...] Trying ::1... Connected to localhost. Escape character is '^]'. nbdkit: debug: accepted connection nbdkit: null[1]: debug: newstyle negotiation: flags: global 0x3 NBDMAGICIHAVEOPT xx nbdkit: null[1]: debug: newstyle negotiation: client flags: 0xd0a7878 nbdkit: null[1]: error: client requested unknown flags 0xd0a7878 Connection closed by foreign host. nbdkit: debug: null: unload plugin Signed-off-by: Richard W.M. Jones <rjones@redhat.com> (cherry picked from commit c05686f)
- Loading branch information
Showing
5 changed files
with
57 additions
and
62 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters