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

Empty values confuse eyaml decrypt #179

Open
amosshapira opened this issue Nov 4, 2015 · 14 comments
Open

Empty values confuse eyaml decrypt #179

amosshapira opened this issue Nov 4, 2015 · 14 comments

Comments

@amosshapira
Copy link

amosshapira commented Nov 4, 2015

When editing an .eyaml file like this:

---
key_with_non_empty_value_1: 'DEC::GPG[value of key with non empty value 1]!'
key_with_empty_value_1: 'DEC::GPG[]!'
key_with_non_empty_value_2: 'DEC::GPG[value of key with non empty value 2]!'

The encrypted file shows only the first two keys (truncated for brevity):

---
key_with_non_empty_value_1: 'ENC[GPG,...
key_with_empty_value_1: 'ENC[GPG,...

And the third key doesn't appear in the encrypted file.

eyaml edit issue-example.eyaml shows the decrypted file just fine:

#| This is eyaml edit mode. This text (lines starting with #| at the top of the
#| file) will be removed when you save and exit.
#|  - To edit encrypted values, change the content of the DEC(<num>)::PKCS7[]!
#|    block (or DEC(<num>)::GPG[]!).
#|    WARNING: DO NOT change the number in the parentheses.
#|  - To add a new encrypted value copy and paste a new block from the
#|    appropriate example below. Note that:
#|     * the text to encrypt goes in the square brackets
#|     * ensure you include the exclamation mark when you copy and paste
#|     * you must not include a number when adding a new block
#|    e.g. DEC::PKCS7[]! -or- DEC::GPG[]!

---
key_with_non_empty_value_1: 'DEC(1)::GPG[value of key with non empty value 1]!'
key_with_empty_value_1: 'DEC(3)::GPG[]!'
key_with_non_empty_value_2: 'DEC::GPG[value of key with non empty value 2]!'

But notice how the DEC::GPG of the third key (key_with_non_empty_value_2) doesn't have a a number. It feels like it was somehow 'swallowed' by the empty value of the previous key.

Then eyaml decrypt -e issue-example.eyaml outputs:

$ eyaml decrypt -e puppet/hieradata/issue-example.eyaml
[hiera-eyaml-core] Loaded config from /Users/amos.shapira/.eyaml/config.yaml

---
key_with_non_empty_value_1: 'DEC::GPG[value of key with non empty value 1]!'
key_with_empty_value_1: 'DEC::GPG[]!'
key_with_non_empty_value_2: 'DEC::GPG[value of key with non empty value 2]!'

Which looks OK (except again - the missing number on the third key suggest that something is broken).
But eyaml decrupt -f issue-example.eyaml comes up with broken values:

$ eyaml decrypt -f puppet/hieradata/issue-example.eyaml
[hiera-eyaml-core] Loaded config from /Users/amos.shapira/.eyaml/config.yaml

---
key_with_non_empty_value_1: 'value of key with non empty value 1'
key_with_empty_value_1: ']!'
key_with_non_empty_value_2: 'DEC::GPG[value of key with non empty value 2'

I'm trying to use -f because it doesn't include the eyaml-specific quoting so I can pass this decrypted .yaml file to regular hiera (e.g. when using master-less setup, or in Packer like image burning scenario).

Whether I'm wrong in using -f on a .eyaml file or not, the encrypted file seems to be broken.

Without looking at the code yet, my hunch is that the regular expression used to match the inside of the decrypted value expects at least one character before the closing ]! and therefore misses the first one and continues until the end of the next encrypted value.

@amosshapira
Copy link
Author

A very quick look at the code, perhaps the +? in https://github.com/TomPoulton/hiera-eyaml/blob/master/lib/hiera/backend/eyaml/parser/encrypted_tokens.rb#L86 triggers a minimum requirement of one character in the value.
Replacing the empty value by a space corrects the problem (but makes the value ' ' instead of empty).

A temporary work-around that I found is to set the value to ENC::GPG['']!. That way eyaml is happy with non-empty value but hiera/yaml gets an empty string.

@elyscape
Copy link
Contributor

elyscape commented Nov 4, 2015

That's likely the problem, yeah, but not on that line. I don't believe it's valid to have the encrypted values be blank. Decrypted values, though, should be fine when blank. As such, lines 116 and 126 should probably be changed to use *? instead of +?. Could you test that out on your local machine and report back on if it works?

@amosshapira
Copy link
Author

Your proposed fix on lines 116 and 126 works. I think that empty values are OK since I use it outside eyaml too.

Results with the fix:

  1. The encrypted file has all expected entries.
  2. edited file is:
---
key_with_non_empty_value_1: 'DEC(1)::GPG[value of key with non empty value 1]!'
key_with_empty_value_1: DEC(3)::GPG[]!
key_with_non_empty_value_2: 'DEC(5)::GPG[value of key with non empty value 2]!'
  1. eyaml decrypt -e works right:
$ eyaml decrypt -e puppet/hieradata/issue-example.eyaml
---
key_with_non_empty_value_1: 'DEC::GPG[value of key with non empty value 1]!'
key_with_empty_value_1: DEC::GPG[]!
key_with_non_empty_value_2: 'DEC::GPG[value of key with non empty value 2]!'
  1. eyaml decrypt -f also works right:
$ eyaml decrypt -f puppet/hieradata/issue-example.eyaml
---
key_with_non_empty_value_1: 'value of key with non empty value 1'
key_with_empty_value_1:
key_with_non_empty_value_2: 'value of key with non empty value 2'

This is based on the following versions of hiera-eyaml and hiera-eyaml-gpg:

hiera-eyaml (2.0.8)
hiera-eyaml-gpg (0.6)

Thanks.

@sihil
Copy link
Collaborator

sihil commented Nov 4, 2015

I never considered empty strings when I wrote the parsing code, but can't
think of a good reason to not support it. The ciphertext will still be
sizable.

As you both say, I expect that changing the two regex will solve the issue

  • worth adding a test as well if you are raising a PR.

On Wed, 4 Nov 2015 00:22 Amos Shapira [email protected] wrote:

Will try to test and report back.


Reply to this email directly or view it on GitHub
#179 (comment)
.

@elyscape
Copy link
Contributor

elyscape commented Nov 4, 2015

I'll make a PR shortly.

@elyscape
Copy link
Contributor

elyscape commented Nov 4, 2015

I've run into some other problems with getting it to register as an empty string vs. a nil value. I have some ideas for how to fix it but it'll take a little bit longer that initially anticipated.

@RoUS
Copy link

RoUS commented Nov 5, 2015

key: DEC::GPG['']! is probably not the workaround. Hiera doesn't get an empty string, it gets a string consisting of two ticks.

We've been encountering this as well. :-)

@amosshapira
Copy link
Author

@RoUS you might be right about the literal value being '', but perhaps what works for me is that the way I use it it's treated as an empty string. YMMV.

@mjburling
Copy link

Any movement on this issue? Thanks much in advance.

@rnelson0
Copy link
Sponsor Member

rnelson0 commented Aug 7, 2017

@mjburling Thanks for poking this issue. It doesn't look like anyone opened a related PR, yet. Is that something you're interested in doing? If not, I can look into it but it may not happen very soon. Thanks!

@mjburling
Copy link

@rnelson0 I came to the same conclusion as @sihil and see no reason for this not to be supported. For my specific use case, it's a crumby default empty-string password for ephemeral environments. For hygiene's sake, I'd like to obscure that fact.

If @elyscape could point me toward his/her work toward this issue, I would be interested in continuing to completion.

@pillarsdotnet
Copy link

pillarsdotnet commented Jul 29, 2021

@rnelson0 -- I submitted #322 adding tests for blank values and #323 to fix the parsing problem. The fix is trivial:

--- a/lib/hiera/backend/eyaml/parser/encrypted_tokens.rb
+++ b/lib/hiera/backend/eyaml/parser/encrypted_tokens.rb
@@ -108,7 +108,7 @@ class Hiera

         class EncHieraTokenType < EncTokenType
           def initialize
-            @regex = %r{ENC\[(\w+,)?([a-zA-Z0-9+/ =\n]+?)\]}
+            @regex = %r{ENC\[(\w+,)?([a-zA-Z0-9+/ =\n]*?)\]}
             @string_token_type = EncStringTokenType.new
           end

@@ -119,7 +119,7 @@ class Hiera

         class EncStringTokenType < EncTokenType
           def initialize
-            @regex = %r{ENC\[(\w+,)?([a-zA-Z0-9+/=]+?)\]}
+            @regex = %r{ENC\[(\w+,)?([a-zA-Z0-9+/=]*?)\]}
           end

           def create_token(string)

pillarsdotnet pushed a commit to pillarsdotnet/hiera-eyaml that referenced this issue Jul 29, 2021
pillarsdotnet pushed a commit to pillarsdotnet/hiera-eyaml that referenced this issue Jul 29, 2021
@pillarsdotnet
Copy link

@rnelson0 - Can I get a review?

@pillarsdotnet
Copy link

@rnelson0 @kenyon @bastelfreak -- Is there anybody who is familiar enough with cucumber to help with testing? I modified the tests in #322 but the tests passed, which indicates that either the tests ae inadequate or the reported bug does not actually exist.

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

No branches or pull requests

7 participants