passlib segfaults when keystone is sent a large password

[Dan Prince]

Using the latest keystone Essex builds (KSL).

I can segfault keystone by sending in a large value for password in the following request to http://localhost:5000/v2.0/tokens:

{"auth":{"tenantName":"admin","passwordCredentials":{"username":"admin","password":"A REALLY LARGE TEXT STRING"}}}

The issue appears to stem code in common/utils.py where we UTF8 encode the raw password string and then try to verify the hash via passlib.hash.sha512_crypt.verify.

--------

def check_password(password, hashed):
    """Check that a plaintext password matches hashed.

    hashpw returns the salt value concatenated with the actual hash value.
    It extracts the actual salt if this value is then passed as the salt.

    """
    if password is None:
        return False
    password_utf8 = password.encode('utf-8')
    val = passlib.hash.sha512_crypt.verify(password_utf8, hashed)
    return val

--------

Sample Ruby script to reproduce the issue:

require 'rubygems'
require 'openstack/compute'

USERNAME=ENV['NOVA_USERNAME']
API_KEY=ENV['NOVA_API_KEY']
API_URL=ENV['NOVA_URL']

bigboy = "0" * 9999999

conn=OpenStack::Compute::Connection.new(:username => USERNAME, :api_key => bigboy, :auth_url => API_URL, :service_name => 'compute', :retry_auth => false, :is_debug => true)

[Russell Bryant (russellb)]

The root cause appears to be a bug in passlib. We should write a simple reproducer and report it upstream.

We should still work around the bug in keystone, obviously. Since this version of keystone hasn't been "released" yet, I don't think we need to go through the information embargo and security advisory on this. Thoughts?

Also, is this function used anywhere else in OpenStack that may also need a workaround put in place?

[Russell Bryant (russellb)]

Also, as discussed on IRC, can you check diablo keystone?

[Dan Prince (dan-prince)]

One more thing to add to the initial bug report. I'm using python-passlib-1.5.3-1.fc16.noarch (on Fedora 16).

Russell: Will check on diablo...

[Thierry Carrez (ttx)]

It's OK to open and fix directly in Keystone if it doesn't affect diablo.
Adding PTL

[Dan Prince (dan-prince)]

Just tried in on Fedora 16 using Keystone Diablo. Boom!

[root@login keystone]# keystone
Starting the RAX-KEY extension
Starting the Legacy Authentication component
Service API listening on 0.0.0.0:5000
Admin API listening on 0.0.0.0:35357
Segmentation fault

[Thierry Carrez (ttx)]

@Dan: any chance you could attach patches here that would minimally fix the issue in old-Keystone and current-keystone ?

[Thierry Carrez (ttx)]

Then we can have Keystone coredevs pre-approve the patch and push it down to the downstream stakeholders with an embargo compatible with RC1 publication.

[Thierry Carrez (ttx)]

Option 1 is to silently patch it in RC1 while we work on coordinated disclosure with passlib / fix in Diablo.

[Dan Prince (dan-prince)]

ttx: yes. I'll attach some patches to work around the issue this evening.

[Dan Prince (dan-prince)]

Attached is a patch for Essex with adds a MAX_PASSWORD_LENGTH global to common/utils.py. Includes a test case which will segfault the test runner if the associated code in trunc_password is removed or short circuited.

[Dan Prince (dan-prince)]

Attached patch fix for diablo.

[Thierry Carrez (ttx)]

We will work around the issue in Essex for RC1, and coordinate/embargo the fix with passlib (together with the Diablo fix).

[Dan Prince (dan-prince)]

ttx:

work around proposal for Essex is here:

https://review.openstack.org/#change,5507

Associated work around bug:

https://bugs.launchpad.net/keystone/+bug/959288

[Thierry Carrez (ttx)]

Adding Dolph and Jesse to get reviewers in the know.

[Joseph Heck (heckj)]

Just added a review to 5507 +2

[Jesse Andrews (anotherjesse)]

Thanks for the heads up, I'm approving 5507

[Thierry Carrez (ttx)]

Updated title. Assigning to Russell who will coordinate disclosure with stakeholders and upstream passlib.

[Russell Bryant (russellb)]

After further analysis and discussion, the root cause is actually in glibc. Pádraig Brady pointed out that this issue has been discussed in the public before:

    http://www.openwall.com/lists/oss-security/2011/11/15/1

It's not clear whether this has ever been reported to glibc, though. I'm still working on that. At first I was thinking we could just proceed since it has been brought up in the public, but now I'm thinking we should wait on a response from glibc maintainers before proceeding with anything public here. I definitely don't want to get caught up in a situation where anyone thinks we mishandled a security issue in glibc.

In the meantime, we need explicit approval of the diablo patch from keystone-core on this bug prior to notification being sent to downstream stakeholders. Joseph, can you take a look at the diablo patch?

[Russell Bryant (russellb)]

The report to glibc was deemed to not be a security issue, so we can proceed. Here is a proposed vulnerability description:

Title: Extremely long passwords can crash Keystone
Impact: High
Reporter: Dan Prince <email address hidden>
Products: Keystone
Affects: All versions

Description:
Dan Prince reported a vulnerability in Keystone. He discovered that you can remotely trigger a crash in Keystone by sending an extremely long password. When Keystone is validating the password, glibc allocates space on the stack for the entire password. If the password is long enough, stack space can be exhausted, resulting in a crash. This vulnerability is mitigated by a patch to impose a reasonable limit on password length (4 kB).

[Joseph Heck (heckj)]

Looked at the Diablo patch- makes good sense to me.

I'm not entirely clear on our internal process (or my responsibilities to it) to enable that patch into existing for a previous release. Do we submit it in through gerrit and approve per normal, or is there a different mechanism for the prior releases?

[Russell Bryant (russellb)]

Thanks for looking. You can find detailed information on the process for these types of issues here:

http://wiki.openstack.org/VulnerabilityManagement

The next thing for you to do with the patch will be to help get the patch expedited through Gerrit when the time comes. That time will be the CRD (Coordinated Release Date) that we decide on. What will happen is I will email this patch and the vulnerability description to a list of stakeholders that include packagers and some public cloud deployments. That email will get sent out as soon as the vulnerability description is approved. Then, everyone will release the patch on the same day.

Regarding the description, I need that approved by both you (as the PTL) and the reporter (Dan), and then I'll send it out.

For the CRD, I propose Tuesday, March 27th.

[Russell Bryant (russellb)]

Actually, Dan is out today. I'm sure he won't mind if I send it out without his ack on the description, as long as I get an ack from you guys on it.

[Joseph Heck (heckj)]

I'm good with that description.

[Russell Bryant (russellb)]

Vulnerability notification email has been sent to downstream stakeholders. The CRD for this issue is Tuesday, March 27th, 1500 UTC.

[Thierry Carrez (ttx)]

Adding stable/diablo maintainers in the loop

[Russell Bryant (russellb)]

Made the issue public since the patch is now up on gerrit

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/5865
Committed: http://github.com/openstack/keystone/commit/7b07f870702de5675d4423042e8b018e3fc4b931
Submitter: Jenkins
Branch: stable/diablo

commit 7b07f870702de5675d4423042e8b018e3fc4b931
Author: Dan Prince <email address hidden>
Date: Fri Mar 16 23:38:19 2012 -0400

    Add MAX_PASSWORD_LENGTH check in backendutils.

    Add a check for max password length to password hash/check
    functions in backendutils.

    Fixes an issue where large passwords can cause segfaults in
    keystone diablo.

    Fixes LP Bug #957359.

    Change-Id: I24de1a295d5f54a070750315e78db968658898d3

[Russell Bryant (russellb)]

For the record, here is the related patch to glibc:

http://sourceware.org/ml/libc-alpha/2012-03/msg01138.html
http://sourceware.org/ml/libc-alpha/2012-03/msg01158.html