[libvirt] [PATCH 04/15] Generic module for handling TLS encryption and x509 certs

[Eric Blake]

On 12/16/2010 04:21 AM, Daniel P. Berrange wrote:
> This provides two modules for handling TLS
> 
>  * virNetTLSContext provides the process-wide state, in particular
>    all the x509 credentials, DH params and x509 whitelists
>  * virNetTLSSession provides the per-connection state, ie the
>    TLS session itself.
> 
> The virNetTLSContext provides APIs for validating a TLS session's
> x509 credentials. The virNetTLSSession includes APIs for performing
> the initial TLS handshake and sending/recving encrypted data
> 
> * src/Makefile.am: Add to libvirt-net-rpc.la
> * src/rpc/virnettlscontext.c, src/rpc/virnettlscontext.h: Generic
>   TLS handling code

> +/* Check DN is on tls_allowed_dn_list. */
> +static int
> +virNetTLSContextCheckDN(virNetTLSContextPtr ctxt,
> +                        const char *dname)
> +{
> +    const char *const*wildcards;
> +
> +    /* If the list is not set, allow any DN. */
> +    wildcards = ctxt->x509dnWhitelist;
> +    if (!wildcards)
> +        return 1;
> +
> +    while (*wildcards) {
> +        if (fnmatch (*wildcards, dname, 0) == 0)
> +            return 1;
> +        wildcards++;
> +    }

Should this function return -1 if fnmatch returns nonzero other than
FNM_NOMATCH (for example, if wildcards contained an ill-formed entry)?

> +static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
> +    if (status != 0) {
> +        const char *reason = _("Invalid certificate");
> +
> +        if (status & GNUTLS_CERT_INVALID)
> +            reason = _("The certificate is not trusted.");
> +
> +        if (status & GNUTLS_CERT_SIGNER_NOT_FOUND)
> +            reason = _("The certificate hasn't got a known issuer.");

Can status return multiple simultaneous errors, and if so, is this
ordering the desired precedence where the last setting of reason is the
best to present to the user?
> +ssize_t virNetTLSSessionWrite(virNetTLSSessionPtr sess,
> +                              const char *buf, size_t len)
> +{
> +    int ret;

s/int/ssize_t/

> +ssize_t virNetTLSSessionRead(virNetTLSSessionPtr sess,
> +                             char *buf, size_t len)
> +{
> +    int ret;

s/int/ssize_t/

> +
> +    ret = gnutls_record_recv(sess->session, buf, len);
> +
> +    switch (ret) {
> +    case GNUTLS_E_AGAIN:
> +        errno = EAGAIN;
> +        break;
> +    case GNUTLS_E_INTERRUPTED:
> +        errno = EINTR;
> +        break;
> +    case 0:
> +        break;
> +    default:
> +        errno = EIO;

Rather than pollute errno on success, this style of switch statement
(for both Write and Read) should be written:

case GNUTLS_E_AGAIN:...
case GNUTLS_E_INTERRUPTED:...
default:
    if (ret < 0)
        errno = EIO;
    break;

That is, a return of 0 should not be the only value that leaves errno
untouched.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101216/32e809e4/attachment-0001.sig>