OpenGamePanel / OGP-Agent-Linux

GNU General Public License v2.0
85 stars 33 forks source link

executable file isn't locked when manual update option used #15

Closed TAPL-93 closed 7 years ago

TAPL-93 commented 7 years ago

The executable file isn't locked if the option "Install/Update manually" was used.

rocco27 commented 7 years ago

I don't know what you are doing or how your root configurated and works, but on my Linux dedicated server I have no and never had any unlocked binary files even if I use Steam Install/Update manually function.

Can you post some screenshots and share details so we can check and start to think of it...

Zorrototo commented 7 years ago

Manual update is not the same as Steam update. Look at games like minecraft I think they offer the manual update function, where you can put URL of zip file to decompress to the server home folder.

Maybe because it is MANUAL install, it requires MANUAL locking of the file though. It is maybe intended to work like that. Anyway, if you start the server it should auto lock the executable file I think.

TAPL-93 commented 7 years ago

The settings of WHMCS module have four installation types which are steam, rsync, manual and master. A field for URL available as well.

if you set the installation type as manual this obviously would be a security risk as any server created with this installation type will have the executable file unlocked.

Zorrototo commented 7 years ago

That's right, but the WHMCS mod is a mod for OGP it is not officially part of OGP base Panel.

With that being said, maybe the manual update function can still be updated to take your issue into consideration. Wait for devs to come here and see what they think about the issue.

own3mall commented 7 years ago

It should be locked when the server starts, so how is this a security risk?

TAPL-93 commented 7 years ago

Sure, it will be locked when the server starts, BUT the user who have the server assigned to him also have access to the files of the server and can easily modify the executable file BEFORE starting the server, so it still a security risk.

rocco27 commented 7 years ago

Oh my mistake, I thought he talks about Steam Installation, sorry ^^.

Then the name says everything, if you enable manual installation, then users can install anything what they want. After first start OGP will lock the startup/binary file, but it could be a possible way to lock the exec file when the manual update finished, so there is no risky period beetwen the manual installation and the first startup.

own3mall commented 7 years ago

I'm not going to modify the remote method signature being called for manual downloads, but the API should lock the files now if your agent, WHMCS mod, and game panel are up-to-date. Let me know if that worked... you get to test it.

TAPL-93 commented 7 years ago

The file now locked if the option "Install/Update manually" was used inside open game panel, however it still unlocked if the server was created using WHMCS.

own3mall commented 7 years ago

And you updated the agent and WHMCS mod?

TAPL-93 commented 7 years ago

Yes, i did.

own3mall commented 7 years ago

Stop the ogp_agent service:

sudo service ogp_agent stop

Disable auto update on the Linux agent in the OGP agent install directory Cfg/bash_prefs.cfg:

agent_auto_update=0

Try using this version of ogp_agent.pl. If it doesn't work, please post your ogp_agent.log file.

ogp_agent.pl.zip

After replacing the ogp_agent.pl file with the one in the zip file above, start the agent:

sudo service ogp_agent start

Test it again, and let me know.

TAPL-93 commented 7 years ago

Mon Oct 23 08:33:39 2017 Reading startup flags from /usr/share/ogp_agent/startups Mon Oct 23 08:33:39 2017 Open Game Panel - Agent started - 2843799f668105ee0b989a5c189c905197cadeda - port 12679 - PID 16076 Mon Oct 23 08:33:40 2017 Error reading tasks file No such file or directory Mon Oct 23 08:34:38 2017 Starting to download URL http://******/MTA1.5.5.zip. Destination: /home/ogp_agent/OGP_User_Files/whmcs/5 - Filename: MTA1.5.5.zip Mon Oct 23 08:34:38 2017 Creating destination directory. Mon Oct 23 08:34:38 2017 Running postscript commands. Mon Oct 23 08:34:38 2017 Postscript command received "". Mon Oct 23 08:34:38 2017 Postscript command received "chattr +i /home/ogp_agent/OGP_User_Files/whmcs/5/mta-server". Mon Oct 23 08:34:39 2017 Successfully fetched http://******/MTA1.5.5.zip and stored it to /home/ogp_agent/OGP_User_Files/whmcs/5/MTA1.5.5.zip. Retval: 200 OK Mon Oct 23 08:34:39 2017 Starting file uncompress as ordered. Mon Oct 23 08:34:39 2017 Uncompression called for file /home/ogp_agent/OGP_User_Files/whmcs/5/MTA1.5.5.zip to dir /home/ogp_agent/OGP_User_Files/whmcs/5. Mon Oct 23 08:34:40 2017 File uncompressed/extracted successfully. Mon Oct 23 08:34:40 2017 Running postscript commands. Mon Oct 23 08:34:40 2017 Postscript command received "". Mon Oct 23 08:34:40 2017 Postscript command received "chattr +i /home/ogp_agent/OGP_User_Files/whmcs/5/mta-server". Mon Oct 23 08:34:41 2017 Download process for /home/ogp_agent/OGP_User_Files/whmcs/5/MTA1.5.5.zip has pid number 16092.

own3mall commented 7 years ago

So it looks like it is working just fine?

/home/ogp_agent/OGP_User_Files/whmcs/5/mta-server is the server binary is it not? chattr +i should be locking it.

TAPL-93 commented 7 years ago

Yes, mta-server is the binary but it still unlocked. I'm no expert here but my guess chattr +i was used before the download/uncompress completed, so some check for this would be necessary.

own3mall commented 7 years ago

Except your log says:

Mon Oct 23 08:34:40 2017 File uncompressed/extracted successfully.
Mon Oct 23 08:34:40 2017 Running postscript commands.
Mon Oct 23 08:34:40 2017 Postscript command received "".
Mon Oct 23 08:34:40 2017 Postscript command received "chattr +i /home/ogp_agent/OGP_User_Files/whmcs/5/mta-server".

So it supposedly ran it after it was extracted. Why it shows up as running the post commands twice though, I'm not sure. I'll do some more digging.

own3mall commented 7 years ago

Ok, try this ogp_agent.pl file. I would expect it to work. I guess I better understand how fork() works in perl now... (unless it still doesn't work :P)

ogp_agent.pl.zip

TAPL-93 commented 7 years ago

I modified the first ogp_agent.pl file you uploaded, i added sleep 10 above if ($post_script ne ""), i also added "1" and "2" to logger so you know where those twice commands running from in the log, but you already figured it out as it appeared something with fork().

https://pastebin.com/jw5XaWST https://pastebin.com/hTLZaWUp

The binary file now locked with sleep 10 which confirm my guess about the completion of the download and uncompress.

Your last ogp_agent.pl doesn't lock the binary file with or without sleep 10 and it leaves postinstall.sh file inside the server directory.

own3mall commented 7 years ago

Sleep is not an acceptable solution. It should work. If the postinstall.sh file is still present, that means the script never ran...

I'll see if I can find out why.

own3mall commented 7 years ago

Try this one:

ogp_agent.pl_more_logging.zip

If it still doesn't work, add this to the logging below these lines:

sub sudo_exec_without_decrypt
{
    my ($sudo_exec) = @_;
    $sudo_exec =~ s/('+)/'"$1"'/g;
    logger "Running the following command \"" . $sudo_exec . "\" with sudo.";
    my $command = "echo '$SUDOPASSWD'|sudo -kS -p \"\" su -c '$sudo_exec;echo \$?' root 2>&1";

Add this line below the last line above:

logger "Running the following complete command \"" . $command . "\".";

Save and try it again. Paste contents of your log file but be sure to REMOVE YOUR SUDO PASSWORD from the log before posting the comment containing your logs.

TAPL-93 commented 7 years ago

It doesn't work. i did some testing and when i commented those lines it works, the binary file locked.

print FILE "while kill -0 $pid >/dev/null 2>&1\n"; print FILE "do\n"; print FILE " sleep 1\n"; print FILE "done\n";

https://prnt.sc/h16szp Not sure what the purpose of those lines, but pid already equal to zero elsif ($pid == 0)

own3mall commented 7 years ago

Yep, that will work. That needs to be removed since it now runs in the child process. I didn't have a good sample to test. Anyways, I'll make a commit soon. Thanks for your help.

TAPL-93 commented 7 years ago

Actually the removal of the codes related to "screen" affected the addons, when installing an addon the logs related to the addon doesn't shows, instead an error appears "Log file missing, started new log".

Also i noticed any file downloaded (with wget) via the addon post-install script will have the owner group root instead of ogp_agent.

own3mall commented 7 years ago

Only way to "lock" files is to use sudo. Guess we'll have to come up with a different solution.

TAPL-93 commented 7 years ago

Well, there's a possibilities that the way to lock the files here https://github.com/OpenGamePanel/OGP-Website/commit/9c6812dbfc1734bccfae4af47f4b554699a8fcfa

could be applied here https://github.com/OpenGamePanel/OGP-WHMCS/blob/master/ogp/api.php

own3mall commented 7 years ago

Try it now (update WHMCS and agent)

TAPL-93 commented 7 years ago

The file locked, but now the addon post-install script does not execute and the file postinstall.sh left inside the server directory.

The logs related to the addon doesn't shows and this line still visible "Log file missing, started new log", probably because the file postinstall.sh was never executed.

own3mall commented 7 years ago

I don't see how that could happen. Paste your log contents.

own3mall commented 7 years ago

Oh, if the script file already exists and has the wrong permissions on it from previous changes, just delete it and try again.

TAPL-93 commented 7 years ago

There was something incorrect with my post-install script (still really it shouldn't leave postinstall.sh inside the server directory).

Anyway, i looked at ogp_agent.pl code and i see the problem, pid here equal to zero.

elsif ($pid == 0) .. .. my $screen_id = create_screen_id("post_script", $pid);

own3mall commented 7 years ago

It won't leave it there. It should be deleted after it runs. And how is pid = 0 a problem? The code to run the post script is here for when PID is 0.

TAPL-93 commented 7 years ago

The file postinstall.sh will be deleted IF the contain of the post-install didn't lead to something that require user intervention.

Anyway let me explain to you why pid shouldn't be zero, i created an addon with the following Post-install script: echo "Test ............."

I tried to install the addon, the following show up "Log file missing, started new log". I checked /usr/share/ogp_agent/screenlogs and found two files: screenlog.OGP_post_script_000000000 It contain the following "Test ............." screenlog.OGP_post_script_000029859 It contain the following "Log file missing, started new log"

Clearly the pid should have been 29859, not 0.

own3mall commented 7 years ago

Post install shouldn't have anything that involves user interaction.

own3mall commented 7 years ago

Why do you need a log file for your addon? I guess we just have to move the post install script back to where it was then.

own3mall commented 7 years ago

but that still may not work in all instances

TAPL-93 commented 7 years ago

Check this out, everything seems to works.

https://pastebin.com/P87Z4s2P

own3mall commented 7 years ago

I don't like it because post commands should be run in the forked thread only after the download and extraction have finished. Else, it's not executing at the right time.

Please provide me with detailed instructions on how to setup and reproduce your issue so I can better handle this.

TAPL-93 commented 7 years ago

I guess we just have to move the post install script back to where it was then.

This exactly what i did, this is how it was before any modification related to this issue.

before: https://github.com/OpenGamePanel/OGP-Agent-Linux/blob/f2c3eb535dcffb7435228a218789d30c7fb54d61/ogp_agent.pl after: https://pastebin.com/P87Z4s2P

own3mall commented 7 years ago

From what I could tell, it wasn't executing appropriately there anyways. It was in the wrong order. Or at least, there was no guarantee it would execute when it was done unzipping. So, moving back to where it was isn't correct either.

I think I will adjust the logic to return PID 0 log depending on if it's using start_remote_download. Where it is now is truly the correct place.

own3mall commented 7 years ago

Update your agent and try again. Does it work as you'd expect now and in all cases?

TAPL-93 commented 7 years ago

Everything now works as expected.

own3mall commented 7 years ago

Great. Thanks for reporting!