Re: [Nbd] Almost ready for release?

[Wouter Verhelst <w@...112...>]

On Wed, Nov 09, 2016 at 10:02:55AM +0000, Alex Bligh wrote:
> Wouter,
> 
> > On 8 Nov 2016, at 23:39, Wouter Verhelst <w@...112...> wrote:
> > 
> > Hi folks,
> > 
> > The STARTTLS implementation in nbd-server is almost ready. Still TODO:
> > 
> > - Interoperability testing against qemu (yes, yes, I said I'd do that
> >  earlier, but hey)
> > - Client certificate validation
> > - Enforcing TLS1.2 by default
> > - Allowing versions of GnuTLS older than 3.3
> 
> I think we should do at least the last of those prior to release
> because it will stop nbd compiling on several distros otherwise.

All four of those points need to be done before release IMO :-)

My mail was as much a request for review (which you've started -- thanks
for that) as it was for patches implementing the stuff on the TODO list.
I think it's time we have this, but I don't have all that much time, and
little experience with GnuTLS, so help is certainly welcome.

> The change is pretty simple I think - and even if it's not it's
> already done for you in my proxy code. Not sure whether you are
> suggesting the release should wait for that or not (as
> "Other than that I think we're good" implies yes, "I think
> it's time to release" implies no").
> 
> My preference would be to wait until the above are fixed (and
> I need to apologise as I said I'd do qemu interop testing too and
> didn't), but if not, we should at least fix the last and mark
> TLS support as beta.

I would prefer not to go down that route. It's not like it's urgent or
anything (well, apart from "Debian is about to go into freeze", but
hey), and if something is "not ready yet", we should probably not
release it in the first place.

> I have one bug filed against my proxy code (obscure error
> handling issue) I have been procrastinating looking at. I will
> copy that over if it needs fixing.

Saw the commit, thanks.

> > Other than that, I think we're good.
> > 
> > Alex: I pulled the nbd-client and nbd-tester-client implementation from
> > your 'add-tls-support' branch, and it seems to work well; at the very
> > least, the current implementation passes the test suite. Man page
> > updates have been written too. I've had to make a few small changes to
> > make it work on current git HEAD, but most of it is pretty much
> > unchanged from your code.
> 
> Yup. On the server side, there is still one reference to tls_dir
> in the man page (whereas the remainder shows it has similar
> parameters to the client).

Fixed that now, thanks for pointing it out.

> The man page also refers to client certificate checking (which is
> correct - it just needs implementing and testing).

Exactly :)

> > In total, there's 80 commits since 3.14 up to current master, a number
> > of new features (multiple connections, STARTTLS, splice), and a few bug
> > fixes.
> > 
> > I think it's time to release.
> 
> TLS bits aside, I agree, though
>   https://github.com/NetworkBlockDevice/nbd/issues/35
> is a bit annoying.

I changed that (earlier) so that the server at least logs a message now.
That should help with debugging that situation; we can implement a
proper fix later.

-- 
< 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