[libvirt] [PATCH] ESX: Don't automatically follow redirects.
The default transport for the VI API is HTTPS. If the server redirects from HTTPS to HTTP the driver would silently follow that redirection. The user assumes to communicate with the server over a secure transport but isn't. This patch disables automatical redirection following. The driver reports an error if the server tries to redirect. * src/esx/esx_vi.c: refactor the call to curl_easy_perform() into a function and do error handling there, disable automatical redirection following for curl * src/esx/esx_vi.h: change the type of responseCode to int --- src/esx/esx_vi.c | 166 +++++++++++++++++++++++++++--------------------------- src/esx/esx_vi.h | 2 +- 2 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index dc51e20..bcf110f 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -216,6 +216,68 @@ esxVI_CURL_Debug(CURL *curl ATTRIBUTE_UNUSED, curl_infotype type, } #endif +static int +esxVI_CURL_Perform(virConnectPtr conn, esxVI_Context *ctx, const char *url) +{ + CURLcode errorCode; + long responseCode = 0; +#if LIBCURL_VERSION_NUM >= 0x071202 /* 7.18.2 */ + const char *redirectUrl = NULL; +#endif + + errorCode = curl_easy_perform(ctx->curl_handle); + + if (errorCode != CURLE_OK) { + ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "curl_easy_perform() returned an error: %s (%d)", + curl_easy_strerror(errorCode), errorCode); + return -1; + } + + errorCode = curl_easy_getinfo(ctx->curl_handle, CURLINFO_RESPONSE_CODE, + &responseCode); + + if (errorCode != CURLE_OK) { + ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "curl_easy_getinfo(CURLINFO_RESPONSE_CODE) returned an " + "error: %s (%d)", curl_easy_strerror(errorCode), + errorCode); + return -1; + } + + if (responseCode < 0) { + ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "curl_easy_getinfo(CURLINFO_RESPONSE_CODE) returned a " + "negative response code"); + return -1; + } + + if (responseCode == 301) { +#if LIBCURL_VERSION_NUM >= 0x071202 /* 7.18.2 */ + errorCode = curl_easy_getinfo(ctx->curl_handle, CURLINFO_REDIRECT_URL, + &redirectUrl); + + if (errorCode != CURLE_OK) { + ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "curl_easy_getinfo(CURLINFO_REDIRECT_URL) returned " + "an error: %s (%d)", curl_easy_strerror(errorCode), + errorCode); + } else { + ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "The server redirects from '%s' to '%s'", url, + redirectUrl); + } +#else + ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "The server redirects from '%s'", url); +#endif + + return -1; + } + + return responseCode; +} + int esxVI_Context_Connect(virConnectPtr conn, esxVI_Context *ctx, const char *url, const char *ipAddress, const char *username, @@ -269,7 +331,7 @@ esxVI_Context_Connect(virConnectPtr conn, esxVI_Context *ctx, const char *url, curl_easy_setopt(ctx->curl_handle, CURLOPT_URL, ctx->url); curl_easy_setopt(ctx->curl_handle, CURLOPT_USERAGENT, "libvirt-esx"); curl_easy_setopt(ctx->curl_handle, CURLOPT_HEADER, 0); - curl_easy_setopt(ctx->curl_handle, CURLOPT_FOLLOWLOCATION, 1); + curl_easy_setopt(ctx->curl_handle, CURLOPT_FOLLOWLOCATION, 0); curl_easy_setopt(ctx->curl_handle, CURLOPT_SSL_VERIFYPEER, noVerify ? 0 : 1); curl_easy_setopt(ctx->curl_handle, CURLOPT_SSL_VERIFYHOST, noVerify ? 0 : 2); curl_easy_setopt(ctx->curl_handle, CURLOPT_COOKIEFILE, ""); @@ -433,8 +495,7 @@ esxVI_Context_DownloadFile(virConnectPtr conn, esxVI_Context *ctx, const char *url, char **content) { virBuffer buffer = VIR_BUFFER_INITIALIZER; - CURLcode errorCode; - long responseCode; + int responseCode = 0; if (content == NULL || *content != NULL) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); @@ -448,27 +509,19 @@ esxVI_Context_DownloadFile(virConnectPtr conn, esxVI_Context *ctx, curl_easy_setopt(ctx->curl_handle, CURLOPT_UPLOAD, 0); curl_easy_setopt(ctx->curl_handle, CURLOPT_HTTPGET, 1); - errorCode = curl_easy_perform(ctx->curl_handle); - - if (errorCode != CURLE_OK) { - ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "curl_easy_perform() returned an error: %s (%d)", - curl_easy_strerror(errorCode), errorCode); - goto unlock; - } + responseCode = esxVI_CURL_Perform(conn, ctx, url); - errorCode = curl_easy_getinfo(ctx->curl_handle, CURLINFO_RESPONSE_CODE, - &responseCode); + virMutexUnlock(&ctx->curl_lock); - if (errorCode != CURLE_OK) { + if (responseCode < 0) { + goto failure; + } else if (responseCode != 200) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "curl_easy_getinfo() returned an error: %s (%d)", - curl_easy_strerror(errorCode), errorCode); - goto unlock; + "HTTP response code %d while trying to download '%s'", + responseCode, url); + goto failure; } - virMutexUnlock(&ctx->curl_lock); - if (virBufferError(&buffer)) { virReportOOMError(conn); goto failure; @@ -476,32 +529,19 @@ esxVI_Context_DownloadFile(virConnectPtr conn, esxVI_Context *ctx, *content = virBufferContentAndReset(&buffer); - if (responseCode != 200) { - ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "HTTP response code %d while trying to download '%s'", - (int)responseCode, url); - goto failure; - } - return 0; failure: free(virBufferContentAndReset(&buffer)); return -1; - - unlock: - virMutexUnlock(&ctx->curl_lock); - - goto failure; } int esxVI_Context_UploadFile(virConnectPtr conn, esxVI_Context *ctx, const char *url, const char *content) { - CURLcode errorCode; - long responseCode; + int responseCode = 0; if (content == NULL) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); @@ -515,40 +555,20 @@ esxVI_Context_UploadFile(virConnectPtr conn, esxVI_Context *ctx, curl_easy_setopt(ctx->curl_handle, CURLOPT_UPLOAD, 1); curl_easy_setopt(ctx->curl_handle, CURLOPT_INFILESIZE, strlen(content)); - errorCode = curl_easy_perform(ctx->curl_handle); - - if (errorCode != CURLE_OK) { - ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "curl_easy_perform() returned an error: %s (%d)", - curl_easy_strerror(errorCode), errorCode); - goto unlock; - } - - errorCode = curl_easy_getinfo(ctx->curl_handle, CURLINFO_RESPONSE_CODE, - &responseCode); - - if (errorCode != CURLE_OK) { - ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "curl_easy_getinfo() returned an error: %s (%d)", - curl_easy_strerror(errorCode), errorCode); - goto unlock; - } + responseCode = esxVI_CURL_Perform(conn, ctx, url); virMutexUnlock(&ctx->curl_lock); - if (responseCode != 200 && responseCode != 201) { + if (responseCode < 0) { + return -1; + } else if (responseCode != 200 && responseCode != 201) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "HTTP response code %d while trying to upload to '%s'", - (int)responseCode, url); + responseCode, url); return -1; } return 0; - - unlock: - virMutexUnlock(&ctx->curl_lock); - - return -1; } int @@ -558,7 +578,6 @@ esxVI_Context_Execute(virConnectPtr conn, esxVI_Context *ctx, { virBuffer buffer = VIR_BUFFER_INITIALIZER; esxVI_Fault *fault = NULL; - CURLcode errorCode; if (request == NULL || response == NULL || *response != NULL) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); @@ -577,27 +596,14 @@ esxVI_Context_Execute(virConnectPtr conn, esxVI_Context *ctx, curl_easy_setopt(ctx->curl_handle, CURLOPT_POSTFIELDS, request); curl_easy_setopt(ctx->curl_handle, CURLOPT_POSTFIELDSIZE, strlen(request)); - errorCode = curl_easy_perform(ctx->curl_handle); - - if (errorCode != CURLE_OK) { - ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "curl_easy_perform() returned an error: %s (%d)", - curl_easy_strerror(errorCode), errorCode); - goto unlock; - } + (*response)->responseCode = esxVI_CURL_Perform(conn, ctx, ctx->url); - errorCode = curl_easy_getinfo(ctx->curl_handle, CURLINFO_RESPONSE_CODE, - &(*response)->responseCode); + virMutexUnlock(&ctx->curl_lock); - if (errorCode != CURLE_OK) { - ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "curl_easy_getinfo() returned an error: %s (%d)", - curl_easy_strerror(errorCode), errorCode); - goto unlock; + if ((*response)->responseCode < 0) { + goto failure; } - virMutexUnlock(&ctx->curl_lock); - if (virBufferError(&buffer)) { virReportOOMError(conn); goto failure; @@ -695,8 +701,7 @@ esxVI_Context_Execute(virConnectPtr conn, esxVI_Context *ctx, } } else if ((*response)->responseCode != 200) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, - "HTTP response code %d", (int)(*response)->responseCode); - + "HTTP response code %d", (*response)->responseCode); goto failure; } @@ -708,11 +713,6 @@ esxVI_Context_Execute(virConnectPtr conn, esxVI_Context *ctx, esxVI_Fault_Free(&fault); return -1; - - unlock: - virMutexUnlock(&ctx->curl_lock); - - goto failure; } diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 4892fde..4b3005e 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -110,7 +110,7 @@ int esxVI_Context_Execute(virConnectPtr conn, esxVI_Context *ctx, */ struct _esxVI_Response { - long responseCode; /* required */ + int responseCode; /* required */ char *content; /* required */ xmlDocPtr document; /* optional */ xmlXPathContextPtr xpathContext; /* optional */ -- 1.6.0.4
On Wed, Oct 28, 2009 at 09:12:06PM +0100, Matthias Bolte wrote:
The default transport for the VI API is HTTPS. If the server redirects from HTTPS to HTTP the driver would silently follow that redirection. The user assumes to communicate with the server over a secure transport but isn't.
Good catch, this is definitely something we don't want to happen.
This patch disables automatical redirection following. The driver reports an error if the server tries to redirect.
Is the user likely to hit any redirects in the real world, or is this just an edge case. If they're likely to hit redirects, then we might want to allow a redirect if it points to another paths on the same server as the original URI, and is using HTTPS. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2009/10/28 Daniel P. Berrange <berrange@redhat.com>:
On Wed, Oct 28, 2009 at 09:12:06PM +0100, Matthias Bolte wrote:
The default transport for the VI API is HTTPS. If the server redirects from HTTPS to HTTP the driver would silently follow that redirection. The user assumes to communicate with the server over a secure transport but isn't.
Good catch, this is definitely something we don't want to happen.
This patch disables automatical redirection following. The driver reports an error if the server tries to redirect.
Is the user likely to hit any redirects in the real world, or is this just an edge case. If they're likely to hit redirects, then we might want to allow a redirect if it points to another paths on the same server as the original URI, and is using HTTPS.
Daniel
As far as I can tell it's an edge case. The available transports can be configured on the ESX server. Default is HTTPS-only, but you can configure it to use HTTPS+HTTP or HTTP-only. The ESX server redirects you to the other protocol if you try to access it via a disabled one. I'm not aware of any other situation that results in a redirect. Matthias
2009/10/29 Matthias Bolte <matthias.bolte@googlemail.com>:
2009/10/28 Daniel P. Berrange <berrange@redhat.com>:
On Wed, Oct 28, 2009 at 09:12:06PM +0100, Matthias Bolte wrote:
The default transport for the VI API is HTTPS. If the server redirects from HTTPS to HTTP the driver would silently follow that redirection. The user assumes to communicate with the server over a secure transport but isn't.
Good catch, this is definitely something we don't want to happen.
This patch disables automatical redirection following. The driver reports an error if the server tries to redirect.
Is the user likely to hit any redirects in the real world, or is this just an edge case. If they're likely to hit redirects, then we might want to allow a redirect if it points to another paths on the same server as the original URI, and is using HTTPS.
Daniel
As far as I can tell it's an edge case.
The available transports can be configured on the ESX server. Default is HTTPS-only, but you can configure it to use HTTPS+HTTP or HTTP-only. The ESX server redirects you to the other protocol if you try to access it via a disabled one. I'm not aware of any other situation that results in a redirect.
Matthias
If not doubts are left then I'm going to push this 5 ESX patches. Matthias
On Mon, Nov 02, 2009 at 05:24:38PM +0100, Matthias Bolte wrote:
2009/10/29 Matthias Bolte <matthias.bolte@googlemail.com>:
2009/10/28 Daniel P. Berrange <berrange@redhat.com>:
On Wed, Oct 28, 2009 at 09:12:06PM +0100, Matthias Bolte wrote:
The default transport for the VI API is HTTPS. If the server redirects from HTTPS to HTTP the driver would silently follow that redirection. The user assumes to communicate with the server over a secure transport but isn't.
Good catch, this is definitely something we don't want to happen.
This patch disables automatical redirection following. The driver reports an error if the server tries to redirect.
Is the user likely to hit any redirects in the real world, or is this just an edge case. If they're likely to hit redirects, then we might want to allow a redirect if it points to another paths on the same server as the original URI, and is using HTTPS.
Daniel
As far as I can tell it's an edge case.
The available transports can be configured on the ESX server. Default is HTTPS-only, but you can configure it to use HTTPS+HTTP or HTTP-only. The ESX server redirects you to the other protocol if you try to access it via a disabled one. I'm not aware of any other situation that results in a redirect.
Matthias
If not doubts are left then I'm going to push this 5 ESX patches.
ACK, works for me Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Nov 02, 2009 at 05:24:38PM +0100, Matthias Bolte wrote:
2009/10/29 Matthias Bolte <matthias.bolte@googlemail.com>:
2009/10/28 Daniel P. Berrange <berrange@redhat.com>:
On Wed, Oct 28, 2009 at 09:12:06PM +0100, Matthias Bolte wrote:
The default transport for the VI API is HTTPS. If the server redirects from HTTPS to HTTP the driver would silently follow that redirection. The user assumes to communicate with the server over a secure transport but isn't.
Good catch, this is definitely something we don't want to happen.
This patch disables automatical redirection following. The driver reports an error if the server tries to redirect.
Is the user likely to hit any redirects in the real world, or is this just an edge case. If they're likely to hit redirects, then we might want to allow a redirect if it points to another paths on the same server as the original URI, and is using HTTPS.
Daniel
As far as I can tell it's an edge case.
The available transports can be configured on the ESX server. Default is HTTPS-only, but you can configure it to use HTTPS+HTTP or HTTP-only. The ESX server redirects you to the other protocol if you try to access it via a disabled one. I'm not aware of any other situation that results in a redirect.
Matthias
If not doubts are left then I'm going to push this 5 ESX patches.
Fine by me ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Matthias Bolte