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

Re: [libvirt] [PATCH v2 6/7] storage: Support "chap" authentication for iscsi pool



On 10/07/13 03:10, John Ferlan wrote:
From: Osier Yang <jyang redhat com>

Although the XML for "chap" authentication with plain "password"
was introduced long ago, the function was never implemented.
This patch completes it by performing the authentication during
findPoolSources() processing of pools with an authType of
VIR_STORAGE_POOL_AUTH_CHAP specified.

There are two types of CHAP configurations supported for iSCSI
authentication:

   * Initiator Authentication
       Forward, one-way; The initiator is authenticated by the target.

   * Target Authentication
       Reverse, Bi-directional, mutual, two-way; The target is authenticated
       by the initiator; This method also requires Initiator Authentication

This only supports the "Initiator Authentication". (I don't have any
enterprise iSCSI env for testing, only have a iSCSI target setup with
tgtd, which doesn't support "Target Authentication").

"Discovery authentication" is not supported by tgt yet too. So this only
setup the session authentication by executing 3 iscsiadm commands, E.g:

% iscsiadm -m node --target "iqn.2013-05.test:iscsi.foo" --name \
   "node.session.auth.authmethod" -v "CHAP" --op update

% iscsiadm -m node --target "iqn.2013-05.test:iscsi.foo" --name \
   "node.session.auth.username" -v "Jim" --op update

% iscsiadm -m node --target "iqn.2013-05.test:iscsi.foo" --name \
   "node.session.auth.password" -v "Jimsecret" --op update

Update storage_backend_rbd.c to use the internal API to get the secret
value instead and error out if the secret value is not found. Without the
flag VIR_SECRET_GET_VALUE_INTERNAL_CALL, there is no way to get the value
of private secret.

Update the formatstorage.html to describe the usage of the secret element
to mention that the secret type "iscsi" and "ceph" can be used
to storage pool too.

Update the formatsecret.html to include a reference to the storage pool
---
  docs/formatsecret.html.in           |  10 ++--
  docs/formatstorage.html.in          |  31 +++++++++-
  src/storage/storage_backend_iscsi.c | 113 +++++++++++++++++++++++++++++++++++-
  src/storage/storage_backend_rbd.c   |  13 ++++-
  4 files changed, 159 insertions(+), 8 deletions(-)

diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in
index 50c9533..3e306b5 100644
--- a/docs/formatsecret.html.in
+++ b/docs/formatsecret.html.in
@@ -64,8 +64,9 @@
        a single <code>name</code> element that specifies a usage name
        for the secret.  The Ceph secret can then be used by UUID or by
        this usage name via the <code>&lt;auth&gt;</code> element of
-      a <a href="formatdomain.html#elementsDisks">disk
-      device</a>. <span class="since">Since 0.9.7</span>.
+      a <a href="formatdomain.html#elementsDisks">disk device</a> or
+      a <a href="formatstorage.html">storage pool (rbd)</a>.
+      <span class="since">Since 0.9.7</span>.
      </p>
<h3>Usage type "iscsi"</h3>
@@ -76,8 +77,9 @@
        a single <code>target</code> element that specifies a usage name
        for the secret.  The iSCSI secret can then be used by UUID or by
        this usage name via the <code>&lt;auth&gt;</code> element of
-      a <a href="formatdomain.html#elementsDisks">disk
-      device</a>. <span class="since">Since 1.0.4</span>.
+      a <a href="formatdomain.html#elementsDisks">disk device</a> or
+      a <a href="formatstorage.html">storage pool (iscsi)</a>.
+      <span class="since">Since 1.0.4</span>.
      </p>
<h2><a name="example">Example</a></h2>
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index d702eb1..cf028dd 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -72,6 +72,9 @@
          &lt;source&gt;
            &lt;host name="iscsi.example.com"/&gt;
            &lt;device path="demo-target"/&gt;
+          &lt;auth type='chap' username='myname'&gt;
+            &lt;secret type='iscsi' usage='mycluster_myname'/&gt;
+          &lt;/auth&gt;
            &lt;vendor name="Acme"/&gt;
            &lt;product name="model"/&gt;
          &lt;/source&gt;
@@ -80,7 +83,6 @@
      <pre>
          ...
          &lt;source&gt;
-        &lt;source&gt;
            &lt;adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/&gt;
          &lt;/source&gt;
          ...</pre>
@@ -123,6 +125,33 @@
          which is the hostname or IP address of the server. May optionally
          contain a <code>port</code> attribute for the protocol specific
          port number. <span class="since">Since 0.4.1</span></dd>
+      <dt><code>auth</code></dt>
+      <dd>If present, the <code>auth</code> element provides the
+        authentication credentials needed to access the source by the
+        setting of the <code>type</code> attribute. The <code>type</code>
+        must be either "chap" or "ceph". For a "chap" <code>type</code>
+        attribute, mandatory attributes <code>login</code> or
+        <code>username</code> and either a <code>passwd</code> or a
+        <code>secret</code> sub-element are used to provide the
+        authentication identification. <span class="since">Since 1.1.1,</span>
+        when writing out the XML, libvirt will utilize the
+        <code>username</code> instead of <code>login</code>.
+        For a "ceph" <code>type</code> attribute a mandatory attribute
+        <code>username</code>, which identifies the username to use during
+        authentication as well as a sub-element <code>secret</code> with
+        a mandatory attribute <code>type</code>, to tie back to a
+        <a href="formatsecret.html">libvirt secret object</a> that
+        holds the actual password or other credentials. The domain XML
+        intentionally does not expose the password, only the reference
+        to the object that does manage the password. The secret element
+        <code>type</code> must be either "ceph" or "iscsi". Use "ceph" for
+        Ceph RBD (rados block device) network sources and use "iscsi" for CHAP
+        (Challenge-Handshake Authentication Protocol) iSCSI targets.
+        The <code>secret</code> element requires either a <code>uuid</code>
+        attribute with the UUID of the secret object or a <code>usage</code>
+        attribute matching the key that was specified in the
+        secret object.  <span class="since">Since 0.9.7</span>
+      </dd>
        <dt><code>name</code></dt>
        <dd>Provides the source for pools backed by storage from a
          named element (e.g., a logical volume group name).
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
index 0a4cd22..673ca16 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -32,6 +32,8 @@
  #include <unistd.h>
  #include <sys/stat.h>
+#include "datatypes.h"
+#include "driver.h"
  #include "virerror.h"
  #include "storage_backend_scsi.h"
  #include "storage_backend_iscsi.h"
@@ -39,6 +41,7 @@
  #include "virlog.h"
  #include "virfile.h"
  #include "vircommand.h"
+#include "virobject.h"
  #include "virrandom.h"
  #include "virstring.h"
@@ -545,9 +548,112 @@ cleanup:
      return ret;
  }
+static int
+virStorageBackendISCSINodeUpdate(const char *portal,
+                                 const char *target,
+                                 const char *name,
+                                 const char *value)
+{
+     virCommandPtr cmd = NULL;
+     int status;
+     int ret = -1;
+
+     cmd = virCommandNewArgList(ISCSIADM,
+                                "--mode", "node",
+                                "--portal", portal,
+                                "--target", target,
+                                "--op", "update",
+                                "--name", name,
+                                "--value", value,
+                                NULL);
+
+    if (virCommandRun(cmd, &status) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to update '%s' of node mode for target '%s'"),
+                       name, target);
+        goto cleanup;
+    }
+
+    ret = 0;
+cleanup:
+    virCommandFree(cmd);
+    return ret;
+}
+
+static int
+virStorageBackendISCSISetAuth(const char *portal,
+                              virConnectPtr conn,
+                              virStoragePoolSourcePtr source)
+{
+    virSecretPtr secret = NULL;
+    unsigned char *secret_value = NULL;
+    const char *passwd = NULL;
+    virStoragePoolAuthChap chap;
+    int ret = -1;
+
+    if (source->authType == VIR_STORAGE_POOL_AUTH_NONE)
+        return 0;
+
+    if (source->authType != VIR_STORAGE_POOL_AUTH_CHAP) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("iscsi pool only supports 'chap' auth type"));
+        return -1;
+    }
+
+    chap = source->auth.chap;
+    if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_SECRET) {
+        if (chap.u.secret.uuidUsable)
+            secret = virSecretLookupByUUID(conn, chap.u.secret.uuid);
+        else
+            secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI,
+                                            chap.u.secret.usage);
+
+        if (secret) {
+            size_t secret_size;
+            secret_value =
+                conn->secretDriver->secretGetValue(secret, &secret_size, 0,
+                                                   VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+            if (!secret_value) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("could not get the value of the secret "
+                                 "for username %s"), chap.login);
+                goto cleanup;
+            }
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("username '%s' specified but secret not found"),
+                           chap.login);
+            goto cleanup;
+        }
+        passwd = (const char *)secret_value;
+    } else if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD) {
+        passwd = chap.u.passwd;
+    }
+
+    if (virStorageBackendISCSINodeUpdate(portal,
+                                         source->devices[0].path,
+                                         "node.session.auth.authmethod",
+                                         "CHAP") < 0 ||
+        virStorageBackendISCSINodeUpdate(portal,
+                                         source->devices[0].path,
+                                         "node.session.auth.username",
+                                         source->auth.chap.login) < 0 ||
+        virStorageBackendISCSINodeUpdate(portal,
+                                         source->devices[0].path,
+                                         "node.session.auth.password",
+                                         passwd) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+cleanup:
+    virObjectUnref(secret);
+    VIR_FREE(secret_value);
+    return ret;
+}
static char *
-virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStorageBackendISCSIFindPoolSources(virConnectPtr conn,
                                        const char *srcSpec,
                                        unsigned int flags)
  {
@@ -590,6 +696,9 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
                                            &ntargets, &targets) < 0)
          goto cleanup;
+ if (virStorageBackendISCSISetAuth(portal, conn, source) < 0)
+        goto cleanup;
+

i think the chap auth iscsi pool won't  be able to start with this change,
findPoolSources is an api for discovering the pool sources. however,
we want the chap auth are set up before connecting to the iscsi target
when starting the pool, otherwise the pool starting will fail on authentication.
note that startPool and findPoolSources are independant apis, they don't
invoke each other.

with this change, if one wants to start the pool successfully, he has to
invoke the findPoolSources first, this dependancy is incorrect. as an
example, in virsh layer, one has to execute find-storage-pool-sources
or find-storage-pool-sources-as before pool-start.

as an alternative way to have the non-null 'conn' for startPool, we can
create a connection in storageDriverAutostart (like qemu driver does),
which is the only place pass an null 'conn' to startPool,

regards
osier


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