[libvirt] [PATCH 4/4] Fix validation of CA certificate chains

[Michal Privoznik]

On 06.08.2013 13:35, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> The code added to validate CA certificates did not take into
> account the possibility that the cacert.pem file can contain
> multiple (concatenated) cert data blocks. Extend the code for
> loading CA certs to use the gnutls APIs for loading cert lists.
> Add test cases to check that multi-level trees of certs will
> validate correctly.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/rpc/virnettlscontext.c   | 73 ++++++++++++++++++++++++++++++++++----------
>  tests/virnettlscontexttest.c | 59 +++++++++++++++++++++++++++++++++++
>  tests/virnettlshelpers.c     | 34 +++++++++++++++++++++
>  tests/virnettlshelpers.h     |  3 ++
>  tests/virnettlssessiontest.c | 63 ++++++++++++++++++++++++++++++++++++--
>  5 files changed, 214 insertions(+), 18 deletions(-)
> 
> diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> index af0ec21..55fb7d0 100644
> --- a/src/rpc/virnettlscontext.c
> +++ b/src/rpc/virnettlscontext.c

> +#define MAX_CERTS 16
>  static int virNetTLSContextSanityCheckCredentials(bool isServer,
>                                                    const char *cacertFile,
>                                                    const char *certFile)
>  {
>      gnutls_x509_crt_t cert = NULL;
> -    gnutls_x509_crt_t cacert = NULL;
> +    gnutls_x509_crt_t cacerts[MAX_CERTS];
> +    size_t ncacerts = MAX_CERTS;
> +    size_t i;
>      int ret = -1;
>  
>      if ((access(certFile, R_OK) == 0) &&
> -        !(cert = virNetTLSContextLoadCertFromFile(certFile, isServer, false)))
> +        !(cert = virNetTLSContextLoadCertFromFile(certFile, isServer)))
>          goto cleanup;
>      if ((access(cacertFile, R_OK) == 0) &&
> -        !(cacert = virNetTLSContextLoadCertFromFile(cacertFile, isServer, false)))
> +        virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts, &ncacerts) < 0)
>          goto cleanup;
>  
>      if (cert &&
>          virNetTLSContextCheckCert(cert, certFile, isServer, false) < 0)
>          goto cleanup;
>  
> -    if (cacert &&
> -        virNetTLSContextCheckCert(cacert, cacertFile, isServer, true) < 0)
> -        goto cleanup;
> +    for (i = 0 ; i < ncacerts ; i++) {

Spacing

> +        if (virNetTLSContextCheckCert(cacerts[i], cacertFile, isServer, true) < 0)
> +            goto cleanup;
> +    }
>  
> -    if (cert && cacert &&
> -        virNetTLSContextCheckCertPair(cert, certFile, cacert, cacertFile, isServer) < 0)
> +    VIR_DEBUG("Here");
> +    if (cert && ncacerts &&
> +        virNetTLSContextCheckCertPair(cert, certFile, cacerts, ncacerts, cacertFile, isServer) < 0) {
> +        VIR_DEBUG("there");
>          goto cleanup;
> +    }
>  
>      ret = 0;
>  
>  cleanup:
>      if (cert)
>          gnutls_x509_crt_deinit(cert);
> -    if (cacert)
> -        gnutls_x509_crt_deinit(cacert);
> +    for (i = 0 ; i < ncacerts ; i++)

Spacing

> +        gnutls_x509_crt_deinit(cacerts[i]);
>      return ret;
>  }
>  

> diff --git a/tests/virnettlshelpers.c b/tests/virnettlshelpers.c
> index 8236e82..baf043a 100644
> --- a/tests/virnettlshelpers.c
> +++ b/tests/virnettlshelpers.c
> @@ -406,6 +406,40 @@ testTLSGenerateCert(struct testTLSCertReq *req,
>  }
>  
>  
> +void testTLSWriteCertChain(const char *filename,
> +                           gnutls_x509_crt_t *certs,
> +                           size_t ncerts)
> +{
> +    size_t i;
> +    int fd;
> +    int err;
> +    static char buffer[1024*1024];
> +    size_t size;
> +
> +    if ((fd = open(filename, O_WRONLY|O_CREAT, 0600)) < 0) {
> +        VIR_WARN("Failed to open %s", filename);
> +        abort();
> +    }
> +
> +    for (i = 0 ; i < ncerts ; i++) {

Spacing

> +        size = sizeof(buffer);
> +        if ((err = gnutls_x509_crt_export(certs[i], GNUTLS_X509_FMT_PEM, buffer, &size) < 0)) {
> +            VIR_WARN("Failed to export certificate %s", gnutls_strerror(err));
> +            unlink(filename);
> +            abort();
> +        }
> +

Michal