[libvirt] [PATCH] remote: Fix TLS transport on Windows
gnulib wraps Windows' SOCKET handle based send() and recv() functions into file descriptor based ones that are use in libvirt. By default GnuTLS uses the SOCKET handle based send() and recv() on Windows. This makes gnutls_handshake() fail internally with a WSAENOTSOCK error because libvirt passes a file descriptor; GnuTLS needs the SOCKET handle. To avoid this mismatch make sure that GnuTLS uses gnulib's replacment functions, by setting custom pull() and push() functions for GnuTLS. --- src/remote/remote_driver.c | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f45476a..87977ab 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1212,6 +1212,20 @@ initialize_gnutls(void) static int verify_certificate (virConnectPtr conn, struct private_data *priv, gnutls_session_t session); +#if HAVE_WINSOCK2_H +static ssize_t +custom_gnutls_push(void *s, const void *buf, size_t len) +{ + return send((int)(long)s, buf, len, 0); +} + +static ssize_t +custom_gnutls_pull(void *s, void *buf, size_t len) +{ + return recv((int)(long)s, buf, len, 0); +} +#endif + static gnutls_session_t negotiate_gnutls_on_connection (virConnectPtr conn, struct private_data *priv, @@ -1266,6 +1280,13 @@ negotiate_gnutls_on_connection (virConnectPtr conn, gnutls_transport_set_ptr (session, (gnutls_transport_ptr_t) (long) priv->sock); +#if HAVE_WINSOCK2_H + /* Make sure GnuTLS uses gnulib's replacment functions for send() and + * recv() on Windows */ + gnutls_transport_set_push_function(session, custom_gnutls_push); + gnutls_transport_set_pull_function(session, custom_gnutls_pull); +#endif + /* Perform the TLS handshake. */ again: err = gnutls_handshake (session); -- 1.7.0.4
On Sat, Nov 20, 2010 at 06:10:21PM +0100, Matthias Bolte wrote:
gnulib wraps Windows' SOCKET handle based send() and recv() functions into file descriptor based ones that are use in libvirt. By default GnuTLS uses the SOCKET handle based send() and recv() on Windows. This makes gnutls_handshake() fail internally with a WSAENOTSOCK error because libvirt passes a file descriptor; GnuTLS needs the SOCKET handle.
This doesn't entirely make any sense to me. GNUTLS also uses GNULIB, including all its socket wrappers for send/recv. If the push/pull function is NULL, gnulib does this if (session->internals._gnutls_push_func == NULL) { i = send (GNUTLS_POINTER_TO_INT (fd), &ptr[n - left], left, 0); And this 'send' impl maps to GNULIBs replacement in GNUTLS tree, which is identical to libvirt's 'send' impl So AFAICT, the syscall behaviour will be identical both with & without your proposed patch. The only potential difference I can see is that when push/pull are NULL, GNUTLS calls into WSAGetLastError (); to set the errno which is somewhat redundant as GNULIB has already called that and used it to set errno. Daniel
2010/11/22 Daniel P. Berrange <berrange@redhat.com>:
On Sat, Nov 20, 2010 at 06:10:21PM +0100, Matthias Bolte wrote:
gnulib wraps Windows' SOCKET handle based send() and recv() functions into file descriptor based ones that are use in libvirt. By default GnuTLS uses the SOCKET handle based send() and recv() on Windows. This makes gnutls_handshake() fail internally with a WSAENOTSOCK error because libvirt passes a file descriptor; GnuTLS needs the SOCKET handle.
This doesn't entirely make any sense to me. GNUTLS also uses GNULIB, including all its socket wrappers for send/recv. If the push/pull function is NULL, gnulib does this
if (session->internals._gnutls_push_func == NULL) { i = send (GNUTLS_POINTER_TO_INT (fd), &ptr[n - left], left, 0);
And this 'send' impl maps to GNULIBs replacement in GNUTLS tree, which is identical to libvirt's 'send' impl
So AFAICT, the syscall behaviour will be identical both with & without your proposed patch.
The only potential difference I can see is that when push/pull are NULL, GNUTLS calls into WSAGetLastError (); to set the errno which is somewhat redundant as GNULIB has already called that and used it to set errno.
Daniel
You're right GNUTLS uses GNULIB. I missed that fact. But why does my patch make a difference then? Without it the TLS transport doesn't work and with the patch it works. I'll have to investigate. Matthias
2010/11/22 Matthias Bolte <matthias.bolte@googlemail.com>:
2010/11/22 Daniel P. Berrange <berrange@redhat.com>:
On Sat, Nov 20, 2010 at 06:10:21PM +0100, Matthias Bolte wrote:
gnulib wraps Windows' SOCKET handle based send() and recv() functions into file descriptor based ones that are use in libvirt. By default GnuTLS uses the SOCKET handle based send() and recv() on Windows. This makes gnutls_handshake() fail internally with a WSAENOTSOCK error because libvirt passes a file descriptor; GnuTLS needs the SOCKET handle.
This doesn't entirely make any sense to me. GNUTLS also uses GNULIB, including all its socket wrappers for send/recv. If the push/pull function is NULL, gnulib does this
if (session->internals._gnutls_push_func == NULL) { i = send (GNUTLS_POINTER_TO_INT (fd), &ptr[n - left], left, 0);
And this 'send' impl maps to GNULIBs replacement in GNUTLS tree, which is identical to libvirt's 'send' impl
So AFAICT, the syscall behaviour will be identical both with & without your proposed patch.
The only potential difference I can see is that when push/pull are NULL, GNUTLS calls into WSAGetLastError (); to set the errno which is somewhat redundant as GNULIB has already called that and used it to set errno.
Daniel
You're right GNUTLS uses GNULIB. I missed that fact. But why does my patch make a difference then? Without it the TLS transport doesn't work and with the patch it works. I'll have to investigate.
Matthias
Okay, yes GnuTLS uses gnulib, but they explicitly don't use gnulib's replacements for send() and recv() on Windows. See lib/gnutls_buffers.c: /* We need to disable gnulib's replacement wrappers to get native Windows interfaces. */ #undef recv #undef send GnuTLS decided to use the native Windows versions of send() and recv(). This cannot be changed, as that would break existing applications using GnuTLS on Windows relying on GnuTLS using the native Windows versions of send() and recv(). Therefore, I think my patch is necessary, as libvirt requires GnuTLS to use gnulib's replacement functions. Matthias
On 11/22/2010 01:42 PM, Matthias Bolte wrote:
This doesn't entirely make any sense to me. GNUTLS also uses GNULIB, including all its socket wrappers for send/recv. If the push/pull function is NULL, gnulib does this
if (session->internals._gnutls_push_func == NULL) { i = send (GNUTLS_POINTER_TO_INT (fd), &ptr[n - left], left, 0);
Okay, yes GnuTLS uses gnulib, but they explicitly don't use gnulib's replacements for send() and recv() on Windows. See lib/gnutls_buffers.c:
/* We need to disable gnulib's replacement wrappers to get native Windows interfaces. */ #undef recv #undef send
GnuTLS decided to use the native Windows versions of send() and recv(). This cannot be changed, as that would break existing applications using GnuTLS on Windows relying on GnuTLS using the native Windows versions of send() and recv(). Therefore, I think my patch is necessary, as libvirt requires GnuTLS to use gnulib's replacement functions.
Makes sense to me. However, why the double cast? +#if HAVE_WINSOCK2_H +static ssize_t +custom_gnutls_push(void *s, const void *buf, size_t len) +{ + return send((int)(long)s, buf, len, 0); +} + +static ssize_t +custom_gnutls_pull(void *s, void *buf, size_t len) +{ + return recv((int)(long)s, buf, len, 0); +} +#endif Wouldn't send((size_t)s, ...) be better than send((int)(long)s,...)? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/11/22 Eric Blake <eblake@redhat.com>:
On 11/22/2010 01:42 PM, Matthias Bolte wrote:
This doesn't entirely make any sense to me. GNUTLS also uses GNULIB, including all its socket wrappers for send/recv. If the push/pull function is NULL, gnulib does this
if (session->internals._gnutls_push_func == NULL) { i = send (GNUTLS_POINTER_TO_INT (fd), &ptr[n - left], left, 0);
Okay, yes GnuTLS uses gnulib, but they explicitly don't use gnulib's replacements for send() and recv() on Windows. See lib/gnutls_buffers.c:
/* We need to disable gnulib's replacement wrappers to get native Windows interfaces. */ #undef recv #undef send
GnuTLS decided to use the native Windows versions of send() and recv(). This cannot be changed, as that would break existing applications using GnuTLS on Windows relying on GnuTLS using the native Windows versions of send() and recv(). Therefore, I think my patch is necessary, as libvirt requires GnuTLS to use gnulib's replacement functions.
Makes sense to me. However, why the double cast?
+#if HAVE_WINSOCK2_H +static ssize_t +custom_gnutls_push(void *s, const void *buf, size_t len) +{ + return send((int)(long)s, buf, len, 0); +} + +static ssize_t +custom_gnutls_pull(void *s, void *buf, size_t len) +{ + return recv((int)(long)s, buf, len, 0); +} +#endif
Wouldn't send((size_t)s, ...) be better than send((int)(long)s,...)?
I just used what'a in curl, as directly casting from void* to int would give this error on 64bit platform: error: cast from pointer to integer of different size [-Wpointer-to-int-cast] (Yes, this is tested on Linux 64bit, as I didn't setup mingw-w64 yet) Casting to size_t works too. Here's a v2 that casts to size_t and has an improved commit message. Matthias
On 11/22/2010 02:50 PM, Matthias Bolte wrote:
I just used what'a in curl, as directly casting from void* to int would give this error on 64bit platform:
error: cast from pointer to integer of different size [-Wpointer-to-int-cast]
And casting to long is no better on windows (where long is still 32-bit even though pointers are 64-bit).
(Yes, this is tested on Linux 64bit, as I didn't setup mingw-w64 yet)
Casting to size_t works too.
Not only does it avoid the warning, but it also matches the signature of send() and recv().
Here's a v2 that casts to size_t and has an improved commit message.
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/11/22 Eric Blake <eblake@redhat.com>:
On 11/22/2010 02:50 PM, Matthias Bolte wrote:
I just used what'a in curl, as directly casting from void* to int would give this error on 64bit platform:
error: cast from pointer to integer of different size [-Wpointer-to-int-cast]
And casting to long is no better on windows (where long is still 32-bit even though pointers are 64-bit).
(Yes, this is tested on Linux 64bit, as I didn't setup mingw-w64 yet)
Casting to size_t works too.
Not only does it avoid the warning, but it also matches the signature of send() and recv().
Here's a v2 that casts to size_t and has an improved commit message.
ACK.
Thanks, pushed. Matthias
On Mon, Nov 22, 2010 at 10:50:14PM +0100, Matthias Bolte wrote:
2010/11/22 Eric Blake <eblake@redhat.com>:
On 11/22/2010 01:42 PM, Matthias Bolte wrote:
This doesn't entirely make any sense to me. GNUTLS also uses GNULIB, including all its socket wrappers for send/recv. If the push/pull function is NULL, gnulib does this
if (session->internals._gnutls_push_func == NULL) { i = send (GNUTLS_POINTER_TO_INT (fd), &ptr[n - left], left, 0);
Okay, yes GnuTLS uses gnulib, but they explicitly don't use gnulib's replacements for send() and recv() on Windows. See lib/gnutls_buffers.c:
/* We need to disable gnulib's replacement wrappers to get native Windows interfaces. */ #undef recv #undef send
GnuTLS decided to use the native Windows versions of send() and recv(). This cannot be changed, as that would break existing applications using GnuTLS on Windows relying on GnuTLS using the native Windows versions of send() and recv(). Therefore, I think my patch is necessary, as libvirt requires GnuTLS to use gnulib's replacement functions.
Makes sense to me. However, why the double cast?
+#if HAVE_WINSOCK2_H +static ssize_t +custom_gnutls_push(void *s, const void *buf, size_t len) +{ + return send((int)(long)s, buf, len, 0); +} + +static ssize_t +custom_gnutls_pull(void *s, void *buf, size_t len) +{ + return recv((int)(long)s, buf, len, 0); +} +#endif
Wouldn't send((size_t)s, ...) be better than send((int)(long)s,...)?
I just used what'a in curl, as directly casting from void* to int would give this error on 64bit platform:
error: cast from pointer to integer of different size [-Wpointer-to-int-cast]
(Yes, this is tested on Linux 64bit, as I didn't setup mingw-w64 yet)
Casting to size_t works too.
Here's a v2 that casts to size_t and has an improved commit message.
ACK, that makes sense now.
On 11/20/2010 06:10 PM, Matthias Bolte wrote:
gnulib wraps Windows' SOCKET handle based send() and recv() functions into file descriptor based ones that are use in libvirt. By default GnuTLS uses the SOCKET handle based send() and recv() on Windows. This makes gnutls_handshake() fail internally with a WSAENOTSOCK error because libvirt passes a file descriptor; GnuTLS needs the SOCKET handle.
To avoid this mismatch make sure that GnuTLS uses gnulib's replacment functions, by setting custom pull() and push() functions for GnuTLS. --- src/remote/remote_driver.c | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f45476a..87977ab 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1212,6 +1212,20 @@ initialize_gnutls(void)
static int verify_certificate (virConnectPtr conn, struct private_data *priv, gnutls_session_t session);
+#if HAVE_WINSOCK2_H +static ssize_t +custom_gnutls_push(void *s, const void *buf, size_t len) +{ + return send((int)(long)s, buf, len, 0); +} + +static ssize_t +custom_gnutls_pull(void *s, void *buf, size_t len) +{ + return recv((int)(long)s, buf, len, 0); +} +#endif + static gnutls_session_t negotiate_gnutls_on_connection (virConnectPtr conn, struct private_data *priv, @@ -1266,6 +1280,13 @@ negotiate_gnutls_on_connection (virConnectPtr conn, gnutls_transport_set_ptr (session, (gnutls_transport_ptr_t) (long) priv->sock);
+#if HAVE_WINSOCK2_H + /* Make sure GnuTLS uses gnulib's replacment functions for send() and + * recv() on Windows */ + gnutls_transport_set_push_function(session, custom_gnutls_push); + gnutls_transport_set_pull_function(session, custom_gnutls_pull); +#endif + /* Perform the TLS handshake. */ again: err = gnutls_handshake (session);
Acked-By: Paolo Bonzini <pbonzini@redhat.com> Paolo
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Paolo Bonzini