3
\$\begingroup\$

This is my first shell script, so I am trying to see if I can get some suggestions. It extracts saved wifi passwords from Linux.

##################################################################
# This script is used to copy wifi password files from /etc/NetworkManager/system-connections folder
# check if the wifi_pass_dir exists, and delete it if it does
if [ -d "wifi_pass_dir" ]
then
 rm -rf wifi_pass_dir
fi
# Create wifi_pass_dir folder
mkdir wifi_pass_dir
#copy the files from NetworkManager to the folder created, these files can only be read by the root user so use sudo
sudo cp /etc/NetworkManager/system-connections/* ./wifi_pass_dir
# change the file permission from read only by root to read and write by any user (666) 
sudo chmod 666 ./wifi_pass_dir/*
#create a list so we know what wifi passwords are saved
sudo ls /etc/NetworkManager/system-connections > ./wifi_pass_dir/pass_list.txt
# Read name of wifi from pass_list.txt and print on screen 
cd wifi_pass_dir
filename="pass_list.txt"
while read -r line
do 
 name="$line"
 echo $name 
 cat "$name" | grep psk=
done < "$filename"
##################################################################
Toby Speight
87.1k14 gold badges104 silver badges322 bronze badges
asked Nov 1, 2017 at 6:45
\$\endgroup\$
4
  • \$\begingroup\$ @Asdeth : Just a minor issue: Your script will produce several error messages if the directory system-connections is empty. \$\endgroup\$ Commented Nov 1, 2017 at 7:08
  • \$\begingroup\$ @user1934428 Thank you. To be honest I don't actually know what a directory system-connect is. I will definitely look up for it. However would you be kind enough to point me which particular line would this affect and any suggestions on what I could do differently? [I do not get error when I run in on my computer but obviously I haven't had the opportunity to have it run on anyone else's] \$\endgroup\$ Commented Nov 1, 2017 at 7:12
  • \$\begingroup\$ It's system-connections, and you mention it in your script, so if you don't know what it is, who else would? .... For verifying whether a directory contains files, see for instance here \$\endgroup\$ Commented Nov 1, 2017 at 12:50
  • \$\begingroup\$ Oh now I get it. Thank you. So i need to add an error checking code to see if the folder is empty. Sort of like , erm, a sanity check. Got it cheers =) Appreciate the input mate. \$\endgroup\$ Commented Nov 1, 2017 at 13:17

1 Answer 1

2
\$\begingroup\$

General

Always include a shebang, and set options to exit the script when a command fails or when we use a variable that's not set:

#!/bin/bash
set -eu

Test for existence with -e, not -d

If wifi_pass_dir exists, but is not a directory (perhaps it's a plain file), it will not be deleted, and the mkdir will fail. It might be better not to check (rm -f will succeed if the file doesn't exist):

# Ensure the wifi_pass_dir exists
rm -rf wifi_pass_dir
mkdir wifi_pass_dir

However, I don't believe it's necessary to copy the config files to an insecure location and make them world readable (and writeable) - see later.

sudo is for interactive use, not scripts

Avoid using sudo in a script - it expects to be interactive, and won't necessarily do what you want if not connected to a terminal. It's easier to require that the script be run under sudo (and we can then use su to change to a non-privileged user where necessary).

Don't parse the output of ls

Filenames you don't control can contain all sorts of characters, including newlines. Here, we don't need to do that, as we're reading from a directory. The while read loop can simply be a for f in * loop.

Don't over-exercise the cat

This cat isn't needed:

cat "$name" | grep psk=

Just tell grep to take its input from $name directly:

grep psk= "$name"

We don't even need a loop, as we can pass all the filenames to grep in one go.


Simplified code

#!/bin/bash
exec grep 'psk=' /etc/NetworkManager/system-connections/*

Yes, that's the whole program. Since we only execute one command, we don't need set -e, and as we use no variables, neither do we need set -u.

answered Jul 17, 2018 at 13:49
\$\endgroup\$
2
  • \$\begingroup\$ Toby Speight , thank you buddy.. seriously thank you. I really appreciate the time and effort you put in this. Thank you so much it really helps :) \$\endgroup\$ Commented Jul 19, 2018 at 20:51
  • \$\begingroup\$ If you found it useful, you can accept the answer - select the tick mark underneath the voting buttons. \$\endgroup\$ Commented Jul 20, 2018 at 7:54

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.