The counter gets incremented on each unauthenticated client added to the server and decremented whenever the client authenticates. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- daemon/remote.c | 21 +++++++++++++-------- src/rpc/virnetserver.c | 36 +++++++++++++++++++++++++++++++++--- src/rpc/virnetserver.h | 2 ++ 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index decaecc..8354376 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2298,7 +2298,7 @@ cleanup: /*-------------------------------------------------------------*/ static int -remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthList(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2328,6 +2328,7 @@ remoteDispatchAuthList(virNetServerPtr server ATTRIBUTE_UNUSED, goto cleanup; VIR_INFO("Bypass polkit auth for privileged client %s", ident); virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; VIR_FREE(ident); } @@ -2443,7 +2444,8 @@ authfail: * Returns 0 if ok, -1 on error, -2 if rejected */ static int -remoteSASLFinish(virNetServerClientPtr client) +remoteSASLFinish(virNetServerPtr server, + virNetServerClientPtr client) { const char *identity; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); @@ -2468,6 +2470,7 @@ remoteSASLFinish(virNetServerClientPtr client) return -2; virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virNetServerClientSetSASLSession(client, priv->sasl); VIR_DEBUG("Authentication successful %d", virNetServerClientGetFD(client)); @@ -2489,7 +2492,7 @@ error: * This starts the SASL authentication negotiation. */ static int -remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthSaslStart(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2547,7 +2550,7 @@ remoteDispatchAuthSaslStart(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 0; } else { /* Check username whitelist ACL */ - if ((err = remoteSASLFinish(client)) < 0) { + if ((err = remoteSASLFinish(server, client)) < 0) { if (err == -2) goto authdeny; else @@ -2587,7 +2590,7 @@ error: static int -remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthSaslStep(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2645,7 +2648,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 0; } else { /* Check username whitelist ACL */ - if ((err = remoteSASLFinish(client)) < 0) { + if ((err = remoteSASLFinish(server, client)) < 0) { if (err == -2) goto authdeny; else @@ -2730,7 +2733,7 @@ remoteDispatchAuthSaslStep(virNetServerPtr server ATTRIBUTE_UNUSED, #if WITH_POLKIT1 static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2822,6 +2825,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 1; virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virMutexUnlock(&priv->lock); virCommandFree(cmd); VIR_FREE(pkout); @@ -2862,7 +2866,7 @@ authdeny: } #elif WITH_POLKIT0 static int -remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchAuthPolkit(virNetServerPtr server, virNetServerClientPtr client, virNetMessagePtr msg ATTRIBUTE_UNUSED, virNetMessageErrorPtr rerr, @@ -2977,6 +2981,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, ret->complete = 1; virNetServerClientSetAuth(client, 0); + virNetServerClientAuth(server, false); virMutexUnlock(&priv->lock); VIR_FREE(ident); return 0; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 8907768..1b2c6d4 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -87,9 +87,10 @@ struct _virNetServer { size_t nprograms; virNetServerProgramPtr *programs; - size_t nclients; - size_t nclients_max; - virNetServerClientPtr *clients; + size_t nclients; /* Current clients count */ + virNetServerClientPtr *clients; /* Clients */ + size_t nclients_max; /* Max allowed clients count */ + size_t nclients_unauth; /* Unauthenticated clients count */ int keepaliveInterval; unsigned int keepaliveCount; @@ -117,6 +118,8 @@ static virClassPtr virNetServerClass; static void virNetServerDispose(void *obj); static void virNetServerUpdateServicesLocked(virNetServerPtr srv, bool enabled); +static size_t virNetServerClientAuthLocked(virNetServerPtr srv, + bool need_auth); static int virNetServerOnceInit(void) { @@ -272,6 +275,9 @@ static int virNetServerAddClient(virNetServerPtr srv, srv->clients[srv->nclients-1] = client; virObjectRef(client); + if (virNetServerClientNeedAuth(client)) + virNetServerClientAuthLocked(srv, true); + if (srv->nclients == srv->nclients_max) { /* Temporarily stop accepting new clients */ VIR_DEBUG("Temporarily suspending services due to max_clients"); @@ -1135,6 +1141,9 @@ void virNetServerRun(virNetServerPtr srv) srv->nclients = 0; } + if (virNetServerClientNeedAuth(client)) + virNetServerClientAuthLocked(srv, false); + /* Enable services if we can accept a new client. * The new client can be accepted if we are at the limit. */ if (srv->nclients == srv->nclients_max - 1) { @@ -1231,3 +1240,24 @@ bool virNetServerKeepAliveRequired(virNetServerPtr srv) virObjectUnlock(srv); return required; } + +static size_t +virNetServerClientAuthLocked(virNetServerPtr srv, + bool need_auth) +{ + if (need_auth) + srv->nclients_unauth++; + else + srv->nclients_unauth--; + return srv->nclients_unauth; +} + +size_t virNetServerClientAuth(virNetServerPtr srv, + bool need_auth) +{ + size_t ret; + virObjectLock(srv); + ret = virNetServerClientAuthLocked(srv, need_auth); + virObjectUnlock(srv); + return ret; +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 1a85c02..703a733 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -97,4 +97,6 @@ void virNetServerClose(virNetServerPtr srv); bool virNetServerKeepAliveRequired(virNetServerPtr srv); +size_t virNetServerClientAuth(virNetServerPtr srv, + bool need_auth); #endif -- 1.8.5.1

https://bugzilla.redhat.com/show_bug.cgi?id=981729 This config tunable allows users to determine the maximum number of accepted but yet not authenticated users. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- daemon/libvirtd-config.c | 1 + daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 3 +++ daemon/test_libvirtd.aug.in | 1 + src/locking/lock_daemon.c | 4 ++-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 37 +++++++++++++++++++++++++++++++++++-- src/rpc/virnetserver.h | 1 + 10 files changed, 47 insertions(+), 5 deletions(-) diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index c816fda..04482c5 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -415,6 +415,7 @@ daemonConfigLoadOptions(struct daemonConfig *data, GET_CONF_INT(conf, filename, max_workers); GET_CONF_INT(conf, filename, max_clients); GET_CONF_INT(conf, filename, max_queued_clients); + GET_CONF_INT(conf, filename, max_anonymous_clients); GET_CONF_INT(conf, filename, prio_workers); diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h index a24d5d2..66dc80b 100644 --- a/daemon/libvirtd-config.h +++ b/daemon/libvirtd-config.h @@ -64,6 +64,7 @@ struct daemonConfig { int max_workers; int max_clients; int max_queued_clients; + int max_anonymous_clients; int prio_workers; diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug index 70fce5c..5a0807c 100644 --- a/daemon/libvirtd.aug +++ b/daemon/libvirtd.aug @@ -57,6 +57,7 @@ module Libvirtd = | int_entry "max_workers" | int_entry "max_clients" | int_entry "max_queued_clients" + | int_entry "max_anonymous_clients" | int_entry "max_requests" | int_entry "max_client_requests" | int_entry "prio_workers" diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 49c42ad..61c706c 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1367,6 +1367,7 @@ int main(int argc, char **argv) { config->max_workers, config->prio_workers, config->max_clients, + config->max_anonymous_clients, config->keepalive_interval, config->keepalive_count, !!config->keepalive_required, diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 5353927..28d3735 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -263,6 +263,9 @@ # connection succeeds. #max_queued_clients = 1000 +# The maximum length of queue of accepted but not yet not +# authenticated clients. +#max_anonymous_clients = 20 # The minimum limit sets the number of workers to start up # initially. If the number of active clients exceeds this, diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in index a7e8515..b03451c 100644 --- a/daemon/test_libvirtd.aug.in +++ b/daemon/test_libvirtd.aug.in @@ -36,6 +36,7 @@ module Test_libvirtd = } { "max_clients" = "20" } { "max_queued_clients" = "1000" } + { "max_anonymous_clients" = "20" } { "min_workers" = "5" } { "max_workers" = "20" } { "prio_workers" = "5" } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 35ccb4e..c97bc61 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -143,8 +143,8 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) } if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, - -1, 0, - false, NULL, + config->max_clients, + -1, 0, false, NULL, virLockDaemonClientNew, virLockDaemonClientPreExecRestart, virLockDaemonClientFree, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index c013147..578a135 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -736,7 +736,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) LXC_STATE_DIR, ctrl->name) < 0) return -1; - if (!(ctrl->server = virNetServerNew(0, 0, 0, 1, + if (!(ctrl->server = virNetServerNew(0, 0, 0, 1, 1, -1, 0, false, NULL, virLXCControllerClientPrivateNew, diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 1b2c6d4..bbb91d4 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -91,6 +91,7 @@ struct _virNetServer { virNetServerClientPtr *clients; /* Clients */ size_t nclients_max; /* Max allowed clients count */ size_t nclients_unauth; /* Unauthenticated clients count */ + size_t nclients_unauth_max; /* Max allowed unauth clients count */ int keepaliveInterval; unsigned int keepaliveCount; @@ -278,6 +279,13 @@ static int virNetServerAddClient(virNetServerPtr srv, if (virNetServerClientNeedAuth(client)) virNetServerClientAuthLocked(srv, true); + if (srv->nclients_unauth == srv->nclients_unauth_max) { + /* Temporarily stop accepting new clients */ + VIR_DEBUG("Temporarily suspending services " + "due to max_anonymous_clients"); + virNetServerUpdateServicesLocked(srv, false); + } + if (srv->nclients == srv->nclients_max) { /* Temporarily stop accepting new clients */ VIR_DEBUG("Temporarily suspending services due to max_clients"); @@ -361,6 +369,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t priority_workers, size_t max_clients, + size_t max_anonymous_clients, int keepaliveInterval, unsigned int keepaliveCount, bool keepaliveRequired, @@ -387,6 +396,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, goto error; srv->nclients_max = max_clients; + srv->nclients_unauth_max = max_anonymous_clients; srv->keepaliveInterval = keepaliveInterval; srv->keepaliveCount = keepaliveCount; srv->keepaliveRequired = keepaliveRequired; @@ -506,6 +516,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, if (!(srv = virNetServerNew(min_workers, max_clients, priority_workers, max_clients, + max_clients, keepaliveInterval, keepaliveCount, keepaliveRequired, mdnsGroupName, clientPrivNew, clientPrivPreExecRestart, @@ -1145,8 +1156,16 @@ void virNetServerRun(virNetServerPtr srv) virNetServerClientAuthLocked(srv, false); /* Enable services if we can accept a new client. - * The new client can be accepted if we are at the limit. */ - if (srv->nclients == srv->nclients_max - 1) { + * The new client can be accepted if both max_clients and + * max_anonymous_clients wouldn't get overcommitted by + * accepting it. */ + VIR_DEBUG("Considering re-enabling services: " + "nclients=%zu nclients_max=%zu " + "nclients_unauth=%zu nclients_unauth_max=%zu", + srv->nclients, srv->nclients_max, + srv->nclients_unauth, srv->nclients_unauth_max); + if ((srv->nclients == srv->nclients_max - 1) && + (srv->nclients_unauth < srv->nclients_unauth_max)) { /* Now it makes sense to accept() a new client. */ VIR_DEBUG("Re-enabling services"); virNetServerUpdateServicesLocked(srv, true); @@ -1257,7 +1276,21 @@ size_t virNetServerClientAuth(virNetServerPtr srv, { size_t ret; virObjectLock(srv); + ret = virNetServerClientAuthLocked(srv, need_auth); + + VIR_DEBUG("Considering re-enabling services: " + "nclients=%zu nclients_max=%zu " + "nclients_unauth=%zu nclients_unauth_max=%zu", + srv->nclients, srv->nclients_max, + srv->nclients_unauth, srv->nclients_unauth_max); + if ((srv->nclients < srv->nclients_max) && + (srv->nclients_unauth == srv->nclients_unauth_max - 1)) { + /* Now it makes sense to accept() a new client. */ + VIR_DEBUG("Re-enabling services"); + virNetServerUpdateServicesLocked(srv, true); + } + virObjectUnlock(srv); return ret; } diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 703a733..4b165c3 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -39,6 +39,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t priority_workers, size_t max_clients, + size_t max_anonymous_clients, int keepaliveInterval, unsigned int keepaliveCount, bool keepaliveRequired, -- 1.8.5.1

On Mon, Dec 09, 2013 at 03:35:53PM +0100, Michal Privoznik wrote: Please mention what's the default here. Since the default (when there's nothing in the config) is 0, it should mean the feature is disabled). If that's what you've intended, wouldn't it make more sense to use 0 so it's visible that the feature is not in effect? Same here? Here it should be taken from the JSON object (and 0 if not found) which should be populated from the save file. Also update that save file populating so the value is kept throughout all restarts. Spurious parentheses, plus even considering your squash-in, this and similar conditions don't count on nclients_unauth_max being 0; consider squashing this in: diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 62490a7..332641e 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -279,7 +279,8 @@ static int virNetServerAddClient(virNetServerPtr srv, if (virNetServerClientNeedAuth(client)) virNetServerClientAuthLocked(srv, true); - if (srv->nclients_unauth == srv->nclients_unauth_max) { + if (srv->nclients_unauth_max && + srv->nclients_unauth == srv->nclients_unauth_max) { /* Temporarily stop accepting new clients */ VIR_DEBUG("Temporarily suspending services " "due to max_anonymous_clients"); @@ -1164,8 +1165,9 @@ void virNetServerRun(virNetServerPtr srv) "nclients_unauth=%zu nclients_unauth_max=%zu", srv->nclients, srv->nclients_max, srv->nclients_unauth, srv->nclients_unauth_max); - if ((srv->nclients < srv->nclients_max) && - (srv->nclients_unauth < srv->nclients_unauth_max)) { + if (srv->nclients < srv->nclients_max && + (!srv->nclients_unauth_max || + srv->nclients_unauth < srv->nclients_unauth_max)) { /* Now it makes sense to accept() a new client. */ VIR_DEBUG("Re-enabling services"); virNetServerUpdateServicesLocked(srv, true); @@ -1284,8 +1286,9 @@ size_t virNetServerClientAuth(virNetServerPtr srv, "nclients_unauth=%zu nclients_unauth_max=%zu", srv->nclients, srv->nclients_max, srv->nclients_unauth, srv->nclients_unauth_max); - if ((srv->nclients < srv->nclients_max) && - (srv->nclients_unauth < srv->nclients_unauth_max)) { + if (srv->nclients < srv->nclients_max && + (!srv->nclients_unauth_max || + srv->nclients_unauth < srv->nclients_unauth_max)) { /* Now it makes sense to accept() a new client. */ VIR_DEBUG("Re-enabling services"); virNetServerUpdateServicesLocked(srv, true); -- Martin