SQL injection through limit parameter

Note that this issue raises a new vulnerability process question: should we release advisories for a project that was never officially released ? (My take on it is that it's sufficiently serious for us to do an advisory even if no "official" version exists)

Adding Ziad as keystone PTL -- please look at the patch and pre-approve it. Please do not make this issue public until the end of the embargoed period (still undetermined).

I wouldn't be surprised if the CVE allocation folks would consider this all to be an SQLAlchemy vulnerability. After all, it should be the responsibility of the ORM to protect you against this kind of stuff. We'll see how it goes.

Proposed description for stakeholders:

Title: SQLAlchemy/Keystone SQL injection through limit handling
Impact: Critical
Reporter: Nikita Savin
Products: Keystone
Affects: All versions

Description:
Nikita Savin from GridDynamics reported a vulnerability in Keystone's usage of SQLAlchemy. Before version 0.7.0, SQLAlchemy did not sanitize the contents of the limit clause, potentially allowing SQL injection. Keystone directly passes user-provided values to the SQLAlchemy call, triggering SQL injection on vulnerable SQLAlchemy versions, potentially resulting in complete compromise of the authentication database. Only setups using SQLAlchemy < 0.7.0 are affected.

Patch: The attached patch works around the issue in Keystone itself, by coercing limit values to integer. Alternatively, this can be fixed by upgrading to SQLAlchemy >= 0.7.0.

Note: This could all be considered a vulnerability in SQLAlchemy, and a CVE assigned against that (silently-fixed) issue.

I do agree that this is a SQLAlchemy vulnerability and not so much a keystone vulnerability. I can work this issue from that angle. I'll look into getting a CVE allocated for it. I think it's still worth posting an OpenStack security advisory about this to help make users aware of the problem. The advisory can just reference the SQLAlchemy CVE.

As to the general process question, I think that if a project is a core Openstack project, it's a good idea to release a security advisory, even if there hasn't been an official release yet.

I have contacted the security response team at Red Hat about the SQLAlchemy issue. They will handle allocating a CVE for it and coordinating with linux-distros as necessary.

    https://access.redhat.com/security/team/
    https://access.redhat.com/security/team/contact/

Once I hear something back, I'll note it here so that it can be referenced in the OpenStack advisory.

I'll incorporate Nikita's patch into the code for E3 (patch already submitted)

Russell: +1
We just need to rewrite the description so that's it's clear the security advisory is about an SQLAlchemy vulnerability that affects Keystone negatively, and not a Keystone issue.

Maybe something like:

Title: SQLAlchemy SQL injection through limit hangling
Impact: Critical
Reporter: Nikita Savin
Products: Keystone
Affects: Versions prior to Keystone Essex-3 milestone

Description:
Nikita Savin from GridDynamics reported a vulnerability in SQLAlchemy. Before version 0.7.0, SQLAlchemy did not sanitize the contents of the limit clause, potentially allowing SQL injection. Since Keystone passes user-provided values to the SQLAlchemy limit calls, it may be abused to trigger that SQL injection on vulnerable SQLAlchemy versions, potentially resulting in complete compromise of the authentication database. Only setups using SQLAlchemy < 0.7.0 are affected.

Solution:
Users are advised to upgrade to unaffected versions of SQLAlchemy (>= 0.7.0). Keystone Essex-3 milestone also contains a workaround that solves the issue for affected versions of SQLAlchemy.

For sure this is issue in SQLAlchemy and it has to sanitize limit().
But this is partially issue in Keystone too - this is usually very good idea to sanitize all user's input. Especially in public API.
For example, glance. It supports limit in API. And it works with the same buggy SQLAlchemy. But glance API control user input, so no sqlinjection like in keystone, but rather meaningful error message:
> curl 'localhost:9191/images?limit=1"'
<html>
 <head>
  <title>400 Bad Request</title>
 </head>
 <body>
  <h1>400 Bad Request</h1>
  The server could not comply with the request since it is either malformed or otherwise incorrect.<br /><br />
limit param must be an integer

 </body>
</html>

@Nikita: I certainly agree. The description must walk a thin line between blaming keystone for making reasonable assumptions about SQLAlchemy and ignoring Keystone vulnerability completely. In this case I'd say the vulnerability is in SQLAlchemy, and the lack of a parameter sanitization layer in Keystone made it worse.

@Russell: the fix is public now... this bug should be opened. How is the linux-distros disclosure going ? If we do an advisory, it would need to go out ASAP.

oops, changed status to "Fix Released" by mistake and can't return it back. Can anybody fix ?

I've asked for an update from the Red Hat security team. It looks like they did verify the vulnerability in SQLAlchemy, assigned a CVE (CVE-2012-0805), and are now working on coordinating a release. I'll report back when I get a date. I know the fix is already out, but ideally I think this bug should stay private until the coordinated release for the CVE.

After thinking about it some more, I don't feel strongly that we need to an OpenStack advisory for this. Do we want to release an advisory every time a project that OS uses has a vulnerability? I wouldn't think so, except perhaps for extreme circumstances.

If they are coordinating a release, better keep that one below the radar until that's done. I also agree that we should not issue an OpenStack advisory on this... but that doesn't prevent us from sending an information e-mail.

The coordinated release date is Tuesday, January 31st. We can open this bug and post a note to the list then.

@Russell: we are way past Jan 31st now. Could you open this and post a "security notice" to the list ?

opened up and message sent to to the list, sorry for the delay