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

`encrypt-file` overwrites previous secure keys when invoked from same working folder and causes file decryption failures when used with multiple files #627

Open
chronosis opened this Issue Sep 25, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@chronosis
Copy link

chronosis commented Sep 25, 2018

When calling travis encrypt-file for multiple files, from the same folder, it causes the cli to overwrite the secure variable that is used for the file.

This causes problems as mentioned in both #239 and #583 --

The trouble seems to be caused here -- https://github.com/travis-ci/travis.rb/blob/master/lib/travis/cli/encrypt_file.rb#L73-L82

As this code creates or updates the secure environment variable for encryption based from the working folder Dir.pwd that travis-cli is being executed from, instead of the full path of the file that is being encrypted.

@env_prefix ||= "encrypted_#{Digest.hexencode(Digest::SHA1.digest(Dir.pwd)[0..5])}"

As a result, calling the encrypt-file from the same working directory results in overwritten key and iv values and causes Travis the issue seen --

bad decrypt
140043714328224:error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt:evp_enc.c:539:
The command "openssl aes-256-cbc -K $encrypted_xxxxxxxxxxxx_key -iv $encrypted_xxxxxxxxxxxx_iv -in file.enc -out file -d" failed and exited with 1 during .

There is a mention of the env variables being overwritten in the documentation; however, there is no mention of the conditions which cause the values to be overwritten.

Rather than using the working directory for this behavior, which is both undocumented and unintuitive, it would make the most sense if the input_path was used for generating env for the key and iv values. It seems doubtful that this behavior would be done intentionally, as it would be extremely easy to circumvent the behavior by simply running the cli from a different working folder, like so:

base-project$ travis encrypt-file file1 --add
base-project$ mkdir -p tmp1
base-project$ cd tmp1
base-project/tmp1$ travis encrypt-file ../file2 --add
base-project/tmp1$ cd ..
base-project$ rmdir tmp1
base-project$ mkdir -p tmp2
base-project$ cd tmp2
base-project/tmp2$ travis encrypt-file ../file3 --add
base-project/tmp2$ cd ..
base-project$ rmdir tmp2

...and so on...

It is a simple fix to -- https://github.com/travis-ci/travis.rb/blob/master/lib/travis/cli/encrypt_file.rb -- to correct the behavior to use the input_path. See pull request #628.

@chronosis chronosis changed the title `encrypt-file` overwrites previous secure keys when invoked from same working folder and causes file decryption failures `encrypt-file` overwrites previous secure keys when invoked from same working folder and causes file decryption failures when used with multiple files Sep 25, 2018

chronosis added a commit to chronosis/travis.rb that referenced this issue Sep 25, 2018

DarthGandalf added a commit to kvirc/KVIrc that referenced this issue Nov 20, 2018

@lenaschimmel

This comment has been minimized.

Copy link

lenaschimmel commented Jan 5, 2019

Just to be sure I understand what's happening…

In September @chronosis supposed a simple fix which would just allow to encrypt any number of files: #628

In November there was a PR to make it clearer that encrypting multiple files is not supported, and there was a lengthy discussion about the wording, but it was finally merged: #2136

Yet notbody commented on the actual fix, which would have prevented the documentation problem in the first place?

@DarthGandalf

This comment has been minimized.

Copy link

DarthGandalf commented Jan 5, 2019

Yep :(
I had to send the documentation PR because I saw that the fix is not being merged for whatever reason.

@chronosis

This comment has been minimized.

Copy link

chronosis commented Jan 8, 2019

Purely conjecture here, but I suspect that the reason it's not being merged is one of the following:

  • Lack of development resources; or,
  • Product Management decision aimed to reduce overhead; or,
  • Perceived fear of a breaking change; or,
  • A belief that requested functionality is not possible

Lack of Development Resources

TravisCI may not currently have developer resources allocated towards maintaining the CLI. If this is the case then the fix does not have visibility. The makes the most sense to me, as I have seen both issues and PRs sit open within the Travis-CI organization on GitHub for months at a time without an official development team response. It is unfortunate, but it is common for businesses who have Open Source offering but that are also in the process of outgrowing their startup phase to have these sorts of developer resource allocation issues. Tools that were developed when the product and business were less mature fail to get timely updates as developers leave the company or the product management team reallocates developer resources towards tasks that have a larger ROI. In these scenarios, without some sort of escalation through their support and product teams (to create Feature Request or Bugs) it is unlikely that the CLI is getting any resources to deal with issues like this proposed fix. From a business perspective, it's easier to update the documentation and say it's "Unsupported" than to allocate development hours which are costly.

Product Management Decision Aimed at Overhead Reduction

It is possible that the Product Management team at Travis has made the decision not to support encryption of multiple files because of the overhead created with additional Travis build environment (i.e. managed variables). I find this to be unlikely, but if it is the case then the rationale is dubious. There are some build environments that have dozens of managed build variables. The issues here is that the CLI overwrites the existing environment variables which it simply should not do. The fact that it does, even with a warning, creates a poor user experience -- enough so that it has been commented on in multiple issues over the course of several years and is what led me to research the cause of the problem in the first place. The fact of the matter is that the additional overhead of two managed variables for each file is trivial and doesn't justify a negative user experience when there is a trivial fix.

Perceived Fear of a Breaking Change

It is also possible (likely due to a lack of development resources) that the product management or development teams believe that this patch will create a breaking change. If this is the case, then it would be a belief that isn't supported by fact. The CLI only supports decryption using the --decrypt flag when both the Key and IV are also provided on the command line. Since the patch neither affects how they Key and IV files are generated nor the presence of existing keys, it can not create a break in the decryption of files that are currently encrypted. A user would need to pull both the Key and IV values used to encrypt the file to decrypt it. Additionally, when a build is running in the Travis environment, OpenSSL is used to decrypt the files. The patch does not break the insertion of script commands that use openssl to decrypt the files. The workaround provided above again shows that multiple files can be encrypted and decrypted with no consequences.

Belief that Requested Functionality is Not Possible

Finally, it is also possible (also likely due to lack of development resources) that there is missing expertise on the CLI codebase which is leading to the belief that multiple file encryption is not possible and therefore unsupported. If this is the case, then it is a belief disproven by the existence of the workaround (noted above). Further, the workaround shows precisely the issue in the current CLI logic and why it is possible to support.

Some Background

My organization is a paid Travis customer and we have some build environments which have dozens of managed variables. When we ran into the "Not Support" behavior of multiple files and noticed that the reason was because it would overwrite the Key / IV variables with subsequent calls from the same working folder, I felt compelled to investigate why the behavior was as such. Unfortunately, although the fix to the issue is trivial, Travis is being reticent on why they are maintaining the position that this is "Unsupported".

Workaround

I usually make a shell script to perform the encryption of all the files I need, paths are shown for clarity.

(base-project)$ travis encrypt-file file1 --add
(base-project)$ mkdir -p tmp1
(base-project)$ cd tmp1
(base-project/tmp1)$ travis encrypt-file ../file2 --add
(base-project/tmp1)$ cd ..
(base-project)$ rmdir tmp1
(base-project)$ mkdir -p tmp2
(base-project)$ cd tmp2
(base-project/tmp2)$ travis encrypt-file ../file3 --add
(base-project/tmp2)$ cd ..
(base-project)$ rmdir tmp2
# ...and so on

# Clean up .travis.yml values of '../' created in the encryption steps above to './'
(base-project)$ sed -i -E "s/\.\.\//\.\//g" .travis.yml
(base-project)$ rm .travis.yml-E

Summary

Regardless, my recommendation would be to try to escalate the issue through Travis' normal support channels in an effort to increase visibility into the feature/bug in the very likely case that this hasn't gotten traction due to lack of visibility. Until then, I suggest using the workaround provided above as a stop gap.

@chronosis

This comment has been minimized.

Copy link

chronosis commented Jan 8, 2019

Alternative Workaround:

You can completely bypass the Travis CLI file encryption which exists mostly as a convenience and encrypt multiple files directly with OpenSSL. This is essentially what the Travis CLI does on its own. Using this method also allows you to reuse the same key / iv to encrypt multiple files.

Use openssl to create a key & iv values:

$ openssl enc -aes-256-cbc -k secret -P -md sha1

Then copy the Key and IV values.

And variables and encrypt files:

$ travis env set YOUR_KEY key_value_in_hex -p
$ travis env set YOUR_IV iv_value_in_hex -p
$ openssl aes-256-cbc -K key_value_in_hex -iv iv_value_in_hex -in file_to_enc.data -out file_to_enc.data.enc
$ openssl aes-256-cbc -K key_value_in_hex -iv iv_value_in_hex -in file2_to_enc.data -out file2_to_enc.data.enc

Add lines to .travis.yml:
Inside the .travis.yml file manually add the lines to before_install: section.

- openssl aes-256-cbc -K $YOUR_KEY -iv $YOUR_IV -in file_to_enc.data.enc -out file_to_enc.data -d
- openssl aes-256-cbc -K $YOUR_KEY -iv $YOUR_IV -in file2_to_enc.data.enc -out file2_to_enc.data -d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment