[libvirt] [PATCH 0/7] Admin API: Add support for client identity info retrieval

This series adds support for client identity retrieval, i.e. information like remote IP (if connected remotely), uid,gid,pid, as well as username if connected locally and also information regarding authentication (if used). The series is rebased on the listing clients series, because it relies on the gendispatch stuff, so for testing purposes checkout my remote branch https://github.com/eskultety/libvirt/tree/list-clients-info-disconnect which also covers the next series about client disconnect. Erik Skultety (7): admin: Introduce virAdmServerLookupClient admin: include: Introduce some client's identity related typed params macros virnetsocket: Provide socket address format in a more standard form virneserverclient: Introduce virNetServerClientHasSASLSession virnetserverclient: Add an internal method to retrieve client's identity admin: Introduce virAdmClientGetInfo API virt-admin: Introduce command client-info daemon/admin.c | 59 ++++++++++++++++++ daemon/admin_server.c | 102 +++++++++++++++++++++++++++++++ daemon/admin_server.h | 9 +++ daemon/remote.c | 13 +++- include/libvirt/libvirt-admin.h | 130 ++++++++++++++++++++++++++++++++++++++++ include/libvirt/virterror.h | 1 + src/admin/admin_protocol.x | 34 ++++++++++- src/admin/admin_remote.c | 47 +++++++++++++++ src/admin_protocol-structs | 20 +++++++ src/libvirt-admin.c | 75 +++++++++++++++++++++++ src/libvirt_admin_private.syms | 4 ++ src/libvirt_admin_public.syms | 2 + src/remote/remote_driver.c | 7 +++ src/rpc/virnetclient.c | 10 ++++ src/rpc/virnetclient.h | 2 + src/rpc/virnetserver.c | 23 +++++++ src/rpc/virnetserver.h | 3 + src/rpc/virnetserverclient.c | 48 +++++++++++++++ src/rpc/virnetserverclient.h | 6 ++ src/rpc/virnetsocket.c | 17 +++++- src/rpc/virnetsocket.h | 2 + src/util/virerror.c | 6 ++ src/util/virsocketaddr.c | 24 ++++++-- tests/virnetsockettest.c | 10 ++-- tools/virt-admin.c | 91 ++++++++++++++++++++++++++++ 25 files changed, 731 insertions(+), 14 deletions(-) -- 2.4.11

Just like with server-related APIs, before any of client-based APIs can be called, a reference to a client-side client object needs to be obtained. For this purpose, a lookup method should exist. Apart from the client retrieval logic, a new error code for non-existent client had to be added as well. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin_server.c | 10 ++++++++++ daemon/admin_server.h | 4 ++++ include/libvirt/libvirt-admin.h | 5 +++++ include/libvirt/virterror.h | 1 + src/admin/admin_protocol.x | 17 ++++++++++++++++- src/admin_protocol-structs | 9 +++++++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + src/rpc/virnetserver.c | 23 +++++++++++++++++++++++ src/rpc/virnetserver.h | 3 +++ src/util/virerror.c | 6 ++++++ 12 files changed, 116 insertions(+), 1 deletion(-) diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 0bd142c..9ea1164 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -201,3 +201,13 @@ adminServerListClients(virNetServerPtr srv, virObjectListFreeCount(clts, ret); return ret; } + +virNetServerClientPtr +adminServerLookupClient(virNetServerPtr srv, + unsigned long long id, + unsigned int flags) +{ + virCheckFlags(0, NULL); + + return virNetServerGetClient(srv, id); +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h index 3e4786e..a593251 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -50,4 +50,8 @@ int adminServerListClients(virNetServerPtr srv, virNetServerClientPtr **clients, unsigned int flags); +virNetServerClientPtr adminServerLookupClient(virNetServerPtr srv, + unsigned long long id, + unsigned int flags); + #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 20768e2..e51f2ba 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -219,6 +219,11 @@ int virAdmServerListClients(virAdmServerPtr srv, virAdmClientPtr **clients, unsigned int flags); +virAdmClientPtr +virAdmServerLookupClient(virAdmServerPtr srv, + unsigned long long id, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index c6abdf7..2ec560e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -314,6 +314,7 @@ typedef enum { VIR_ERR_MIGRATE_FINISH_OK = 93, /* Finish API succeeded but it is expected to return NULL */ VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */ VIR_ERR_NO_SERVER = 95, /* Server was not found */ + VIR_ERR_NO_CLIENT = 96, /* Client was not found */ } virErrorNumber; /** diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index e77ce9e..da21db8 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -138,6 +138,16 @@ struct admin_server_list_clients_ret { /* insert@1 */ unsigned int ret; }; +struct admin_server_lookup_client_args { + admin_nonnull_server srv; + unsigned hyper id; + unsigned int flags; +}; + +struct admin_server_lookup_client_ret { + admin_nonnull_client clnt; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -198,5 +208,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_SERVER_LIST_CLIENTS = 8 + ADMIN_PROC_SERVER_LIST_CLIENTS = 8, + + /** + * @generate: both + */ + ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 055ddd5..dc2220e 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -87,6 +87,14 @@ struct admin_server_list_clients_ret { } clients; u_int ret; }; +struct admin_server_lookup_client_args { + admin_nonnull_server srv; + uint64_t id; + u_int flags; +}; +struct admin_server_lookup_client_ret { + admin_nonnull_client clnt; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -96,4 +104,5 @@ enum admin_procedure { ADMIN_PROC_SERVER_GET_THREADPOOL_PARAMETERS = 6, ADMIN_PROC_SERVER_SET_THREADPOOL_PARAMETERS = 7, ADMIN_PROC_SERVER_LIST_CLIENTS = 8, + ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index d4143e4..5d0755f 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -889,3 +889,39 @@ virAdmServerListClients(virAdmServerPtr srv, virDispatchError(NULL); return -1; } + +/** + * virAdmServerLookupClient: + * @srv: a valid server object reference + * @id: ID of the client to lookup on server @srv + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Try to lookup a client on the given server based on @id. + * + * virAdmClientFree() should be used to free the resources after the + * client object is no longer needed. + * + * Returns the requested client or NULL in case of failure. If the + * client could not be found, then VIR_ERR_NO_CLIENT error is raised. + */ +virAdmClientPtr +virAdmServerLookupClient(virAdmServerPtr srv, + unsigned long long id, + unsigned int flags) +{ + virAdmClientPtr ret = NULL; + + VIR_DEBUG("srv=%p, id=%llu, flags=%x", srv, id, flags); + virResetLastError(); + + virCheckAdmServerGoto(srv, error); + virCheckFlagsGoto(0, error); + + if (!(ret = remoteAdminServerLookupClient(srv, id, flags))) + goto error; + + return ret; + error: + virDispatchError(NULL); + return NULL; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 621b502..3d7ecbc 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -16,6 +16,8 @@ xdr_admin_server_get_threadpool_parameters_args; xdr_admin_server_get_threadpool_parameters_ret; xdr_admin_server_list_clients_args; xdr_admin_server_list_clients_ret; +xdr_admin_server_lookup_client_args; +xdr_admin_server_lookup_client_ret; xdr_admin_server_set_threadpool_parameters_args; # datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index bf6643a..066ae0c 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -29,6 +29,7 @@ LIBVIRT_ADMIN_1.3.0 { virAdmServerGetName; virAdmServerGetThreadPoolParameters; virAdmServerFree; + virAdmServerLookupClient; virAdmConnectLookupServer; virAdmServerSetThreadPoolParameters; virAdmServerListClients; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index fcc79f4..60541cb 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -972,3 +972,26 @@ virNetServerGetClients(virNetServerPtr srv, virObjectUnlock(srv); return ret; } + +virNetServerClientPtr +virNetServerGetClient(virNetServerPtr srv, + unsigned long long id) +{ + size_t i; + virNetServerClientPtr ret = NULL; + + virObjectLock(srv); + + for (i = 0; i < srv->nclients; i++) { + virNetServerClientPtr client = srv->clients[i]; + if (virNetServerClientGetID(client) == id) + ret = virObjectRef(client); + } + + virObjectUnlock(srv); + + if (!ret) + virReportError(VIR_ERR_NO_CLIENT, + _("No client with matching ID '%llu'"), id); + return ret; +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index f5bb200..993bda7 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -105,6 +105,9 @@ int virNetServerSetThreadPoolParameters(virNetServerPtr srv, unsigned long long virNetServerNextClientID(virNetServerPtr srv); +virNetServerClientPtr virNetServerGetClient(virNetServerPtr srv, + unsigned long long id); + int virNetServerGetClients(virNetServerPtr srv, virNetServerClientPtr **clients); diff --git a/src/util/virerror.c b/src/util/virerror.c index 4766e13..5d875e3 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1388,6 +1388,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("Server not found: %s"); break; + case VIR_ERR_NO_CLIENT: + if (info == NULL) + errmsg = _("Client not found"); + else + errmsg = _("Client not found: %s"); + break; } return errmsg; } -- 2.4.11

On Fri, Apr 29, 2016 at 03:45:53PM +0200, Erik Skultety wrote:
Just like with server-related APIs, before any of client-based APIs can be called, a reference to a client-side client object needs to be obtained. For this purpose, a lookup method should exist. Apart from the client retrieval logic, a new error code for non-existent client had to be added as well.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin_server.c | 10 ++++++++++ daemon/admin_server.h | 4 ++++ include/libvirt/libvirt-admin.h | 5 +++++ include/libvirt/virterror.h | 1 + src/admin/admin_protocol.x | 17 ++++++++++++++++- src/admin_protocol-structs | 9 +++++++++ src/libvirt-admin.c | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + src/rpc/virnetserver.c | 23 +++++++++++++++++++++++ src/rpc/virnetserver.h | 3 +++ src/util/virerror.c | 6 ++++++ 12 files changed, 116 insertions(+), 1 deletion(-)
+virAdmClientPtr +virAdmServerLookupClient(virAdmServerPtr srv, + unsigned long long id, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index c6abdf7..2ec560e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -314,6 +314,7 @@ typedef enum { VIR_ERR_MIGRATE_FINISH_OK = 93, /* Finish API succeeded but it is expected to return NULL */ VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */
VIR_ERR_NO_SERVER = 95, /* Server was not found */ + VIR_ERR_NO_CLIENT = 96, /* Client was not found */ } virErrorNumber;
Should these be added to the whitelist in daemonErrorLogFilter? Jan

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index c6abdf7..2ec560e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -314,6 +314,7 @@ typedef enum { VIR_ERR_MIGRATE_FINISH_OK = 93, /* Finish API succeeded but it is expected to return NULL */ VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */
VIR_ERR_NO_SERVER = 95, /* Server was not found */ + VIR_ERR_NO_CLIENT = 96, /* Client was not found */ } virErrorNumber;
Should these be added to the whitelist in daemonErrorLogFilter?
Jan
Good catch, I pushed a fix under trivial rule, thanks for noticing that. Erik

This patch could easily be squashed with the virAdmClientGetInfo method introduced later one, but the idea was to split the logic to as many preferably independent patches as possible. As the subject hints, this patch defines some public typed params-related macros used within virAdmClientGetInfo method. The thing is, there's one identity attribute missing in the set -- SELinux context, which libvirt internally supports in virIdentity, but it doesn't seem to do much (or anything at all), so there's some room to extend the set in the future. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- include/libvirt/libvirt-admin.h | 120 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index e51f2ba..5c30aae 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -224,6 +224,126 @@ virAdmServerLookupClient(virAdmServerPtr srv, unsigned long long id, unsigned int flags); +/* Client identity info */ + +/** + * VIR_CLIENT_INFO_READONLY: + * Macro represents client's connection permission, whether the client is + * connected in read-only mode or just the opposite - read-write, + * as VIR_TYPED_PARAM_BOOLEAN. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_READONLY "readonly" + +/** + * VIR_CLIENT_INFO_SOCKET_ADDR: + * Macro represents clients network socket address in a standard URI format: + * (IPv4|[IPv6]):port, as VIR_TYPED_PARAM_STRING. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_SOCKET_ADDR "sock_addr" + +/** + * VIR_CLIENT_INFO_SASL_USER_NAME: + * Macro represents client's SASL user name, if SASL authentication is enabled + * on the remote host, as VIR_TYPED_PARAM_STRING. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_SASL_USER_NAME "sasl_user_name" + +/** + * VIR_CLIENT_INFO_X509_DISTINGUISHED_NAME: + * Macro represents the 'distinguished name' field in X509 certificate the + * client used to establish a TLS session with remote host, as + * VIR_TYPED_PARAM_STRING. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_X509_DISTINGUISHED_NAME "tls_x509_dname" + +/** + * VIR_CLIENT_INFO_UNIX_USER_ID: + * Macro represents UNIX UID the client process is running with. Only relevant + * for clients connected locally, i.e. via a UNIX socket, + * as VIR_TYPED_PARAM_INT. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_UNIX_USER_ID "unix_user_id" + +/** + * VIR_CLIENT_INFO_UNIX_USER_NAME: + * Macro represents the user name that is bound to the client process's UID it + * is running with. Only relevant for clients connected locally, i.e. via a + * UNIX socket, as VIR_TYPED_PARAM_STRING. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_UNIX_USER_NAME "unix_user_name" + +/** + * VIR_CLIENT_INFO_UNIX_GROUP_ID: + * Macro represents UNIX GID the client process is running with. Only relevant + * for clients connected locally, i.e. via a UNIX socket, + * as VIR_TYPED_PARAM_INT. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_UNIX_GROUP_ID "unix_group_id" + +/** + * VIR_CLIENT_INFO_UNIX_GROUP_NAME: + * Macro represents the group name that is bound to the client process's GID it + * is running with. Only relevant for clients connected locally, i.e. via a + * UNIX socket, as VIR_TYPED_PARAM_STRING. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_UNIX_GROUP_NAME "unix_group_name" + +/** + * VIR_CLIENT_INFO_UNIX_PROCESS_ID: + * Macro represents the client process's pid it is running with. Only relevant + * for clients connected locally, i.e. via a UNIX socket, + * as VIR_TYPED_PARAM_INT. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_UNIX_PROCESS_ID "unix_process_id" + +/** + * VIR_CLIENT_INFO_SELINUX_CONTEXT: + * Macro represents the client's (peer's) SELinux context and this can either + * be at socket layer or at transport layer, depending on the connection type, + * as VIR_TYPED_PARAM_STRING. + * + * NOTE: This attribute is read-only and any attempt to set it will be denied + * by daemon + */ + +# define VIR_CLIENT_INFO_SELINUX_CONTEXT "selinux_context" + # ifdef __cplusplus } # endif -- 2.4.11

Our socket address format is in a rather non-standard format and that is because sasl library requires the IP address and service to be delimited by a semicolon. The string form is a completely internal matter, however once the admin interfaces to retrieve client identity information are merged, we should return the socket address string in a common format, e.g. format defined by URI rfc-3986, i.e. the IP address and service are delimited by a colon and in case of an IPv6 address, square brackets are added: Examples: 127.0.0.1:1234 [::1]:1234 This patch changes our default format to the one described above, while adding separate methods to request the non-standard SASL format using semicolon as a delimiter. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- 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 ++ src/util/virsocketaddr.c | 24 ++++++++++++++++++++---- tests/virnetsockettest.c | 10 +++++----- 10 files changed, 87 insertions(+), 13 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index fde029d..b2a420b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2937,6 +2937,8 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, virNetSASLSessionPtr sasl = NULL; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + char *localAddr = NULL; + char *remoteAddr = NULL; virMutexLock(&priv->lock); @@ -2947,10 +2949,17 @@ remoteDispatchAuthSaslInit(virNetServerPtr server ATTRIBUTE_UNUSED, goto authfail; } + localAddr = virNetServerClientLocalAddrFormatSASL(client); + remoteAddr = virNetServerClientRemoteAddrFormatSASL(client); + sasl = virNetSASLSessionNewServer(saslCtxt, "libvirt", - virNetServerClientLocalAddrString(client), - virNetServerClientRemoteAddrString(client)); + localAddr, + remoteAddr); + + VIR_FREE(localAddr); + VIR_FREE(remoteAddr); + if (!sasl) goto authfail; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 6bed2c5..e3cf5fb 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3684,6 +3684,8 @@ 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; @@ -3702,6 +3704,9 @@ 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", @@ -3889,6 +3894,8 @@ 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 d8ed15b..26b02fa 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -954,6 +954,16 @@ 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 38f929c..4b78677 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -123,6 +123,8 @@ 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 a9d70e1..a7b3b15 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -911,6 +911,19 @@ 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 55a8af1..267e4ec 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -139,6 +139,8 @@ 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 d909b94..a90cc55 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, ";"))) + !(sock->localAddrStr = virSocketAddrFormatFull(localAddr, true, NULL))) goto error; if (remoteAddr && - !(sock->remoteAddrStr = virSocketAddrFormatFull(remoteAddr, true, ";"))) + !(sock->remoteAddrStr = virSocketAddrFormatFull(remoteAddr, true, NULL))) goto error; sock->client = isClient; @@ -1465,6 +1465,19 @@ 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 5de3d92..4eb7187 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -150,6 +150,8 @@ 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/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 4b45681..a0c92ea 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -339,9 +339,11 @@ virSocketAddrFormat(const virSocketAddr *addr) * @withService: if true, then service info is appended * @separator: separator between hostname & service. * - * Returns a string representation of the given address - * Returns NULL on any error - * Caller must free the returned string + * Returns a string representation of the given address. If a format conforming + * to URI specification is required, NULL should be passed to separator. + * Set @separator only if non-URI format is required, e.g. passing ';' for + * @separator if the address should be used with SASL. + * Caller must free the returned string. */ char * virSocketAddrFormatFull(const virSocketAddr *addr, @@ -383,8 +385,22 @@ virSocketAddrFormatFull(const virSocketAddr *addr, } if (withService) { - if (virAsprintf(&addrstr, "%s%s%s", host, separator, port) == -1) + char *ipv6_host = NULL; + /* sasl_new_client demands the socket address to be in an odd format: + * a.b.c.d;port or e:f:g:h:i:j:k:l;port, so use square brackets for + * IPv6 only if no separator is passed to the function + */ + if (!separator && VIR_SOCKET_ADDR_FAMILY(addr) == AF_INET6) { + if (virAsprintf(&ipv6_host, "[%s]", host) < 0) + goto error; + } + + if (virAsprintf(&addrstr, "%s%s%s", + ipv6_host ? ipv6_host : host, + separator ? separator : ":", port) == -1) goto error; + + VIR_FREE(ipv6_host); } else { if (VIR_STRDUP(addrstr, host) < 0) goto error; diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 5786870..c2bc4e7 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.4.11

We do have a similar method, serving the same purpose, for TLS, but we lack one for SASL. So introduce one, in order for other modules to be able to find out, if a SASL session is active, or better said, that a SASL session exists at all. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/rpc/virnetserverclient.c | 9 +++++++++ src/rpc/virnetserverclient.h | 1 + 2 files changed, 10 insertions(+) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index a7b3b15..396b0cc 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -863,6 +863,15 @@ virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr clie virObjectUnlock(client); return sasl; } + +bool virNetServerClientHasSASLSession(virNetServerClientPtr client) +{ + bool has = false; + virObjectLock(client); + has = !!client->sasl; + virObjectUnlock(client); + return has; +} #endif diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 267e4ec..b576fde 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -91,6 +91,7 @@ int virNetServerClientGetTLSKeySize(virNetServerClientPtr client); # endif # ifdef WITH_SASL +bool virNetServerClientHasSASLSession(virNetServerClientPtr client); void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionPtr sasl); virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client); -- 2.4.11

On 29.04.2016 15:45, Erik Skultety wrote:
We do have a similar method, serving the same purpose, for TLS, but we lack one for SASL. So introduce one, in order for other modules to be able to find out, if a SASL session is active, or better said, that a SASL session exists at all.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/rpc/virnetserverclient.c | 9 +++++++++ src/rpc/virnetserverclient.h | 1 + 2 files changed, 10 insertions(+)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index a7b3b15..396b0cc 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -863,6 +863,15 @@ virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr clie virObjectUnlock(client); return sasl; } + +bool virNetServerClientHasSASLSession(virNetServerClientPtr client) +{ + bool has = false; + virObjectLock(client); + has = !!client->sasl; + virObjectUnlock(client); + return has; +} #endif
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 267e4ec..b576fde 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -91,6 +91,7 @@ int virNetServerClientGetTLSKeySize(virNetServerClientPtr client); # endif
# ifdef WITH_SASL +bool virNetServerClientHasSASLSession(virNetServerClientPtr client); void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionPtr sasl); virNetSASLSessionPtr virNetServerClientGetSASLSession(virNetServerClientPtr client);
This symbol should be exposed in src/libvirt_sasl.syms too. Michal

This method just aggregates various client object attributes, like socket address, connection type (RO/RW), and some TCP/TLS/UNIX identity in an atomic manner. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/rpc/virnetserverclient.c | 26 ++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 3 +++ 2 files changed, 29 insertions(+) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 396b0cc..ef1b568 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1612,3 +1612,29 @@ virNetServerClientGetTransport(virNetServerClientPtr client) return ret; } + +int +virNetServerClientGetInfo(virNetServerClientPtr client, + bool *readonly, const char **sock_addr, + virIdentityPtr *identity) +{ + int ret = -1; + + virObjectLock(client); + *readonly = client->readonly; + + if (!(*sock_addr = virNetServerClientRemoteAddrString(client))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No network socket associated with client")); + goto cleanup; + } + + *identity = NULL; + if (client->identity) + *identity = virObjectRef(client->identity); + + ret = 0; + cleanup: + virObjectUnlock(client); + return ret; +} diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index b576fde..2fbf60c 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -148,5 +148,8 @@ int virNetServerClientSendMessage(virNetServerClientPtr client, bool virNetServerClientNeedAuth(virNetServerClientPtr client); int virNetServerClientGetTransport(virNetServerClientPtr client); +int virNetServerClientGetInfo(virNetServerClientPtr client, + bool *readonly, const char **sock_addr, + virIdentityPtr *identity); #endif /* __VIR_NET_SERVER_CLIENT_H__ */ -- 2.4.11

On 29.04.2016 15:45, Erik Skultety wrote:
This method just aggregates various client object attributes, like socket address, connection type (RO/RW), and some TCP/TLS/UNIX identity in an atomic manner.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/rpc/virnetserverclient.c | 26 ++++++++++++++++++++++++++ src/rpc/virnetserverclient.h | 3 +++ 2 files changed, 29 insertions(+)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 396b0cc..ef1b568 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1612,3 +1612,29 @@ virNetServerClientGetTransport(virNetServerClientPtr client)
return ret; } + +int +virNetServerClientGetInfo(virNetServerClientPtr client, + bool *readonly, const char **sock_addr, + virIdentityPtr *identity) +{ + int ret = -1; + + virObjectLock(client); + *readonly = client->readonly; + + if (!(*sock_addr = virNetServerClientRemoteAddrString(client))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No network socket associated with client")); + goto cleanup; + } + + *identity = NULL; + if (client->identity) + *identity = virObjectRef(client->identity);
Should we drop this if() and therefore set *identity to NULL if there's none? I guess it's better to be safe than sorry. Moreover, if we don't do that and return 0 it's hard for callers of this function to determine if this argument has been updated or not. In general I think that function should either update all args or none.
+ + ret = 0; + cleanup: + virObjectUnlock(client); + return ret; +} diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index b576fde..2fbf60c 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -148,5 +148,8 @@ int virNetServerClientSendMessage(virNetServerClientPtr client,
bool virNetServerClientNeedAuth(virNetServerClientPtr client); int virNetServerClientGetTransport(virNetServerClientPtr client); +int virNetServerClientGetInfo(virNetServerClientPtr client, + bool *readonly, const char **sock_addr, + virIdentityPtr *identity);
#endif /* __VIR_NET_SERVER_CLIENT_H__ */
Don't forget to export this symbol in src/libvirt_remote.syms too. Michal

+int +virNetServerClientGetInfo(virNetServerClientPtr client, + bool *readonly, const char **sock_addr, + virIdentityPtr *identity) +{ + int ret = -1; + + virObjectLock(client); + *readonly = client->readonly; + + if (!(*sock_addr = virNetServerClientRemoteAddrString(client))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No network socket associated with client")); + goto cleanup; + } + + *identity = NULL; + if (client->identity) + *identity = virObjectRef(client->identity);
Should we drop this if() and therefore set *identity to NULL if there's none? I guess it's better to be safe than sorry. Moreover, if we don't do that and return 0 it's hard for callers of this function to determine if this argument has been updated or not. In general I think that function should either update all args or none.
Not sure I got your point. If no identity is present, pointer will be set to NULL and 0 is returned, so I'm not sure about the safe and sorry part. Anyway, I do agree that we shouldn't touch caller's args if an error occurs and since not having any identity information for a client clearly is one, I suggest removing "*identity = NULL" and checking if client identity exists at all and if not, returning -1 and raising an error that no identity was available for a client...would that work for you?
+ + ret = 0; + cleanup: + virObjectUnlock(client); + return ret; +} diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index b576fde..2fbf60c 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -148,5 +148,8 @@ int virNetServerClientSendMessage(virNetServerClientPtr client,
bool virNetServerClientNeedAuth(virNetServerClientPtr client); int virNetServerClientGetTransport(virNetServerClientPtr client); +int virNetServerClientGetInfo(virNetServerClientPtr client, + bool *readonly, const char **sock_addr, + virIdentityPtr *identity);
#endif /* __VIR_NET_SERVER_CLIENT_H__ */
Don't forget to export this symbol in src/libvirt_remote.syms too.
Michal
Yep, will add the symbol, thanks. Erik

Expose a public API to retrieve some identity and connection information about a client connected to the specified server on daemon. The identity info retrieved is mostly connection transport dependent, i.e. there won't be any socket address returned for a local (UNIX socket) connection, while on the other hand, when connected through TLS or unencrypted TCP, obviously no UNIX process identification will be present in the returned data. All supported values that can be returned in typed params are exposed and documented in include/libvirt/libvirt-admin.h Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin.c | 59 ++++++++++++++++++++++++++ daemon/admin_server.c | 92 +++++++++++++++++++++++++++++++++++++++++ daemon/admin_server.h | 5 +++ include/libvirt/libvirt-admin.h | 5 +++ src/admin/admin_protocol.x | 19 ++++++++- src/admin/admin_remote.c | 47 +++++++++++++++++++++ src/admin_protocol-structs | 11 +++++ src/libvirt-admin.c | 39 +++++++++++++++++ src/libvirt_admin_private.syms | 2 + src/libvirt_admin_public.syms | 1 + 10 files changed, 279 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index fcbdee5..3de09ca 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -236,4 +236,63 @@ adminDispatchServerSetThreadpoolParameters(virNetServerPtr server ATTRIBUTE_UNUS virObjectUnref(srv); return rv; } + +static int +adminDispatchClientGetInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + struct admin_client_get_info_args *args, + struct admin_client_get_info_ret *ret) +{ + int rv = -1; + virNetServerPtr srv = NULL; + virNetServerClientPtr clnt = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + struct daemonAdmClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!(srv = virNetDaemonGetServer(priv->dmn, args->clnt.srv.name))) { + virReportError(VIR_ERR_NO_SERVER, + _("no server with matching name '%s' found"), + args->clnt.srv.name); + goto cleanup; + } + + if (!(clnt = virNetServerGetClient(srv, args->clnt.id))) { + virReportError(VIR_ERR_NO_CLIENT, + _("no client with matching id '%lu' found"), + args->clnt.id); + goto cleanup; + } + + if (adminClientGetInfo(clnt, ¶ms, &nparams, args->flags) < 0) + goto cleanup; + + if (nparams > ADMIN_CLIENT_INFO_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of client info parameters %d exceeds max " + "allowed limit: %d"), nparams, + ADMIN_CLIENT_INFO_PARAMETERS_MAX); + goto cleanup; + } + + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) + goto cleanup; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + + virTypedParamsFree(params, nparams); + virObjectUnref(clnt); + virObjectUnref(srv); + return rv; +} #include "admin_dispatch.h" diff --git a/daemon/admin_server.c b/daemon/admin_server.c index 9ea1164..2fc4675 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -27,6 +27,7 @@ #include "datatypes.h" #include "viralloc.h" #include "virerror.h" +#include "viridentity.h" #include "virlog.h" #include "virnetdaemon.h" #include "virnetserver.h" @@ -211,3 +212,94 @@ adminServerLookupClient(virNetServerPtr srv, return virNetServerGetClient(srv, id); } + +int +adminClientGetInfo(virNetServerClientPtr client, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int ret = -1; + int maxparams = 0; + bool readonly; + const char *sock_addr = NULL; + const char *attr = NULL; + virTypedParameterPtr tmpparams = NULL; + virIdentityPtr identity = NULL; + + virCheckFlags(0, -1); + + if (virNetServerClientGetInfo(client, &readonly, + &sock_addr, &identity) < 0) + goto cleanup; + + if (virTypedParamsAddBoolean(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_READONLY, + readonly) < 0) + goto cleanup; + + if (!virNetServerClientIsLocal(client)) { + if (virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_SOCKET_ADDR, + sock_addr) < 0) + goto cleanup; + + if (virIdentityGetSASLUserName(identity, &attr) < 0 || + (attr && + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_SASL_USER_NAME, + attr) < 0)) + goto cleanup; + + if (virIdentityGetX509DName(identity, &attr) < 0 || + (attr && + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_X509_DISTINGUISHED_NAME, + attr) < 0)) + goto cleanup; + } else { + pid_t pid; + uid_t uid; + gid_t gid; + if (virIdentityGetUNIXUserID(identity, &uid) < 0 || + virTypedParamsAddInt(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_UNIX_USER_ID, uid) < 0) + goto cleanup; + + if (virIdentityGetUNIXUserName(identity, &attr) < 0 || + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_UNIX_USER_NAME, + attr) < 0) + goto cleanup; + + if (virIdentityGetUNIXGroupID(identity, &gid) < 0 || + virTypedParamsAddInt(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_UNIX_GROUP_ID, gid) < 0) + goto cleanup; + + if (virIdentityGetUNIXGroupName(identity, &attr) < 0 || + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_UNIX_GROUP_NAME, + attr) < 0) + goto cleanup; + + if (virIdentityGetUNIXProcessID(identity, &pid) < 0 || + virTypedParamsAddInt(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_UNIX_PROCESS_ID, pid) < 0) + goto cleanup; + } + + if (virIdentityGetSELinuxContext(identity, &attr) < 0 || + (attr && + virTypedParamsAddString(&tmpparams, nparams, &maxparams, + VIR_CLIENT_INFO_SELINUX_CONTEXT, attr) < 0)) + goto cleanup; + + *params = tmpparams; + tmpparams = NULL; + ret = 0; + + cleanup: + virObjectUnref(identity); + return ret; +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h index a593251..95c76b9 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -54,4 +54,9 @@ virNetServerClientPtr adminServerLookupClient(virNetServerPtr srv, unsigned long long id, unsigned int flags); +int adminClientGetInfo(virNetServerClientPtr client, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 5c30aae..0a1ea61 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -344,6 +344,11 @@ virAdmServerLookupClient(virAdmServerPtr srv, # define VIR_CLIENT_INFO_SELINUX_CONTEXT "selinux_context" +int virAdmClientGetInfo(virAdmClientPtr client, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index da21db8..67bdbf3 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -42,6 +42,9 @@ const ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX = 32; /* Upper limit on list of clients */ const ADMIN_CLIENT_LIST_MAX = 16384; +/* Upper limit on number of client info parameters */ +const ADMIN_CLIENT_INFO_PARAMETERS_MAX = 64; + /* A long string, which may NOT be NULL. */ typedef string admin_nonnull_string<ADMIN_STRING_MAX>; @@ -148,6 +151,15 @@ struct admin_server_lookup_client_ret { admin_nonnull_client clnt; }; +struct admin_client_get_info_args { + admin_nonnull_client clnt; + unsigned int flags; +}; + +struct admin_client_get_info_ret { /* insert@1 */ + admin_typed_param params<ADMIN_CLIENT_INFO_PARAMETERS_MAX>; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -213,5 +225,10 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9 + ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9, + + /** + * @generate: none + */ + ADMIN_PROC_CLIENT_GET_INFO = 10 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index b833ea4..40fcddb 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -67,6 +67,16 @@ make_nonnull_server(admin_nonnull_server *srv_dst, virAdmServerPtr srv_src) srv_dst->name = srv_src->name; } +static void +make_nonnull_client(admin_nonnull_client *client_dst, + virAdmClientPtr client_src) +{ + client_dst->id = client_src->id; + client_dst->transport = client_src->transport; + client_dst->timestamp = client_src->timestamp; + make_nonnull_server(&client_dst->srv, client_src->srv); +} + static int callFull(virAdmConnectPtr conn ATTRIBUTE_UNUSED, remoteAdminPrivPtr priv, @@ -308,3 +318,40 @@ remoteAdminServerSetThreadPoolParameters(virAdmServerPtr srv, virObjectUnlock(priv); return rv; } + +static int +remoteAdminClientGetInfo(virAdmClientPtr client, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remoteAdminPrivPtr priv = client->srv->conn->privateData; + admin_client_get_info_args args; + admin_client_get_info_ret ret; + + args.flags = flags; + make_nonnull_client(&args.clnt, client); + + memset(&ret, 0, sizeof(ret)); + virObjectLock(priv); + + if (call(client->srv->conn, 0, ADMIN_PROC_CLIENT_GET_INFO, + (xdrproc_t)xdr_admin_client_get_info_args, (char *) &args, + (xdrproc_t)xdr_admin_client_get_info_ret, (char *) &ret) == -1) + goto cleanup; + + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + ADMIN_CLIENT_INFO_PARAMETERS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + xdr_free((xdrproc_t)xdr_admin_client_get_info_ret, (char *) &ret); + + cleanup: + virObjectUnlock(priv); + return rv; +} diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index dc2220e..ea9adf6 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -95,6 +95,16 @@ struct admin_server_lookup_client_args { struct admin_server_lookup_client_ret { admin_nonnull_client clnt; }; +struct admin_client_get_info_args { + admin_nonnull_client clnt; + u_int flags; +}; +struct admin_client_get_info_ret { + struct { + u_int params_len; + admin_typed_param * params_val; + } params; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -105,4 +115,5 @@ enum admin_procedure { ADMIN_PROC_SERVER_SET_THREADPOOL_PARAMETERS = 7, ADMIN_PROC_SERVER_LIST_CLIENTS = 8, ADMIN_PROC_SERVER_LOOKUP_CLIENT = 9, + ADMIN_PROC_CLIENT_GET_INFO = 10, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 5d0755f..01094e6 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -925,3 +925,42 @@ virAdmServerLookupClient(virAdmServerPtr srv, virDispatchError(NULL); return NULL; } + +/** + * virAdmClientGetInfo: + * @client: a client object reference + * @params: pointer to a list of typed parameters which will be allocated + * to store all returned parameters + * @nparams: pointer which will hold the number of params returned in @params + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Extract information about a client. This information regards connection + * parameters, namely transport, authentication method, pid, gid, uid, etc. + * + * Returns 0 if the information has been successfully retrieved or -1 in case + * of an error. + */ +int +virAdmClientGetInfo(virAdmClientPtr client, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("client=%p, params=%p, nparams=%p, flags=%x", + client, params, nparams, flags); + + virResetLastError(); + virCheckAdmClientReturn(client, -1); + virCheckNonNullArgGoto(params, error); + virCheckFlagsGoto(0, error); + + if ((ret = remoteAdminClientGetInfo(client, params, nparams, flags)) < 0) + goto error; + + return ret; + error: + virDispatchError(NULL); + return -1; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 3d7ecbc..affe8c1 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -6,6 +6,8 @@ # # admin/admin_protocol.x +xdr_admin_client_get_info_args; +xdr_admin_client_get_info_ret; xdr_admin_connect_get_lib_version_ret; xdr_admin_connect_list_servers_args; xdr_admin_connect_list_servers_ret; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 066ae0c..27e4a1d 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -33,4 +33,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectLookupServer; virAdmServerSetThreadPoolParameters; virAdmServerListClients; + virAdmClientGetInfo; }; -- 2.4.11

Wire-up the client identity getter into virt-admin tool. This patch adjusts man-page accordingly. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index f0fecdd..72c54ae 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -655,6 +655,91 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) return ret; } +/* ------------------- + * Command client-info + * ------------------- + */ + +static const vshCmdInfo info_client_info[] = { + {.name = "help", + .data = N_("retrieve client's identity info from <server>") + }, + {.name = "desc", + .data = N_("Retrieve identity details about <client> from <server>") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_client_info[] = { + {.name = "client", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("client which to retrieve identity information for"), + }, + {.name = "server", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("server to which <client> is connected to"), + }, + {.name = NULL} +}; + +static bool +cmdClientInfo(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + size_t i; + unsigned long long id; + const char *srvname = NULL; + char *timestr = NULL; + virAdmServerPtr srv = NULL; + virAdmClientPtr clnt = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + vshAdmControlPtr priv = ctl->privData; + + if (vshCommandOptULongLong(ctl, cmd, "client", &id) < 0) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + return false; + + if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)) || + !(clnt = virAdmServerLookupClient(srv, id, 0))) + goto cleanup; + + /* Retrieve client identity info */ + if (virAdmClientGetInfo(clnt, ¶ms, &nparams, 0) < 0) { + vshError(ctl, _("failed to retrieve client identity information for " + "client '%llu' connected to server '%s'"), + id, virAdmServerGetName(srv)); + goto cleanup; + } + + if (vshAdmGetTimeStr(ctl, virAdmClientGetTimestamp(clnt), ×tr) < 0) + goto cleanup; + + /* this info is provided by the client object itself */ + vshPrint(ctl, "%-15s: %llu\n", "id", virAdmClientGetID(clnt)); + vshPrint(ctl, "%-15s: %s\n", "connection_time", timestr); + vshPrint(ctl, "%-15s: %s\n", "transport", + vshAdmClientTransportToString(virAdmClientGetTransport(clnt))); + + for (i = 0; i < nparams; i++) { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + virAdmServerFree(srv); + virAdmClientFree(clnt); + VIR_FREE(timestr); + return ret; +} static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -966,6 +1051,12 @@ static const vshCmdDef monitoringCmds[] = { .info = info_srv_clients_list, .flags = 0 }, + {.name = "client-info", + .handler = cmdClientInfo, + .opts = opts_client_info, + .info = info_client_info, + .flags = 0 + }, {.name = NULL} }; -- 2.4.11

On 29.04.2016 15:45, Erik Skultety wrote:
Wire-up the client identity getter into virt-admin tool. This patch adjusts man-page accordingly.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- tools/virt-admin.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+)
Missing documentation.
diff --git a/tools/virt-admin.c b/tools/virt-admin.c index f0fecdd..72c54ae 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -655,6 +655,91 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) return ret; }
+/* ------------------- + * Command client-info + * ------------------- + */ + +static const vshCmdInfo info_client_info[] = { + {.name = "help", + .data = N_("retrieve client's identity info from <server>") + }, + {.name = "desc", + .data = N_("Retrieve identity details about <client> from <server>") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_client_info[] = { + {.name = "client", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("client which to retrieve identity information for"), + }, + {.name = "server", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("server to which <client> is connected to"), + }, + {.name = NULL}
I wonder if we should reverse the order of these two. Server is more important than client. But maybe I'm bikeshedding, so whatever is your gut feeling.
+}; + +static bool +cmdClientInfo(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + size_t i; + unsigned long long id; + const char *srvname = NULL; + char *timestr = NULL; + virAdmServerPtr srv = NULL; + virAdmClientPtr clnt = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + vshAdmControlPtr priv = ctl->privData; + + if (vshCommandOptULongLong(ctl, cmd, "client", &id) < 0) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + return false; + + if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)) || + !(clnt = virAdmServerLookupClient(srv, id, 0))) + goto cleanup; + + /* Retrieve client identity info */ + if (virAdmClientGetInfo(clnt, ¶ms, &nparams, 0) < 0) { + vshError(ctl, _("failed to retrieve client identity information for " + "client '%llu' connected to server '%s'"), + id, virAdmServerGetName(srv)); + goto cleanup; + } + + if (vshAdmGetTimeStr(ctl, virAdmClientGetTimestamp(clnt), ×tr) < 0) + goto cleanup; + + /* this info is provided by the client object itself */ + vshPrint(ctl, "%-15s: %llu\n", "id", virAdmClientGetID(clnt)); + vshPrint(ctl, "%-15s: %s\n", "connection_time", timestr); + vshPrint(ctl, "%-15s: %s\n", "transport", + vshAdmClientTransportToString(virAdmClientGetTransport(clnt))); + + for (i = 0; i < nparams; i++) { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + virAdmServerFree(srv); + virAdmClientFree(clnt); + VIR_FREE(timestr); + return ret; +} static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -966,6 +1051,12 @@ static const vshCmdDef monitoringCmds[] = { .info = info_srv_clients_list, .flags = 0 }, + {.name = "client-info", + .handler = cmdClientInfo, + .opts = opts_client_info, + .info = info_client_info, + .flags = 0 + }, {.name = NULL} };
Michal

On 29.04.2016 15:45, Erik Skultety wrote:
This series adds support for client identity retrieval, i.e. information like remote IP (if connected remotely), uid,gid,pid, as well as username if connected locally and also information regarding authentication (if used).
The series is rebased on the listing clients series, because it relies on the gendispatch stuff, so for testing purposes checkout my remote branch https://github.com/eskultety/libvirt/tree/list-clients-info-disconnect which also covers the next series about client disconnect.
Erik Skultety (7): admin: Introduce virAdmServerLookupClient admin: include: Introduce some client's identity related typed params macros virnetsocket: Provide socket address format in a more standard form virneserverclient: Introduce virNetServerClientHasSASLSession virnetserverclient: Add an internal method to retrieve client's identity admin: Introduce virAdmClientGetInfo API virt-admin: Introduce command client-info
daemon/admin.c | 59 ++++++++++++++++++ daemon/admin_server.c | 102 +++++++++++++++++++++++++++++++ daemon/admin_server.h | 9 +++ daemon/remote.c | 13 +++- include/libvirt/libvirt-admin.h | 130 ++++++++++++++++++++++++++++++++++++++++ include/libvirt/virterror.h | 1 + src/admin/admin_protocol.x | 34 ++++++++++- src/admin/admin_remote.c | 47 +++++++++++++++ src/admin_protocol-structs | 20 +++++++ src/libvirt-admin.c | 75 +++++++++++++++++++++++ src/libvirt_admin_private.syms | 4 ++ src/libvirt_admin_public.syms | 2 + src/remote/remote_driver.c | 7 +++ src/rpc/virnetclient.c | 10 ++++ src/rpc/virnetclient.h | 2 + src/rpc/virnetserver.c | 23 +++++++ src/rpc/virnetserver.h | 3 + src/rpc/virnetserverclient.c | 48 +++++++++++++++ src/rpc/virnetserverclient.h | 6 ++ src/rpc/virnetsocket.c | 17 +++++- src/rpc/virnetsocket.h | 2 + src/util/virerror.c | 6 ++ src/util/virsocketaddr.c | 24 ++++++-- tests/virnetsockettest.c | 10 ++-- tools/virt-admin.c | 91 ++++++++++++++++++++++++++++ 25 files changed, 731 insertions(+), 14 deletions(-)
ACK series Michal
participants (3)
-
Erik Skultety
-
Ján Tomko
-
Michal Privoznik