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

2019.2.0 binary pillar unicode error returns. #51879

Closed
whytewolf opened this issue Feb 27, 2019 · 5 comments

Comments

@whytewolf
Copy link
Contributor

commented Feb 27, 2019

Description of Issue/Question

It looks like #46934 has come back in 2019.2.0

2019-02-27 11:34:43,831 [salt.pillar      :741 ][CRITICAL][31367] Rendering SLS 'foo' failed, render error:
'ascii' codec can't decode byte 0x8b in position 0: ordinal not in range(128)
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/salt/pillar/__init__.py", line 736, in render_pstate
    **defaults)
  File "/usr/lib/python2.7/site-packages/salt/template.py", line 101, in compile_template
    ret = render(input_data, saltenv, sls, **render_kwargs)
  File "/usr/lib/python2.7/site-packages/salt/renderers/gpg.py", line 350, in render
    return _decrypt_object(gpg_data, translate_newlines=translate_newlines)
  File "/usr/lib/python2.7/site-packages/salt/renderers/gpg.py", line 329, in _decrypt_object
    translate_newlines=translate_newlines)
  File "/usr/lib/python2.7/site-packages/salt/renderers/gpg.py", line 325, in _decrypt_object
    return _decrypt_ciphertexts(obj, translate_newlines=translate_newlines)
  File "/usr/lib/python2.7/site-packages/salt/renderers/gpg.py", line 307, in _decrypt_ciphertexts
    ret, num = GPG_CIPHERTEXT.subn(lambda m: _decrypt_ciphertext(m.group()), cipher)
UnicodeDecodeError: 'ascii' codec can't decode byte 0x8b in position 0: ordinal not in range(128)
2019-02-27 11:34:44,223 [salt.pillar      :1036][CRITICAL][31367] Pillar render error: Rendering SLS 'foo' failed. Please see master log for details.

Setup

Same setup as #46934

Steps to Reproduce Issue

binary gpg render setup.

Versions Report

Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: 1.11.5
       cherrypy: Not Installed
       dateutil: 1.5
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Oct 30 2018, 23:45:53)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.6.1810 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-957.1.3.el7.x86_64
         system: Linux
        version: CentOS Linux 7.6.1810 Core
@whytewolf whytewolf added the ZD label Feb 27, 2019
@whytewolf

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

ZD-3312

@waynew

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

I'm still trying to wrap my head around whether or not this truly is a bug, or it's just a feature that we don't yet support.

My understanding is that we only support base64 encoded binary data in pillars. It appears that what's happening here is that when binary data is being decrypted (and contains some non-ascii values), it breaks the pillar - which currently is expected - if I create this pillar:

foopillar: "Hey dude ~K~Kllo"

(where those ~K are literally 0x8b chars. In vim edit your pillar that's got say, Hey dude hello and then run :%!xxd, change the two he bytes to 8b8b then run :%!xxd -r)

and try to run state.apply, I also get a UnicodeDecodeError.


It's actually super interesting to me that this used to be a thing that maybe worked - from what I can tell that's actually more of an accident due python2 treating bytes and text the same. I've been able to change the gpg renderer to return the decrypted pillar, but the failure ends out percolating up because it expects pillar data to be ascii/unicode, and not binary data.

@waynew

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

Oh. This is interesting. Apparently if you stick a null byte in your binary data, it will happily dump the contents of the pillar, at least with the few changes I've made. I still have to gpg the text, but it will actually call it a binary file. It may make sense that if we fail decoding pillar contents that we should assume that it's binary data...

@waynew

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

Looks like my test changes were definitely part of letting it get that far. I'll discuss with @saltstack/team-core about what approach I should take here.

@KChandrashekhar KChandrashekhar added this to In progress in Salt Open Workspace Mar 18, 2019
@waynew waynew added this to In Progress in Salt Core Workspace Mar 20, 2019
@waynew

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

I've run into another bit of a hurdle.

It's clear that we have some challenges to get to a point where we have cohesive bytes/text story. The original behavior only existed in the first place because Python2 was terribly lazy at text vs. binary data, so it was pretty implicit about just writing strings including binary data.

When we started to support (now) legacy Python and Python3 it looks like there was an implicit understanding that pillars would be textual data (that is, ASCII or Unicode), along with basically everything else. It's unfortunate, because technically speaking there's no real reason that we can't be sticking binary data into pillars - in fact, YAML has a binary datatype.

From what it looks, there is a possibility that serializing binary data via JSON could be problematic, though - which may only come into play for salt-ssh but is something that we should definitely be aware of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Salt Open Workspace
  
In progress
4 participants
You can’t perform that action at this time.