[Libvir] PATCH: 3/4: Wire encryption with SASL

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> On Fri, Nov 02, 2007 at 01:09:26AM +0000, Daniel P. Berrange wrote:
>> With the TLS socket, all data is encrypted on the wire. The TCP socket though
>> is still clear text.  Fortunately some SASL authentication mechanism can
...
>>  qemud/internal.h      |   17 ++-
>>  qemud/qemud.c         |  207 +++++++++++++++++++++++++++++-------
>>  qemud/remote.c        |   97 ++++++++++++++++-
>>  src/remote_internal.c |  283 ++++++++++++++++++++++++++++++++++++++++----------

Hi Dan,

I like the design and have started looking at the implementation.
I have two small suggestions so far:

> +#if HAVE_SASL
> +static int qemudClientReadSASL(struct qemud_server *server,
> +                               struct qemud_client *client) {
> +    int got, want;
...
> +    if (want > got)
> +        want = got;

Barely worth mentioning, but...
The first time I read that, I didn't recognize right away that
it's essentially "want = MIN(want, got)".  For such a test, I find
it more readable to reverse the inequality:

   if (got < want)
       want = got;

whatever.

...
> +#if HAVE_SASL
> +static int qemudClientWriteSASL(struct qemud_server *server,
> +                                struct qemud_client *client) {
> +    int ret;
> +
> +    /* Not got any pending encoded data, so we need to encode raw stuff */
> +    if (client->saslEncoded == NULL) {
> +        int err;
> +        err = sasl_encode(client->saslconn,
> +                          client->buffer + client->bufferOffset,
> +                          client->bufferLength - client->bufferOffset,
> +                          &client->saslEncoded,
> +                          &client->saslEncodedLength);
> +
> +        client->saslEncodedOffset = 0;
> +    }

The documentation for sasl_encode suggests that it can fail in many ways.
I guess it needs the same "if (err != SASL_OK) { ... }" test as used in the
latter parts of this patch.