hiera-eyaml icon indicating copy to clipboard operation
hiera-eyaml copied to clipboard

Empty values confuse `eyaml decrypt`

Open amosshapira opened this issue 8 years ago • 14 comments

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 avatar Nov 04 '15 00:11 amosshapira

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.

amosshapira avatar Nov 04 '15 00:11 amosshapira

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?

elyscape avatar Nov 04 '15 00:11 elyscape

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.

amosshapira avatar Nov 04 '15 00:11 amosshapira

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 https://github.com/TomPoulton/hiera-eyaml/issues/179#issuecomment-153532229 .

sihil avatar Nov 04 '15 09:11 sihil

I'll make a PR shortly.

elyscape avatar Nov 04 '15 18:11 elyscape

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.

elyscape avatar Nov 04 '15 21:11 elyscape

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. :-)

RoUS avatar Nov 05 '15 19:11 RoUS

@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.

amosshapira avatar Nov 06 '15 03:11 amosshapira

Any movement on this issue? Thanks much in advance.

mjburling avatar Aug 05 '17 20:08 mjburling

@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!

rnelson0 avatar Aug 07 '17 13:08 rnelson0

@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.

mjburling avatar Aug 07 '17 14:08 mjburling

@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 avatar Jul 29 '21 15:07 pillarsdotnet

@rnelson0 - Can I get a review?

pillarsdotnet avatar Jul 29 '21 22:07 pillarsdotnet

@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.

pillarsdotnet avatar Aug 05 '21 16:08 pillarsdotnet