passlib failure to sanitize env variables PASSLIB_MAX_PASSWORD_SIZE

[Kurt Seifried]

Grant Murphy originally reported:

* Usage of passlib

  The keystone server does not appear to sanitize the environment when
  starting. This means that an unintended value can be set for
  PASSLIB_MAX_PASSWORD_SIZE. Which will overwrite the default value of
  4096 and potentially cause an unhandled passlib.exc.PasswordSizeError.
  We should ensure sensible defaults are applied here prior to loading passlib.

If this is exploitable it will need a CVE, if not we should still harden it so it can't be monkeyed with in the future.

[Thierry Carrez (ttx)]

I don't see this as exploitable, as you'd have to locally control the environment for the keystone user, which means control of that user which means pretty much controlling Keystone anyway ?

Fully agree that we can strengthen that part to avoid it being monkeyed with in the future. With your permission, I'd open this bug publicly and let it be strengthened in public patches.

[Thierry Carrez (ttx)]

Will open tomorrow unless someone raises a flag.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/43322

[David Stanek (dstanek)]

How should we proceed here? The patch submitted for this just hard codes the value of PASSLIB_MAX_PASSWORD_SIZE to 4096. Is that the correct thing to do or should we allow it to be configured?

If it can be configured do we need to have boundaries we can validate against (like a lower bounds of 1024 and and upper bounds of 16384)?

[Dolph Mathews (dolph)]

I would assume it should be end-user configurable (which it is today), but that seems to be completely at odds with the bug report. I can't really think of a simpler solution than the one proposed above; I'd appreciate feedback from Grant Murphy on the patch itself.

[Grant Murphy (gmurphy)]

So passlib defines the maximum password size as -

MAX_PASSWORD_SIZE = int(os.environ.get("PASSLIB_MAX_PASSWORD_SIZE") or 4096)

In this situation -

import passlib.utils
if CONF.identity.max_password_length > passlib.utils.MAX_PASSWORD_SIZE:
   # we might have problems.

One potential alternative to the proposed fix is to explicitly set the PASSLIB_MAX_PASSWORD_SIZE to the size defined in the configuration file. This would also act as a failsafe for any cases where max password length checks were missed.

[Morgan Fainberg (mdrnstm)]

Instead of returning HTTP 500 (ISE), the simplest fix is to just return HTTP 400. Lets be fair, if a deployer configures a longer maxium password than passlib can handle, it's either a 500 or a 400. In this case we can determine what the correct size would be and we should raise up a 400 Bad Request.

This is an edge case, but handling it elegantly is better than letting it just fail in an ugly way.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/98296

[OpenStack Infra (hudson-openstack)]

Change abandoned by Morgan Fainberg (<email address hidden>) on branch: master
Review: https://review.openstack.org/98296
Reason: Based upon discussions with David Stanek, This should just be setting a max in oslo.config which is waiting for a fix to be released.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/217449

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/217449
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=a7235fc0511c643a8441efd3d21fc334535066e2
Submitter: Jenkins
Branch: master

commit a7235fc0511c643a8441efd3d21fc334535066e2
Author: Eric Brown <email address hidden>
Date: Wed Aug 26 17:38:04 2015 -0700

    Set max on max_password_length to passlib max

    With this patch if someone overrides the PASSLIB_MAX_PASSWORD_SIZE
    environment variable, keystone will fail to start with a config
    error instead of a passlib.exc.PasswordSizeError when creating
    a user.

    Change-Id: Ic59a7964d8044ba3ab7cb6539fecca1d190dbbcc
    Closes-Bug: #1175905