[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Pulp-list] Auth Issues



On 11/15/2010 10:01 AM, Jason Dobies wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeff and I ended up digging pretty deep into our auth subsystem and came
up with a bunch of issues. I'm thinking next sprint we should take some
time to revamp our auth handling to clean this up.


definitely. I think the work may even span more than one sprint but agreed nonetheless.

- - The two CLIs improperly use each other's certificates. Currently, both
utilize the same preference scheme for determining the type of auth to
use (username/pass, admin cert, consumer cert). This leads to some wonky
situations where a machine with both a consumer and admin cert will use
the admin cert for consumer operations, which leads to (among other
things) incorrect reporting of who initiated the action.

The desired behavior should be based on which CLI is being invoked:
* pulp-admin: if a username/password are specified to the executable,
use those; otherwise, use an admin certificate if one exists.

+1

* pulp-client: all commands require a consumer certificate except for
consumer create; username/password should be moved from being an
executable-level option to specific only to consumer create. In other
words, you use a username/password to get the consumer cert, but then
the consumer cert is the only way to do consumer operations from
pulp-client from there on.


+1 as well.

- - The role checking lines between admin (read: user) and consumer are
blurred. Currently, we try to log you in as an admin. If we cannot, we
try to log you in as a consumer. I know we're reusing the same APIs for
both admin and consumer operations (in other words, if a consumer is
binding itself or an admin is binding a consumer, we use the same call),
but this just feels wrong. It allows the bug we face today, that admin
credentials are used for consumer initiated operations. The two
concepts, users v. consumers, are apples and oranges, but we don't
reflect that properly.


RBAC *should* solve this problem. We will specify what type of access (permission(s) and object type) the user requires for each API call and the pre-bundled Role that is included in Pulp that includes that access.

The role_check class is intended to be a proof of concept and a path to get to RBAC but definitely not our final solution. I'm thinking that the class itself may be largely rewritten entirely to handle the more advanced authorization model we need. Right now it is approaching an unwieldy size and would only get worse if we tried to jam RBAC into the existing structure.

I haven't looked yet, but I doubt our API docs properly reflect this as
well. The consumer ID is required in all cases of the API, but I don't
think we document the auth requirements to make the call (those
requirements read something like "You either need to have the consumer
cert of the consumer with the same ID as the call edits OR you need to
have admin credentials"... and this is all before we have anything
role-related in place, which further complicates this).


I did attempt to have the docs accurately reflect the credentials required for each call back when I added the role checking to Pulp.

User APIs require username/pass:

https://fedorahosted.org/pulp/wiki/ContentAPI-Users

"Authentication:
    username/pass"

vs say on Consumer we see some requiring uname/pass and some requiring either:

https://fedorahosted.org/pulp/wiki/ContentAPI-Consumers

"Path: /consumers/
Authentication:
    username/pass"

vs:

"Path: /consumers/
Authentication:
    username/pass or Consumer Certificate"

that said, the pointer to the Auth design doc at the top level of our API page:

https://fedorahosted.org/pulp/wiki/ContentAPI

points to:

https://fedorahosted.org/pulp/wiki/Authentication

is far too 'designy' and doesn't really give a good 'how to' for developers trying to use our API. We need more of a walkthrough of how to auth to our API using either username and password or a cert.

- - Any consumer can edit another consumer. Our consumer verification
ensures that the consumer ID in your certificate is a valid consumer,
but doesn't actually check to make sure it's the consumer the API call
is acting on. Big security hole here.

actually, that isn't entirely true:

# If the ID should be explicitly verified against a provided list, the ID in the
    # certificate will be verified against the ID found in the certificate
    good_certificate = False
    if check_id:
        for arg in fargs:
            LOG.error("Checking ID in cert [%s] against expected ID [%s]" %
                      (consumer_cert_uid, arg))
            if arg == consumer_cert_uid:
                good_certificate = True
                break
        else:
            good_certificate = True

we check the arguments passed in to the method to verify that one of them is a consumer_id.

This algorithm is somewhat weak because we are checking *all* the args for *any* match vs checking specific arguments. That said, it isn't as wide open as you state above.


- - The principal handling (auth.py) has a few issues:
- -- It is only set for admin users; the principal is left at the default
when a consumer initiated (pulp-client) command is run.
- -- The SystemPrincipal implementation is wonky. It's not a subclass of
User, yet is meant to be treated as one. What this leads to is checking
elsewhere in the code where you get the principal but then have to
determine if it's an actual user before being able to access data on it.
- -- The principal is not reset on each call to role check, which means if
a thread with a principal is set and tossed back into the pool, the next
call that uses that thread again is by default logged in.

nasty bugs we gotta fix..

- - All certificates, both admin and consumer, are generated with the same
serial number. This prevents us from ever implementing a certificate
revocation list (I just filed a bug on this this morning).


+1

--
Mike McCune
mmccune AT redhat.com
Red Hat Engineering       | Portland, OR
Systems Management        | 650.254.4248


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]