Closed killermosi closed 7 years ago
Thanks for the feedback! About the security: Although much of your comment makes sense (And I don't disagree one bit) An admin panel should NEVER be public facing EVER. (Which is why I am just running apache as a user with sudo access.) About page.php: I put them in functions because I planned on doing more with pages like better auth/token checking, brute force protection, etc. About the $_GET['filename']: Although I should be checking (I think I am after by replacing ".." with "" later in the code before processing) please see point 1 about internet facing admin panels. As for the scripts: this was meant to be as simple to install as possible (Could probably be even easier tbh). I have a set of scripts that need to run as root in order to modify, or get info that is needed from the system.
You do however make a good point about security. I know that apache should not have sudo rights. For now I will put a warning up on the instructions page to let people know that apache should not be internet facing. I do however apprieciate that you will not trust the code. Especially after you see all the "security" issues in it. I understand that. However, given that a RPi is not usually internet facing (Is yours really internet facing?), and the fact that I have been using mine with this code for months, and using mine to dev this code, with no issues should help calm your mind about the apache issue.
Please let me know if there is anything else I can do for you. You can contact me on my website if you have any other comments for concerns: https://urgero.org and again thank you so much for commenting about your concerns.
Thank you for taking the time to read the wall of text instead of simply saying "don't use it if you don't like it". I appreciate it.
My concern stems from my own Pi setup: I have several public-accessible applications on it, one being the web server, which serves a photo gallery, among other things. Switching Apache to a sudo
able user is a big no for me.
True, an administration panel should not be publicly-accessible, especially for a private system, but while I can restrict access to the administration panel from the local network only, I need to keep Apache accessible from outside, and this opens the door for bugs in the other applications that I have installed.
Also, you are assuming that the user pi
is available. Again, while this is most likely true for a Pi accessible only locally, I have deleted mine right after installing Raspbian on it, a long time ago.
Yea - Any user with ALL=(ALL) NOPASSWD:ALL
in the sudoers file will work (pi is used as an example and ease of setup as it will most likely exist as you said)
However - Any webserver that can run PHP and be run as a user with sudo nopasswd access will work (Like nginx as an example.) and you can run both nginx and apache side by side to keep apache on the public interface without a sudo user. Like I said before I understand the security issues. Eventually I will be able to fix them. :)
I skimmed the code a bit before going to the installation instructions page, and I stopped at the second step numbered
1
:Once that is done, run "sudo nano /etc/apache2/apache2.conf" Edit the User and Group to the user/group pi
Here are my observations:
sudo
able user? I understand that you need to get a hold of some root-only files from/etc
, but there is a good reason that web server runs sand-boxed by default: bugs in the applicationsroot
only commands directly from a public-facing PHP script - like making some scripts that run as superuser and that give you back only what info you need (not everything) / or execute specific commands, ensuring that even if your application has a bug, the risk of a malicious person wreaking havoc in the system is decreased.index.pxp
(the only PHP file that needs to be accessible to the public), and all the media/javascript needed by the application to properly function.session_start(); if(!isset($_SESSION['username'])) die("You must be logged in to view this page!");
lines and vastly improve the security of your application.Some minor observations:
$file = '/home/'.$iuser.'/ovpns/'.$_GET['filename'];
Imagine that I setfilename
to somethig like../../../etc/passwd
. Always make sure that the resource is supposed to be accessible.page.php
you have a hugeswitch
statement, each calling a small function that does only one thing: it includes a file. You can include the file directly in theswitch
, or better yet, define a mapping of'pageName' => 'file/to/include'
and do a simpleif (isset($map[$_POST['page']])) include $map[$_POST['page']] else echo '404';
Those are things that have jumped at me. I'm pretty sure that there are a lot of other improvements that can be done.
If I may suggest, use OOP and a small framework for your application - I would recommend CodeIgniter for this. I'm pretty sure this is the framework best suited because I have created at some point a small bare bones framework containing a basic router and MVC structure as a learning example for a friend, example which was inspired from my experience with CI.
Bottom line is that after looking at your application, as it stands now, I would not trust the security of my Pi to it. Apologies if this sounds harsh (I know that you put a lot of time and love into it), but please look at the issues I have pointed out and try to fix them.
If I can help with further information, please ask. I'll be happy to help.