[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 2/3] Fix checking of key usage/purpose data



From: "Daniel P. Berrange" <berrange redhat com>

If key usage or purpose data is not present in the cert, the
RFC recommends that access be allowed. Also fix checking of
key usage to include requirements for client/server certs,
and fix key purpose checking to treat data as a list of bits
---
 src/rpc/virnettlscontext.c |   78 +++++++++++++++++++++++++------------------
 1 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 402029f..1ee5ab2 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -111,6 +111,7 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
     char *buffer = NULL;
     size_t size;
     unsigned int usage;
+    bool allowClient = false, allowServer = false;
 
     VIR_DEBUG("isServer %d isCA %d certFile %s",
               isServer, isCA, certFile);
@@ -193,24 +194,34 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
 
     VIR_DEBUG("Cert %s key usage status %d usage %d", certFile, status, usage);
     if (status < 0) {
-        virNetError(VIR_ERR_SYSTEM_ERROR,
-                    _("Unable to query certificate %s key usage %s"),
-                    certFile, gnutls_strerror(status));
-        goto cleanup;
+        if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
+            usage = isCA ? GNUTLS_KEY_KEY_CERT_SIGN :
+                GNUTLS_KEY_DIGITAL_SIGNATURE|GNUTLS_KEY_KEY_ENCIPHERMENT;
+        } else {
+            virNetError(VIR_ERR_SYSTEM_ERROR,
+                        _("Unable to query certificate %s key usage %s"),
+                        certFile, gnutls_strerror(status));
+            goto cleanup;
+        }
     }
 
-    if (usage & GNUTLS_KEY_KEY_CERT_SIGN) {
-        if (!isCA) {
-            virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
-                        _("Certificate %s usage is for certificate signing, but wanted a server certificate") :
-                        _("Certificate %s usage is for certificate signing, but wanted a client certificate"),
+    if (isCA) {
+        if (!(usage & GNUTLS_KEY_KEY_CERT_SIGN)) {
+            virNetError(VIR_ERR_SYSTEM_ERROR,
+                        _("Certificate %s usage does not permit certificate signing"),
                         certFile);
             goto cleanup;
         }
     } else {
-        if (isCA) {
+        if (!(usage & GNUTLS_KEY_DIGITAL_SIGNATURE)) {
+            virNetError(VIR_ERR_SYSTEM_ERROR,
+                        _("Certificate %s usage does not permit digital signature"),
+                        certFile);
+            goto cleanup;
+        }
+        if (!(usage & GNUTLS_KEY_KEY_ENCIPHERMENT)) {
             virNetError(VIR_ERR_SYSTEM_ERROR,
-                        _("Certificate %s usage is for not certificate signing"),
+                        _("Certificate %s usage does not permit key encipherment"),
                         certFile);
             goto cleanup;
         }
@@ -221,7 +232,11 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
         status = gnutls_x509_crt_get_key_purpose_oid(cert, i, buffer, &size, NULL);
 
         if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
-            VIR_DEBUG("No key purpose data available, skipping checks");
+            VIR_DEBUG("No key purpose data available at slot %d", i);
+
+            /* If there is no data at all, then we must allow client/server to pass */
+            if (i == 0)
+                allowServer = allowClient = true;
             break;
         }
         if (status != GNUTLS_E_SHORT_MEMORY_BUFFER) {
@@ -246,34 +261,31 @@ static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
 
         VIR_DEBUG("Key purpose %d %s", status, buffer);
         if (STREQ(buffer, GNUTLS_KP_TLS_WWW_SERVER)) {
-            if (isCA || !isServer) {
-                virNetError(VIR_ERR_SYSTEM_ERROR, isCA ?
-                            _("Certificate %s purpose is TLS server, but wanted a CA certificate") :
-                            _("Certificate %s client purpose is TLS server, but wanted a TLS client certificate"),
-                            certFile);
-                goto cleanup;
-            }
+            allowServer = true;
         } else if (STREQ(buffer, GNUTLS_KP_TLS_WWW_CLIENT)) {
-            if (isCA || isServer) {
-                virNetError(VIR_ERR_SYSTEM_ERROR, isCA ?
-                            _("Certificate %s purpose is TLS client, but wanted a CA certificate") :
-                            _("Certificate %s server purpose is TLS client, but wanted a TLS server certificate"),
-                            certFile);
-                goto cleanup;
-            }
+            allowClient = true;
         } else if (STRNEQ(buffer, GNUTLS_KP_ANY)) {
-            virNetError(VIR_ERR_SYSTEM_ERROR, (isCA ?
-                        _("Certificate %s purpose is wrong, wanted a CA certificate") :
-                        (isServer ?
-                         _("Certificate %s purpose is wrong, wanted a TLS server certificate") :
-                         _("Certificate %s purpose is wrong, wanted a TLS client certificate"))),
-                        certFile);
-            goto cleanup;
+            allowServer = allowClient = true;
         }
 
         VIR_FREE(buffer);
     }
 
+    if (!isCA) { /* No purpose checks required for CA certs */
+        if (isServer && !allowServer) {
+            virNetError(VIR_ERR_SYSTEM_ERROR,
+                        _("Certificate %s purpose does not allow use for with a TLS server"),
+                        certFile);
+            goto cleanup;
+        }
+        if (!isServer && !allowClient) {
+            virNetError(VIR_ERR_SYSTEM_ERROR,
+                        _("Certificate %s purpose does not allow use for with a TLS client"),
+                        certFile);
+            goto cleanup;
+        }
+    }
+
 
     ret = 0;
 
-- 
1.7.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]