Skip to content
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

Handling intelmqctl timeouts #195

Open
bernhard-herzog opened this issue May 4, 2020 · 5 comments
Open

Handling intelmqctl timeouts #195

bernhard-herzog opened this issue May 4, 2020 · 5 comments

Comments

@bernhard-herzog
Copy link
Contributor

The PHP backend tries to make sure that the intelmqctl calls do not
take too long by implementing some kind of timeout. It tries two
mechanisms, which both fail to work AFAICT.

One is using PHP's set_time_limit function. Unfortunately, time spent
executing subprocesses is not included in that timeout. The Note section
in https://www.php.net/manual/en/function.set-time-limit.php says:

Any time spent on activity that happens outside the execution of the
script such as system calls using system(), [...] is not included when
determining the maximum time that the script has been running.

The other method is some shell code that tries to kill the subprocess
after 20s using kill. This doesn't work in the usual setup where the
web-server code uses sudo to run intelmqctl as the intelmq user
because the kill fails due to insufficient permissions (kill: Operation not permitted).
It would probably work when intelmqctl is run without sudo.

All this does make me wonder why the fix for #164 works, because AFAICT
it shouldn't work, and in my tests (making intelmqctl artificially slow
with a sleep) set_time_limit indeed doesn't cause a timeout. The kill
in the shell code doesn't stop the process and doesn't make PHP send the
response faster, but its error message makes the PHP code produce a 500
response.

I noticed this while working on ther Python-API (#80). The obvious way
to implement a subprocess timeout in Python is the timeout parameter
of the subprocess module. This doesn't work for exactly the same reason
the shell approach in the PHP code doesn't work: The Python code is not
permitted to send signals to a subprocess that uses sudo.

Before I put more time into coming up with ways to implement timeouts
anyway, it's probably best to decide how important this timeout feature
actually is. Given that it doesn't work in the PHP implementation, and
doesn't seem to be a problem in practice, it seems to be acceptable for
now at least not to drop it.

@bernhard-herzog
Copy link
Contributor Author

In #80 (comment) @wagner-certat asks:

Could the timeout maybe easier be implemented in intelmqctl itself?

Yes, that would be a solution.

Reading more about sudo and signals just now, it turns out that sudo
does forward some signals to the process it runs. Maybe this can be done
with signals after all. I'll look into this a bit more.

@ghost
Copy link

ghost commented May 7, 2020

IMHO Please don’t waste time if we can solve that significantly easier in other components at a later point time

@bernhard-herzog
Copy link
Contributor Author

Reading more about sudo and signals just now, it turns out that sudo
does forward some signals to the process it runs. Maybe this can be
done with signals after all. I'll look into this a bit more.

Well, that doesn't work. According to the manpage sudo does forward
some of the signals it receives. For instance it would forward SIGTERM
but not SIGKILL (SIGKILL cannot be caught so it cannot be forwarded).
However, it can only forward signals it receives and the user www-data
cannot send signals to a process running as root, so of course sudo
cannot receive any of those signals.

@ghost
Copy link

ghost commented Dec 7, 2020

is that still relevant in intelmq-api @schacht-certat ?

@ghost
Copy link

ghost commented Dec 9, 2020

is that still relevant in intelmq-api @schacht-certat ?

Yes, if used with sudo it is still a problem. Though splitting out the API makes it probably easier to use intelmqctl without sudo, which means setting a timeout would work.

@ghost ghost changed the title Handling intemqctl timeouts Handling intelmqctl timeouts Jun 7, 2021
@ghost ghost added the help wanted label Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant