Re: [PATCH v3 7/7] storage: Support "chap" authentication for iscsi pool

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

 



On Mon, Jul 15, 2013 at 09:04:35AM -0400, John Ferlan wrote:
> From: Osier Yang <jyang@xxxxxxxxxx>
> 
> 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.
> ---
>  src/storage/storage_backend_iscsi.c | 107 +++++++++++++++++++++++++++++++++++-
>  src/storage/storage_backend_rbd.c   |  13 ++++-
>  2 files changed, 117 insertions(+), 3 deletions(-)
> 
> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
> index ba4f388..c0f6032 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"
>  
> @@ -538,9 +541,106 @@ 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;
> +    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.secret.uuidUsable)
> +        secret = virSecretLookupByUUID(conn, chap.secret.uuid);
> +    else
> +        secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI,
> +                                        chap.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.username);
> +            goto cleanup;
> +        }
> +    } else {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("username '%s' specified but secret not found"),
> +                       chap.username);
> +        goto cleanup;
> +    }
> +
> +    if (virStorageBackendISCSINodeUpdate(portal,
> +                                         source->devices[0].path,
> +                                         "node.session.auth.authmethod",
> +                                         "CHAP") < 0 ||
> +        virStorageBackendISCSINodeUpdate(portal,
> +                                         source->devices[0].path,
> +                                         "node.session.auth.username",
> +                                         chap.username) < 0 ||
> +        virStorageBackendISCSINodeUpdate(portal,
> +                                         source->devices[0].path,
> +                                         "node.session.auth.password",
> +                                         (const char *)secret_value) < 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)
>  {
> @@ -583,6 +683,9 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                            &ntargets, &targets) < 0)
>          goto cleanup;
>  
> +    if (virStorageBackendISCSISetAuth(portal, conn, source) < 0)
> +        goto cleanup;
> +
>      if (VIR_ALLOC_N(list.sources, ntargets) < 0)
>          goto cleanup;
>  

As Osier mentioned, I think you need to be calling this somewhere
in the virStorageBackendISCSIStartPool method or its call path.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
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]