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

rest_cherrypy eauth can't handle some characters #33023

Closed
cmclaughlin opened this issue May 3, 2016 · 7 comments
Closed

rest_cherrypy eauth can't handle some characters #33023

cmclaughlin opened this issue May 3, 2016 · 7 comments
Assignees
Labels
Documentation Relates to Salt documentation expected-behavior intended functionality fixed-pls-verify fix is linked, bug author to confirm fix Salt-API
Milestone

Comments

@cmclaughlin
Copy link
Contributor

cmclaughlin commented May 3, 2016

I think I found a bug... some system passwords don't work with rest_cherrypy. I guess eauth can be used for other things... but I'm just using it for the API... so I'm not certain the scope of this bug.

My master is configured for the like this (I have apache in front, but I've tested with and without it):

 rest_cherrypy:
   port: 8000
   host: 127.0.0.1
   disable_ssl: True
   webhook_disable_auth: True
   webhook_url: /hook

external_auth:
  pam:
    myapiuser:
      - .*

If the myapiuser system user's password includes any of these characters auth fails:

+ & ;

I tested all the special characters on my keyboard and only observed the problem when one of those three is in the password.

Here's how I'm authenticating against the API:

curl -ksi http://localhost:8000/login -H "Accept: application/json" -d username='myapiuser' -d password='1234+' -d eauth='pam'

I use the system passwd command to set the user's password... simply 1234+ in this example.

I tried running the api in debug mode and don't see anything interesting... nothing in syslog, etc. I just see this in the master log:

2016-05-03 19:10:08,526 [salt.master      ][WARNING ][16829] Authentication failure of type "eauth" occurred.

Here's my system report:

Salt Version:
           Salt: 2015.8.3

Dependency Versions:
         Jinja2: 2.7.2
       M2Crypto: Not Installed
           Mako: 0.9.1
         PyYAML: 3.10
          PyZMQ: 14.0.1
         Python: 2.7.6 (default, Mar 22 2014, 22:59:56)
           RAET: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.4
           cffi: Not Installed
       cherrypy: 3.2.2
       dateutil: 2.4.2
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
        libnacl: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.3.0
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
   python-gnupg: 0.3.6
          smmap: 0.8.2
        timelib: Not Installed

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-48-generic
         system: Ubuntu 14.04 trusty
@whiteinge
Copy link
Contributor

Thanks for the thorough report!

@whiteinge whiteinge added Bug broken, incorrect, or confusing behavior Salt-API labels May 3, 2016
@whiteinge
Copy link
Contributor

I can reproduce this using PAM on OS X. Doesn't affect the Salt CLI. Looks specific to rest_cherrypy and Saltnado.

@whiteinge whiteinge added severity-low 4th level, cosemtic problems, work around exists P4 Priority 4 labels May 3, 2016
@whiteinge whiteinge added this to the Approved milestone May 3, 2016
@whiteinge whiteinge self-assigned this May 3, 2016
@whiteinge
Copy link
Contributor

Narrowing it down. This is running afoul of the urlencoded content type. A quick workaround is to send JSON instead:

curl -ksi http://localhost:8000/login \
    -H "Accept: application/json" \
    -H "Content-type: application/json" \
    -d'{"username": "myapiuser", "password": "1234+", "eauth": "pam"}'

@whiteinge
Copy link
Contributor

whiteinge commented May 4, 2016

So turns out this is a curl thing -- and news to me. The -d flag will set the content-type of the request to x-www-form-urlencoded but it does not actually urlencode the input. You need to use the --data-urlencode flag for that. 😝

The following should do it:

curl -ksi http://localhost:8000/login \
    -H "Accept: application/json" \
    -d username='myapiuser' \
    --data-urlencode password='1234+' \
    -d eauth='pam'

@whiteinge whiteinge added expected-behavior intended functionality and removed Bug broken, incorrect, or confusing behavior P4 Priority 4 severity-low 4th level, cosemtic problems, work around exists labels May 4, 2016
@cmclaughlin
Copy link
Contributor Author

Thanks for figuring that out. I was thinking it was something like that, but I didn't think + or ; meant anything special with URLs.

Maybe I should update the docs... The examples given don't use a password that would be a problem, but it could save someone else some time.

@whiteinge
Copy link
Contributor

Yeah, good call. How about "A note about curl" under the "Usage" section in the docs?

@rallytime rallytime added the Documentation Relates to Salt documentation label May 4, 2016
jacobhammons added a commit to jacobhammons/salt that referenced this issue Jun 14, 2016
Removes the "Full list of builtin ..." from each module reference list, leaving just the module type for scanability.

Refs saltstack#12470
Refs saltstack#10206
Refs saltstack#10480
Refs saltstack#23522
Refs saltstack#33023
@jacobhammons
Copy link
Contributor

@cmclaughlin I was in the source yesterday so I went ahead and added this: https://docs.saltstack.com/en/latest/ref/netapi/all/salt.netapi.rest_cherrypy.html#a-note-about-curl

If you think this fix is sufficient please close. Thanks!

@jacobhammons jacobhammons added the fixed-pls-verify fix is linked, bug author to confirm fix label Jun 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Relates to Salt documentation expected-behavior intended functionality fixed-pls-verify fix is linked, bug author to confirm fix Salt-API
Projects
None yet
Development

No branches or pull requests

4 participants