-
Notifications
You must be signed in to change notification settings - Fork 224
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
Dev branch #354
base: dev-branch
Are you sure you want to change the base?
Dev branch #354
Conversation
Just my notes: functions.sh
** Changed almost every instance of $ipaddress to $hostname (where applicable) index.php
installfog.sh
ldap.class.php
processlogin.class.php
|
packages/web/index.php
Outdated
@@ -19,5 +19,5 @@ | |||
* @license http://opensource.org/licenses/gpl-3.0 GPLv3 | |||
* @link https://fogproject.org | |||
*/ | |||
header('Location: ./management/index.php'); | |||
header('Location: /fog/management/index.php'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to change this to go through /fog/ that will cause redirects to do:
http://ipoffogserver/fog/fog/fog/fog/fog/fog/management/index.php
You should be able to maintain it with ./management/index.php unless there's a specific reason for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be valid maintaining with ./management/index.php as by the time you're accessing this, you would already be in /fog/index.php so the redirect would just append ./management/index.php which would leave you in /fog/management/index.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, okay. Yeah, the only reason I did it was I made a change in functions.sh to copy index.php to the root of httpd so I could just go to http://fog.domain.com/ and it would redirect to /fog/management/index.php and not the Apache default splash screen. The laziest was to change the index.php in /var/www/html/fog directory and then cp it to /var/www/html/.
I just tried to go to http://fog.domain.com/fog and it redirected properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Mind changing that back as well?
Also, wanted to add I am working on getting OCSP configuration in order. Doing some testing and will be adding in the code tomorrow, hopefully. |
Okay, I got the OCSP configuration sorted out and uploaded. There are a few caveats for more secure environments.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Thanks again for working on this and sending in your changes! There are a couple of things we will consider adding to the official code base. Though we can't just merge it in as is because it would break thinks for some users I think.
I'd prefer to just pull in some of your changes but also drop parts of the pull request. Would you be fine with that or should we rather discuss all the things to the point where we agree to merge it into the official code base as is?
@@ -630,7 +709,7 @@ while [[ -z $blGo ]]; do | |||
echo | |||
echo " This can be done by opening a web browser and going to:" | |||
echo | |||
echo " ${httpproto}://${ipaddress}${webroot}management" | |||
echo " ${httpproto}://${hostname}${webroot}management" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 You probably changed to hostname because you use a proper SSL certificate with hostname in it. I think we'll stick to IP address for now because we cannot rely on people having a proper DNS configuration for their server. We'd probably need to ask during setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! That's my bad. I'm just so used to having DNS as being authoritative, I made those changes for my own sanity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we provide both instances:
This can be done by opening a browser to either on or the other......
If you built your own certificate you can test this by using the hostname.
${httpproto}://${ipaddress}${webroot}management
${httpproto}://${hostname}${webroot}management
@@ -1054,7 +1097,7 @@ configureMySql() { | |||
[[ -n $snmysqlpass ]] && options=( "${options[@]}" "--password=$snmysqlpass" ) | |||
sqlescsnmysqlpass=$(echo "$snmysqlpass" | sed -e s/\'/\'\'/g) # Replace every ' with '' for full MySQL escaping | |||
sql="UPDATE mysql.user SET plugin='mysql_native_password' WHERE User='root';" | |||
mysql "${options}" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1 | |||
mysql -s --host="$snmysqlhost" --user="$snmysqluser" --password="$snmysqlpass" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 I have done a fair amount of work on the mysql stuff. This will be merged into the code soon. I am not sure why you changed it from using the $options array to single parameters?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason the $options variable didn't build properly and I was getting errors stating the mysql code was attempting to log in as root with no password. I did add a password to mysql and I added it to .fogsettings, but $options never built up properly even though the variables that constitute $options were passing through correctly. Very strange...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 This issue is known and as mentioned I did a complete rewrite of the database stuff in branch db-security
- will merge that into dev-branch
in the next days. This should fix the problem altogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 May I ask you to revert all those mysql changes? As mentioned I have pushed a big change for this part of the code already.
lib/common/functions.sh
Outdated
mkdir -p $webdirdest/management/other/ssl >>$workingdir/error_logs/fog_error_${version}.log 2>&1 | ||
cp -f "$serverCert" $webdirdest/management/other/ssl/srvpublic.crt >>$workingdir/error_logs/fog_error_${version}.log 2>&1 | ||
cp -f "$serverKey" $sslpath/.srvprivate.key >>$workingdir/error_logs/fog_error_${version}.log 2>&1 | ||
cp -f "$externalCA" /etc/pki/ca-trust/source/anchors/chain.pem >>$workingdir/error_logs/fog_error_${version}.log 2>&1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 The path /etc/pki/ca-trust/...
is only valid fog RedHat based systems. We'll definitely make this more general for Debian/Ubuntu and ARCH Linux. Why did you decide to copy your CA to the system CA folder? Wouldn't it be easier to copy to /opt/fog/...
because that path is the same for all Linux systems that use FOG. But possibly there are other programs that look for the CA in the system folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put in the /etc/pki... folder so when update-ca-trust ran (which I added later in the code), it would add the custom certificate authorities to the complete listing of CAs on the machine and the machine certificate would be trusted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll work on getting this more generalized for Debian to start with. I'm working on generalizing the firewall rules first and then I will move on to this. Anyone, is there anything else I've added that is specifically RedHat that needs to be generalized? I didn't see anything, but want to make sure.
I'll move on to ARCH once I test Debian.
lib/common/functions.sh
Outdated
echo " SSLCertificateKeyFile $sslprivkey" >> "$etcconf" | ||
echo " SSLCertificateChainFile $webdirdest/management/other/ca.cert.der" >> "$etcconf" | ||
if [ ! -z "$externalCA" ]; then | ||
echo " SSLCACertificateFile /etc/pki/ca-trust/source/anchors/chain.pem" >> "$etcconf" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 I am not exactly sure but I think the options SSLCACertificateFile
and SSLCertificateChainFile
are specific to Apache version. The later might even be wrong in the context we use it. See here: https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile
SSLCertificateChainFile became obsolete with version 2.4.8, when SSLCertificateFile was extended to also load intermediate CA certificates from the server certificate file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SSLCertificateChainFile is the default code you guys put in, so I just reused it if the user does not input an external CA certificate chain. Is that not right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 This part of the code is likely old (from pre 2.4.x Apache days). So my guess is this was put in and not touched since then as it didn't cause any harm. Apache seems to work fine even without this option being correct.
lib/common/functions.sh
Outdated
@@ -1906,6 +1975,7 @@ configureHttpd() { | |||
esac | |||
fi | |||
dots "Setting up Apache and PHP files" | |||
echo "ServerName $hostname" | sudo tee -a /etc/httpd/conf/httpd.conf >> /dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 If you run the installer more than once (either because of an error or on upgrades) it will append that same line over and over again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! I'll need to do a check on that line's existence and then react to that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I think I fixed it. I uploaded a new version of functions.sh
$this->_username = $uname; | ||
$this->_password = $upass; | ||
global $currentUser; | ||
$user = $this->hasValidCert(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 I think there is a cleaner way of doing an auto login and not show the login page at all. I will let you know when I have a good solution to this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10-4, sounds good. Thanks!
Also, just making sure you can see my first post? I can't tell since I'm so new to Github... It's where I put an overview of changes and a general reasoning as to why I made the changes in some locations.
. 'required="" id="upass"/>'; | ||
//echo '<input type="password" class="form-control" name="upass" ' | ||
// . 'required="" id="upass"/>'; | ||
echo '<input type="password" class="form-control" name="upass" id="upass"/>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about handling this with a little javascript and maybe a "login method" drop down?
For now there'd only be 2 dropdowns (username/password, certificate)
If certificate is selected, the password field becomes hidden using the javascript, if username/password is selected the field becomes visible/required.
Just thoughts. Using the drop down method would also allow us to add other authentication mechanisms in the future as well.
lib/common/functions.sh
Outdated
@@ -750,6 +774,25 @@ checkSELinux() { | |||
esac | |||
done | |||
} | |||
rulesFirewall() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is very Redhat centric right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Haha I'll work on the Debian-based stuff soon. I don't have much experience with Debian, but I'll give it a shot.
lib/common/functions.sh
Outdated
@@ -1906,6 +1975,9 @@ configureHttpd() { | |||
esac | |||
fi | |||
dots "Setting up Apache and PHP files" | |||
if (! grep -Fxq "ServerName \"$hostname\"" /etc/httpd/conf/httpd.conf); then | |||
echo "ServerName \"$hostname\"" | sudo tee -a /etc/httpd/conf/httpd.conf >> /dev/null | |||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I have a variable to get the httpd conf, which is ${httpdconf}
This said, you're forcing the Servername to the main httpd.conf, why not do this for the fog configuration file we generate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I was seeing this error in Apache's log for a while and then it just stopped over a week ago. I think it's because I flipped the ServerName and ServerAlias when fog.conf is written. The original has the IP address as the ServerName and the hostname as the ServerAlias. I removed this line and reuploaded functions.sh
Scrapped the firewall rules. I can't figure out how to properly use iptables and it seems like it's very difficult to get it to work properly with NFS. Firewall-cmd doesn't work on the latest version of iptables on Debian, so I just said forget it for now. I did add a few checks for Debian and tried to tailor the functions to that OS. It seems Arch and Red Hat share common functions for everything used by FOG, so I lumped them together. Not sure if that is entirely accurate, so please let me know if I messed anything up. Removed the check for server certificate and the wget calls since I couldn't get that to work properly either right now. |
lib/common/functions.sh
Outdated
@@ -36,7 +36,7 @@ registerStorageNode() { | |||
if [[ $storageNodeExists != exists ]]; then | |||
[[ -z $maxClients ]] && maxClients=10 | |||
dots "Node being registered" | |||
wget --no-check-certificate -qO - $httpproto://$ipaddress/${webroot}/maintenance/create_update_node.php --post-data="newNode&name=$(echo -n $ipaddress| base64)&path=$(echo -n $storageLocation|base64)&ftppath=$(echo -n $storageLocation|base64)&snapinpath=$(echo -n $snapindir|base64)&sslpath=$(echo -n $sslpath|base64)&ip=$(echo -n $ipaddress|base64)&maxClients=$(echo -n $maxClients|base64)&user=$(echo -n $username|base64)&pass=$(echo -n $password|base64)&interface=$(echo -n $interface|base64)&bandwidth=$(echo -n $interface|base64)&webroot=$(echo -n $webroot|base64)&fogverified" | |||
wget --no-check-certificate -qO - $httpproto://$ipaddress/${webroot}/maintenance/create_update_node.php --post-data="newNode&name=$(echo -n $ipaddress| base64)&path=$(echo -n $storageLocation|base64)&ftppath=$(echo -n $storageLocation|base64)&snapinpath=$(echo -n $snapindir|base64)&sslpath=$(echo -n $sslpath|base64)&ip=$(echo -n $ipaddress|base64)&maxClients=$(echo -n $maxClients|base64)&user=$(echo -n $username|base64)&pass=$(echo -n $password|base64)&interface=$(echo -n $interface|base64)&bandwidth=$(echo -n $interface|base64)&webroot=$(echo -n $webroot|base64)&fogverified" | |||
echo "Done" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Please correct indent here and on the following wget call lines (spaces at the beginning of the line).
lib/common/functions.sh
Outdated
@@ -46,13 +46,14 @@ updateStorageNodeCredentials() { | |||
[[ -z $webroot ]] && webroot="/" | |||
dots "Ensuring node username and passwords match" | |||
wget --no-check-certificate -qO - $httpproto://$ipaddress${webroot}maintenance/create_update_node.php --post-data="nodePass&ip=$(echo -n $ipaddress|base64)&user=$(echo -n $username|base64)&pass=$(echo -n $password|base64)&fogverified" | |||
fi | |||
echo "Done" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 This if
statement might cause a script error I reckon.
lib/common/functions.sh
Outdated
@@ -417,7 +418,7 @@ configureFTP() { | |||
} | |||
configureDefaultiPXEfile() { | |||
[[ -z $webroot ]] && webroot='/' | |||
echo -e "#!ipxe\ncpuid --ext 29 && set arch x86_64 || set arch \${buildarch}\nparams\nparam mac0 \${net0/mac}\nparam arch \${arch}\nparam platform \${platform}\nparam product \${product}\nparam manufacturer \${product}\nparam ipxever \${version}\nparam filename \${filename}\nparam sysuuid \${uuid}\nisset \${net1/mac} && param mac1 \${net1/mac} || goto bootme\nisset \${net2/mac} && param mac2 \${net2/mac} || goto bootme\n:bootme\nchain ${httpproto}://$ipaddress${webroot}service/ipxe/boot.php##params" > "$tftpdirdst/default.ipxe" | |||
echo -e "#!ipxe\ncpuid --ext 29 && set arch x86_64 || set arch \${buildarch}\nparams\nparam mac0 \${net0/mac}\nparam arch \${arch}\nparam platform \${platform}\nparam product \${product}\nparam manufacturer \${product}\nparam ipxever \${version}\nparam filename \${filename}\nparam sysuuid \${uuid}\nisset \${net1/mac} && param mac1 \${net1/mac} || goto bootme\nisset \${net2/mac} && param mac2 \${net2/mac} || goto bootme\n:bootme\nchain http://${hostname}/fog/service/ipxe/boot.php##params" > "$tftpdirdst/default.ipxe" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Please switch back to use ${httpproto}
variable.
@@ -1054,7 +1097,7 @@ configureMySql() { | |||
[[ -n $snmysqlpass ]] && options=( "${options[@]}" "--password=$snmysqlpass" ) | |||
sqlescsnmysqlpass=$(echo "$snmysqlpass" | sed -e s/\'/\'\'/g) # Replace every ' with '' for full MySQL escaping | |||
sql="UPDATE mysql.user SET plugin='mysql_native_password' WHERE User='root';" | |||
mysql "${options}" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1 | |||
mysql -s --host="$snmysqlhost" --user="$snmysqluser" --password="$snmysqlpass" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 May I ask you to revert all those mysql changes? As mentioned I have pushed a big change for this part of the code already.
@@ -1728,19 +1749,37 @@ EOF | |||
echo " SetHandler \"proxy:fcgi://127.0.0.1:9000/\"" >> "$etcconf" | |||
fi | |||
echo " </FilesMatch>" >> "$etcconf" | |||
echo " ServerName $ipaddress" >> "$etcconf" | |||
echo " ServerAlias $hostname" >> "$etcconf" | |||
echo " ServerName $hostname" >> "$etcconf" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Please switch back name and alias as most FOG users still use IP address.
cp -f $webdirsrc/index.php /var/www/ | ||
else | ||
cp -f $webdirsrc/index.php /var/www/html/ | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Why was this added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when I tested with Debian, Apache's root directory is different from Red Hat's.
lib/common/functions.sh
Outdated
@@ -2094,7 +2143,7 @@ class Config | |||
*/ | |||
private static function _initSetting() | |||
{ | |||
define('TFTP_HOST', \"${ipaddress}\"); | |||
define('TFTP_HOST', \"${hostname}\"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Please change back to IP for the official code. A lot of our users have a hard time setting FOG up and I think it's too much for a lot of them to get DNS right on top of that.
packages/web/index.php
Outdated
@@ -19,5 +19,5 @@ | |||
* @license http://opensource.org/licenses/gpl-3.0 GPLv3 | |||
* @link https://fogproject.org | |||
*/ | |||
header('Location: ./management/index.php'); | |||
header('Location: /fog/management/index.php'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ty9000 Mind changing that back as well?
@Ty9000 Are you still active and want to get this merged into the code? |
c77f5f3
to
ed156a1
Compare
No description provided.