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

Osier Yang jyang at redhat.com
Mon Jul 15 14:50:30 UTC 2013


On 15/07/13 22:47, Osier Yang wrote:
> On 15/07/13 21:04, John Ferlan wrote:
>> From: Osier Yang <jyang at 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.
>> ---
>>   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;
>> +
>>
>
> is it a mistake or? since i just submitted the comments on v2, which 
> says this
> will lead the pool starting to failure. :\  or there are differences 
> with v2 in some
> other places?
>
oh, i see, the delay is too much, you posted the v3 set before my 
comments on
v2, however, it just showed up on my mailbox.




More information about the libvir-list mailing list