"Daniel P. Berrange" <berrange@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.