[libvirt] [PATCH 0/7] Fix SASL authentication regression
Introduced by commit 9b45c9f, released in v1.3.5. https://bugzilla.redhat.com/show_bug.cgi?id=1345743 Ján Tomko (7): Revert "virnetsocket: Provide socket address format in a more standard form" Introduce virNetSocketRemoteAddrStringURI Introduce virNetServerClientRemoteAddrStringURI virnetsockettest: fix error messages virNetSocket: rename AddrStr to AddrStrSASL Add SASL to virNetSocket{Local,Remote}AddrString Rename virNetServerClient*AddrString daemon/remote.c | 13 ++----------- src/libvirt_remote.syms | 10 ++++++---- src/remote/remote_driver.c | 7 ------- src/rpc/virnetclient.c | 14 ++------------ src/rpc/virnetclient.h | 2 -- src/rpc/virnetserver.c | 2 +- src/rpc/virnetserverclient.c | 21 +++++++-------------- src/rpc/virnetserverclient.h | 7 +++---- src/rpc/virnetsocket.c | 41 +++++++++++++++++++---------------------- src/rpc/virnetsocket.h | 7 +++---- tests/virnetsockettest.c | 26 ++++++++++++++++++-------- 11 files changed, 61 insertions(+), 89 deletions(-) -- 2.7.3
This partially reverts commit 9b45c9f049a7e9b6c1abfa6988b63b760714e169. It changed the default format of socket address from the one SASL requires, but did not adjust all the callers. It also removed the test coverage for it. Revert most of the changes except the virSocketAddrFormatFull support for URI-formatted strings. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1345743 while reverting the format used by virt-admin's client-info command from the URI one to the SASL one. https://bugzilla.redhat.com/show_bug.cgi?id=1345743 --- daemon/remote.c | 13 ++----------- src/remote/remote_driver.c | 7 ------- src/rpc/virnetclient.c | 10 ---------- src/rpc/virnetclient.h | 2 -- src/rpc/virnetserverclient.c | 13 ------------- src/rpc/virnetserverclient.h | 2 -- src/rpc/virnetsocket.c | 17 ++--------------- src/rpc/virnetsocket.h | 2 -- tests/virnetsockettest.c | 10 +++++----- 9 files changed, 9 insertions(+), 67 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 4e2aff8..ea4753f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3016,8 +3016,6 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, virNetSASLSessionPtr sasl = NULL; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); - char *localAddr = NULL; - char *remoteAddr = NULL; virMutexLock(&priv->lock); @@ -3028,17 +3026,10 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, goto authfail; } - localAddr = virNetServerClientLocalAddrFormatSASL(client); - remoteAddr = virNetServerClientRemoteAddrFormatSASL(client); - sasl = virNetSASLSessionNewServer(saslCtxt, "libvirt", - localAddr, - remoteAddr); - - VIR_FREE(localAddr); - VIR_FREE(remoteAddr); - + virNetServerClientLocalAddrString(client), + virNetServerClientRemoteAddrString(client)); if (!sasl) goto authfail; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 84b6d58..0315c6e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3809,8 +3809,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, sasl_callback_t *saslcb = NULL; int ret = -1; const char *mechlist; - char *localAddr = NULL; - char *remoteAddr = NULL; virNetSASLContextPtr saslCtxt; virNetSASLSessionPtr sasl = NULL; struct remoteAuthInteractState state; @@ -3829,9 +3827,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, saslcb = NULL; } - localAddr = virNetClientLocalAddrFormatSASL(priv->client); - remoteAddr = virNetClientRemoteAddrFormatSASL(priv->client); - /* Setup a handle for being a client */ if (!(sasl = virNetSASLSessionNewClient(saslCtxt, "libvirt", @@ -4019,8 +4014,6 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, cleanup: VIR_FREE(serverin); - VIR_FREE(localAddr); - VIR_FREE(remoteAddr); remoteAuthInteractStateClear(&state, true); VIR_FREE(saslcb); diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 3d59990..9c0d190 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -954,16 +954,6 @@ const char *virNetClientRemoteAddrString(virNetClientPtr client) return virNetSocketRemoteAddrString(client->sock); } -char *virNetClientLocalAddrFormatSASL(virNetClientPtr client) -{ - return virNetSocketLocalAddrFormatSASL(client->sock); -} - -char *virNetClientRemoteAddrFormatSASL(virNetClientPtr client) -{ - return virNetSocketRemoteAddrFormatSASL(client->sock); -} - #if WITH_GNUTLS int virNetClientGetTLSKeySize(virNetClientPtr client) { diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 4b78677..38f929c 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -123,8 +123,6 @@ bool virNetClientIsOpen(virNetClientPtr client); const char *virNetClientLocalAddrString(virNetClientPtr client); const char *virNetClientRemoteAddrString(virNetClientPtr client); -char *virNetClientLocalAddrFormatSASL(virNetClientPtr client); -char *virNetClientRemoteAddrFormatSASL(virNetClientPtr client); # ifdef WITH_GNUTLS int virNetClientGetTLSKeySize(virNetClientPtr client); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index ef83507..2bc058c 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -918,19 +918,6 @@ const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client) return virNetSocketRemoteAddrString(client->sock); } -char *virNetServerClientLocalAddrFormatSASL(virNetServerClientPtr client) -{ - if (!client->sock) - return NULL; - return virNetSocketLocalAddrFormatSASL(client->sock); -} - -char *virNetServerClientRemoteAddrFormatSASL(virNetServerClientPtr client) -{ - if (!client->sock) - return NULL; - return virNetSocketRemoteAddrFormatSASL(client->sock); -} void virNetServerClientDispose(void *obj) { diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 594803b..c8b8dc1 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -140,8 +140,6 @@ int virNetServerClientStartKeepAlive(virNetServerClientPtr client); const char *virNetServerClientLocalAddrString(virNetServerClientPtr client); const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client); -char *virNetServerClientLocalAddrFormatSASL(virNetServerClientPtr client); -char *virNetServerClientRemoteAddrFormatSASL(virNetServerClientPtr client); int virNetServerClientSendMessage(virNetServerClientPtr client, virNetMessagePtr msg); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index a90cc55..d909b94 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -262,11 +262,11 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, if (localAddr && - !(sock->localAddrStr = virSocketAddrFormatFull(localAddr, true, NULL))) + !(sock->localAddrStr = virSocketAddrFormatFull(localAddr, true, ";"))) goto error; if (remoteAddr && - !(sock->remoteAddrStr = virSocketAddrFormatFull(remoteAddr, true, NULL))) + !(sock->remoteAddrStr = virSocketAddrFormatFull(remoteAddr, true, ";"))) goto error; sock->client = isClient; @@ -1465,19 +1465,6 @@ const char *virNetSocketRemoteAddrString(virNetSocketPtr sock) return sock->remoteAddrStr; } -/* These helper functions return a SASL-formatted socket addr string, - * caller is responsible for freeing the string. - */ -char *virNetSocketLocalAddrFormatSASL(virNetSocketPtr sock) -{ - return virSocketAddrFormatFull(&sock->localAddr, true, ";"); -} - -char *virNetSocketRemoteAddrFormatSASL(virNetSocketPtr sock) -{ - return virSocketAddrFormatFull(&sock->remoteAddr, true, ";"); -} - #if WITH_GNUTLS static ssize_t virNetSocketTLSSessionWrite(const char *buf, diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 4eb7187..5de3d92 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -150,8 +150,6 @@ bool virNetSocketHasPendingData(virNetSocketPtr sock); const char *virNetSocketLocalAddrString(virNetSocketPtr sock); const char *virNetSocketRemoteAddrString(virNetSocketPtr sock); -char *virNetSocketLocalAddrFormatSASL(virNetSocketPtr sock); -char *virNetSocketRemoteAddrFormatSASL(virNetSocketPtr sock); int virNetSocketListen(virNetSocketPtr sock, int backlog); int virNetSocketAccept(virNetSocketPtr sock, diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index bbecb3c..9df90a9 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -249,7 +249,7 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) if (virNetSocketNewListenUNIX(path, 0700, -1, getegid(), &lsock) < 0) goto cleanup; - if (STRNEQ(virNetSocketLocalAddrString(lsock), "127.0.0.1:0")) { + if (STRNEQ(virNetSocketLocalAddrString(lsock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } @@ -265,12 +265,12 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) if (virNetSocketNewConnectUNIX(path, false, NULL, &csock) < 0) goto cleanup; - if (STRNEQ(virNetSocketLocalAddrString(csock), "127.0.0.1:0")) { + if (STRNEQ(virNetSocketLocalAddrString(csock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } - if (STRNEQ(virNetSocketRemoteAddrString(csock), "127.0.0.1:0")) { + if (STRNEQ(virNetSocketRemoteAddrString(csock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } @@ -282,12 +282,12 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) } - if (STRNEQ(virNetSocketLocalAddrString(ssock), "127.0.0.1:0")) { + if (STRNEQ(virNetSocketLocalAddrString(ssock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } - if (STRNEQ(virNetSocketRemoteAddrString(ssock), "127.0.0.1:0")) { + if (STRNEQ(virNetSocketRemoteAddrString(ssock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } -- 2.7.3
On 06/20/2016 10:27 AM, Ján Tomko wrote:
This partially reverts commit 9b45c9f049a7e9b6c1abfa6988b63b760714e169.
It changed the default format of socket address from the one SASL requires, but did not adjust all the callers.
It also removed the test coverage for it.
Revert most of the changes except the virSocketAddrFormatFull support for URI-formatted strings.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1345743 while reverting the format used by virt-admin's client-info command from the URI one to the SASL one.
https://bugzilla.redhat.com/show_bug.cgi?id=1345743 --- daemon/remote.c | 13 ++----------- src/remote/remote_driver.c | 7 ------- src/rpc/virnetclient.c | 10 ---------- src/rpc/virnetclient.h | 2 -- src/rpc/virnetserverclient.c | 13 ------------- src/rpc/virnetserverclient.h | 2 -- src/rpc/virnetsocket.c | 17 ++--------------- src/rpc/virnetsocket.h | 2 -- tests/virnetsockettest.c | 10 +++++----- 9 files changed, 9 insertions(+), 67 deletions(-)
So prior to Erik's changes, we provided/used SASL addresses. Erik's change was to provide/use URI addresses. Can I assume the "missed" change was perhaps in remoteAuthSASL where localAddr/remoteAddr weren't used? ACK - John
On Wed, Jun 22, 2016 at 07:41:39PM -0400, John Ferlan wrote:
On 06/20/2016 10:27 AM, Ján Tomko wrote:
This partially reverts commit 9b45c9f049a7e9b6c1abfa6988b63b760714e169.
It changed the default format of socket address from the one SASL requires, but did not adjust all the callers.
It also removed the test coverage for it.
Revert most of the changes except the virSocketAddrFormatFull support for URI-formatted strings.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1345743 while reverting the format used by virt-admin's client-info command from the URI one to the SASL one.
https://bugzilla.redhat.com/show_bug.cgi?id=1345743 --- daemon/remote.c | 13 ++----------- src/remote/remote_driver.c | 7 ------- src/rpc/virnetclient.c | 10 ---------- src/rpc/virnetclient.h | 2 -- src/rpc/virnetserverclient.c | 13 ------------- src/rpc/virnetserverclient.h | 2 -- src/rpc/virnetsocket.c | 17 ++--------------- src/rpc/virnetsocket.h | 2 -- tests/virnetsockettest.c | 10 +++++----- 9 files changed, 9 insertions(+), 67 deletions(-)
So prior to Erik's changes, we provided/used SASL addresses. Erik's change was to provide/use URI addresses. Can I assume the "missed" change was perhaps in remoteAuthSASL where localAddr/remoteAddr weren't used?
Yes, that would have been the minimal fix, but I also wanted to restore the tests and get rid of the extra allocation. Jan
Add a test case to virnetsockettest. --- src/libvirt_remote.syms | 1 + src/rpc/virnetsocket.c | 10 ++++++++++ src/rpc/virnetsocket.h | 1 + tests/virnetsockettest.c | 10 ++++++++++ 4 files changed, 22 insertions(+) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 1a88fff..f3cf65d 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -227,6 +227,7 @@ virNetSocketPreExecRestart; virNetSocketRead; virNetSocketRecvFD; virNetSocketRemoteAddrString; +virNetSocketRemoteAddrStringURI; virNetSocketRemoveIOCallback; virNetSocketSendFD; virNetSocketSetBlocking; diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index d909b94..f10b62b 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -87,6 +87,7 @@ struct _virNetSocket { virSocketAddr remoteAddr; char *localAddrStr; char *remoteAddrStr; + char *remoteAddrStrURI; #if WITH_GNUTLS virNetTLSSessionPtr tlsSession; @@ -269,6 +270,10 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, !(sock->remoteAddrStr = virSocketAddrFormatFull(remoteAddr, true, ";"))) goto error; + if (remoteAddr && + !(sock->remoteAddrStrURI = virSocketAddrFormatFull(remoteAddr, true, NULL))) + goto error; + sock->client = isClient; PROBE(RPC_SOCKET_NEW, @@ -1204,6 +1209,7 @@ void virNetSocketDispose(void *obj) VIR_FREE(sock->localAddrStr); VIR_FREE(sock->remoteAddrStr); + VIR_FREE(sock->remoteAddrStrURI); } @@ -1465,6 +1471,10 @@ const char *virNetSocketRemoteAddrString(virNetSocketPtr sock) return sock->remoteAddrStr; } +const char *virNetSocketRemoteAddrStringURI(virNetSocketPtr sock) +{ + return sock->remoteAddrStrURI; +} #if WITH_GNUTLS static ssize_t virNetSocketTLSSessionWrite(const char *buf, diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 5de3d92..25ca14e 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -150,6 +150,7 @@ bool virNetSocketHasPendingData(virNetSocketPtr sock); const char *virNetSocketLocalAddrString(virNetSocketPtr sock); const char *virNetSocketRemoteAddrString(virNetSocketPtr sock); +const char *virNetSocketRemoteAddrStringURI(virNetSocketPtr sock); int virNetSocketListen(virNetSocketPtr sock, int backlog); int virNetSocketAccept(virNetSocketPtr sock, diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 9df90a9..8cd8ede 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -275,6 +275,11 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) goto cleanup; } + if (STRNEQ(virNetSocketRemoteAddrStringURI(csock), "127.0.0.1:0")) { + VIR_DEBUG("Unexpected remote address"); + goto cleanup; + } + if (virNetSocketAccept(lsock, &ssock) < 0) { VIR_DEBUG("Unexpected client socket missing"); @@ -292,6 +297,11 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) goto cleanup; } + if (STRNEQ(virNetSocketRemoteAddrStringURI(ssock), "127.0.0.1:0")) { + VIR_DEBUG("Unexpected remote address"); + goto cleanup; + } + ret = 0; -- 2.7.3
On 06/20/2016 10:27 AM, Ján Tomko wrote:
Add a test case to virnetsockettest.
Commit message is a bit light... Would be nice to indicate what this does ACK John
--- src/libvirt_remote.syms | 1 + src/rpc/virnetsocket.c | 10 ++++++++++ src/rpc/virnetsocket.h | 1 + tests/virnetsockettest.c | 10 ++++++++++ 4 files changed, 22 insertions(+)
Use it in virNetServerClientGetInfo to switch back to using the URI-format (separated by ':') instead of the SASL format (separated by ';'). Also use it in the error message reported byvirNetServerAddClient. --- src/libvirt_remote.syms | 1 + src/rpc/virnetserver.c | 2 +- src/rpc/virnetserverclient.c | 8 +++++++- src/rpc/virnetserverclient.h | 1 + 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index f3cf65d..fd80e46 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -145,6 +145,7 @@ virNetServerClientNew; virNetServerClientNewPostExecRestart; virNetServerClientPreExecRestart; virNetServerClientRemoteAddrString; +virNetServerClientRemoteAddrStringURI; virNetServerClientRemoveFilter; virNetServerClientSendMessage; virNetServerClientSetAuth; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 4c4b144..8c8af97 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -243,7 +243,7 @@ int virNetServerAddClient(virNetServerPtr srv, if (srv->nclients >= srv->nclients_max) { virReportError(VIR_ERR_RPC, _("Too many active clients (%zu), dropping connection from %s"), - srv->nclients_max, virNetServerClientRemoteAddrString(client)); + srv->nclients_max, virNetServerClientRemoteAddrStringURI(client)); goto error; } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 2bc058c..15715a9 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -918,6 +918,12 @@ const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client) return virNetSocketRemoteAddrString(client->sock); } +const char *virNetServerClientRemoteAddrStringURI(virNetServerClientPtr client) +{ + if (!client->sock) + return NULL; + return virNetSocketRemoteAddrStringURI(client->sock); +} void virNetServerClientDispose(void *obj) { @@ -1608,7 +1614,7 @@ virNetServerClientGetInfo(virNetServerClientPtr client, virObjectLock(client); *readonly = client->readonly; - if (!(*sock_addr = virNetServerClientRemoteAddrString(client))) { + if (!(*sock_addr = virNetServerClientRemoteAddrStringURI(client))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No network socket associated with client")); goto cleanup; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index c8b8dc1..bb9c937 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -140,6 +140,7 @@ int virNetServerClientStartKeepAlive(virNetServerClientPtr client); const char *virNetServerClientLocalAddrString(virNetServerClientPtr client); const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client); +const char *virNetServerClientRemoteAddrStringURI(virNetServerClientPtr client); int virNetServerClientSendMessage(virNetServerClientPtr client, virNetMessagePtr msg); -- 2.7.3
On 06/20/2016 10:27 AM, Ján Tomko wrote:
Use it in virNetServerClientGetInfo to switch back to using the URI-format (separated by ':') instead of the SASL format (separated by ';').
Also use it in the error message reported byvirNetServerAddClient.
s/byvir/by vir/ So this restores the functionality adjusted by reverted commit '9b45c9f0' and subsequent API addition in commit id '8420a53e' (Just making sure) ACK John
--- src/libvirt_remote.syms | 1 + src/rpc/virnetserver.c | 2 +- src/rpc/virnetserverclient.c | 8 +++++++- src/rpc/virnetserverclient.h | 1 + 4 files changed, 10 insertions(+), 2 deletions(-)
--- tests/virnetsockettest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 8cd8ede..4d78d27 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -271,7 +271,7 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) } if (STRNEQ(virNetSocketRemoteAddrString(csock), "127.0.0.1;0")) { - VIR_DEBUG("Unexpected local address"); + VIR_DEBUG("Unexpected remote address"); goto cleanup; } @@ -293,7 +293,7 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) } if (STRNEQ(virNetSocketRemoteAddrString(ssock), "127.0.0.1;0")) { - VIR_DEBUG("Unexpected local address"); + VIR_DEBUG("Unexpected remote address"); goto cleanup; } -- 2.7.3
Make it more obvious that these are in the SASL format. --- src/rpc/virnetsocket.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index f10b62b..00dc417 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -85,8 +85,8 @@ struct _virNetSocket { virSocketAddr localAddr; virSocketAddr remoteAddr; - char *localAddrStr; - char *remoteAddrStr; + char *localAddrStrSASL; + char *remoteAddrStrSASL; char *remoteAddrStrURI; #if WITH_GNUTLS @@ -263,11 +263,11 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, if (localAddr && - !(sock->localAddrStr = virSocketAddrFormatFull(localAddr, true, ";"))) + !(sock->localAddrStrSASL = virSocketAddrFormatFull(localAddr, true, ";"))) goto error; if (remoteAddr && - !(sock->remoteAddrStr = virSocketAddrFormatFull(remoteAddr, true, ";"))) + !(sock->remoteAddrStrSASL = virSocketAddrFormatFull(remoteAddr, true, ";"))) goto error; if (remoteAddr && @@ -279,7 +279,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, PROBE(RPC_SOCKET_NEW, "sock=%p fd=%d errfd=%d pid=%lld localAddr=%s, remoteAddr=%s", sock, fd, errfd, (long long) pid, - NULLSTR(sock->localAddrStr), NULLSTR(sock->remoteAddrStr)); + NULLSTR(sock->localAddrStrSASL), NULLSTR(sock->remoteAddrStrSASL)); return sock; @@ -1207,8 +1207,8 @@ void virNetSocketDispose(void *obj) virProcessAbort(sock->pid); - VIR_FREE(sock->localAddrStr); - VIR_FREE(sock->remoteAddrStr); + VIR_FREE(sock->localAddrStrSASL); + VIR_FREE(sock->remoteAddrStrSASL); VIR_FREE(sock->remoteAddrStrURI); } @@ -1463,12 +1463,12 @@ int virNetSocketSetBlocking(virNetSocketPtr sock, const char *virNetSocketLocalAddrString(virNetSocketPtr sock) { - return sock->localAddrStr; + return sock->localAddrStrSASL; } const char *virNetSocketRemoteAddrString(virNetSocketPtr sock) { - return sock->remoteAddrStr; + return sock->remoteAddrStrSASL; } const char *virNetSocketRemoteAddrStringURI(virNetSocketPtr sock) -- 2.7.3
On 06/20/2016 10:27 AM, Ján Tomko wrote:
Make it more obvious that these are in the SASL format. --- src/rpc/virnetsocket.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
ACK, John
Rename them to virNetSocket{Local,Remote}AddrStringSASL to make their format more obvious. --- src/libvirt_remote.syms | 4 ++-- src/rpc/virnetclient.c | 4 ++-- src/rpc/virnetserverclient.c | 4 ++-- src/rpc/virnetsocket.c | 4 ++-- src/rpc/virnetsocket.h | 4 ++-- tests/virnetsockettest.c | 12 ++++++------ 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index fd80e46..3858f4d 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -212,7 +212,7 @@ virNetSocketHasPassFD; virNetSocketHasPendingData; virNetSocketIsLocal; virNetSocketListen; -virNetSocketLocalAddrString; +virNetSocketLocalAddrStringSASL; virNetSocketNewConnectCommand; virNetSocketNewConnectExternal; virNetSocketNewConnectLibSSH2; @@ -227,7 +227,7 @@ virNetSocketNewPostExecRestart; virNetSocketPreExecRestart; virNetSocketRead; virNetSocketRecvFD; -virNetSocketRemoteAddrString; +virNetSocketRemoteAddrStringSASL; virNetSocketRemoteAddrStringURI; virNetSocketRemoveIOCallback; virNetSocketSendFD; diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 9c0d190..c43cd08 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -946,12 +946,12 @@ void virNetClientRemoveStream(virNetClientPtr client, const char *virNetClientLocalAddrString(virNetClientPtr client) { - return virNetSocketLocalAddrString(client->sock); + return virNetSocketLocalAddrStringSASL(client->sock); } const char *virNetClientRemoteAddrString(virNetClientPtr client) { - return virNetSocketRemoteAddrString(client->sock); + return virNetSocketRemoteAddrStringSASL(client->sock); } #if WITH_GNUTLS diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 15715a9..5aa7634 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -907,7 +907,7 @@ const char *virNetServerClientLocalAddrString(virNetServerClientPtr client) { if (!client->sock) return NULL; - return virNetSocketLocalAddrString(client->sock); + return virNetSocketLocalAddrStringSASL(client->sock); } @@ -915,7 +915,7 @@ const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client) { if (!client->sock) return NULL; - return virNetSocketRemoteAddrString(client->sock); + return virNetSocketRemoteAddrStringSASL(client->sock); } const char *virNetServerClientRemoteAddrStringURI(virNetServerClientPtr client) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 00dc417..405f5ba 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1461,12 +1461,12 @@ int virNetSocketSetBlocking(virNetSocketPtr sock, } -const char *virNetSocketLocalAddrString(virNetSocketPtr sock) +const char *virNetSocketLocalAddrStringSASL(virNetSocketPtr sock) { return sock->localAddrStrSASL; } -const char *virNetSocketRemoteAddrString(virNetSocketPtr sock) +const char *virNetSocketRemoteAddrStringSASL(virNetSocketPtr sock) { return sock->remoteAddrStrSASL; } diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 25ca14e..ec064bb 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -148,8 +148,8 @@ void virNetSocketSetSASLSession(virNetSocketPtr sock, bool virNetSocketHasCachedData(virNetSocketPtr sock); bool virNetSocketHasPendingData(virNetSocketPtr sock); -const char *virNetSocketLocalAddrString(virNetSocketPtr sock); -const char *virNetSocketRemoteAddrString(virNetSocketPtr sock); +const char *virNetSocketLocalAddrStringSASL(virNetSocketPtr sock); +const char *virNetSocketRemoteAddrStringSASL(virNetSocketPtr sock); const char *virNetSocketRemoteAddrStringURI(virNetSocketPtr sock); int virNetSocketListen(virNetSocketPtr sock, int backlog); diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 4d78d27..cafdca9 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -249,12 +249,12 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) if (virNetSocketNewListenUNIX(path, 0700, -1, getegid(), &lsock) < 0) goto cleanup; - if (STRNEQ(virNetSocketLocalAddrString(lsock), "127.0.0.1;0")) { + if (STRNEQ(virNetSocketLocalAddrStringSASL(lsock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } - if (virNetSocketRemoteAddrString(lsock) != NULL) { + if (virNetSocketRemoteAddrStringSASL(lsock) != NULL) { VIR_DEBUG("Unexpected remote address"); goto cleanup; } @@ -265,12 +265,12 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) if (virNetSocketNewConnectUNIX(path, false, NULL, &csock) < 0) goto cleanup; - if (STRNEQ(virNetSocketLocalAddrString(csock), "127.0.0.1;0")) { + if (STRNEQ(virNetSocketLocalAddrStringSASL(csock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } - if (STRNEQ(virNetSocketRemoteAddrString(csock), "127.0.0.1;0")) { + if (STRNEQ(virNetSocketRemoteAddrStringSASL(csock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected remote address"); goto cleanup; } @@ -287,12 +287,12 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) } - if (STRNEQ(virNetSocketLocalAddrString(ssock), "127.0.0.1;0")) { + if (STRNEQ(virNetSocketLocalAddrStringSASL(ssock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected local address"); goto cleanup; } - if (STRNEQ(virNetSocketRemoteAddrString(ssock), "127.0.0.1;0")) { + if (STRNEQ(virNetSocketRemoteAddrStringSASL(ssock), "127.0.0.1;0")) { VIR_DEBUG("Unexpected remote address"); goto cleanup; } -- 2.7.3
On 06/20/2016 10:27 AM, Ján Tomko wrote:
Rename them to virNetSocket{Local,Remote}AddrStringSASL to make their format more obvious. --- src/libvirt_remote.syms | 4 ++-- src/rpc/virnetclient.c | 4 ++-- src/rpc/virnetserverclient.c | 4 ++-- src/rpc/virnetsocket.c | 4 ++-- src/rpc/virnetsocket.h | 4 ++-- tests/virnetsockettest.c | 12 ++++++------ 6 files changed, 16 insertions(+), 16 deletions(-)
ACK John
Add SASL at the end to make the format obvious. --- daemon/remote.c | 4 ++-- src/libvirt_remote.syms | 4 ++-- src/rpc/virnetserverclient.c | 4 ++-- src/rpc/virnetserverclient.h | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ea4753f..5ee82bb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3028,8 +3028,8 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, sasl = virNetSASLSessionNewServer(saslCtxt, "libvirt", - virNetServerClientLocalAddrString(client), - virNetServerClientRemoteAddrString(client)); + virNetServerClientLocalAddrStringSASL(client), + virNetServerClientRemoteAddrStringSASL(client)); if (!sasl) goto authfail; diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 3858f4d..a6192ef 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -139,12 +139,12 @@ virNetServerClientInitKeepAlive; virNetServerClientIsClosed; virNetServerClientIsLocal; virNetServerClientIsSecure; -virNetServerClientLocalAddrString; +virNetServerClientLocalAddrStringSASL; virNetServerClientNeedAuth; virNetServerClientNew; virNetServerClientNewPostExecRestart; virNetServerClientPreExecRestart; -virNetServerClientRemoteAddrString; +virNetServerClientRemoteAddrStringSASL; virNetServerClientRemoteAddrStringURI; virNetServerClientRemoveFilter; virNetServerClientSendMessage; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 5aa7634..39fe860 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -903,7 +903,7 @@ void virNetServerClientSetDispatcher(virNetServerClientPtr client, } -const char *virNetServerClientLocalAddrString(virNetServerClientPtr client) +const char *virNetServerClientLocalAddrStringSASL(virNetServerClientPtr client) { if (!client->sock) return NULL; @@ -911,7 +911,7 @@ const char *virNetServerClientLocalAddrString(virNetServerClientPtr client) } -const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client) +const char *virNetServerClientRemoteAddrStringSASL(virNetServerClientPtr client) { if (!client->sock) return NULL; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index bb9c937..c243a68 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -138,8 +138,8 @@ bool virNetServerClientCheckKeepAlive(virNetServerClientPtr client, virNetMessagePtr msg); int virNetServerClientStartKeepAlive(virNetServerClientPtr client); -const char *virNetServerClientLocalAddrString(virNetServerClientPtr client); -const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client); +const char *virNetServerClientLocalAddrStringSASL(virNetServerClientPtr client); +const char *virNetServerClientRemoteAddrStringSASL(virNetServerClientPtr client); const char *virNetServerClientRemoteAddrStringURI(virNetServerClientPtr client); int virNetServerClientSendMessage(virNetServerClientPtr client, -- 2.7.3
On 06/20/2016 10:27 AM, Ján Tomko wrote:
Add SASL at the end to make the format obvious. --- daemon/remote.c | 4 ++-- src/libvirt_remote.syms | 4 ++-- src/rpc/virnetserverclient.c | 4 ++-- src/rpc/virnetserverclient.h | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index ea4753f..5ee82bb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3028,8 +3028,8 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED,
sasl = virNetSASLSessionNewServer(saslCtxt, "libvirt", - virNetServerClientLocalAddrString(client), - virNetServerClientRemoteAddrString(client)); + virNetServerClientLocalAddrStringSASL(client), + virNetServerClientRemoteAddrStringSASL(client)); if (!sasl) goto authfail;
Based purely on my comment from 1/7 - Does remoteAuthSASL need to be adjusted too? ACK for this, but I wanted to make sure to ask. John
On Wed, Jun 22, 2016 at 07:43:08PM -0400, John Ferlan wrote:
On 06/20/2016 10:27 AM, Ján Tomko wrote:
Add SASL at the end to make the format obvious. --- daemon/remote.c | 4 ++-- src/libvirt_remote.syms | 4 ++-- src/rpc/virnetserverclient.c | 4 ++-- src/rpc/virnetserverclient.h | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index ea4753f..5ee82bb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3028,8 +3028,8 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED,
sasl = virNetSASLSessionNewServer(saslCtxt, "libvirt", - virNetServerClientLocalAddrString(client), - virNetServerClientRemoteAddrString(client)); + virNetServerClientLocalAddrStringSASL(client), + virNetServerClientRemoteAddrStringSASL(client)); if (!sasl) goto authfail;
Based purely on my comment from 1/7 - Does remoteAuthSASL need to be adjusted too?
Right, I forgot to rename the virNetClient*AddrString functions.
ACK for this, but I wanted to make sure to ask.
Thanks, I've pushed the series and sent the missing rename separately. Jan
participants (2)
-
John Ferlan -
Ján Tomko