nova passes incorrect authentication info to cinderclient

[Divya K Konoor]

There are multiple problems with the authentication information that nova/volume/cinder code passes to cinderclient:

1. nova/volume/cinder.py passes 'cinder endpoint publicURL' as the auth_url to cinderclient for credential authentication instead of the keystone auth_url .This happens here:

get_cinder_client_version(context) sets the value for global CINDER_URL and passes it to
c = cinder_client.Client(version,
                             context.user_id,
                             context.auth_token,
                             project_id=context.project_id,
                             auth_url=CINDER_URL,
                             insecure=CONF.cinder.api_insecure,
                             retries=CONF.cinder.http_retries,
                             timeout=CONF.cinder.http_timeout,
                             cacert=CONF.cinder.ca_certificates_file)

c.client.auth_token = context.auth_token or '%s:%s' % (context.user_id,
                                                           context.project_id)

Under normal circumstances ( i e in cases where the context has auth_token) , the auth_url is never used/required. So this is required only when the token expires and an attempt to do fresh authentication is made here:

def _cs_request(self, url, method, **kwargs):
        auth_attempts = 0
        attempts = 0
        backoff = 1
        while True:
            attempts += 1
            if not self.management_url or not self.auth_token:
                self.authenticate()
            kwargs.setdefault('headers', {})['X-Auth-Token'] = self.auth_token
            if self.projectid:
                kwargs['headers']['X-Auth-Project-Id'] = self.projectid
            try:
                resp, body = self.request(self.management_url + url, method,
                                          **kwargs)
                return resp, body
            except exceptions.BadRequest as e:
                if attempts > self.retries:
                    raise
            except exceptions.Unauthorized:
                if auth_attempts > 0:
                    raise
                self._logger.debug("Unauthorized, reauthenticating.")
                self.management_url = self.auth_token = None
                # First reauth. Discount this attempt.
                attempts -= 1
                auth_attempts += 1
                continue

2. nova/volume.cinderclient.py >> cinderclient method passes context.auth_token instead of the password.Due to this HttpClient.password attribute is set to the auth token instead of the password.

3. There are other problems around this which is summarized as below:

cinderclient should really support a way of passing an auth_token in on the __init__ so it is explicitly supported for the caller to specify an auth_token, rather than forcing this hack that nova is currently using of setting the auth_token itself after creating the cinderclient instance. That's not strictly required, but it would be a much better design. At that point, cinderclient should also stop requiring the auth_url parameter (it currently raises an exception if that isn't specified) if an auth_token is specified and retries==0, since in that case the auth_url would never be used. Userid and password would also not be required in that case.

nova needs to either start passing a valid userid and password and a valid auth_url so that retries will work, or stop setting retries to a non-zero number (it's using a conf setting to determine the number of retries, and the default is 3). If the decision is to get retries working, then we have to figure out what to pass for the userid and password. Nova won't know the end-user's user/password that correspond to the auth_token it initially uses, and we wouldn't want to be using a different user on retries than we do on the initial requests, so I don't think retries should be supported unless nova is going to make ALL requests with a service userid rather than with the end-user's userid... and I don't think that fits with the current OpenStack architecture. So that leaves us with not supporting retries. In that case, nova should still stop passing the auth_token in as the password so that someone doesn't stumble over that later when retry support is added. Similarly for the auth_url it should start passing the correct keystone auth_url, or at least make it clear that it's passing an invalid auth_url so someone doesn't stumble over that when trying to add retry support later. And it definitely needs to stop setting retries to a non-zero number.

[Morgan Fainberg (mdrnstm)]

This looks to be using the user's token meaning a couple things: Nova could never pass a username/password to cinderclient, and refreshing the token is not working since you don't have a username/password [today].

There are a couple approaches to fix this (no particular order):

1) as outlined, don't set retries

2) Keystone team needs to evaluate allowing a "refresh" of a token with a longer expires in some cases (this however has a lot of security implications, and is unlikely to go through).

3) (as discussed at the summit), when policy.py is graduated (in process) to it's own library we can add the ability to the rules engine to accept expired tokens (explicitly configured) for some calls (this also requires some changes to auth token middleware). This would need to also be reviewed by the security teams, but some cases it would be ok (checking status of things) be allowed to use an expired token. Some mechanics on this approach likely need to still be worked out.

4) Migrate this call to use a service user nova actually controls.

The short answer is that as of today, the only easy option is to not retry, since the user's token isn't guaranteed to be valid the entire time and we can't refresh it.

[Jamie Lennox (jamielennox)]

The code mentioned has been replaced. Looking at master calling client looks like:

    return cinder_client.Client(version,
                                session=_SESSION,
                                auth=auth,
                                endpoint_override=endpoint_override,
                                connect_retries=CONF.cinder.http_retries,
                                **service_parameters)

The auth plugin specified by auth knows that it got this from a user token and will not attempt to reauthenticate.

Similarly for problem 2, the auth plugin sorts this out, there is a token within the plugin that is used for requests. This negates the need for all those horrible cinderclient context hacks.

Retries are a known problem throughout all of OpenStack, however we can't pass a user_id and password to cinder from nova because this is the user's data - and nova never knows the user's password.

[Matthew Edmonds (edmondsw)]

Looks to me like the changes referenced by Jaime address the issues on the nova side. As for cinderclient... It is great that nova switched to using cinderclient.client.SessionClient, but there remain a bunch of issues with cinderclient.client.HTTPClient which will affect other things that have not made that switch. For HTTPClient cases, we lack a way to pass an auth_token in on the __init__, which led to nova's horrible hack of setting it after the fact (before it switched to using sessions). HTTPClient should also stop requiring auth_url be passed in when retries is None or 0. It's also concerning that HTTPClient's authenticate() method doesn't support keystone v3. Since https://review.openstack.org/#/c/95305/ purports to add keystone v3 support to cinderclient, but didn't address that method, I assume whoever wrote that expects SessionClient to be used instead of HTTPClient when using keystone v3. That seems like an invalid assumption, though, or at least something that should be checked and a nice error returned in the event that keystone v3 was tried.

[Matthew Edmonds (edmondsw)]

I opened a separate defect for cinderclient (https://bugs.launchpad.net/python-cinderclient/+bug/1424763) since that doesn't really seem to fit here. I think this one can probably be closed out, given the changes that have already gone into nova addressing the original issue, and the new cinderclient bug used to track what remains there.

[Sean McGinnis (sean-mcginnis)]

Closing stale bug. This has be Incomplete status for over 90 days. If this is still an issue please reopen.