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

Re: [libvirt] [PATCH] esx: do not store escaped password in esxVI_Context.



Hi Michal,

Thanks for reviewing. I'll send v2 soon to handle EnsureSession as
well.

Regards,
Dawid

On Wed, 2016-05-25 at 14:33 +0200, Michal Privoznik wrote:
> On 23.05.2016 23:32, Dawid Zamirski wrote:
> > This patch fixes an issue where screenshot API call was failing
> > when
> > the esx/vcenter password contains special characters such as
> > apostrophee. The reason for failures was that passwords were
> > escaped
> > for XML and stored in esxVI_Context which was then passed to raw
> > CURL API
> > calls where the password must be passed in original form to
> > authenticate successfully. So this patch addresses this by storing
> > original passwords in the esxVI_Context struct and escape only for
> > esxVI_Login call.
> > ---
> >  src/esx/esx_driver.c | 22 ++++------------------
> >  src/esx/esx_vi.c     | 13 ++++++++++++-
> >  2 files changed, 16 insertions(+), 19 deletions(-)
> > 
> > diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> > index 00d0e0a..031c666 100644
> > --- a/src/esx/esx_driver.c
> > +++ b/src/esx/esx_driver.c
> > @@ -617,7 +617,6 @@ esxConnectToHost(esxPrivate *priv,
> >      int result = -1;
> >      char ipAddress[NI_MAXHOST] = "";
> >      char *username = NULL;
> > -    char *unescapedPassword = NULL;
> >      char *password = NULL;
> >      char *url = NULL;
> >      esxVI_String *propertyNameList = NULL;
> > @@ -647,18 +646,13 @@ esxConnectToHost(esxPrivate *priv,
> >          }
> >      }
> >  
> > -    unescapedPassword = virAuthGetPassword(conn, auth, "esx",
> > username, conn->uri->server);
> > +    password = virAuthGetPassword(conn, auth, "esx", username,
> > conn->uri->server);
> >  
> > -    if (!unescapedPassword) {
> > +    if (!password) {
> >          virReportError(VIR_ERR_AUTH_FAILED, "%s", _("Password
> > request failed"));
> >          goto cleanup;
> >      }
> >  
> > -    password = esxUtil_EscapeForXml(unescapedPassword);
> > -
> > -    if (!password)
> > -        goto cleanup;
> > -
> >      if (virAsprintf(&url, "%s://%s:%d/sdk", priv->parsedUri-
> > >transport,
> >                      conn->uri->server, conn->uri->port) < 0)
> >          goto cleanup;
> > @@ -705,7 +699,6 @@ esxConnectToHost(esxPrivate *priv,
> >  
> >   cleanup:
> >      VIR_FREE(username);
> > -    VIR_FREE(unescapedPassword);
> >      VIR_FREE(password);
> >      VIR_FREE(url);
> >      esxVI_String_Free(&propertyNameList);
> > @@ -726,7 +719,6 @@ esxConnectToVCenter(esxPrivate *priv,
> >      int result = -1;
> >      char ipAddress[NI_MAXHOST] = "";
> >      char *username = NULL;
> > -    char *unescapedPassword = NULL;
> >      char *password = NULL;
> >      char *url = NULL;
> >  
> > @@ -752,18 +744,13 @@ esxConnectToVCenter(esxPrivate *priv,
> >          }
> >      }
> >  
> > -    unescapedPassword = virAuthGetPassword(conn, auth, "esx",
> > username, hostname);
> > +    password = virAuthGetPassword(conn, auth, "esx", username,
> > hostname);
> >  
> > -    if (!unescapedPassword) {
> > +    if (!password) {
> >          virReportError(VIR_ERR_AUTH_FAILED, "%s", _("Password
> > request failed"));
> >          goto cleanup;
> >      }
> >  
> > -    password = esxUtil_EscapeForXml(unescapedPassword);
> > -
> > -    if (!password)
> > -        goto cleanup;
> > -
> >      if (virAsprintf(&url, "%s://%s:%d/sdk", priv->parsedUri-
> > >transport,
> >                      hostname, conn->uri->port) < 0)
> >          goto cleanup;
> > @@ -799,7 +786,6 @@ esxConnectToVCenter(esxPrivate *priv,
> >  
> >   cleanup:
> >      VIR_FREE(username);
> > -    VIR_FREE(unescapedPassword);
> >      VIR_FREE(password);
> >      VIR_FREE(url);
> >  
> > diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> > index bf6f228..872cb7d 100644
> > --- a/src/esx/esx_vi.c
> > +++ b/src/esx/esx_vi.c
> > @@ -996,6 +996,8 @@ esxVI_Context_Connect(esxVI_Context *ctx, const
> > char *url,
> >                        const char *ipAddress, const char *username,
> >                        const char *password, esxUtil_ParsedUri
> > *parsedUri)
> >  {
> > +    char *escapedPassword = NULL;
> > +
> >      if (!ctx || !url || !ipAddress || !username ||
> >          !password || ctx->url || ctx->service || ctx->curl) {
> >          virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid
> > argument"));
> > @@ -1107,7 +1109,16 @@ esxVI_Context_Connect(esxVI_Context *ctx,
> > const char *url,
> >      if (ctx->productLine == esxVI_ProductLine_VPX)
> >          ctx->hasSessionIsActive = true;
> >  
> > -    if (esxVI_Login(ctx, username, password, NULL, &ctx->session)
> > < 0 ||
> > +    escapedPassword = esxUtil_EscapeForXml(password);
> > +
> > +    if (!escapedPassword) {
> > +        VIR_FREE(escapedPassword);
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Failed to escape password for XML"));
> > +        return -1;
> > +    }
> > +
> > +    if (esxVI_Login(ctx, username, escapedPassword, NULL, &ctx-
> > >session) < 0 ||
> >          esxVI_BuildSelectSetCollection(ctx) < 0) {
> >          return -1;
> >      }
> > 
> 
> Looks reasonable, although you forgot about esxVI_EnsureSession(). I
> guess it needs some fixing too.
> 
> Michal


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