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

Portainer does not send auth password correctly if it contains "<" #2199

Closed
halfer opened this issue Aug 21, 2018 · 8 comments
Closed

Portainer does not send auth password correctly if it contains "<" #2199

halfer opened this issue Aug 21, 2018 · 8 comments
Assignees
Milestone

Comments

@halfer
Copy link

halfer commented Aug 21, 2018

I used my password manager to generate a complex password, thus:

WP<FjH\MM]w=@Al2f&Im'nT",~su9I5%Ujlabcef

As you can see, the third character is a standard less-than symbol. This should be a legitimate password as it is very hashable, and indeed it was accepted by the password change system.

I then logged out and logged back in again, and this password (plus the custom username) no longer works. I get an error supplied with a 422 status code:

Invalid credentials

I found out why: the JavaScript to transmit the auth attempt corrupts the password. Using my network tab in Firefox, this is what is sent to the auth AJAX endpoint:

{"username":"my-admin","password":"WP"}

If I amend the JSON POST operation by putting the password in (and escaping special chars /") then I get a 200.

I am running the official image portainer/portainer sha256:ab096b92ed177b47adfa8a9a99e304d36596efa557b9627c066cee164cc39910.

Technical details:

  • Portainer version: 1.19.1
  • Docker version (managed by Portainer): 18.03.1-ce
  • Platform (windows/linux): Linux 4.4.0-109-generic 132-Ubuntu SMP x86_64 x86_64 x86_64 GNU/Linux
  • Command used to start Portainer:
docker run \
    --label portainer-instance \
    --network dockernet \
    --network-alias portainer \
    --detach \
    --restart always \
    --volume /var/run/docker.sock:/var/run/docker.sock \
    --volume portainer_data:/data \
    portainer/portainer
  • Browser: Firefox on Mint

My workaround in the short-term will be to change the password to remove the symbol. I believe this can be forced on the console temporarily.

@deviantony
Copy link
Member

This is due to the fact that we sanitize the username and password when creating a user and when authenticating. I believe that we should remove the sanitization process.

@halfer
Copy link
Author

halfer commented Aug 22, 2018

Have a 9,000th star for your work! 🥇

@chiptus
Copy link
Contributor

chiptus commented Aug 23, 2018

it is $sanitize that changes the password, but I wonder why you can't login. $sanitize runs also when you make a user, change pass, login, create admin

@halfer
Copy link
Author

halfer commented Aug 23, 2018

@chiptus: I tried this a second time, and in fact got the result you describe. For the first go, I was monkeying around with saved passwords and faked JSON requests quite a bit, so I probably got something wrong.

Incidentally, I wonder if there needs to be a notice that goes out with this release, to help manage unexpected issues. For affected users, any usernames/passwords saved in browsers or password keepers will no longer work.

@deviantony
Copy link
Member

Incidentally, I wonder if there needs to be a notice that goes out with this release, to help manage unexpected issues. For affected users, any usernames/passwords saved in browsers or password keepers will no longer work.

Do you mean that if we fix this the actual password will no longer work? We're aware of this and working on it in #2202.
Our idea is to authenticate without the sanitization call first, if it fails then authenticate with the sanitization call. If the second authentication succeed, the user will be asked to update his password (with an information message).

@halfer
Copy link
Author

halfer commented Aug 24, 2018

Do you mean that if we fix this the actual password will no longer work?

Yes, a browser-saved password won't work. In my above example, the browser thinks the password is WP<FjH\MM]w=@Al2f&Im'nT",~su9I5%Ujlabcef, but the password supplied to the auth creation system was WP (so that will be the password that was hashed and stored).

Our idea is to authenticate without the sanitization call first, if it fails then authenticate with the sanitization call. If the second authentication succeed, the user will be asked to update his password (with an information message).

That would certainly be convenient, but it would continue to expose a small number of users to weaker passwords than they intended. I'd not be opposed to this, as long as that code was removed in a few months, otherwise it would prolong the problem indefinitely.

The alternative would be to let users bump into an access-denied message, and give clear instructions in your support channels about how to change the password on the console; while inconvenient, it does fix the weak password issue immediately. We could argue that Portainer users will generally be the kind of people who can fix this themselves, since they will usually be software or operations engineers.

Ultimately security/convenience/support are trade-offs that you'll need to decide on based on principles set out for this project. To some degree it may depend on how many people are affected by this - folks using password generators is pretty rare, so I'd go with 5%, and of course some auto-generated passwords won't contain angle brackets. Do you know roughly how many installs you have (e.g. do Portainer instances check for updates, so you have a rough ongoing idea?)

@deviantony
Copy link
Member

That would certainly be convenient, but it would continue to expose a small number of users to weaker passwords than they intended. I'd not be opposed to this, as long as that code was removed in a few months, otherwise it would prolong the problem indefinitely.

New users would not be affected by this issue anymore. I believe that will be able to remove that legacy support in a few months, yes.

@halfer
Copy link
Author

halfer commented Aug 24, 2018

Great, sounds like a plan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants