Wouter Verhelst [Mon, 21 Nov 2016 08:13:33 +0000 (09:13 +0100)]
Add a "wrong certificate used" test
We want to fail authentication when the certificate in use is one not
signed by the correct CA, so ensure that that happens.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 20 Nov 2016 20:21:50 +0000 (21:21 +0100)]
Re-add the SERVER_PRECEDENCE flag
That's still a good idea, even if we require TLS1.2
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 20 Nov 2016 20:19:03 +0000 (21:19 +0100)]
Swap hostname and tlshostname
The "tlshostname" argument must match the CN attribute on the server's
certificate if we want SNI to work. Since that is set to "localhost", we
should make sure that the -H argument actually matches that, or this
test may fail (depending on GnuTLS version)
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 20 Nov 2016 19:54:17 +0000 (20:54 +0100)]
Reorder tls initialization
If we're going to check whether tlshostname != NULL, then make sure we
don't just always set it to !NULL the line before.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 20 Nov 2016 19:46:39 +0000 (20:46 +0100)]
Update priority string
We want to disallow TLS <1.2, as per spec, so update the priority string
to do so.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 20 Nov 2016 19:44:42 +0000 (20:44 +0100)]
Move DH initialization into the start of the program
Initializing DH parameters takes a while (~1s on a 2.3GHz Intel Haswell
core i7). Rather than doing it once per connection, do it once per
nbd-server run.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Wed, 9 Nov 2016 23:09:12 +0000 (00:09 +0100)]
Update HAVE_GNUTLS checks in nbd-tester-client, too
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Wed, 9 Nov 2016 23:07:53 +0000 (00:07 +0100)]
Improve TLS-related help output
- Make it fit without 80 columns
- Add note about -x parameter, too
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Wed, 9 Nov 2016 23:03:20 +0000 (00:03 +0100)]
Sigh
Add the missing "x"
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Wed, 9 Nov 2016 22:57:47 +0000 (23:57 +0100)]
Fix check for HAVE_GNUTLS
If we always define a variable (but simply set it to 0 sometimes), we
cannot use #ifdef but must use #if instead.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Wed, 9 Nov 2016 22:50:57 +0000 (23:50 +0100)]
Fix shell syntaxis
The "test" command takes one or more arguments, which are then evaluated
in the shell before processing. An assignment results in an exit state,
which as far as test is concerned, is fine enough.
Since we wrote "x$HAVE_GNUTLS=1", that would get expanded to "x0=1",
meaning, we were assigning the value 1 to the variable "x0", which meant
that "test" was seeing one argument rather than the three that we wanted
to give it, and so it was happy.
Fix by adding the required spaces to make test see the equals sign and
perform the comparison.
Shell is a horrible language.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Wed, 9 Nov 2016 22:10:48 +0000 (23:10 +0100)]
Remove last reference to tls_dir option
This option no longer exists, so we shouldn't reference it anymore.
Signed-off-by: Wouter Verhelst <w@uter.be>
Alex Bligh [Wed, 9 Nov 2016 12:25:17 +0000 (12:25 +0000)]
Set GnuTLS 2.12.0 as the minimum version
We now compile against GnuTLS 2.12.0 (probably an earlier version would work
too, but I don't have one right here to test against).
Signed-off-by: Alex Bligh <alex@alex.org.uk>
Alex Bligh [Wed, 9 Nov 2016 12:17:47 +0000 (12:17 +0000)]
gnutls_transport_set_int is only available for gnutls 3.1.9 or later
gnutls_transport_set_int is only available for gnutls 3.1.9 or later so
on earlier versions use gnutls_transport_set_ptr with a cast, or more
accurately two casts (ugly), as that eliminates a warning about a cast
from an integer to a pointer of a different size.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
Alex Bligh [Wed, 9 Nov 2016 12:06:38 +0000 (12:06 +0000)]
GNUTLS_SEC_PARAM_MEDIUM was previously called GNUTLS_SEC_PARAM_NORMAL
Fix one compilation error on early GnuTLS thanks to their non-backwards
compatible API change.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
Alex Bligh [Wed, 9 Nov 2016 11:47:58 +0000 (11:47 +0000)]
Update crypto-gnutls.c to upstream
Update crypto-gnutls.c to upstream to fix bug when GNU_TLS_E_AGAIN
is returned. See:
https://github.com/abligh/tlsproxy/issues/1
Signed-off-by: Alex Bligh <alex@alex.org.uk>
Wouter Verhelst [Tue, 8 Nov 2016 23:36:21 +0000 (00:36 +0100)]
Fix defines again
s/WITH_GNUTLS/HAVE_GNUTLS/g
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 8 Nov 2016 23:27:25 +0000 (00:27 +0100)]
Allow anonymous TLS, too
Certificate authentication is nice, but we shouldn't require it.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 8 Nov 2016 23:27:02 +0000 (00:27 +0100)]
Document nbd-client options
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 8 Nov 2016 23:24:54 +0000 (00:24 +0100)]
Remove hostname confusion
Currently, there is a "hostname" argument (which specifies the server to
connect to, and which is not an option argument) and a '-hostname'
argument (which takes the hostname for SNI connectivity in the TLS
context). The two are only tangentially related, and it could be
confusing to have two "hostname" arguments.
Rename the TLS-related one to tlshostname to clarify.
Signed-off-by: Wouter Verhelst <w@uter.be>
Alex Bligh [Tue, 12 Apr 2016 13:18:27 +0000 (14:18 +0100)]
Add TLS support to NBD client
Signed-off-by: Alex Bligh <alex@alex.org.uk>
(cherry picked from commit
ad3ddfe14bce1c7fca8ab7205d210ab2e312aaac)
[wouter: fix conflict with multiple connections changes]
Signed-off-by: Wouter Verhelst <w@uter.be>
Alex Bligh [Tue, 12 Apr 2016 12:07:26 +0000 (13:07 +0100)]
Add options to nbd-client for TLS support
Signed-off-by: Alex Bligh <alex@alex.org.uk>
(cherry picked from commit
40426ea76ab4c2c37ddda96e56f9145f53c531aa)
[wouter: make --certfile be -F rather than -C, which was already taken by --connections]
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 8 Nov 2016 20:01:46 +0000 (21:01 +0100)]
Reorder code
No need to #endif out of the TLS code block if we're going to #ifdef it again
half a screen further down
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 8 Nov 2016 19:34:56 +0000 (20:34 +0100)]
send_export_info(): use socket_write()
This function was still using plain write() calls rather than socket_write()
ones. Fix.
This makes nbd-server survive the TLS test suite.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 8 Nov 2016 18:50:01 +0000 (19:50 +0100)]
Fix logic error
The combined check for tls-only exports with "is this the right export"
was failing. Additionally, it was complicated code. Rather than trying
to fix it, separate the two checks and use "continue" to move on to the
next export if the tls check fails.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 8 Nov 2016 18:39:54 +0000 (19:39 +0100)]
Don't forget to read and check length of STARTTLS
We forgot to read the "length" field of the STARTTLS command.
Additionally, we need to check that the length is in fact zero
(otherwise the STARTTLS command is invalid)
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sat, 5 Nov 2016 13:36:29 +0000 (14:36 +0100)]
Be a bit clearer on failure
Saying "it wasn't NBD_REP_ACK isn't as useful as "I received foo while I
expected NBD_REP_ACK" for debugging.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sat, 5 Nov 2016 13:35:33 +0000 (14:35 +0100)]
Show some more information on why we failed
When the handshake fails, it's useful to know why exactly it fails, so
produce an error message in that case.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sat, 5 Nov 2016 13:34:42 +0000 (14:34 +0100)]
Don't dereference pointers that may be NULL
There's a test for !client, but that is done after we try to dereference
it. That can't be right.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:52:23 +0000 (19:52 +0100)]
Dereference the correct variable
tlsdir no longer exists, so stop trying to read from it.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:50:14 +0000 (19:50 +0100)]
Update comment to sync with reality
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:49:31 +0000 (19:49 +0100)]
Fix AC_DEFINE usage
Apparently AC_DEFINE does not expand shell variables, so we need to
place it inside an if structure.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sat, 5 Nov 2016 13:41:25 +0000 (14:41 +0100)]
Log something on address family mismatch
Currently, when we refuse to match an authorized_file entry for address
family mismatches, we don't say anything and instead just exit. That
works, but makes debugging harder.
Add a log message, so that users can at least figure out why things
fail.
Should help for github issue #35, although it's not a full fix.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:28:58 +0000 (19:28 +0100)]
Add the set_nonblocking function
Commit
be34514f857d60b453bb12a8dca626ace1dc79a5 (which we don't want)
added the set_nonblocking() function to cliserv.c, which
2a5a4f2058827279247641d2db326079b026ca15 added to nbd-tester-client.
Add this now.
Fixes:
2a5a4f2058827279247641d2db326079b026ca15
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:24:55 +0000 (19:24 +0100)]
Use correct #ifdefs
I forgot to update nbd-tester-client.c to check the correct #ifdefs
(HAVE_GNUTLS rather than WITH_GNUTLS). Fix.
Fixes:
2a5a4f2058827279247641d2db326079b026ca15
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:22:57 +0000 (19:22 +0100)]
Improve upon build system even further
We botched the build system changes a bit, so fix them properly now.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:20:07 +0000 (19:20 +0100)]
Make nbd-client -l work again
We don't need to open an nbd device if all we're going to ask for is the
list of exports.
Also, nbd-client -l is a valid thing to run even if you don't have
access to /dev/nbd* (which the test suite does, for instance), so don't
even try to open the device node unless we really need to.
Fixes:
4e08f11ab9996d0c4a8c52d5acecf8b33890825f
Signed-off-by: Wouter Verhelst <w@uter.be>
Alex Bligh [Mon, 11 Apr 2016 16:46:35 +0000 (17:46 +0100)]
Add TLS testing to nbd-tester-client.c
This commit adds TLS testing to nbd-tester-client and 'make check'.
If TLS is not compiled in, then the test is skipped.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
(cherry picked from commit
48065afaa8dee3eda6221660200ac473becf5c0e)
[wouter: update to make it apply in the face of handshake test]
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:02:41 +0000 (19:02 +0100)]
Clarify which group
We specify the name of the group in an extra parameter, without actually
using it, which causes a compiler warning. While removing that extra
parameter would have fixed this, stating which group we're failing on is
actually useful information, so add that to the output.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:02:08 +0000 (19:02 +0100)]
Use correct function
strcmp doesn't take a length, but strncmp does
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 3 Nov 2016 18:00:26 +0000 (19:00 +0100)]
Fix tls options
Rather than specifying a single directory that contains all files,
specify the three options which we need by individual files.
Based on commit
e851a027fe889779c091233849797718a7ae3792, originally by
Alex Bligh <alex@alex.org.uk>, but significantly altered to make it also
remove my older stuff.
Signed-off-by: Wouter Verhelst <w@uter.be>
Alex Bligh [Sun, 10 Apr 2016 13:33:56 +0000 (14:33 +0100)]
Add GnuTLS infrastructure
Add configure.ac section to detect GnuTLS
Add buffer.[ch] and crypto-gnutls.[ch] from
https://github.com/abligh/tlsproxy
Add Makefile.am changes to link these new files in
Signed-off-by: Alex Bligh <alex@alex.org.uk>
(cherry picked from commit
aac8f6afbaf982431b4b97c978c3d0156badbbe5)
[wouter: updated to cooperate with server-side GnuTLS support that already exists]
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 1 Nov 2016 09:28:51 +0000 (10:28 +0100)]
doc: Allow NBD_OPT_ABORT in FORCEDTLS
It does not make sense to disallow a soft disconnect when the client
cannot do TLS, so make that explicitly legal as well in the FORCEDTLS
situation.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Mon, 31 Oct 2016 02:03:28 +0000 (03:03 +0100)]
Allow setting the number of connections from the nbdtab file, too
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Mon, 31 Oct 2016 01:54:36 +0000 (02:54 +0100)]
Document new options
The last two patches introduced new command-line options for nbd-client
and new config file options for nbd-server, but did not document them.
Fix.
Signed-off-by: Wouter Verhelst <w@uter.be>
Josef Bacik [Thu, 8 Sep 2016 19:30:27 +0000 (12:30 -0700)]
nbd-client: add support for multiple client connections
Now that the kernel provides the ability to have multiple connections per NBD
device, provide an option for the user to specify how many connections to open
up.
Signed-off-by: Josef Bacik <jbacik@fb.com>
(cherry picked from commit
4eec891cd6cd25a420f3fe10b32cd9d561058572)
Signed-off-by: Wouter Verhelst <w@uter.be>
Josef Bacik [Tue, 5 Jul 2016 18:53:20 +0000 (14:53 -0400)]
Add splice support
Using splice can net a pretty big performance improvement. Add a per export
option to enable splice. Using splice will create a pipe for every write to
buffer our write buffer into, and will create a temporary pipe for reads to read
our data into and splice into our socket.
Splice has a few limitations. We are limited to the in kernel pipe size for our
requests, so if any request exceeds those limits we'll fall back to normal
read/writes. Copyonwrite would add to the complexity by having multiple pipes
to deal with any diff files, so to avoid that we simply do not allow copyonwrite
coupled with splice.
Signed-off-by: Josef Bacik <jbacik@fb.com>
(cherry picked from commit
3135b8677efa31069e846d659398abf2e8827c67)
[wouter: modified to apply after STARTTLS patch, and to not trigger when TLS is active]
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 30 Oct 2016 23:38:10 +0000 (00:38 +0100)]
Provide a success message for the handshake test
All other tests produce a message upon successful completion of the
test, so make the handshake test follow suit.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 30 Oct 2016 23:37:06 +0000 (00:37 +0100)]
Move declaration to the right place
A forward declaration that isn't actually a forward one is less than
helpful.
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Thu, 20 Oct 2016 00:48:15 +0000 (19:48 -0500)]
tests: Cover recent bug fixes
Add a new test 'handshake' that intentionally provokes a server error
during option negotiation, before falling back to NBD_OPT_ABORT to
end negotiation, to prove that the server is correctly allowing a
client to fall back to known options; thus covering two recent bug
fixes for a server sending the wrong length in an error reply, and
for a server not reading enough data when replying to an unknown
command.
Signed-off-by: Eric Blake <eblake@redhat.com>
[reflowed in light of
b8852465a]
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Sun, 30 Oct 2016 23:10:03 +0000 (00:10 +0100)]
Properly handle maxconnections
The "maxconnections" setting considers connections to all exports,
rather than "just" the one where the "maxconnections" setting is set.
This is unexpected and not useful.
Fix by creating a socketpair over which the child asks the parent
process whether a connection to the given export is allowed, rather than
doing the check in the client process.
Closes github #35
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 11:51:55 +0000 (13:51 +0200)]
Add missing #endif
Commit e8c8cdc removed the non-GnuTLS alternate implementation of
handle_starttls, but accidentally removed the #endif at the end, causing
failure to compile.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 11:50:17 +0000 (13:50 +0200)]
EXTRA_DIST support/genver.sh
Now that we pass '-I support' through ACLOCAL_AMFLAGS, the "support"
directory needs to actually exist for things to continue working.
Additionally, support/genver.sh needs to exist in non-git directories
for autoconf to do its thing.
Ship genver.sh to kill both flies with one stone.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 08:20:05 +0000 (10:20 +0200)]
Don't forget the NBD_REP_ACK
We're not allowed to start TLS negotiation until *after* we've sent an
ACK to the client. Do so.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 08:18:05 +0000 (10:18 +0200)]
Send a proper error if GnuTLS was not compiled in
Currently we just hung up, which is not cool (and not what the spec wants).
Instead, we should send NBD_REP_ERR_PLATFORM.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 08:16:44 +0000 (10:16 +0200)]
Don't forget the break
After successfully negotiating TLS, we should read the next option, not
send NBD_REP_ERR_UNSUP (in the encrypted channel!) to the client...
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Mon, 17 Oct 2016 20:23:40 +0000 (15:23 -0500)]
server: Read client's TLS length data before next option
Any client attempting to probe support for a new option, such as
NBD_OPT_STARTTLS or NBD_OPT_GO, with plans to do a graceful
fallback to older methods, will fail in its attempt if the server
does not consume the length field and potential payload of the
unrecognized (or rejected) option, because the server will then
be reading out of sync and not see the client's magic for the
next option. While it is true that sane clients are unlikely to
send more than one NBD_OPT_STARTTLS, and thus never trigger some
of the paths in this patch, it is still better to be robust for
all clients.
Furthermore, even if the server requires TLS, and rejects all but
NBD_OPT_STARTTLS as the first valid option, it should still honor
NBD_OPT_ABORT.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Mon, 17 Oct 2016 20:23:38 +0000 (15:23 -0500)]
server: Read client's unknown option length before next option
Any client attempting to probe support for a new option, such as
NBD_OPT_STARTTLS or NBD_OPT_GO, with plans to do a graceful
fallback to older methods, will fail in its attempt if the server
does not consume the length field and potential payload of the
unrecognized (or rejected) option, because the server will then
be reading out of sync and not see the client's magic for the
next option. This bug has been latent in the reference
server since commit 626c2a3 in 2012, even though it is EXACTLY
the bug that NBD_FLAG_FIXED_NEWSTYLE was designed to prevent.
The only reason it has been buggy for so long is that it has
taken us this long to finally want to implement clients that use
a new option.
This patch fixes only the portion of the server that has been
previously released, to make backports easier. The new code for
handling TLS also needs fixing, in the next patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Mon, 17 Oct 2016 20:23:37 +0000 (15:23 -0500)]
server: Swap argument order in consume()
The signature of consume() threw me off. Good design says that if
you are going to have paired parameters (buf and bufsize), you
generally want them adjacent, not separated by an unrelated parameter
(len). Move len to be first, adjusting all callers.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Mon, 17 Oct 2016 20:23:36 +0000 (15:23 -0500)]
server: Fix botched strlen computation of error message
Commit 3b80382 tried to make it easy for the server to send an
error message whose length was determined by strlen(), but ended
up sending a length of UINT32_MAX, causing clients to either
hang up (reply too large) or wait for nearly 4G of data that was
never coming.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 07:55:23 +0000 (09:55 +0200)]
Correct comment
NBD_OPT_EXPORT_NAME cannot be issued before NBD_OPT_STARTTLS, period,
since it needs to be the final option. However, future versions of
nbd-server will support NBD_OPT_SELECT/NBD_OPT_GO, in which case this
*will* be necessary.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 07:54:01 +0000 (09:54 +0200)]
Conditionally compile in GnuTLS
... so that we can still compile on platforms that don't have
GnuTLS >= 3.3
TODO: consider whether allowing an older version of GnuTLS is sensible.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 07:51:35 +0000 (09:51 +0200)]
Set function pointers
The client->socket_read and client->socket_write function pointers need
to be changed after we negotiated TLS. Forgetting this means we
negotiate TLS successfully, and then try to continue to communicate in
cleartext...
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 21 Oct 2016 07:50:58 +0000 (09:50 +0200)]
Fix memory leak
If we don't need the session, we need to deallocate after deiniting it.
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Thu, 20 Oct 2016 19:05:52 +0000 (14:05 -0500)]
build: Ignore copied file during in-tree build
Commit b885246 creates a symlink to work around an automake weakness,
but forgot to ignore the link when doing an in-tree build.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Wed, 19 Oct 2016 19:37:15 +0000 (14:37 -0500)]
build: Silence autogen.sh warnings
Starting from a fresh git checkout, running ./autogen.sh gives a
couple of warnings on my Fedora 24 build tools, one from libtool:
libtoolize: Consider adding '-I support' to ACLOCAL_AMFLAGS in Makefile.am.
and one from automake:
tests/run/Makefile.am:4: warning: source file '$(top_srcdir)/cliserv.c' is in a subdirectory,
tests/run/Makefile.am:4: but option 'subdir-objects' is disabled
automake: warning: possible forward-incompatibility.
automake: At least a source file is in a subdirectory, but the 'subdir-objects'
automake: automake option hasn't been enabled. For now, the corresponding output
automake: object file(s) will be placed in the top-level directory. However,
automake: this behaviour will change in future Automake versions: they will
automake: unconditionally cause object files to be placed in the same subdirectory
automake: of the corresponding sources.
automake: You are advised to start using 'subdir-objects' option throughout your
automake: project, to avoid future incompatibilities.
Following the advice almost works, except that automake 1.15 still
has a nasty bug (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=13928)
where use of $(foo) in a _SOURCES variable coupled with subdir-objects
creates a directory with a literal name $(foo) rather than the intended
name. And while open-coding it (using ../../ instead of $(top_srcdir)/)
works around the problem of bad naming in automake 1.15, it fails for
automake 1.11 (hello CentOS 6), due to the order in which .deps files
are erased vs. included in makefiles. The solution that works across
all automake versions is to only stick files in _SOURCES that do not
live outside of the subtree. Note that we only need to copy cliserv.c
into the tests/run directory; automakes handling of dependencies will
still rebuild against .h file changes even without listing the .h files
or copying them locally.
I also noticed that the build was already leaving behind an untracked
manpage.log file, in addition to the new .dirstamp witness file created
by our new use of subdir-objects.
This patch has been tested with 'make distcheck' across multiple
automake and libtool versions, ranging from CentOS 6 vintage to current
git toolchains.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Wed, 19 Oct 2016 19:37:14 +0000 (14:37 -0500)]
build: Distribute netdb-compat.h without relying on tests
nbd-server depends on netdb-compat.h; however, we were only
including it in the tarball as a side effect of it also being
used by the testsuite. Make the dependency explicit.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Eric Blake [Thu, 20 Oct 2016 14:22:06 +0000 (09:22 -0500)]
maint: Let emacs know our preferred style
Teach emacs that this entire source tree uses Linux style
indentation (8-space indents, but with hard TAB character
instead of spaces). Without this hint, the default emacs
C style tries to use 2-space indentation, making it a pain
to edit correctly when automatic formatting is enabled.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 18:47:29 +0000 (20:47 +0200)]
Improve string
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 18:47:04 +0000 (20:47 +0200)]
Rename the goto label to clarify that we're dealing with a hard close here
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 18:46:47 +0000 (20:46 +0200)]
Don't send an option reply to OPT_EXPORT_NAME
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 18:39:54 +0000 (20:39 +0200)]
Silence compiler warning
size.c: In function ‘main’:
size.c:12:2: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
write(fd, filename, 1);
We don't actually care (if write fails, the size detection will also
fail, so the test will fail), but it doesn't hurt to be somewhat more
explicit.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 00:10:56 +0000 (02:10 +0200)]
Whitespace fix
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 00:08:46 +0000 (02:08 +0200)]
Don't change socket options after negotiation
Now that we may negotiate TLS in the negotiation function, manually
touching the socket after setting up the GnuTLS context is no longer a
good idea.
Move the functions that change socket options so they are called before
negotiate().
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 00:07:55 +0000 (02:07 +0200)]
Send error messages to the client
When an error occurs during negotiation, make the server send a
human-readable error message to the client along with the error reply
message.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Tue, 11 Oct 2016 00:06:53 +0000 (02:06 +0200)]
Allow for easy sending of error messages
While the protocol spec allows it, currently we don't actually send any
data to the client when we send an error message.
Change the send_reply() function to make that easier.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Mon, 10 Oct 2016 23:16:46 +0000 (01:16 +0200)]
Implement STARTTLS server side
This adds support for STARTTLS to nbd-server. Still TODO are:
- Testing
- client-side
- Testing
- Verification that everything works the way it should, also against other
implementations (e.g., qemu)
- Testing
- Add unit tests for STARTTLS implementation, too.
- Testing
Did I mention this hasn't been tested well, yet?
At least it compiles.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Mon, 10 Oct 2016 22:14:56 +0000 (00:14 +0200)]
Resync comment with reality
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Mon, 10 Oct 2016 22:10:20 +0000 (00:10 +0200)]
Speed up the critical path
These socket_read() and socket_write() functions will now be called
everywhere in the critical path (especially socket_write(), which will
be called with a mutex held). Therefore, their performance is critical.
Since a conditional jumps wreak havoc on cache prediction, use function
pointers to avoid them, thereby not negatively impacting performance
too much.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Mon, 10 Oct 2016 22:04:17 +0000 (00:04 +0200)]
Refactor negotiation a bit.
Currently, we did negotiation by way of direct read() and write() calls.
This has two problems:
- The error handling in the implementation is not perfect; it is
possible to have short reads, which would result in the server
assuming a buffer has a given size, when in reality it does not.
- Having direct read() and write() calls makes it impossible to abstract
away whether we're using TLS or not.
The easy fix is to use the new socket_read() and socket_write() calls,
but those require a CLIENT* to be available before they can be used.
Rather than allocating it in handle_export_name(), we now allocate it in
negotiate(), passing the pointer to the export name handler, and
removing its allocation.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Mon, 10 Oct 2016 20:54:24 +0000 (22:54 +0200)]
Remove bashism
Running an EXIT trap on abnormal termination of the shell is a bashism;
dash, for instance, does not run them on SIGINT.
Fix by explicitly adding a trap for SIGINT.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 7 Oct 2016 15:16:07 +0000 (17:16 +0200)]
Re-enable integrityhuge test
This used to cause deadlocks due to the fact that both server and tester
client did single-threaded and blocking I/O. However, this has now been
fixed, so we really should re-enable the check.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 7 Oct 2016 15:10:35 +0000 (17:10 +0200)]
Prepare for allowing STARTTLS
Replace every use of readit() or writeit() on a socket by an abstracting
socket_read()/socket_write() function instead.
This socket_read()/socket_write() function will call
gnutls_record_recv()/gnutls_record_send() if the client is doing TLS,
which we signal by the non-NULL-ness of the new .tls_session member of
the CLIENT struct.
After that, once we add STARTTLS negotiation, we should be able to do
TLS properly.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 7 Oct 2016 15:08:22 +0000 (17:08 +0200)]
Don't claim we're exiting when we're not.
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 7 Oct 2016 15:07:46 +0000 (17:07 +0200)]
Search for and compile in GnuTLS
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Fri, 7 Oct 2016 10:48:12 +0000 (12:48 +0200)]
glibc wants _DEFAULT_SOURCE rather than _BSD_SOURCE nowadays
Signed-off-by: Wouter Verhelst <w@uter.be>
Josef Bacik [Wed, 6 Jul 2016 16:30:15 +0000 (12:30 -0400)]
Work with old autotools
m4_esyscmd_s doesn't exist with old autotools, nor does serial_tests. Fix up
these to use the old variants and not include serial_tests if our autotools
don't have support for it. Also add gthread to the glib check since older glibs
don't get the proper includes/libs by default without it.
Signed-off-by: Josef Bacik <jbacik@fb.com>
(cherry picked from commit
dc6b6e208f597e7e287a3181f1fcbd923d2bf223)
Signed-off-by: Wouter Verhelst <w@uter.be>
Josef Bacik [Wed, 6 Jul 2016 16:25:08 +0000 (12:25 -0400)]
Fix compilation errors on old glib versions
On centos6 the older version of glib needs to have g_thread_init() run before we
create the threadpool, however this is a deprecated function and we don't need
it on newer glibs. So add a configure check to see if our glib is the old one
and add the appropriate includes and g_thread_init() call. Tested on centos6
and Fedora 23 to make sure it did the right thing in both cases.
Signed-off-by: Josef Bacik <jbacik@fb.com>
(cherry picked from commit
92fc4f11aa6c27aeb7ba28a510b550b550d13b60)
Signed-off-by: Wouter Verhelst <w@uter.be>
Wouter Verhelst [Thu, 6 Oct 2016 12:46:16 +0000 (14:46 +0200)]
Clarify that clients cannot expect ordering in the face of multiple connections.
Wouter Verhelst [Sat, 1 Oct 2016 11:34:51 +0000 (13:34 +0200)]
Clarify that it's not really *all* options
Closes github issue#40
Alex Bligh [Mon, 26 Sep 2016 18:36:10 +0000 (19:36 +0100)]
Ensure length of flush command is set to zero
Signed-off-by: Alex Bligh <alex@alex.org.uk>
Carl-Daniel Hailfinger [Mon, 26 Sep 2016 01:22:52 +0000 (03:22 +0200)]
nbd-tester-client: fix flags usage.
Running nbd-tester-client against nbdkit with oldstyle negotiation was fun.
I managed to segfault nbdkit and noticed that nbd-tester-client speaks
the oldstyle protocol incorrectly, ignoring flags sent by the server.
Fix.
Alex Bligh [Thu, 15 Sep 2016 12:25:47 +0000 (13:25 +0100)]
Docs: remove reference to 'write barrier' in NBD_CMD_FLUSH
Reference to a 'write barrier' in NBD_CMD_FLUSH is confusing as it might
appear to be a reference to an old-style linux block layer write barrier
which actually waits for all writes to complete, rather than just requiring
writes that have completed (and for which replies have been sent) have
been persisted.
Signed-off-by: Alex Bligh <alex@alex.org.uk>
Wouter Verhelst [Thu, 11 Aug 2016 17:25:09 +0000 (19:25 +0200)]
Add CII Best Practices badge. No, we're not ready yet.
Wouter Verhelst [Wed, 13 Jul 2016 15:17:30 +0000 (17:17 +0200)]
Don't forget to ship the nbd@.service.tmpl file
Wouter Verhelst [Wed, 13 Jul 2016 08:42:57 +0000 (10:42 +0200)]
Move statements to their correct section
Before belongs in the [Unit] section, not the [Service] one. Also,
remove duplicate RequiredBy statement in Service section.
Wouter Verhelst [Thu, 30 Jun 2016 18:06:24 +0000 (20:06 +0200)]
Add missing closing tag for para element
Wouter Verhelst [Thu, 23 Jun 2016 14:37:31 +0000 (16:37 +0200)]
Force generation of the nbd@.service file
Wouter Verhelst [Thu, 23 Jun 2016 14:27:17 +0000 (16:27 +0200)]
Don't do the symlink things
This confuses automake a bit too much. Instead, use a relative reference
Wouter Verhelst [Thu, 23 Jun 2016 14:23:36 +0000 (16:23 +0200)]
Add systemd directory to autogen, too