Re: [libvirt] [PATCH 05/15] Generic module for handling SASL authentication & encryption
On Thu, Dec 16, 2010 at 04:36:34PM -0700, Eric Blake wrote:
On 12/16/2010 04:21 AM, Daniel P. Berrange wrote:
> This provides two modules for handling SASL
>
> * virNetSASLContext provides the process-wide state, currently
> just a whitelist of usernames on the server and a one time
> library init call
>
> * virNetTLSSession provides the per-connection state, ie the
> SASL session itself. This also include APIs for providing
> data encryption/decryption once the session is established
>
> * src/Makefile.am: Add to libvirt-net-rpc.la
> * src/rpc/virnetsaslcontext.c, src/rpc/virnetsaslcontext.h: Generic
> SASL handling code
> ---
> po/POTFILES.in | 1 +
> src/Makefile.am | 3 +
> src/rpc/virnetsaslcontext.c | 525 +++++++++++++++++++++++++++++++++++++++++++
> src/rpc/virnetsaslcontext.h | 125 ++++++++++
> 4 files changed, 654 insertions(+), 0 deletions(-)
> create mode 100644 src/rpc/virnetsaslcontext.c
> create mode 100644 src/rpc/virnetsaslcontext.h
> +
> +int virNetSASLSessionSecProps(virNetSASLSessionPtr sasl,
> + int minSSF,
> + int maxSSF,
> + bool allowAnonymous)
> +{
> + sasl_security_properties_t secprops;
> + int err;
> +
> + memset (&secprops, 0, sizeof secprops);
> +
> + secprops.min_ssf = minSSF;
> + secprops.max_ssf = maxSSF;
> + secprops.maxbufsize = 100000;
How was this arbitrary number picked? Should it be larger, to
accommodate REMOTE_MESSAGE_MAX (262144)?
Well it was just copied from existing code, but looking at the
detailed SASL library API docs, it seems maxbufsize sets the
maximum len that can be passed to sasl_encode/decode. SASL may
give you back a smaller maxbufsize, so we need to check it after
negotiating the session. Then we need to make sure we don't
call sasl_encode/decode with too large a value. This shouldn't
be too difficult we can just limit the number of bytes and the
code should already do the right thing to encode the rest later.
> +int virNetSASLSessionServerStep(virNetSASLSessionPtr sasl,
> + default:
> + VIR_DEBUG("Foo %s", sasl_errdetail(sasl->conn));
Interesting debug message; should "Foo" have been something more legible?
> +ssize_t virNetSASLSessionEncode(virNetSASLSessionPtr sasl,
> + const char *input,
> + size_t inputLen,
> + const char **output,
> + size_t *outputlen)
> +{
> + unsigned inlen = inputLen;
Should you check and fail if ((unsigned)inputLen != inputLen), since
sasl_* (unlike gnutls_*) used int rather than size_t as the maximum
transaction size? Or are we assuming that libvirt will never try to
exceed a transaction size of REMOTE_MESSAGE_MAX in the first place, so
we don't have to worry about the 2GB limit being abused?
It is worth adding a check there, even though we don't ever send
more than a few hundred KB.
Daniel