asrob-uc3m / robotDevastation

A new-generation shooter with augmented reality and real robots. You can play online with other users with your PC, moving robots in championships and campaigns: all 24/7!
http://asrob-uc3m.github.io/workgroups/2017-05-28-robot-devastation.html
GNU Lesser General Public License v2.1
9 stars 4 forks source link

think about remote shutdown #146

Open jgvictores opened 6 years ago

jgvictores commented 6 years ago

think about shutdown ...but also about privileges: http://how-to.wikia.com/wiki/How_to_allow_non-super_users_to_shutdown_computer_in_Linux ...and avoiding system calls: https://stackoverflow.com/questions/28812514/how-to-shutdown-linux-using-c-or-qt-without-call-to-system

PeterBowman commented 3 years ago

Is it actually desirable to shutdown a PC via API calls? You can easily craft a shell command for that matter (via remote SSH commands), but why would you like to do that from C++ code?

jgvictores commented 3 years ago
PeterBowman commented 3 years ago

Why a remote shutdown? Enabling an RD PC (graphical) client or similar to command a clean shut down to a remote robot seems reasonable; as the idea was always to avoid making the user have to do anything terminal-related.

Yes, I agree, I'm not saying the user should not be able to do that.

Why avoiding system calls? System calls are usually not the best solution (and in this case, would additionally require permissions). In Ubuntu there are many OS-level behaviors that can be solved directly by sending commands to the D-Bus, which seemed kind of elegant. However, I believe that even writing to the D-Bus would require permissions too. :-(

The C way of doing this according to the link you posted earlier also requires permissions. I don't think that exposing a system call (either explicitly via system or masked through low-level C APIs) in a high-level robot motion control API is elegant. Also think of the implementors of said interface. Are we going to implement such calls on any of these? It makes no sense to me.

jgvictores commented 3 years ago

I agree. IRobotManager was originally intended to be catch-all and ambitious. In practice, devices have implemented the movement oriented methods. It now seems like the naming should have been the opposite: in the current state, the base class is catch-all, and the implementations are limited; we could rename the base class to something movement specific (and in the future rename the implementations if they ever become more broad).

In any case: 1) Renaming is low priority as long as we are aware of the scope of the IRobotManager interface. This is, a shutdown is out of the current scope of the class. 2) Exposing a shutdown through an API that maps to a relatively unprotected port seems kind of lame from a cyber-security perspective, where ssh-ing would be better.

PeterBowman commented 3 years ago

Exposing a shutdown through an API that maps to a relatively unprotected port seems kind of lame from a cyber-security perspective, where ssh-ing would be better.

Agreed. SSH-ing here doesn't necessarily mean to log-in and start a remote shell session, though.

SSH calls aside, one could use yarprun for such remote commands.