Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OpenSSL 3 will break transcrypt #133

Closed
Erotemic opened this issue May 7, 2022 · 1 comment · Fixed by #135 · May be fixed by #132
Closed

OpenSSL 3 will break transcrypt #133

Erotemic opened this issue May 7, 2022 · 1 comment · Fixed by #135 · May be fixed by #132

Comments

@Erotemic
Copy link

Erotemic commented May 7, 2022 •
edited

In openssl3 the behavior has changed such that when the -S flag is specified to give a specific salt, the output will no longer be prefixed with "Salted__" and contain the salt in the encoding. Note that when -S is not given and the salt is randomized, the prepend still happens, but we need determinism, so we have to use -S, and unfortunately there is no option to output ciphertext with the 1.x encoding.

https://stackoverflow.com/questions/72149327/openssl-3-0-2-with-custom-salt-doesnt-start-with-salted

I'm working on fixing this in my PR such that if the version of openssl is detected to be 3.x, it prepends the salt encoding to the output. The decryption step still works fine when -S is not given in the decrypt command (which we don't do).

So, it seems to be really tricky to concatenate these encodings together. You can't store the ascii encoding in bash variables because they might contain null bytes, and you can concatenate the base64 encodings because that introduces padding in the middle of the text. It seems like you have to concat the bytes by writing to a file. Here is a MWE for doing this that I came up with.

    #### TESTS
    SALT_PREFIX="Salted__"
    SALT_HEX="deadbeaf00000bad"
    printf "secret" | SECRET_PASSWORD=12345 openssl enc -aes-256-cbc -pbkdf2 -md sha512 -pass env:SECRET_PASSWORD -S "$SALT_HEX" -e -a 
    # OpenSSL 1.x output
    SALTED_CIPHERTEXT_B64_ORIG="U2FsdGVkX1/erb6vAAALrWJ5ujRXTt7kQoCwvQPCjJE="
    # OpenSSL 3.x output
    UNSALTED_CIPHERTEXT_B64="Ynm6NFdO3uRCgLC9A8KMkQ=="

    PART1_ACII="$SALT_PREFIX"
    PART2_HEX="$SALT_HEX"
    PART3_B64="$UNSALTED_CIPHERTEXT_B64"

    b64_to_ascii(){
        base64 -d
    }
    ascii_to_b64(){
        base64
    }
    hex_to_ascii(){
        xxd -r -p 
    }

    TMP_FPATH=$(mktemp)
    printf "" > "$TMP_FPATH"
    # shellcheck disable=SC2059
    printf "$PART1_ACII" > "$TMP_FPATH"
    # shellcheck disable=SC2059
    printf "$PART2_HEX" | hex_to_ascii >> "$TMP_FPATH"
    # shellcheck disable=SC2059
    printf "$PART3_B64" | b64_to_ascii >> "$TMP_FPATH"

    SALTED_CIPHERTEXT_B64_RECON=$(cat "$TMP_FPATH" | ascii_to_b64)

    if [[ "$SALTED_CIPHERTEXT_B64_RECON" == "$SALTED_CIPHERTEXT_B64_ORIG" ]]; then
        echo "correct reconstruction"
    else
        echo "incorrect reconstruction"
    fi

When I go to implement this, I'll take out the extra bash function as it's clear enough what base64 and xxd are doing. I spent way to long trying to get consistent bash conversions between ascii, hex, oct, and baes64 and these are some of the resulting functions.

But if there is a better way that does not involve writing temporary files like this (or there is a better way to write the temporary file) then I'd like to know.

I did find a better way of doing the concatenation without needing to write to a file:

    (printf "Salted__" && printf "deadbeaf00000bad" | xxd -r -p && echo "$UNSALTED_CIPHERTEXT_B64" | base64 -d ) | base64
@Erotemic Erotemic mentioned this issue May 7, 2022
Open
@jmurty jmurty mentioned this issue May 14, 2022
Merged
@jmurty jmurty mentioned this issue Jun 3, 2022
Closed
@jmurty
Copy link
Collaborator

jmurty commented Jun 14, 2022

Fixed in main branch, pending release in version 2.2.0

@jmurty jmurty mentioned this issue Jun 16, 2022
Closed
@jmurty jmurty mentioned this issue Jul 9, 2022
Open
This was referenced Nov 10, 2022
Open
Open
jmurty added a commit that referenced this issue Feb 10, 2023
@jmurty
Detect and fix decryption of files encrypted with doubled "Salted" pr…
40bbe37 
…efixes due to #147

If someone has committed and encrypted a file using Transcrypt
version 2.2.0 on a MacOS 13 Ventura system and using the system-
provided version of `openssl`, the encrypted file will mistakenly
include a doubled "Salted" prefix.

The prefix doubling was caused by Transcrypt applying a work-around
for the OpenSSL project's breaking change (#133) when it didn't need
to, because the LibreSSL project's version 3+ of `openssl` does not
have the same breaking change.

This fix checks for a doubled prefix during decryption (smudge)
operations, and when it finds the mistake will strip out the first
of the doubled prefixes before decrypting.

A proper fix for the issue is to commit a new version of the file
to remove the faulty doubled prefix, but it will be difficult for
users to commit a new version if they only have a faulty decrypted
file to work with.

As an example, a faulty version of this repository's _sensitive_file_
with the doubled prefix would be decrypted like this:

    ��2p͙��g�c�^?Dj6����`�32��\rs to love
    You know the rules and so do I
    A full commitment's what I'm thinking of

With the fix applied in this commit, the decrypted copy becomes:

    We're no strangers to love
    You know the rules and so do I
    A full commitment's what I'm thinking of
jmurty added a commit that referenced this issue Feb 11, 2023
@jmurty
Detect and fix decryption of files encrypted with doubled "Salted" pr…
b380f2c 
…efixes due to #147

If someone has committed and encrypted a file using Transcrypt
version 2.2.0 on a MacOS 13 Ventura system and using the system-
provided version of `openssl`, the encrypted file will mistakenly
include a doubled "Salted" prefix.

The prefix doubling was caused by Transcrypt applying a work-around
for the OpenSSL project's breaking change (#133) when it didn't need
to, because the LibreSSL project's version 3+ of `openssl` does not
have the same breaking change.

This fix checks for a doubled prefix during decryption (smudge)
operations, and when it finds the mistake will strip out the first
of the doubled prefixes before decrypting.

A proper fix for the issue is to commit a new version of the file
to remove the faulty doubled prefix, but it will be difficult for
users to commit a new version if they only have a faulty decrypted
file to work with.

As an example, a faulty version of this repository's _sensitive_file_
with the doubled prefix would be decrypted like this:

    ��2p͙��g�c�^?Dj6����`�32��\rs to love
    You know the rules and so do I
    A full commitment's what I'm thinking of

With the fix applied in this commit, the decrypted copy becomes:

    We're no strangers to love
    You know the rules and so do I
    A full commitment's what I'm thinking of
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@jmurty @Erotemic