OpenVPN Login Bash Shell script for DD-WRT router
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
4
down vote
favorite
This script is made to run on DD-WRT as I am running an OpenVPN server on it just to securely connect to my home network when I'm on the go. Basically, the input ($1
) that goes into this script is a .tmp file generated by OpenVPN when some user wishes to log into the server. The .tmp file consists of 2 lines where the 1st line is the username and the 2nd line is the password.
In this day and age, I'm afraid that potential hackers could inject unintended malicious arguments into the username or password which could exploit my script with the intention to take control over the DD-WRT router (in a way similar to SQL-Injection). So far I have made some huge improvements to the script but I'm not sure if I am finished yet.
#!/bin/sh
#This script was made with OpenVPN via-file in mind
#Location of the Approved Username/Password File
USERS="/somefolder/users"
#Check to see if username and password in the OpenVPN file has any special characters line by line
#Terminate script if special characters are used
while IFS= read -r line
do
case "$line" in *[!-_a-zA-Z0-9]*) exit 1 ;; esac
done < "$1"
Username=`awk 'NR==1' "$1"`
Password=`awk 'NR==2' "$1"`
HASHPASS=`echo -n "$Username$Password" | md5sum | sed s'/ -//'`
i=0
while [ $i -lt 10 ]; do
HASHPASS=`echo -n $HASHPASS$HASHPASS | md5sum | sed s'/ -//'`
i=`expr $i + 1`
done
if grep -q "$Username:$HASHPASS" $USERS; then
echo "User Authenticated."
exit 0
fi
echo "Login credentials failed."
exit 1
security linux authentication shell
 |Â
show 2 more comments
up vote
4
down vote
favorite
This script is made to run on DD-WRT as I am running an OpenVPN server on it just to securely connect to my home network when I'm on the go. Basically, the input ($1
) that goes into this script is a .tmp file generated by OpenVPN when some user wishes to log into the server. The .tmp file consists of 2 lines where the 1st line is the username and the 2nd line is the password.
In this day and age, I'm afraid that potential hackers could inject unintended malicious arguments into the username or password which could exploit my script with the intention to take control over the DD-WRT router (in a way similar to SQL-Injection). So far I have made some huge improvements to the script but I'm not sure if I am finished yet.
#!/bin/sh
#This script was made with OpenVPN via-file in mind
#Location of the Approved Username/Password File
USERS="/somefolder/users"
#Check to see if username and password in the OpenVPN file has any special characters line by line
#Terminate script if special characters are used
while IFS= read -r line
do
case "$line" in *[!-_a-zA-Z0-9]*) exit 1 ;; esac
done < "$1"
Username=`awk 'NR==1' "$1"`
Password=`awk 'NR==2' "$1"`
HASHPASS=`echo -n "$Username$Password" | md5sum | sed s'/ -//'`
i=0
while [ $i -lt 10 ]; do
HASHPASS=`echo -n $HASHPASS$HASHPASS | md5sum | sed s'/ -//'`
i=`expr $i + 1`
done
if grep -q "$Username:$HASHPASS" $USERS; then
echo "User Authenticated."
exit 0
fi
echo "Login credentials failed."
exit 1
security linux authentication shell
After taking a look at it. I guess I could have it so it first looks at the username first and checks to see if it even exists in the USERS file just in case as to not waste cpu cycles on generating a md5 hash if the username never existed in the USERS file at all. Also, I guess what I could also do is have it so it checks that a username and password were entered and not just one or the other to not waste time executing the rest of the script. Do you guys agree with these 2 points (at least for robustness/efficiency)?
â Rocketboy235
Mar 24 at 4:19
2
Nope. That would be a side-channel attack. If the authentication fails faster than others we immediately know that the user doesn't exist and can continue with the next name.
â Zeta
Mar 24 at 7:03
Hmm, good point. Did not think of that...
â Rocketboy235
Mar 24 at 14:09
Tagged bash but with#!/bin/sh
- do you intend this to be POSIX-compliant shell?
â Toby Speight
Mar 26 at 8:54
@TobySpeight I guess I might have confused the terms between sh and bash. Didn't fully understood POSIX-compliant until I looked it up. I chose #!/bin/sh to be run on the DD-WRT router as #!/bin/bash would not work.
â Rocketboy235
Mar 26 at 23:32
 |Â
show 2 more comments
up vote
4
down vote
favorite
up vote
4
down vote
favorite
This script is made to run on DD-WRT as I am running an OpenVPN server on it just to securely connect to my home network when I'm on the go. Basically, the input ($1
) that goes into this script is a .tmp file generated by OpenVPN when some user wishes to log into the server. The .tmp file consists of 2 lines where the 1st line is the username and the 2nd line is the password.
In this day and age, I'm afraid that potential hackers could inject unintended malicious arguments into the username or password which could exploit my script with the intention to take control over the DD-WRT router (in a way similar to SQL-Injection). So far I have made some huge improvements to the script but I'm not sure if I am finished yet.
#!/bin/sh
#This script was made with OpenVPN via-file in mind
#Location of the Approved Username/Password File
USERS="/somefolder/users"
#Check to see if username and password in the OpenVPN file has any special characters line by line
#Terminate script if special characters are used
while IFS= read -r line
do
case "$line" in *[!-_a-zA-Z0-9]*) exit 1 ;; esac
done < "$1"
Username=`awk 'NR==1' "$1"`
Password=`awk 'NR==2' "$1"`
HASHPASS=`echo -n "$Username$Password" | md5sum | sed s'/ -//'`
i=0
while [ $i -lt 10 ]; do
HASHPASS=`echo -n $HASHPASS$HASHPASS | md5sum | sed s'/ -//'`
i=`expr $i + 1`
done
if grep -q "$Username:$HASHPASS" $USERS; then
echo "User Authenticated."
exit 0
fi
echo "Login credentials failed."
exit 1
security linux authentication shell
This script is made to run on DD-WRT as I am running an OpenVPN server on it just to securely connect to my home network when I'm on the go. Basically, the input ($1
) that goes into this script is a .tmp file generated by OpenVPN when some user wishes to log into the server. The .tmp file consists of 2 lines where the 1st line is the username and the 2nd line is the password.
In this day and age, I'm afraid that potential hackers could inject unintended malicious arguments into the username or password which could exploit my script with the intention to take control over the DD-WRT router (in a way similar to SQL-Injection). So far I have made some huge improvements to the script but I'm not sure if I am finished yet.
#!/bin/sh
#This script was made with OpenVPN via-file in mind
#Location of the Approved Username/Password File
USERS="/somefolder/users"
#Check to see if username and password in the OpenVPN file has any special characters line by line
#Terminate script if special characters are used
while IFS= read -r line
do
case "$line" in *[!-_a-zA-Z0-9]*) exit 1 ;; esac
done < "$1"
Username=`awk 'NR==1' "$1"`
Password=`awk 'NR==2' "$1"`
HASHPASS=`echo -n "$Username$Password" | md5sum | sed s'/ -//'`
i=0
while [ $i -lt 10 ]; do
HASHPASS=`echo -n $HASHPASS$HASHPASS | md5sum | sed s'/ -//'`
i=`expr $i + 1`
done
if grep -q "$Username:$HASHPASS" $USERS; then
echo "User Authenticated."
exit 0
fi
echo "Login credentials failed."
exit 1
security linux authentication shell
edited Mar 27 at 7:11
Toby Speight
17.6k13490
17.6k13490
asked Mar 24 at 4:06
Rocketboy235
233
233
After taking a look at it. I guess I could have it so it first looks at the username first and checks to see if it even exists in the USERS file just in case as to not waste cpu cycles on generating a md5 hash if the username never existed in the USERS file at all. Also, I guess what I could also do is have it so it checks that a username and password were entered and not just one or the other to not waste time executing the rest of the script. Do you guys agree with these 2 points (at least for robustness/efficiency)?
â Rocketboy235
Mar 24 at 4:19
2
Nope. That would be a side-channel attack. If the authentication fails faster than others we immediately know that the user doesn't exist and can continue with the next name.
â Zeta
Mar 24 at 7:03
Hmm, good point. Did not think of that...
â Rocketboy235
Mar 24 at 14:09
Tagged bash but with#!/bin/sh
- do you intend this to be POSIX-compliant shell?
â Toby Speight
Mar 26 at 8:54
@TobySpeight I guess I might have confused the terms between sh and bash. Didn't fully understood POSIX-compliant until I looked it up. I chose #!/bin/sh to be run on the DD-WRT router as #!/bin/bash would not work.
â Rocketboy235
Mar 26 at 23:32
 |Â
show 2 more comments
After taking a look at it. I guess I could have it so it first looks at the username first and checks to see if it even exists in the USERS file just in case as to not waste cpu cycles on generating a md5 hash if the username never existed in the USERS file at all. Also, I guess what I could also do is have it so it checks that a username and password were entered and not just one or the other to not waste time executing the rest of the script. Do you guys agree with these 2 points (at least for robustness/efficiency)?
â Rocketboy235
Mar 24 at 4:19
2
Nope. That would be a side-channel attack. If the authentication fails faster than others we immediately know that the user doesn't exist and can continue with the next name.
â Zeta
Mar 24 at 7:03
Hmm, good point. Did not think of that...
â Rocketboy235
Mar 24 at 14:09
Tagged bash but with#!/bin/sh
- do you intend this to be POSIX-compliant shell?
â Toby Speight
Mar 26 at 8:54
@TobySpeight I guess I might have confused the terms between sh and bash. Didn't fully understood POSIX-compliant until I looked it up. I chose #!/bin/sh to be run on the DD-WRT router as #!/bin/bash would not work.
â Rocketboy235
Mar 26 at 23:32
After taking a look at it. I guess I could have it so it first looks at the username first and checks to see if it even exists in the USERS file just in case as to not waste cpu cycles on generating a md5 hash if the username never existed in the USERS file at all. Also, I guess what I could also do is have it so it checks that a username and password were entered and not just one or the other to not waste time executing the rest of the script. Do you guys agree with these 2 points (at least for robustness/efficiency)?
â Rocketboy235
Mar 24 at 4:19
After taking a look at it. I guess I could have it so it first looks at the username first and checks to see if it even exists in the USERS file just in case as to not waste cpu cycles on generating a md5 hash if the username never existed in the USERS file at all. Also, I guess what I could also do is have it so it checks that a username and password were entered and not just one or the other to not waste time executing the rest of the script. Do you guys agree with these 2 points (at least for robustness/efficiency)?
â Rocketboy235
Mar 24 at 4:19
2
2
Nope. That would be a side-channel attack. If the authentication fails faster than others we immediately know that the user doesn't exist and can continue with the next name.
â Zeta
Mar 24 at 7:03
Nope. That would be a side-channel attack. If the authentication fails faster than others we immediately know that the user doesn't exist and can continue with the next name.
â Zeta
Mar 24 at 7:03
Hmm, good point. Did not think of that...
â Rocketboy235
Mar 24 at 14:09
Hmm, good point. Did not think of that...
â Rocketboy235
Mar 24 at 14:09
Tagged bash but with
#!/bin/sh
- do you intend this to be POSIX-compliant shell?â Toby Speight
Mar 26 at 8:54
Tagged bash but with
#!/bin/sh
- do you intend this to be POSIX-compliant shell?â Toby Speight
Mar 26 at 8:54
@TobySpeight I guess I might have confused the terms between sh and bash. Didn't fully understood POSIX-compliant until I looked it up. I chose #!/bin/sh to be run on the DD-WRT router as #!/bin/bash would not work.
â Rocketboy235
Mar 26 at 23:32
@TobySpeight I guess I might have confused the terms between sh and bash. Didn't fully understood POSIX-compliant until I looked it up. I chose #!/bin/sh to be run on the DD-WRT router as #!/bin/bash would not work.
â Rocketboy235
Mar 26 at 23:32
 |Â
show 2 more comments
1 Answer
1
active
oldest
votes
up vote
1
down vote
accepted
Be paranoid about $PATH
It's a good idea to start this script with
PATH=/usr/bin:/bin
Also consider using full paths for programs.
Get the shell to check some errors
set -e -u
Use lower-case for variables
Upper-case is used for communicating well-known environment variables between programs. Prefer lower-case for our own internal shell variables, to avoid any conflicts.
Simplify the valid-character checking
The while
-do
loop could be a simple grep
:
if /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
then exit 1
fi
With set -e
, that's simply
! /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
To be honest, I'm not convinced this checking is needed, provided we don't pass the username as a regular expression - see "Match exactly" below.
Options to echo
are not portable
Consider printf '%s'
instead (or, in Bash, <<<
redirection)
Consider for
instead of while
for counted loop
With Bash, we could use an arithmetic for
loop. For standard shell, consider
for i in $(seq 10)
do
hashpass=$(printf '%s%s' "$hashpass" "$hashpass" |
/usr/bin/md5sum | /usr/bin/cut -d' ' -f1)
done
You could instead write a filter function and just put that in the code 10 times (which might work slightly faster, as parts can work in parallel, and built-in read
is better than starting a process to remove the filename part of the output):
function hashround()
local hash rest
read hash rest
printf '%s%s' "$hash" "$hash"
hashpass=$(printf '%s%s' "$Username" "$Password" | /usr/bin/md5sum
| hashround | hashround | hashround | hashround | hashround
| hashround | hashround | hashround | hashround | hashround
| /usr/bin/cut -d' ' -f1)
(We could even eliminate that final cut
if we agree to use it as $hashpass%% *
to remove the second field as a shell substitution.)
Match exactly
Instead of passing a regular expression to the final grep
, use grep -F
, and also match the entire line (-x
):
if /bin/grep -Fxq "$Username:$hashpass" "$users"
then
echo "User Authenticated." >&2
exit 0
fi
I've also redirected the output to standard error stream, so it doesn't interfere with actual output.
Thanks for all of the tips/suggestions Toby! I have went ahead and modified my script so it uses what you suggest. Only question I have is about when you said it may not be necessary to check for valid characters if we don't pass the username as a regular expression. Would it be possible for someone to hijack it when username is being called elsewhere in the script or actually maybe not since it's in quotes within the printf command. Thanks again.
â Rocketboy235
Mar 30 at 4:40
1
Yes, it might still be a good idea to check that usernames correspond to your expectations, on the principle of Defence in Depth - it could prevent accidents when the script is modified and the username is used in a less-safe context. It's safe where it's used in theprintf
andgrep
commands in this answer - note that we always use%s
withprintf
so that%
is treated as a normal character.
â Toby Speight
Apr 2 at 10:40
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
accepted
Be paranoid about $PATH
It's a good idea to start this script with
PATH=/usr/bin:/bin
Also consider using full paths for programs.
Get the shell to check some errors
set -e -u
Use lower-case for variables
Upper-case is used for communicating well-known environment variables between programs. Prefer lower-case for our own internal shell variables, to avoid any conflicts.
Simplify the valid-character checking
The while
-do
loop could be a simple grep
:
if /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
then exit 1
fi
With set -e
, that's simply
! /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
To be honest, I'm not convinced this checking is needed, provided we don't pass the username as a regular expression - see "Match exactly" below.
Options to echo
are not portable
Consider printf '%s'
instead (or, in Bash, <<<
redirection)
Consider for
instead of while
for counted loop
With Bash, we could use an arithmetic for
loop. For standard shell, consider
for i in $(seq 10)
do
hashpass=$(printf '%s%s' "$hashpass" "$hashpass" |
/usr/bin/md5sum | /usr/bin/cut -d' ' -f1)
done
You could instead write a filter function and just put that in the code 10 times (which might work slightly faster, as parts can work in parallel, and built-in read
is better than starting a process to remove the filename part of the output):
function hashround()
local hash rest
read hash rest
printf '%s%s' "$hash" "$hash"
hashpass=$(printf '%s%s' "$Username" "$Password" | /usr/bin/md5sum
| hashround | hashround | hashround | hashround | hashround
| hashround | hashround | hashround | hashround | hashround
| /usr/bin/cut -d' ' -f1)
(We could even eliminate that final cut
if we agree to use it as $hashpass%% *
to remove the second field as a shell substitution.)
Match exactly
Instead of passing a regular expression to the final grep
, use grep -F
, and also match the entire line (-x
):
if /bin/grep -Fxq "$Username:$hashpass" "$users"
then
echo "User Authenticated." >&2
exit 0
fi
I've also redirected the output to standard error stream, so it doesn't interfere with actual output.
Thanks for all of the tips/suggestions Toby! I have went ahead and modified my script so it uses what you suggest. Only question I have is about when you said it may not be necessary to check for valid characters if we don't pass the username as a regular expression. Would it be possible for someone to hijack it when username is being called elsewhere in the script or actually maybe not since it's in quotes within the printf command. Thanks again.
â Rocketboy235
Mar 30 at 4:40
1
Yes, it might still be a good idea to check that usernames correspond to your expectations, on the principle of Defence in Depth - it could prevent accidents when the script is modified and the username is used in a less-safe context. It's safe where it's used in theprintf
andgrep
commands in this answer - note that we always use%s
withprintf
so that%
is treated as a normal character.
â Toby Speight
Apr 2 at 10:40
add a comment |Â
up vote
1
down vote
accepted
Be paranoid about $PATH
It's a good idea to start this script with
PATH=/usr/bin:/bin
Also consider using full paths for programs.
Get the shell to check some errors
set -e -u
Use lower-case for variables
Upper-case is used for communicating well-known environment variables between programs. Prefer lower-case for our own internal shell variables, to avoid any conflicts.
Simplify the valid-character checking
The while
-do
loop could be a simple grep
:
if /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
then exit 1
fi
With set -e
, that's simply
! /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
To be honest, I'm not convinced this checking is needed, provided we don't pass the username as a regular expression - see "Match exactly" below.
Options to echo
are not portable
Consider printf '%s'
instead (or, in Bash, <<<
redirection)
Consider for
instead of while
for counted loop
With Bash, we could use an arithmetic for
loop. For standard shell, consider
for i in $(seq 10)
do
hashpass=$(printf '%s%s' "$hashpass" "$hashpass" |
/usr/bin/md5sum | /usr/bin/cut -d' ' -f1)
done
You could instead write a filter function and just put that in the code 10 times (which might work slightly faster, as parts can work in parallel, and built-in read
is better than starting a process to remove the filename part of the output):
function hashround()
local hash rest
read hash rest
printf '%s%s' "$hash" "$hash"
hashpass=$(printf '%s%s' "$Username" "$Password" | /usr/bin/md5sum
| hashround | hashround | hashround | hashround | hashround
| hashround | hashround | hashround | hashround | hashround
| /usr/bin/cut -d' ' -f1)
(We could even eliminate that final cut
if we agree to use it as $hashpass%% *
to remove the second field as a shell substitution.)
Match exactly
Instead of passing a regular expression to the final grep
, use grep -F
, and also match the entire line (-x
):
if /bin/grep -Fxq "$Username:$hashpass" "$users"
then
echo "User Authenticated." >&2
exit 0
fi
I've also redirected the output to standard error stream, so it doesn't interfere with actual output.
Thanks for all of the tips/suggestions Toby! I have went ahead and modified my script so it uses what you suggest. Only question I have is about when you said it may not be necessary to check for valid characters if we don't pass the username as a regular expression. Would it be possible for someone to hijack it when username is being called elsewhere in the script or actually maybe not since it's in quotes within the printf command. Thanks again.
â Rocketboy235
Mar 30 at 4:40
1
Yes, it might still be a good idea to check that usernames correspond to your expectations, on the principle of Defence in Depth - it could prevent accidents when the script is modified and the username is used in a less-safe context. It's safe where it's used in theprintf
andgrep
commands in this answer - note that we always use%s
withprintf
so that%
is treated as a normal character.
â Toby Speight
Apr 2 at 10:40
add a comment |Â
up vote
1
down vote
accepted
up vote
1
down vote
accepted
Be paranoid about $PATH
It's a good idea to start this script with
PATH=/usr/bin:/bin
Also consider using full paths for programs.
Get the shell to check some errors
set -e -u
Use lower-case for variables
Upper-case is used for communicating well-known environment variables between programs. Prefer lower-case for our own internal shell variables, to avoid any conflicts.
Simplify the valid-character checking
The while
-do
loop could be a simple grep
:
if /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
then exit 1
fi
With set -e
, that's simply
! /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
To be honest, I'm not convinced this checking is needed, provided we don't pass the username as a regular expression - see "Match exactly" below.
Options to echo
are not portable
Consider printf '%s'
instead (or, in Bash, <<<
redirection)
Consider for
instead of while
for counted loop
With Bash, we could use an arithmetic for
loop. For standard shell, consider
for i in $(seq 10)
do
hashpass=$(printf '%s%s' "$hashpass" "$hashpass" |
/usr/bin/md5sum | /usr/bin/cut -d' ' -f1)
done
You could instead write a filter function and just put that in the code 10 times (which might work slightly faster, as parts can work in parallel, and built-in read
is better than starting a process to remove the filename part of the output):
function hashround()
local hash rest
read hash rest
printf '%s%s' "$hash" "$hash"
hashpass=$(printf '%s%s' "$Username" "$Password" | /usr/bin/md5sum
| hashround | hashround | hashround | hashround | hashround
| hashround | hashround | hashround | hashround | hashround
| /usr/bin/cut -d' ' -f1)
(We could even eliminate that final cut
if we agree to use it as $hashpass%% *
to remove the second field as a shell substitution.)
Match exactly
Instead of passing a regular expression to the final grep
, use grep -F
, and also match the entire line (-x
):
if /bin/grep -Fxq "$Username:$hashpass" "$users"
then
echo "User Authenticated." >&2
exit 0
fi
I've also redirected the output to standard error stream, so it doesn't interfere with actual output.
Be paranoid about $PATH
It's a good idea to start this script with
PATH=/usr/bin:/bin
Also consider using full paths for programs.
Get the shell to check some errors
set -e -u
Use lower-case for variables
Upper-case is used for communicating well-known environment variables between programs. Prefer lower-case for our own internal shell variables, to avoid any conflicts.
Simplify the valid-character checking
The while
-do
loop could be a simple grep
:
if /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
then exit 1
fi
With set -e
, that's simply
! /bin/grep -q '[^-_a-zA-Z0-9]' "$1"
To be honest, I'm not convinced this checking is needed, provided we don't pass the username as a regular expression - see "Match exactly" below.
Options to echo
are not portable
Consider printf '%s'
instead (or, in Bash, <<<
redirection)
Consider for
instead of while
for counted loop
With Bash, we could use an arithmetic for
loop. For standard shell, consider
for i in $(seq 10)
do
hashpass=$(printf '%s%s' "$hashpass" "$hashpass" |
/usr/bin/md5sum | /usr/bin/cut -d' ' -f1)
done
You could instead write a filter function and just put that in the code 10 times (which might work slightly faster, as parts can work in parallel, and built-in read
is better than starting a process to remove the filename part of the output):
function hashround()
local hash rest
read hash rest
printf '%s%s' "$hash" "$hash"
hashpass=$(printf '%s%s' "$Username" "$Password" | /usr/bin/md5sum
| hashround | hashround | hashround | hashround | hashround
| hashround | hashround | hashround | hashround | hashround
| /usr/bin/cut -d' ' -f1)
(We could even eliminate that final cut
if we agree to use it as $hashpass%% *
to remove the second field as a shell substitution.)
Match exactly
Instead of passing a regular expression to the final grep
, use grep -F
, and also match the entire line (-x
):
if /bin/grep -Fxq "$Username:$hashpass" "$users"
then
echo "User Authenticated." >&2
exit 0
fi
I've also redirected the output to standard error stream, so it doesn't interfere with actual output.
edited Mar 26 at 10:14
answered Mar 26 at 9:51
Toby Speight
17.6k13490
17.6k13490
Thanks for all of the tips/suggestions Toby! I have went ahead and modified my script so it uses what you suggest. Only question I have is about when you said it may not be necessary to check for valid characters if we don't pass the username as a regular expression. Would it be possible for someone to hijack it when username is being called elsewhere in the script or actually maybe not since it's in quotes within the printf command. Thanks again.
â Rocketboy235
Mar 30 at 4:40
1
Yes, it might still be a good idea to check that usernames correspond to your expectations, on the principle of Defence in Depth - it could prevent accidents when the script is modified and the username is used in a less-safe context. It's safe where it's used in theprintf
andgrep
commands in this answer - note that we always use%s
withprintf
so that%
is treated as a normal character.
â Toby Speight
Apr 2 at 10:40
add a comment |Â
Thanks for all of the tips/suggestions Toby! I have went ahead and modified my script so it uses what you suggest. Only question I have is about when you said it may not be necessary to check for valid characters if we don't pass the username as a regular expression. Would it be possible for someone to hijack it when username is being called elsewhere in the script or actually maybe not since it's in quotes within the printf command. Thanks again.
â Rocketboy235
Mar 30 at 4:40
1
Yes, it might still be a good idea to check that usernames correspond to your expectations, on the principle of Defence in Depth - it could prevent accidents when the script is modified and the username is used in a less-safe context. It's safe where it's used in theprintf
andgrep
commands in this answer - note that we always use%s
withprintf
so that%
is treated as a normal character.
â Toby Speight
Apr 2 at 10:40
Thanks for all of the tips/suggestions Toby! I have went ahead and modified my script so it uses what you suggest. Only question I have is about when you said it may not be necessary to check for valid characters if we don't pass the username as a regular expression. Would it be possible for someone to hijack it when username is being called elsewhere in the script or actually maybe not since it's in quotes within the printf command. Thanks again.
â Rocketboy235
Mar 30 at 4:40
Thanks for all of the tips/suggestions Toby! I have went ahead and modified my script so it uses what you suggest. Only question I have is about when you said it may not be necessary to check for valid characters if we don't pass the username as a regular expression. Would it be possible for someone to hijack it when username is being called elsewhere in the script or actually maybe not since it's in quotes within the printf command. Thanks again.
â Rocketboy235
Mar 30 at 4:40
1
1
Yes, it might still be a good idea to check that usernames correspond to your expectations, on the principle of Defence in Depth - it could prevent accidents when the script is modified and the username is used in a less-safe context. It's safe where it's used in the
printf
and grep
commands in this answer - note that we always use %s
with printf
so that %
is treated as a normal character.â Toby Speight
Apr 2 at 10:40
Yes, it might still be a good idea to check that usernames correspond to your expectations, on the principle of Defence in Depth - it could prevent accidents when the script is modified and the username is used in a less-safe context. It's safe where it's used in the
printf
and grep
commands in this answer - note that we always use %s
with printf
so that %
is treated as a normal character.â Toby Speight
Apr 2 at 10:40
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190349%2fopenvpn-login-bash-shell-script-for-dd-wrt-router%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
After taking a look at it. I guess I could have it so it first looks at the username first and checks to see if it even exists in the USERS file just in case as to not waste cpu cycles on generating a md5 hash if the username never existed in the USERS file at all. Also, I guess what I could also do is have it so it checks that a username and password were entered and not just one or the other to not waste time executing the rest of the script. Do you guys agree with these 2 points (at least for robustness/efficiency)?
â Rocketboy235
Mar 24 at 4:19
2
Nope. That would be a side-channel attack. If the authentication fails faster than others we immediately know that the user doesn't exist and can continue with the next name.
â Zeta
Mar 24 at 7:03
Hmm, good point. Did not think of that...
â Rocketboy235
Mar 24 at 14:09
Tagged bash but with
#!/bin/sh
- do you intend this to be POSIX-compliant shell?â Toby Speight
Mar 26 at 8:54
@TobySpeight I guess I might have confused the terms between sh and bash. Didn't fully understood POSIX-compliant until I looked it up. I chose #!/bin/sh to be run on the DD-WRT router as #!/bin/bash would not work.
â Rocketboy235
Mar 26 at 23:32