[MIR] keystone

This will require a security team audit.

Rational for requiring security team audit: http://docs.openstack.org/diablo/openstack-identity/admin/content/what-is.html

the dependency on python-sqlite needs to be dropped (build and use sqlite3 from the python standard library)

MIR's are missing for the b-d's/d's, at least passlib, python-coverage, python-memcache

and python-coverage on jquery-goodies and more "goodies" :-/

I have performed a shallow security audit of keystone. The code audit was not deep because "Keystone is very young and developing very fast." -- http://docs.openstack.org/trunk/openstack-identity/admin/content/what-is.html. With that said, here is my review:

Package review:
- Does not run as root
- Has test suite in the build
- Listens on all interfaces by default, on ports 5000/tcp and 35357/tcp (admin interface)
- Documentation on keystone is scant and there are no manpages for the binaries shipped in keystone
- no sudo fragments
- no DBus services
- no setuid/setgid binaries
- By default, does not use ssl. Since access to keystone is necessarily over the network and considering that Keystone/Nova/Glance/Quantum bits are likely on a trusted network (though they should use SSL as well), the most important bit seems to be the User to Keystone interaction, as that is where the password is passed and the token used by the other services is received. If the password or token is snooped then an attacker can do everything as that user.

Code review
- no privileged operations
- no processes spawned
- file handling seems sane
- keystone-control has, which shouldn't strictly be a problem because of Yama, but should be fixed regardless: os.environ['PYTHON_EGG_CACHE'] = '/tmp'
- correct example uses of ssl does not seem to be present. /etc/keystone/ssl.conf is shipped, but this is an example for tests and is confusing. /etc/keystone/keystone.conf has comments for how to use SSL, but only if using a local CA. It would be much better if it were commented to also include a section on using SSL with a proper CA from ca-certificates
- Doesn't seem to be any input validation on 'marker' in get_marker_limit_and_url() from keystone/controllers/__init__.py. It isn't clear at this time if this can be used for SQL injection, but 'marker' is passed unvalidated from get_marker_limit_and_url() all the way to session.query() (part of sqlalchemy) in endpoint_get_by_tenant_get_page() from endpoint_template.py.

Recommendations from security team for MIR approval:
- provide documentation for keystone, especially man pages
- provide documentation on how to correctly use SSL certificates, not just snakeoil or self-signed with your own CA (manpages and server documentation ideally)
- fix the /tmp file issue
- perform input validation an all variables returned in get_marker_limit_and_url() (keystone/controllers/__init__.py) or demonstrate where this is already being performed (and therefore why this is not needed)

Ideally, Ubuntu would also have tools or adjust packaging to properly configure keystone for SSL, with helpers for the various other openstack services utilizing keystone so all could also benefit. Since keystone is an identity service and a key component of openstack, it begs the question why it would be used without SSL at all. ssl-cert may be helpful here.

I believe I all of these issues have been addressed now:

- passlib MIR - https://bugs.launchpad.net/ubuntu/+source/passlib/+bug/911287
- python-sqlite has been dropped
- python-coverage has been dropped
- python-memcache MIR - https://bugs.launchpad.net/ubuntu/+source/python-memcache/+bug/911295
- I believe the security issues that was rasied by Jamie has been addressed upstream.
- I have made it easier to enable ssl by default for the user.

Whats the status of this MIR?

passlib and python-memcache now promoted; jdstrand, please have a look at the changes after your last review.

Hi Jamie,

The keystonelight git repo can be found at: https://github.com/termie/keystonelight/

if you have any questions please let me know.

Regards
chuck

So the reason for the switch to keystonelight is that alot of the developers in openstack has had problems with keystone and so an alternate was created called keystonelight. The discussion has started at:

http://<email address hidden>/msg06998.html

If you have any questions please let me know.

Regards
chuck

After talking to the server team, I am unassigning myself for now while the server team decides which keystone packages they want in main. Leaving as Incomplete for now.

For info, keystone packages after 2012.1~e4-0ubuntu1 (after the prior review), is based on whole new upstream code. Upstream rewrote this project from scratch. Apologies for the doubling of work.

Thanks.

FYI the latest keystone that was uploaded to the archive will be the version that we will be using in Ubuntu (not final release fro precise):

A list of the dependencies are the following:

 pep8 (in main)
 pylint (in main)
 python-all (>= 2.6) (in main)
 python-all-dev (>= 2.6.6-3~) | python-support (in main)
 python-eventlet (in main)
 python-keystoneclient (in universe pending MIR)
 python-ldap (in main)
 python-lxml (in main)
 python-memcache (in main)
 python-migrate (in main)
 python-mox (in main)
 python-nose (in main)
 python-pam (in main)
 python-passlib (in main)
 python-paste (in main)
 python-pastedeploy (in main)
 python-prettytable (in main)
 python-routes (in main)
 python-sphinx (in main)
 python-sqlalchemy (in main)
 python-unittest2 (in main)
 python-webob (in main)
 python-webtest (in main)

I have performed another shallow security audit of keystone. The code audit was
not deep because "Keystone is very young and developing very fast." (even more
so now because of the rewrite. --
http://docs.openstack.org/trunk/openstack-identity/admin/content/what-is.html.
With that said, here is my review:

Package review:
- Does not run as root
- Has test suite in the build, but 139 out of 266 test cases fail. debian/rules
  has:
  override_dh_auto_test:
        bash run_tests.sh -N || true
- Listens on all interfaces by default, on ports 5000/tcp and 35357/tcp (admin
  interface)
- no sudo fragments
- no DBus services
- no setuid/setgid binaries
- By default, does not use ssl. Since access to keystone is necessarily over
the network and considering that Keystone/Nova/Glance/Quantum bits are likely
on a trusted network (though they should use SSL as well), the most important
bit seems to be the User to Keystone interaction, as that is where the password
is passed and the token used by the other services is received. If the password
or token is snooped then an attacker can do everything as that authenticated
user.
- The previous version of keystone had some sort of SSL capabilities, but this
  version doesn't support it in any documentation.

Code review:
- no privileged operations
- process spawning seems sane
- file handling seems sane
- environment handling seems sane
- uses sqlalchemy, which is good

Requirements for main inclusion:
- add to manpage the fact that this must be on a trusted network segment or
  provide proper SSL configuration.
- fix test suite and make it fail the build

Note: As of 2012.1~rc1~20120308.2103-0ubuntu1, keystone.conf now contains a 'bind_host' option to configure which address the servers listen.

With regards to the testsuite for keystone.

The testsuite included in keystone contain both functional and integration tests. An example of this keystone integration with keystoneclient, or swift with keystone. In order to run these tests, the tests do a git checkout of both swift and keystone when the tests are run. This make the tests not use what we will have in the archive for swift and keystone (ie make it less static) which is not good for us and less than ideal. I think we need to have options going forward.

Here are the options I see going forward, in preferred order of precedence:
1. patch the keystone testsuite to work against installed Ubuntu packages (and therefore adding swift and keystoneclient to Build-Depends). This will allow the security, QA and SRU teams to have higher confidence that patches don't break keystone when used against the packages in the archive
2. disable the tests that require git and document somewhere (README.Debian?) how the keystone testsuite can be run against Ubuntu installed packages
3. do nothing

I don't really see '3' as an option since keystone is going to be supported for 5 years and upstream git is going to be constantly evolving while the keystone testsuite being used in the package is going to be static. This means potentially more and more problems when people try to use the integration tests in their personal testing (which we should be doing).

'2' is acceptable but far less desirable as it will also require a QRT script.

I spent some time on this yestreaday, to clarify-

I don't really see the git checkouts an issue. The pending test to swift tests that do this do so only if there is an error importing it from already installed libraries. That is, it should be imported locally instead of cloned remotely if we list it with Build-Depends. The same should be true of keystoneclient, if its not we can probably patch that easily enough (I'm not able to confirm that the keystoneclient tests are actually running, though, as that would have turned up already)

The bigger problem AFAICS is that the python-swift library used makes an unconditional call to setup a logging handler via /dev/log, which is causing these FTBFS:

https://launchpadlibrarian.net/97807065/buildlog_ubuntu-precise-i386.keystone_2012.1%2Bgit201203212030-0ubuntu1_FAILEDTOBUILD.txt.gz

We can either disable these swift tests (since they appear to be more integration tests that are actually exercising swift more than keystone), or we can probably carry a small patch to swift as well as a corresponding patch to any test suite that uses it, to make log setup optional.

Thanks Adam, I'll let you decide on the log FTBFS, but a properly formatted DEP-3 commented patch the describes why we are doing this and disables those specific tests seems fine to me provided that changes to keystone should not affect how it works with swift (ie, it is preferable to not leave a hole in our testing).

The testsuite passes the build now, however the SSL support that was in keystone before the rewrite got removed. I am expecting to see it back in folsom, and when it happens I would be more than happy to backport the patch into essex.

Thanks Chuck. I see that keystone now has its testsuite enabled and the fix-ubuntu-tests.patch looks sane. The only thing that remains is:
- add to manpage the fact that this must be on a trusted network segment or provide proper SSL configuration.

Once you have a patch for that (I suggest keystone.8), feel free to seed.

2012-04-13 19:31:29 INFO Creating lockfile: /var/lock/launchpad-change-override.lock
2012-04-13 19:31:38 INFO Override Component to: 'main'
2012-04-13 19:31:39 INFO 'keystone - 2012.1-0ubuntu1/universe/net' source overridden
2012-04-13 19:31:39 INFO 'keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/amd64
2012-04-13 19:31:39 INFO 'keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/armel
2012-04-13 19:31:39 INFO 'keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/armhf
2012-04-13 19:31:39 INFO 'keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/i386
2012-04-13 19:31:39 INFO 'keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/powerpc
2012-04-13 19:31:39 INFO 'keystone-doc-2012.1-0ubuntu1/universe/doc/EXTRA' binary overridden in precise/amd64
2012-04-13 19:31:39 INFO 'keystone-doc-2012.1-0ubuntu1/universe/doc/EXTRA' binary overridden in precise/armel
2012-04-13 19:31:39 INFO 'keystone-doc-2012.1-0ubuntu1/universe/doc/EXTRA' binary overridden in precise/armhf
2012-04-13 19:31:39 INFO 'keystone-doc-2012.1-0ubuntu1/universe/doc/EXTRA' binary overridden in precise/i386
2012-04-13 19:31:39 INFO 'keystone-doc-2012.1-0ubuntu1/universe/doc/EXTRA' binary overridden in precise/powerpc
2012-04-13 19:31:40 INFO 'python-keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/amd64
2012-04-13 19:31:40 INFO 'python-keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/armel
2012-04-13 19:31:40 INFO 'python-keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/armhf
2012-04-13 19:31:40 INFO 'python-keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/i386
2012-04-13 19:31:40 INFO 'python-keystone-2012.1-0ubuntu1/universe/python/EXTRA' binary overridden in precise/powerpc
Confirm this transaction? [yes, no] yes
2012-04-13 19:31:45 INFO Transaction committed.
2012-04-13 19:31:45 INFO Done.