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.

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

Support OpenSSL 3 which has changed behaviour for "Salted__" prefix #135

Merged
merged 9 commits into from
Jun 14, 2022

Conversation

jmurty
Copy link
Collaborator

@jmurty jmurty commented May 14, 2022

Work in progress to fix #133

…n Openssl

Change output checks in unit tests to work, although with less
precision, when tests are run against newer OpenSSL versions like
1.1 that output the following warning messages:

    *** WARNING : deprecated key derivation used.
    Using -iter or -pbkdf2 would be better.
…enSSL 3

Recognise when a file is encrypted with a version of OpenSSL or
similar that does not include the "Salted__" prefix in the cipher
text prior to Base64 encoding, and add that prefix.

This makes Transcrypt compatible with OpenSSL version 3 which changed
the behaviour of salting files and leaves off the "Salted__" prefix.

This workaround requires another temporary file to store encrypted
data, to check for the prefix before Base64 encoding the cipher text
into Git's index.
@jmurty
Copy link
Collaborator Author

jmurty commented May 14, 2022

Hi @Erotemic @elasticdog this fix for OpenSSL 3 compatibility is working, provably so in the ubuntu-22.04 test workflow which runs against OpenSSL version 3.0.2.

The fix required yet another temporary file to hold the cipher text at encryption time, prior to base64 encoding so we can check for the presence of the Salted__ prefix and add it when it's missing. Because bash variables can't store all possible bytes – no null bytes – I can't think of a better way to handle this.

Aside from the fix this PR includes a lot of test changes so they work despite the noise of the warnings generated by newer (non-ancient) versions of OpenSSL:

*** WARNING : deprecated key derivation used.
Using -iter or -pbkdf2 would be better.

@jmurty jmurty requested a review from elasticdog May 14, 2022 16:15
… to a temporary file

Avoid double-handling of encrypted files via a temporary file, but
also ensure the "Salted__" + salt prefix required by Transcrypt is
included, by:

- manually prepend the required prefix in Transcrypt, so we know it
  is always present
- strip this prefix if the OpenSSL encryption step also included it,
  so it isn't doubled for OpenSSL versions prior to 3
LANG=C tends to work for Linux but not on macOS, LC_ALL is a better
option and the same approach used everywhere else in Transcrypt
(I keep having to re-learn this...)
@jmurty jmurty merged commit c6ec80e into main Jun 14, 2022
@jmurty
Copy link
Collaborator Author

jmurty commented Jun 14, 2022

This fix seems to work – at least in some cases and no-one has said otherwise – so it's time to roll it out in a new version 2.2.0

jmurty added a commit to Erotemic/transcrypt that referenced this pull request Jun 14, 2022
* main:
  Prepare for 2.2.0 release
  Fix when using OpenSSL 3 which no longer embeds salt in output (elasticdog#135)

# Conflicts:
#	CHANGELOG.md
#	contrib/packaging/pacman/PKGBUILD
#	transcrypt
jmurty added a commit that referenced this pull request Oct 15, 2022
# By James Murty (18) and others
# Via GitHub (1) and James Murty (1)
* main: (26 commits)
  Centralise load and save of password into functions #141
  Fix date of 2.2.0 release
  Ensure tests use "main" as default branch name #143
  Use OpenSSL for B64 encoding not `base64` which differs between Linux and Mac #140
  Use core attributesFile from worktree (#137)
  Document `xxd` requirement, and make optional with OpenSSL < 3 (#138)
  Prepare for 2.2.0 release
  Fix when using OpenSSL 3 which no longer embeds salt in output (#135)
  Consolidate all git operation scripts into a single transcrypt script
  Fix handling of small files and files with null in first 8 bytes (#116)
  Improve command hint to fix secret files not encrypted in index (#120) (#130)
  Remove Ubuntu 16.04 LTS from test matrix (#123)
  Configure default Git branch name for macOS tests in GitHub
  Handle rename of primary branch from "master" to "main"
  Ensure Git index is up-to-date before dirty repo  check #37 (#109)
  Fix incorrect salt when partially staged files are commited (#119)
  Use shorthand for grep options for broader compatibility (#121)
  Let user set a custom path to openssl #108
  Install entire transcrypt script into repository
  Change version to indicate development "pre-release" status
  ...

# Conflicts:
#	README.md
#	tests/_test_helper.bash
#	tests/test_cleanup.bats
#	tests/test_crypt.bats
#	tests/test_init.bats
#	tests/test_not_inited.bats
#	transcrypt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenSSL 3 will break transcrypt
1 participant