5
\$\begingroup\$

As I am new to bash scripting, and want to use apt-get in my university. I know that many people have issues when trying to do so. My focus is simplicity and ease of use, but still need to be somewhat robust.

#!/bin/bash 
string="install"
errormsg="\n\tInvalid input\n You should run this script with the following structure:\n\n sudo apt-proxy-install.sh install USERNAME PASSWORD\n\n "
DIRECTORY=~/bin/
if [ "$1" = "$string" ] && { ! ([ -z "$3" ] || [ -z "$3" ]) }
   then 
         if [ ! -d "$DIRECTORY" ]; then
            mkdir ~/bin/
         fi
         printf "#!/bin/bash\nhttp_proxy=""http://$2:$3@10.20.10.50:3128"" sudo apt-get \${@:2}" > ~/bin/apt-proxy
         chmod 777 ~/bin/apt-proxy
         PATH=~/bin:$PATH
         addpath="export $PATH"
         sudo cat ~/.bashrc $addpath > ~/.bashrc
   else
         printf "$errormsg"
fi

My question is: is this code acceptable, or should I improve it? If the answer is not, then please give me some hints.

\$\endgroup\$
  • \$\begingroup\$ Assuming that apt-get should always use the HTTP proxy, configure /etc/apt/apt.conf with Acquire::http::Proxy "http://username:password@10.20.10.50:3128". You might consider the risk of storing a plaintext password to be acceptable if apt.conf is readable only by root. \$\endgroup\$ – 200_success Dec 9 '13 at 20:52
3
\$\begingroup\$

Storing passwords in text files is never a good idea, especially if you give that file world-readable permissions.

Try this:

#!/bin/bash 

mkdir -p ~/bin/

# add ~/bin to PATH if not already there
echo '[[ :"$PATH": == *:"$HOME/bin":* ]] || PATH="$HOME/bin:$PATH"' >> .bashrc

# create the apt-proxy script
cat <<'END_SCRIPT' > ~/bin/apt-proxy
#!/bin/bash

stty -echo
printf "Password for %s: " "$LOGNAME"
read password
stty echo
echo

export http_proxy="http://${LOGNAME}:${password}@10.20.10.50:3128"
sudo -S apt-get "$@" <<< "$password"
END_SCRIPT

chmod 755 ~/bin/apt-proxy

cat <<INSTRUCTIONS
The apt-proxy script has been installed. 
You may need to log out and log back in before you can use it.

usage: apt-proxy package ...
INSTRUCTIONS
  • mkdir -p dir will silently do nothing if the directory already exists
  • you only need to add the user's bin dir to the PATH if it is not already there
  • DO NOT store the user's password. Have the user enter it each time.
  • you should not even need to use sudo to edit files in your home directory.
  • 777 permissions is overly generous: the world does not have to be able to edit the file
  • use sudo -S and pass the user's password via stdin
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Additionally, you almost always want to quote "$@" to expand the command line arguments robustly. \$\endgroup\$ – glenn jackman Dec 9 '13 at 23:22
1
\$\begingroup\$

NO!

Storing plaintext passwords in a world-readable file is not acceptable.

Making an executable file world-writable is not acceptable.

Asking for a password to be entered as a command-line parameter, where it would likely end up in ~/.bash_history, is not acceptable. The password would also be temporarily be visible to all users via /bin/ps, which is also bad practice.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

This line has readability problems:

if [ "$1" = "$string" ] && { ! ([ -z "$3" ] || [ -z "$3" ]) }

You put $3 twice; I assume you meant $2 for one of them.

Since "$string" is just install, you might as well say install instead.

The compound conditional is hard to understand. Apply De Morgan's laws to obtain

if [ "$1" = install ] && ! [ -z "$2" ] && ! [ -z "$3" ]

… which is just

if [ "$1" = install ] && [ -n "$2" ] && [ -n "$3" ]

I would introduce explaining variables

command="$1"
username="$2"
password="$3"
if [ "$command" = install ] && [ -n "$username" ] && [ -n "$password" ]
| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

This line…

sudo cat ~/.bashrc $addpath > ~/.bashrc

… is actually extremely problematic.

You will almost certainly wipe out the existing contents of ~/.bashrc: the shell will likely truncate ~/.bashrc before it runs sudo, and sudo runs cat, and cat reads the file. See Unix Big Redirection Mistake #1. The way to append to a file is using echo "$blah" >> ~/.bashrc.

The $addpath variable is incorrectly defined as addpath="export $PATH". You would be trying to export a variable named ~/bin:/bin:/usr/bin because there is no PATH=… assignment. You meant addpath="export PATH=\"$PATH\"".

Furthermore, cat $addpath has two errors: it's the wrong command (you want echo), and you failed to double-quote its argument. Therefore, it will just try to read files named export and PATH=~/bin:… — if you're lucky. If you're unlucky, $PATH could expand to a string that contains a nasty shell command. That scenario is admittedly farfetched, but that kind of carelessness in quoting is how exploits get introduced. When writing shell scripts, double-quote every variable you use unless you have a good reason not to.

sudo is unnecessary for that operation; it could even backfire if the sudoers doesn't allow cat to be executed. By the way, sudo would only elevate the cat ~/.bashrc to root privileges; the writing would still be done using output redirection in the non-elevated context.

~/.bashrc is not a good place to put a statement to edit $PATH, since it gets executed with every interactive shell. If you run bash from within Bash, the subshell would unexpectedly have its $PATH redefined. A more appropriate place might be ~/.bash_profile.

After correcting for the mistakes above, with the code

PATH="~/bin:$PATH"
addpath="export PATH=\"$PATH\""
echo "$PATH" >> ~/.bash_profile

… you would be "freezing" the current value of $PATH into ~/.bash_profile. Instead, you probably want to prepend ~/bin to whatever $PATH exists when ~/.bash_profile is executed in the future (notice the single quotes instead of double quotes):

echo 'export PATH=~/bin:$PATH' >> ~/.bash_profile
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.