Commit Graph

12 Commits

Author SHA1 Message Date
Daniel P. Berrangé
72874b2d11 tests: validate private data / pre / post exec hooks for RPC APIs
Validate that the virNetServer(Client) RPC APIs are processing the
private data callbacks correctly by passing in non-NULL pointers.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2018-02-02 13:00:48 +00:00
Marc Hartmayer
27e2e855cc tests: virnetdaemontest: Enable testing for 'auth_pending'
Enable testing for 'auth_pending' in the virnetdaemon test case.

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2018-01-04 06:55:31 -05:00
Marc Hartmayer
94bbbcee1f rpc: virnetserver: Fix race on srv->nclients_unauth
There is a race between virNetServerProcessClients (main thread) and
remoteDispatchAuthList/remoteDispatchAuthPolkit/remoteSASLFinish (worker
thread) that can lead to decrementing srv->nclients_unauth when it's
zero. Since virNetServerCheckLimits relies on the value
srv->nclients_unauth the underrun causes libvirtd to stop accepting
new connections forever.

Example race scenario (assuming libvirtd is using policykit and the
client is privileged):
  1. The client calls the RPC remoteDispatchAuthList =>
     remoteDispatchAuthList is executed on a worker thread (Thread
     T1). We're assuming now the execution stops for some time before
     the line 'virNetServerClientSetAuth(client, 0)'
  2. The client closes the connection irregularly. This causes the
     event loop to wake up and virNetServerProcessClient to be
     called (on the main thread T0). During the
     virNetServerProcessClients the srv lock is hold. The condition
     virNetServerClientNeedAuth(client) will be checked and as the
     authentication is not finished right now
     virNetServerTrackCompletedAuthLocked(srv) will be called =>
     --srv->nclients_unauth => 0
  3. The Thread T1 continues, marks the client as authenticated, and
     calls virNetServerTrackCompletedAuthLocked(srv) =>
     --srv->nclients_unauth => --0 => wrap around as nclient_unauth is
     unsigned
  4. virNetServerCheckLimits(srv) will disable the services forever

To fix it, add an auth_pending field to the client struct so that it
is now possible to determine if the authentication process has already
been handled for this client.

Setting the authentication method to none for the client in
virNetServerProcessClients is not a proper way to indicate that the
counter has been decremented, as this would imply that the client is
authenticated.

Additionally, adjust the existing test cases for this new field.

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
2018-01-04 06:55:31 -05:00
Jiri Denemark
7197e5fd3f virjson: Make pretty format more compact
json_reformat uses two spaces for when indenting nested objects, let's
do the same. The result of virJSONValueToString will be exactly the same
as json_reformat would produce.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2016-05-05 10:01:55 +02:00
Erik Skultety
a32135b3b1 rpc: virnetserverclient: Introduce new attribute conn_time to client
Besides ID, libvirt should provide several parameters to help the user
distinguish two clients from each other. One of them is the connection
timestamp. This patch also adds a testcase for proper JSON formatting of the
new attribute too (proper formatting of older clients that did not support
this attribute yet is included in the existing tests) - in order to
testGenerateJSON to work, a mock of time_t time(time_t *timer) needed to be
created.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2016-05-02 22:25:52 +02:00
Erik Skultety
5841d64d25 rpc: virnetserverclient: Identify clients by an integer ID
Admin API needs a way of addressing specific clients. Unlike servers, which we
are happy to address by names both because its name reflects its purpose (to
some extent) and we only have two of them (so far), naming clients doesn't make
any sense, since a) each client is an anonymous, i.e. not recognized after a
disconnect followed by a reconnect, b) we can't predict what kind of requests
it's going to send to daemon, and c) the are loads of them comming and going,
so the only viable option is to use an ID which is of a reasonably wide data
type.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2016-05-02 22:25:51 +02:00
Erik Skultety
ac55654e35 tests: virnetdaemontest: Enable testing for no-keepalive-required
Commit a8743c39 removed keepalive_required attribute from daemon, added a test
case for it, but forgot to enable the test itself in virnetdaemontest.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2016-04-21 12:36:03 +02:00
Erik Skultety
8da7520d22 tests: virnetdaemon: Replace tabs with spaces
We already did this once, but somehow commit 252610f7 managed to bring the tabs
back again.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
2016-04-21 12:36:03 +02:00
Erik Skultety
252610f7dd virnetdaemon: Store servers in a hash table
Since the daemon can manage and add (at fresh start) multiple servers,
we also should be able to add them from a JSON state file in case of a
daemon restart, so post exec restart support for multiple servers is also
provided. Patch also updates virnetdaemontest accordingly.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2016-02-17 12:46:34 +01:00
Erik Skultety
4ac0d545e8 test: Replace tabs with spaces in virnetdaemondata json files
JSON data that are used to initialize tests in virnetdaemontest should
be in a consistent format, i.e. not using tabs for indentation, those
should be replaced by spaces.
2015-08-24 09:30:40 +02:00
Martin Kletzander
a8743c3938 rpc: Remove keepalive_required option
Since its introduction in 2011 (particularly in commit f4324e3292),
the option doesn't work.  It just effectively disables all incoming
connections.  That's because the client private data that contain the
'keepalive_supported' boolean, are initialized to zeroes so the bool is
false and the only other place where the bool is used is when checking
whether the client supports keepalive.  Thus, according to the server,
no client supports keepalive.

Removing this instead of fixing it is better because a) apparently
nobody ever tried it since 2011 (4 years without one month) and b) we
cannot know whether the client supports keepalive until we get a ping or
pong keepalive packet.  And that won't happen until after we dispatched
the ConnectOpen call.

Another two reasons would be c) the keepalive_required was tracked on
the server level, but keepalive_supported was in private data of the
client as well as the check that was made in the remote layer, thus
making all other instances of virNetServer miss this feature unless they
all implemented it for themselves and d) we can always add it back in
case there is a request and a use-case for it.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-08-10 13:15:56 +02:00
Martin Kletzander
a474611458 tests: Adapt virnetservertest to daemon refactor
Rename the test to virnetdaemontest and use virNetDaemon objects instead
of virNetServer inside.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2015-06-16 13:46:20 +02:00