Re: [libvirt] VMware driver: SessionIsActive API / Sessions.ValidateSession permission

2016-02-18 Thread Matthias Bolte
2016-02-16 16:13 GMT+01:00 Richard W.M. Jones :
> On Mon, Feb 15, 2016 at 11:22:27PM +0100, Matthias Bolte wrote:
>> 2016-02-11 12:59 GMT+01:00 Richard W.M. Jones :
>> > Is calling SessionIsActive necessary?  From my (very limited)
>> > understanding, it seems as if we might use 'SessionManager.
>> > currentSession' property instead, which doesn't require admin
>> > permissions.  Actually the code [see link above] already does this
>> > when ctx->hasSessionIsActive is false, but that doesn't apply to
>> > modern vCenter.
>>
>> SessionIsActive Is not necessary here, but it seemed to be the better
>> solution compared to this more hacky way of checking session object. I
>> wasn't aware that is would require admin level privileges.
>
> The patch works fine for me.  I believe the server I'm testing against
> is vCenter 5.5, although I'm not totally sure about that.
>
> ACK.
>
> Rich.

Thanks, pushed.

-- 
Matthias Bolte
http://photron.blogspot.com

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


Re: [libvirt] VMware driver: SessionIsActive API / Sessions.ValidateSession permission

2016-02-16 Thread Richard W.M. Jones
On Mon, Feb 15, 2016 at 11:22:27PM +0100, Matthias Bolte wrote:
> 2016-02-11 12:59 GMT+01:00 Richard W.M. Jones :
> > Is calling SessionIsActive necessary?  From my (very limited)
> > understanding, it seems as if we might use 'SessionManager.
> > currentSession' property instead, which doesn't require admin
> > permissions.  Actually the code [see link above] already does this
> > when ctx->hasSessionIsActive is false, but that doesn't apply to
> > modern vCenter.
> 
> SessionIsActive Is not necessary here, but it seemed to be the better
> solution compared to this more hacky way of checking session object. I
> wasn't aware that is would require admin level privileges.

The patch works fine for me.  I believe the server I'm testing against
is vCenter 5.5, although I'm not totally sure about that.

ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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


Re: [libvirt] VMware driver: SessionIsActive API / Sessions.ValidateSession permission

2016-02-15 Thread Richard W.M. Jones
On Mon, Feb 15, 2016 at 11:22:27PM +0100, Matthias Bolte wrote:
> Here's a patch that basically reverts the offending commit. The patch
> is only compile tested, as I don't have a vCenter at hand to test
> this. Do you have the option to test this in an actual setup?

Yes - I will be able to test this tomorrow.

Matt (Booth) - what do you think of this patch?

Rich.

> -- 
> Matthias Bolte
> http://photron.blogspot.com

> From d94afccfdee014ee97ecbf01f1108e17014b2017 Mon Sep 17 00:00:00 2001
> From: Matthias Bolte 
> Date: Mon, 15 Feb 2016 21:17:49 +0100
> Subject: [PATCH] esx: Avoid using vSphere SessionIsActive function
> 
> A login session with the vSphere API might expire after some idle time.
> The esxVI_EnsureSession function uses the SessionIsActive function to
> check if the current session has expired and a relogin needs to be done.
> 
> But the SessionIsActive function needs the Sessions.ValidateSession
> privilege that is considered as an admin level privilege.
> 
> Only vCenter actually provides the SessionIsActive function. This results
> in requiring an admin level privilege even for read-only operations on
> a vCenter server.
> 
> ESX and VMware Server don't provide the SessionIsActive function and
> the code already works around that. Use the same workaround for vCenter
> again.
> 
> This basically reverts commit 5699034b65afd49d91dff13c46481bea545cbaac.
> ---
>  src/esx/esx_vi.c | 88 
> 
>  1 file changed, 37 insertions(+), 51 deletions(-)
> 
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index af822b1..f7eeeb5 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -2043,11 +2043,21 @@ esxVI_BuildSelectSetCollection(esxVI_Context *ctx)
>  
>  
>  
> +/*
> + * Cannot use the SessionIsActive() function here, because at least
> + * ESX Server 3.5.0 build-64607 and ESX 4.0.0 build-171294 return an
> + * method-not-implemented fault when calling it. The vCenter Server
> + * implements this method, but because it can be used to check any
> + * session it requires the Sessions.ValidateSession privilege that is
> + * considered as an admin privilege.
> + *
> + * Instead query the session manager for the current session of this
> + * connection and re-login if there is no current session.
> + */
>  int
>  esxVI_EnsureSession(esxVI_Context *ctx)
>  {
>  int result = -1;
> -esxVI_Boolean active = esxVI_Boolean_Undefined;
>  esxVI_String *propertyNameList = NULL;
>  esxVI_ObjectContent *sessionManager = NULL;
>  esxVI_DynamicProperty *dynamicProperty = NULL;
> @@ -2065,65 +2075,41 @@ esxVI_EnsureSession(esxVI_Context *ctx)
>  goto cleanup;
>  }
>  
> -if (ctx->hasSessionIsActive) {
> -/*
> - * Use SessionIsActive to check if there is an active session for 
> this
> - * connection, and re-login if there isn't.
> - */
> -if (esxVI_SessionIsActive(ctx, ctx->session->key,
> -  ctx->session->userName, &active) < 0) {
> -goto cleanup;
> -}
> -
> -if (active != esxVI_Boolean_True) {
> -esxVI_UserSession_Free(&ctx->session);
> +if (esxVI_String_AppendValueToList(&propertyNameList,
> +   "currentSession") < 0 ||
> +esxVI_LookupObjectContentByType(ctx, ctx->service->sessionManager,
> +"SessionManager", propertyNameList,
> +&sessionManager,
> +esxVI_Occurrence_RequiredItem) < 0) {
> +goto cleanup;
> +}
>  
> -if (esxVI_Login(ctx, ctx->username, ctx->password, NULL,
> -&ctx->session) < 0) {
> +for (dynamicProperty = sessionManager->propSet; dynamicProperty;
> + dynamicProperty = dynamicProperty->_next) {
> +if (STREQ(dynamicProperty->name, "currentSession")) {
> +if (esxVI_UserSession_CastFromAnyType(dynamicProperty->val,
> +  ¤tSession) < 0) {
>  goto cleanup;
>  }
> -}
> -} else {
> -/*
> - * Query the session manager for the current session of this 
> connection
> - * and re-login if there is no current session for this connection.
> - */
> -if (esxVI_String_AppendValueToList(&propertyNameList,
> -   "currentSession") < 0 ||
> -esxVI_LookupObjectContentByType(ctx, 
> ctx->service->sessionManager,
> -"SessionManager", 
> propertyNameList,
> -&sessionManager,
> -esxVI_Occurrence_RequiredItem) < 
> 0) {
> -goto cleanup;
> -}
> -
> -for (dynamicProperty = sessionManager->propSet; dynamicProperty;
> - dynamicP

Re: [libvirt] VMware driver: SessionIsActive API / Sessions.ValidateSession permission

2016-02-15 Thread Matthias Bolte
2016-02-11 12:59 GMT+01:00 Richard W.M. Jones :
> Is calling SessionIsActive necessary?  From my (very limited)
> understanding, it seems as if we might use 'SessionManager.
> currentSession' property instead, which doesn't require admin
> permissions.  Actually the code [see link above] already does this
> when ctx->hasSessionIsActive is false, but that doesn't apply to
> modern vCenter.

SessionIsActive Is not necessary here, but it seemed to be the better
solution compared to this more hacky way of checking session object. I
wasn't aware that is would require admin level privileges.

> Also, why is it even necessary to check if the session is active here?
> Shouldn't we just log in unconditionally?

This was done a long time ago, I don't remember all the details, but
the gist of it goes like this:

The login session will expire after some idle time. I'm not sure if an
expired session could be detected from the server responses at all or
if it was just too difficult to integrated it that way. Anyway, I
ended up checking if the login session was still good and re-login if
it wasn't at the start of every libvirt driver function.

The driver could probably login at the start of each libvirt driver
function and logout at the end, completely avoiding the issue of
idling login session. On the other hand that would be a major chance
in the way the driver works. So I'm not tempted to chance stuff like
this now and am going to play it save.

As said before, SessionIsActive was used because it seemed to be the
right thing to do and I wasn't aware of the consequences at the time.

Here's a patch that basically reverts the offending commit. The patch
is only compile tested, as I don't have a vCenter at hand to test
this. Do you have the option to test this in an actual setup?

-- 
Matthias Bolte
http://photron.blogspot.com
From d94afccfdee014ee97ecbf01f1108e17014b2017 Mon Sep 17 00:00:00 2001
From: Matthias Bolte 
Date: Mon, 15 Feb 2016 21:17:49 +0100
Subject: [PATCH] esx: Avoid using vSphere SessionIsActive function

A login session with the vSphere API might expire after some idle time.
The esxVI_EnsureSession function uses the SessionIsActive function to
check if the current session has expired and a relogin needs to be done.

But the SessionIsActive function needs the Sessions.ValidateSession
privilege that is considered as an admin level privilege.

Only vCenter actually provides the SessionIsActive function. This results
in requiring an admin level privilege even for read-only operations on
a vCenter server.

ESX and VMware Server don't provide the SessionIsActive function and
the code already works around that. Use the same workaround for vCenter
again.

This basically reverts commit 5699034b65afd49d91dff13c46481bea545cbaac.
---
 src/esx/esx_vi.c | 88 
 1 file changed, 37 insertions(+), 51 deletions(-)

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index af822b1..f7eeeb5 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -2043,11 +2043,21 @@ esxVI_BuildSelectSetCollection(esxVI_Context *ctx)
 
 
 
+/*
+ * Cannot use the SessionIsActive() function here, because at least
+ * ESX Server 3.5.0 build-64607 and ESX 4.0.0 build-171294 return an
+ * method-not-implemented fault when calling it. The vCenter Server
+ * implements this method, but because it can be used to check any
+ * session it requires the Sessions.ValidateSession privilege that is
+ * considered as an admin privilege.
+ *
+ * Instead query the session manager for the current session of this
+ * connection and re-login if there is no current session.
+ */
 int
 esxVI_EnsureSession(esxVI_Context *ctx)
 {
 int result = -1;
-esxVI_Boolean active = esxVI_Boolean_Undefined;
 esxVI_String *propertyNameList = NULL;
 esxVI_ObjectContent *sessionManager = NULL;
 esxVI_DynamicProperty *dynamicProperty = NULL;
@@ -2065,65 +2075,41 @@ esxVI_EnsureSession(esxVI_Context *ctx)
 goto cleanup;
 }
 
-if (ctx->hasSessionIsActive) {
-/*
- * Use SessionIsActive to check if there is an active session for this
- * connection, and re-login if there isn't.
- */
-if (esxVI_SessionIsActive(ctx, ctx->session->key,
-  ctx->session->userName, &active) < 0) {
-goto cleanup;
-}
-
-if (active != esxVI_Boolean_True) {
-esxVI_UserSession_Free(&ctx->session);
+if (esxVI_String_AppendValueToList(&propertyNameList,
+   "currentSession") < 0 ||
+esxVI_LookupObjectContentByType(ctx, ctx->service->sessionManager,
+"SessionManager", propertyNameList,
+&sessionManager,
+esxVI_Occurrence_RequiredItem) < 0) {
+goto cleanup;
+}
 
-if (esxVI_Login(ctx, ctx->username, ctx->password, NULL,
-  

[libvirt] VMware driver: SessionIsActive API / Sessions.ValidateSession permission

2016-02-11 Thread Richard W.M. Jones
The VMware driver currently calls the SessionIsActive API, which
requires the vCenter Sessions.ValidateSession permission.

https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/esx/esx_vi.c;h=af822b14cfc5ba93c9c2ab4dfa2cb72a23a74a1a;hb=HEAD#l2068

This causes a problem that you have to give this permission to any
libvirt client accessing VMware, and you have to give it from the very
top level of vCenter, all the way down through the Cluster, Folder,
hypervisor levels.  This has caused a bit of pushback from virt-v2v
users who consider that the SessionIsActive API is an "admin" API
which they don't want to give out to roles using v2v.

Is calling SessionIsActive necessary?  From my (very limited)
understanding, it seems as if we might use 'SessionManager.
currentSession' property instead, which doesn't require admin
permissions.  Actually the code [see link above] already does this
when ctx->hasSessionIsActive is false, but that doesn't apply to
modern vCenter.

See also
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=5699034b65afd49d91dff13c46481bea545cbaac
which doesn't really explain why this was added.

Also, why is it even necessary to check if the session is active here?
Shouldn't we just log in unconditionally?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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