patternbashMinor
Automated owncloud installation script
Viewed 0 times
automatedinstallationscriptowncloud
Problem
Here is a shell script which installs and configure owncloud on a Debian server. It also installs apache2 and MySQL as dependencies and fail2ban with a specific rule for owncloud. Apache2 is also configured to use a self signed SSL certificate.
The script seems to work properly (everything is installed and works fine) my question is about the possible security issues that could be created:
The official Github repo of this file is here.
```
#!/bin/bash
#Some bash script optimization for robustness (More information: www.davidpashley.com/articles/writing-robust-shell-scripts/)
#Break if the script uses unset variables
set -o nounset
#Break if a command has a non-true return value
set -o errexit
echo_info () {
echo "--------------------------------------"
echo "| INFO: $1"
echo "--------------------------------------"
}
echo_error () {
echo "--------------------------------------"
echo "| ERROR: $1"
echo "--------------------------------------"
}
is_installed () {
#Check if a package is installed (More information: https://askubuntu.com/questions/319307/reliably-check-if-a-package-is-installed-or-not)
if dpkg --get-selections | grep -q "^$1[[:space:]]*install$" >/dev/null; then
echo_error "$1 is already installed"
exit 1
fi
}
check_not_installed () {
echo_info "Check if some package is already installed. If this is the case the script stops because it could not be performed without risc."
is_installed apache2
is_installed mysql-server-5.5
is_installed owncloud
is_installed fail2ban
}
check_root () {
if [ "$(id -u)" != "0" ]; then
echo_error "The script must be called as root user!"
exit 1
fi
}
install_owncloud () {
cd /tmp
#Automatic installation of owncloud (More information: https://softwar
The script seems to work properly (everything is installed and works fine) my question is about the possible security issues that could be created:
- Is the ssl certificate correctly issued and installed?
- Is there a potential issue with the handling of mysql?
- Is there something else that I should see?
The official Github repo of this file is here.
```
#!/bin/bash
#Some bash script optimization for robustness (More information: www.davidpashley.com/articles/writing-robust-shell-scripts/)
#Break if the script uses unset variables
set -o nounset
#Break if a command has a non-true return value
set -o errexit
echo_info () {
echo "--------------------------------------"
echo "| INFO: $1"
echo "--------------------------------------"
}
echo_error () {
echo "--------------------------------------"
echo "| ERROR: $1"
echo "--------------------------------------"
}
is_installed () {
#Check if a package is installed (More information: https://askubuntu.com/questions/319307/reliably-check-if-a-package-is-installed-or-not)
if dpkg --get-selections | grep -q "^$1[[:space:]]*install$" >/dev/null; then
echo_error "$1 is already installed"
exit 1
fi
}
check_not_installed () {
echo_info "Check if some package is already installed. If this is the case the script stops because it could not be performed without risc."
is_installed apache2
is_installed mysql-server-5.5
is_installed owncloud
is_installed fail2ban
}
check_root () {
if [ "$(id -u)" != "0" ]; then
echo_error "The script must be called as root user!"
exit 1
fi
}
install_owncloud () {
cd /tmp
#Automatic installation of owncloud (More information: https://softwar
Solution
Naming
Some of the function names could be improved, for example this one:
I expect a function named
so that I can build conditional statements with it.
But this is not that kind of function,
it validates something or else exits the program.
I'd name this
Other minor issues
You don't need to redirect the output of
Every time you
So you could move the
and then probably rename it (I usually call it
The base URL is common in both of these commands,
so I would put it in a variable to make it easy to change later if needed.
Backticks are archaic, use
This is a tedious and fragile way to construct a longer string.
You could pass a here-document directly to
This overwrites the
You can perform multiple replacements in a single
and overwrite the file only once:
This extremely long line is unreadable.
Consider using a here-document instead:
Some of the function names could be improved, for example this one:
is_installed () {
#Check if a package is installed (More information: https://askubuntu.com/questions/319307/reliably-check-if-a-package-is-installed-or-not)
if dpkg --get-selections | grep -q "^$1[[:space:]]*install$" >/dev/null; then
echo_error "$1 is already installed"
exit 1
fi
}I expect a function named
is_X to behave like a boolean,so that I can build conditional statements with it.
But this is not that kind of function,
it validates something or else exits the program.
I'd name this
verify_not_installed or similar.Other minor issues
if dpkg --get-selections | grep -q "^$1[[:space:]]*install$" >/dev/null; thenYou don't need to redirect the output of
grep -q, the -q flag already takes care of that.echo_error "$1 is already installed"
exit 1Every time you
echo_error, you also exit 1.So you could move the
exit 1 inside the function,and then probably rename it (I usually call it
fatal).echo 'deb http://download.owncloud.org/download/repositories/8.2/Debian_8.0/ /' >> /etc/apt/sources.list.d/owncloud.list
wget -nv https://download.owncloud.org/download/repositories/8.2/Debian_8.0/Release.key -O Release.keyThe base URL is common in both of these commands,
so I would put it in a variable to make it easy to change later if needed.
mysql=`which mysql`Backticks are archaic, use
$(...) instead.Q1="CREATE DATABASE IF NOT EXISTS $ocDb;"
Q2="GRANT USAGE ON *.* TO $ocDbUser@localhost IDENTIFIED BY '$ocDbUserPw';"
Q3="GRANT ALL PRIVILEGES ON $ocDb.* TO $ocDbUser@localhost;"
Q4="FLUSH PRIVILEGES;"
SQL="${Q1}${Q2}${Q3}${Q4}"This is a tedious and fragile way to construct a longer string.
You could pass a here-document directly to
mysql:mysql -uroot -p$mysqlRootPw <<EOF
CREATE DATABASE IF NOT EXISTS $ocDb;
GRANT USAGE ON *.* TO $ocDbUser@localhost IDENTIFIED BY '$ocDbUserPw';
...
EOFsed -i "s/php_value upload_max_filesize .*/php_value upload_max_filesize $maxFileSize/" /var/www/owncloud/.htaccess
sed -i "s/php_value post_max_size .*/php_value post_max_size $maxFileSize/" /var/www/owncloud/.htaccess
sed -i "s/php_value memory_limit .*/php_value memory_limit $maxFileSize/" /var/www/owncloud/.htaccessThis overwrites the
/var/www/owncloud/.htaccess three times. You can perform multiple replacements in a single
sed call,and overwrite the file only once:
sed -i \
-e "s/php_value upload_max_filesize .*/php_value upload_max_filesize $maxFileSize/" \
-e "s/php_value post_max_size .*/php_value post_max_size $maxFileSize/" \
-e "s/php_value memory_limit .*/php_value memory_limit $maxFileSize/" \
/var/www/owncloud/.htaccessecho -e "[Definition]\nfailregex={\"app\":\"core\",\"message\":\"Login failed: user '.*' , wrong password, IP:\",\"level\":2,\"time\":\".*\"}\n {\"app\":\"core\",\"message\":\"Login failed: '.*' \(Remote IP: '', X-Forwarded-For: '.*'\)\",\"level\":2,\"time\":\".*\"}\n {\"reqId\":\".*\",\"remoteAddr\":\"\",\"app\":\"core\",\"message\":\"Login failed: .*\",\"level\":2,\"time\":\".*\"}" > /etc/fail2ban/filter.d/owncloud.confThis extremely long line is unreadable.
Consider using a here-document instead:
cat /etc/fail2ban/filter.d/owncloud.conf
[Definition]
failregex={"app":"core","message":"Login failed: user '.*' , wrong password, IP:","level":2,"time":".*"}
{"app":"core","message":"Login failed: '.*' \(Remote IP: '', X-Forwarded-For: '.*'\)","level":2,"time":".*"}
{"reqId":".*","remoteAddr":"","app":"core","message":"Login failed: .*","level":2,"time":".*"}
EOFCode Snippets
is_installed () {
#Check if a package is installed (More information: https://askubuntu.com/questions/319307/reliably-check-if-a-package-is-installed-or-not)
if dpkg --get-selections | grep -q "^$1[[:space:]]*install$" >/dev/null; then
echo_error "$1 is already installed"
exit 1
fi
}if dpkg --get-selections | grep -q "^$1[[:space:]]*install$" >/dev/null; thenecho_error "$1 is already installed"
exit 1echo 'deb http://download.owncloud.org/download/repositories/8.2/Debian_8.0/ /' >> /etc/apt/sources.list.d/owncloud.list
wget -nv https://download.owncloud.org/download/repositories/8.2/Debian_8.0/Release.key -O Release.keymysql=`which mysql`Context
StackExchange Code Review Q#117956, answer score: 2
Revisions (0)
No revisions yet.