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

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; } -- 2.7.4

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

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(-)
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);
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;
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- - - if (!password) - goto cleanup; - if (virAsprintf(&url, "%s://%s:%d/sdk", priv->parsedUri- + 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

2016-05-23 23:32 GMT+02:00 Dawid Zamirski <dzamirski@datto.com>:
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.
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);
No need to free it here, because it was never allocated in this path.
+ 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) {
But you need to free it here
return -1; }
and here, otherwise you'll leak memory. And as Michal already mentioned, you missed the login call in esxVI_EnsureSession. -- Matthias Bolte http://photron.blogspot.com
participants (3)
-
Dawid Zamirski
-
Matthias Bolte
-
Michal Privoznik