6
\$\begingroup\$

Tonight, I tried to write a Bash OpenSSL file decryption script. Encryption script will follow in highly similar manner, so I think it's not necessary now.

Let's suppose we have an encrypted large file using:

openssl enc -aes-256-cbc -salt -in somefile -out somefile.enc 

You correctly noticed I always add .enc to the file name.


The meaning of the script is:

  • take one, and only one, argument as a file

  • ask user for password

  • decrypt that file with openssl with a shown progress

  • save it without the .enc extension, or if it didn't have that extension, give it .dec extension

  • not to overwrite existing files


Though I wrote it with maximum effort, a review is more than welcome.

The script follows:

#!/bin/bash if [ "$#" -ne 1 ] || [ ! -f "$1" ] then echo "A single existing file needs to be given as an argument. Exiting." else input_filename="$1" filepath=$(dirname "$1") filename_extracted_from_path=$(basename "$1") filename_without_enc_extension="${filename_extracted_from_path%.enc}" if [[ "$filename_extracted_from_path" == "$filename_without_enc_extension" ]] then output_filename="$filepath/$filename_extracted_from_path".dec else output_filename="$filepath/$filename_without_enc_extension" fi if [ -f "$output_filename" ] then echo "Destination file exists. Exiting." else pv "$input_filename" | openssl enc -aes-256-cbc -d -salt -out "$output_filename" fi fi 

EDIT1:

  • I decided to put the script on GitHub (when done), therefore it must be POSIX compliant, so I have made minor changes.

  • I found out, the script lacks destination directory writable check, so I integrated it.

  • Also, it now checks if it is run as root (or with sudo).

But the structure remains and new version of the script follows:

#!/bin/sh if [ "$#" -ne 1 ] || [ ! -f "$1" ] then echo "A single existing file needs to be given as an argument. Exiting." else input_filename="$1" filepath=$(dirname "$1") filename_extracted_from_path=$(basename "$1") filename_without_enc_extension="${filename_extracted_from_path%.enc}" if [ "$filename_extracted_from_path" = "$filename_without_enc_extension" ] then output_filename="$filepath/$filename_extracted_from_path".dec else output_filename="$filepath/$filename_without_enc_extension" fi if [ -f "$output_filename" ] then echo "Destination file exists. Exiting." else if [ ! -w "$filepath" ] && [ "$(id -u)" -ne 0 ] then echo "Destination directory is not writable. Exiting." else pv --wait "$input_filename" | openssl enc -aes-256-cbc -d -salt -out "$output_filename" fi fi fi 

Uploaded on GitHub: https://git.io/fxslm

\$\endgroup\$
0

    1 Answer 1

    4
    \$\begingroup\$

    Get rid of unnecessary nested and combined ifs

    While here it goes to no extreme, it's advisable to check for errors first, and to do that one by one - if possible, which is exactly this case.

    Do some extra checks

    • Only in case of 0 arguments, the script should output usage message.

    • This script does not support multiple arguments, so if given more than 1, inform the user.

    • The script only checks for existence of the file, but what if it's not readable by the user.

    Use tput for handling text colors instead of escape sequences

    Those escape sequences are non-portable and the fact it works in dash does not mean it would do the job in Cygwin for instance; tput is clearly more portable.

    bold=$(tput bold) red=$(tput setaf 1) yellow=$(tput setaf 3) nocolor=$(tput sgr0) bold_red=${bold}${red} bold_yellow=${bold}${yellow} 

    Write a function for handling errors

    print_error_and_exit() { echo "${bold_red}$1 Exit code = $2.${nocolor}" exit "$2" } 

    Shorten test commands, since there are only if ... then

    [ "$#" -eq 0 ] \ && print_error_and_exit "One argument needs to be given. Both relative, and absolute file paths are supported." 1 

    Declare and initialize variables just before they're used

    The following declarations:

    input_filename="$1" filepath=$(dirname "$1") filename_extracted_from_path=$(basename "$1") filename_without_enc_extension="${filename_extracted_from_path%.enc}" 

    Should be split in order not to declare / initialize variables the script may not use in event of a failure.

    Use exit return codes

    The script should give the system feedback for example for automated processing. And be sure to use different exit codes in every failure event.

    Root account check is unnecessary

    The following test:

    [ "$(id -u)" -ne 0 ] 

    Does not serve the intended purpose and should be omitted.

    Comment code, that is not self-explanatory

    The following code:

    if [ "$filename_extracted_from_path" = "$filename_without_enc_extension" ] then output_filename="$filepath/$filename_extracted_from_path".dec else output_filename="$filepath/$filename_without_enc_extension" fi 

    May be a little unclear, so a short comment is advisable.

    The script should give feedback to the user too

    The core code:

    pv --wait "$input_filename" | openssl enc -aes-256-cbc -d -salt -out "$output_filename" 

    Should say something like:

    Decryption failed / successful. 

    Clean after failed decryption

    In the event of failed decryption, there is an empty output file present.

    (Optional) The decryption status message could use some colors

    In the end, if the decryption fails, it shows something similar to:

    bad decrypt 140673166227096:error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt:evp_enc.c:529: 

    Though it might suffice to say the above mentioned message, some users might prefer it in color.


    Based on the above code, I would re-write it as follows:

    #!/bin/sh bold=$(tput bold) red=$(tput setaf 1) yellow=$(tput setaf 3) nocolor=$(tput sgr0) bold_red=${bold}${red} bold_yellow=${bold}${yellow} print_error_and_exit() { echo "${bold_red}$1 Exit code = $2.${nocolor}" exit "$2" } [ "$#" -eq 0 ] \ && print_error_and_exit "One argument needs to be given. Both relative, and absolute file paths are supported." 1 [ "$#" -gt 1 ] \ && print_error_and_exit "Multiple arguments are not supported." 2 [ ! -f "$1" ] \ && print_error_and_exit "The given argument is not an existing file." 3 input_filename="$1" [ ! -r "$input_filename" ] \ && print_error_and_exit "Input file is not readable by you." 4 input_filepath=$(dirname "$input_filename") [ ! -w "$input_filepath" ] \ && print_error_and_exit "Destination directory is not writable by you." 5 filename_extracted_from_path=$(basename "$input_filename") filename_without_enc_extension="${filename_extracted_from_path%.enc}" if [ "$filename_extracted_from_path" = "$filename_without_enc_extension" ] then # the file has a different than .enc extension or no extension at all # what we do now, is that we append .dec extention to the file name output_filename="$input_filepath/$filename_extracted_from_path".dec else # the file has the .enc extension # what we do now, is that we use the file name without .enc extension output_filename="$input_filepath/$filename_without_enc_extension" fi [ -f "$output_filename" ] \ && print_error_and_exit "Destination file exists." 6 if ! pv -W "$input_filename" | openssl enc -aes-256-cbc -md sha256 -salt -out "$output_filename" -d 2> /dev/null then [ -f "$output_filename" ] && rm "$output_filename" print_error_and_exit "Decryption failed." 7 else echo "${bold_yellow}Decryption successful.${nocolor}" exit 0 fi 
    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.