-
Notifications
You must be signed in to change notification settings - Fork 584
Abort connections with no valid endpoint #10415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Can you please update the PR description with a brief summary of the problem? In particular, #10405 describes a much more complex scenario than what should be necessary to reproduce this.
Also, please share examples of how this behaves now. |
@julianbrost Please see the updated description. I hope this makes things clearer. |
Please verify whether ApiUsers authenticated using a TLS client certificate still work, see https://icinga.com/docs/icinga-2/latest/doc/12-icinga2-api/#icinga2-api-authentication |
4407a89
to
f942abc
Compare
@julianbrost You are right, a certificate based ApiUser could no longer connect with this PR. I've just pushed an updated version that should fix this. It returns a bit later and only for verified JSON-RPC connections. Now both verified connections with no endpoint (i.e. ApiUser with Are there any other corner cases I'm not thinking about that need further testing? |
f942abc
to
1b3a0a8
Compare
1b3a0a8
to
353386f
Compare
Problem
In #10405 the problem is that incoming connections with valid certificates, but from endpoints that are not defined locally will get added as anonymous clients (via
ApiListener::AddAnonymousClient()
) and then hang around essentially forever since the check inJsonRpcConnection::CheckLiveness()
only puts anonymous connections on a timeout if they are unauthenticated.To Reproduce
#10405 describes a more complicated setup in detail, but the simplest setup to reproduce the issue is to have a working, authenticated master/agent or master/satellite setup and then comment out the master endpoint in the
zones.conf
of the agent/satellite and restart.Solution
Abort connections early when no endpoint is defined for the incoming connection. This is done by returning early from
ApiListener::NewClientHandlerInternal
when the certificate is validated, but no endpoint is configured for the remote.Caveats
Since the client closes the connection very early it is possible that the other side tries to read from or write to the socket, which then fails. For example this message+stracktrace can appear in the log:
A more complex solution that does not close the connection so abruptly for the remote would involve both sides of the
JsonRpcConnection
confirming the connection via an exchange of messages and should be considered in a future refactoring of theNewClientHandler
code.For now this closes #10405 by making the cluster checks fail reliably and keeps the parent from blindly sending requests to clients that just silently discard them.