Re: [PATCH 2/3] Allow certificate sanity checking to be disabled

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

 



On 07/21/2011 06:30 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@xxxxxxxxxx>

When libvirtd starts it it will sanity check its own certs,
and before libvirt clients connect to a remote server they
will sanity check their own certs. This patch allows such
sanity checking to be skipped. There is no strong reason to
need todo this, other than to bypass possible libvirt bugs

s/todo/to do/

in sanity checking, or for testing purposes.

libvirt.conf gains tls_no_sanity_certificate parameter to
go along with tls_no_verify_certificate. The remote driver
client URIs gain a no_sanity URI parameter

Makes sense.

+++ b/src/remote/remote_driver.c
@@ -342,7 +342,7 @@ doRemoteOpen (virConnectPtr conn,
       */
      char *name = NULL, *command = NULL, *sockname = NULL, *netcat = NULL;
      char *port = NULL, *authtype = NULL, *username = NULL;
-    int no_verify = 0, no_tty = 0;
+    bool no_sanity = false, no_verify = false, no_tty = false;

Double negatives.  Yuck.  Can we instead go with:

bool sanity = true, ...

      char *pkipath = NULL;

      /* Return code from this function, and the private data. */
@@ -416,11 +416,14 @@ doRemoteOpen (virConnectPtr conn,
                  netcat = strdup (var->value);
                  if (!netcat) goto out_of_memory;
                  var->ignore = 1;
+            } else if (STRCASEEQ (var->name, "no_sanity")) {
+                no_sanity = atoi (var->value) != 0;
+                var->ignore = 1;

sanity = atoi(var->value) == 0; ...

@@ -500,7 +503,7 @@ doRemoteOpen (virConnectPtr conn,
      case trans_tls:
          priv->tls = virNetTLSContextNewClientPath(pkipath,
                                                    geteuid() != 0 ? true : false,
-                                                  no_verify ? false : true);
+                                                  no_sanity, no_verify);

..., !sanity, !verify)

Oops - logic bug. Here, you passed no_sanity (true to skip sanity checking)...

@@ -851,6 +852,7 @@ out_of_memory:
  static virNetTLSContextPtr virNetTLSContextNewPath(const char *pkipath,
                                                     bool tryUserPkiPath,
                                                     const char *const*x509dnWhitelist,
+                                                   bool sanityCheckCert,

but here, you accept sanityCheckCert (true to perform sanity checking).

See why I hate double negatives?

@@ -1048,10 +1055,12 @@ int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt,
  {
      if (virNetTLSContextValidCertificate(ctxt, sess)<  0) {
          if (ctxt->requireValidCert) {
-            virNetError(VIR_ERR_AUTH_FAILED, "%s",
-                        _("Failed to verify peer's certificate"));
+            if (0)
+                virNetError(VIR_ERR_AUTH_FAILED, "%s",
+                            _("Failed to verify peer's certificate"));

Debugging hunk?  Why are we leaving if(0) in?

--
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]