Hi Alex,

I'm going to reply to this series in more detail later (have to go to
work soon), but some quick notes for now:

- I'm not sure I like the idea of having a proxy to do TLS *at the
  server side*, although I do agree that there's an upside of "more
  shared code with client". To be discussed (and I have some more
  thoughts on this that I don't currently have the time to write down).
- The check for GnuTLS in configure.ac should probably be done after the
  AC_PROG_* checks (you shouldn't check for a library before you've
  checked for the compiler etc), and should probably use
  PKG_CHECK_MODULES rather than AC_CHECK_LIB.
- I prefer using an AM_CONDITIONAL to compile certain features
  conditionally over using an #ifdef for an entire file.

On Sun, Apr 10, 2016 at 07:59:05PM +0100, Alex Bligh wrote:
> This is an RFC patch to introduce TLS support on nbdserver.
> 
> This is *NOT* production ready by any means, and is submitted for comment.
> 
> I have added crypto-gnutls.[ch] from:
>   github.com/abligh/tlsproxy
> which is my attempt at an MIT licenced GnuTLS proxy. The proxy element
> is standalone, and is incorporated here. Whilst it's not GPL licensed,
> MIT is compatible. Also it uses GNU format indentation (sorry about that).
> However, it (together with buffer.[ch]) is almost entirely self-contained.
> 
> The same approach (believe it or not) could be taken for nbdclient (which
> is my plan). As the proxy runs as an independent process, nbdclient can
> launch it, then call the DOIT ioctl() on one end of the created socketpair().
> The proxy process then drops into the background and closes after
> either the kernel closes the socket or the other end closes the socket.
> 
> I have tested this to a minimal extent against qemu-img (i.e. qemu
> acting as a client). The problem (see nbdgeneral ad nauseam) we have
> with NBD_CMD_DISC means that we see false reports of 'magic number mismatch'.
> This appears to be because read() returns 0 in negotiate(), and nbdserver
> does not check for this. It then reverses (again) the *previous* magic
> number, using ntohll(), and this causes the 'magic number mismatch' issue.
> This isn't really a problem but causes confusing errors.
> 
> The first two patches are preparatory work, and the third patch actually
> adds NBD_OPT_STARTTLS. The comment in that patch shows what is left to
> figure out.
> 
> Alex Bligh (3):
>   Add GnuTLS infrastructure
>   Add options for TLS support for server
>   Add TLS support to server
> 
>  Makefile.am              |   4 +-
>  buffer.c                 | 225 +++++++++++++++++
>  buffer.h                 |  45 ++++
>  cliserv.h                |   1 +
>  configure.ac             |  11 +
>  crypto-gnutls.c          | 615 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  crypto-gnutls.h          |  43 ++++
>  man/nbd-server.5.in.sgml |  65 +++++
>  nbd-server.c             | 204 +++++++++++++---
>  nbdsrv.h                 |   1 +
>  10 files changed, 1185 insertions(+), 29 deletions(-)
>  create mode 100644 buffer.c
>  create mode 100644 buffer.h
>  create mode 100644 crypto-gnutls.c
>  create mode 100644 crypto-gnutls.h
> 
> -- 
> 1.9.1
> 
> 
> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications Manager
> Applications Manager provides deep performance insights into multiple tiers of
> your business applications. It resolves application problems quickly and
> reduces your MTTR. Get your free trial! http://pubads.g.doubleclick.net/
> gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532
> _______________________________________________
> Nbd-general mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial! http://pubads.g.doubleclick.net/
gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532
_______________________________________________
Nbd-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/nbd-general

Reply via email to