Mailing-List: contact dev-help@couchdb.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@couchdb.apache.org
Received-SPF: neutral (athena.apache.org: local policy)
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Apple Message framework v1077)
Subject: Re: DB ACLs (was Re: 0.11 Release / Feature Freeze for 1.0)
From: Jan Lehnardt <jan@apache.org>
In-Reply-To: <e282921e1002030921g53af1711r3b46ebe31f4cb38b@mail.gmail.com>
Date: Wed, 3 Feb 2010 10:22:36 -0800
Content-Transfer-Encoding: quoted-printable
Message-Id: <0BEBE42F-84A1-4156-AEC4-5D9CAA68C845@apache.org>
References: <e282921e1002030921g53af1711r3b46ebe31f4cb38b@mail.gmail.com>
To: dev@couchdb.apache.org

On 3 Feb 2010, at 09:21, Chris Anderson wrote:
>> I do see logic in keeping the admin/reader authorizations for a =
database
>> within the database itself. The problems are:
>>=20
>> (1) _all_dbs currently shows everything - even those databases you =
don't
>> have access to.
>>=20
>> I believe that in its current form, _all_dbs simply won't scale to =
millions
>> of databases on a box if you want to limit it to accessible dbs only.
>>=20
>=20
> This is an interesting one. _all_dbs won't scale indefinitely even
> before this patch, because it has no built-in pagination abilities.
> Enhancing this feature to look into each file and keep going to till
> it finds N that can be listed isn't hard to code. It will be a little
> more work to make _all_dbs respect startkey and endkey.
>=20
> I'd be thrilled if someone would patch _all_dbs to act correctly.

I think I'm leaning towards making _all_dbs a admin-only resource.

Brian, thanks for taking the time to criticise Chris's patch and thanks
Chris for the thoughtful reply :)

Cheers
Jan
--


>=20
>> (2) _readers is a single monolithic object. I believe that it won't =
scale to
>> millions of users having access to the same database.
>=20
> It's not meant to support this use case. If you have millions of users
> with the same access rights, give them a common role and give that
> role access to the database.
>=20
>>=20
>> (3) _readers has no concurrency control. One admin making an ACL =
change in
>> futon (say) will silently overwrite changes made around the same time =
by
>> another admin. This will get worse the more frequently users are =
added and
>> removed.
>=20
> _readers / _admins / _security are stored as a raw object without
> concurrency control, because keeping them as a document adds too much
> performance overhead on each request. Concurrency control is a
> tradeoff we make here.
>=20
>>=20
>> For me, those are serious problems. I sketched a design for an =
alternative
>> approach, using the _user db to hold the authorizations in terms of
>> database-specific roles.  Unfortunately I didn't have time to =
contribute an
>> implementation of this.  If there's a chance this alternative =
approach would
>> be used then I will try to steal the time from somewhere.  The ideas =
behind
>> it weren't explicitly rejected, but neither were they acknowledged as =
a good
>> approach.
>=20
> The database-specfic roles and names don't belong in the users db. The
> users db is for answering the question: "who is the user and what
> roles do they have". The ACLs say which names and roles can read or
> admin a given database.
>=20
> It's a fact of life that users can rsync db-files around. If the names
> / roles are in the users db, they get wrong when databases are moved
> to another host or renamed on the current host.
>=20
>=20
>>=20
>> (4) An "admin" is not a "reader", and this is clearly intentional =
from
>> comments in the code. However, someone who has an "admin" role =
without
>> "reader" role is unable to perform ACL changes, which for me defeats =
the
>> whole purpose of the "admin" role.
>>=20
>>=20
>> So I'd propose that the relaxed approach is that a database "admin" =
should
>> inherit "reader" rights. Isn't that true for a server-level admin =
anyway?
>=20
> I considered this, but dropped it because it seemed like it'd send the
> wrong message (if you put yourself as the only db-admin, this doesn't
> make the db private, to do that you need to edit the readers.)
>=20
> Looking at your shell transcript makes me think it'd make life easier
> if we grant db-admins the reader privilege as well (so long as it
> remains clear that dbs with admins but no readers are public).
>=20
> 4 is fixed.
>=20
>>=20
>> (5) Non-admin readers can view the entire _readers, _admins and =
_security
>> resources.  I think this is quite a severe privacy concern, but it is =
easily
>> fixed.
>=20
> They can also read the design document. I'm not sure why this is a
> privacy concern. A user may need to contact a db admin for help with
> something, it's handy to be able to get a list of them. And it only
> makes sense that you should be able to see the list of users who can
> also access the same db you can.
>=20
> If there's consensus that this is indeed an issue, it's not a hard
> thing to change in the code.
>=20
>>=20
>> (6) Databases are created world-readable by default, which means a =
race to
>> get the _readers set before someone else starts inserting documents. =
I think
>> a PUT /dbname option to set a non-empty readers list would be a good =
idea
>> (and a corresponding checkbox in futon)
>=20
> Yes, I'd like to add some UI to the create-database form in Futon that
> immediately sets a reader =3D=3D the current user. The UI would be a
> checkbox that says "make database private" or something. Making this
> synchronous with db-creation would be a neat trick, too.
>=20
>>=20
>> (7) Couchdb accepts nonsense _readers documents, e.g.
>>=20
>> $ curl -X PUT -d '{"names":{"foo":"bar"},"roles":456}'
>> http://admin:admin@127.0.0.1:5984/briantest/_readers
>> {"ok":true}
>>=20
>> The effect is to reset the _readers document to its permit-all =
default, thus
>> opening up the database to the world.
>>=20
>> $ curl http://127.0.0.1:5984/briantest/_readers
>> {"names":[],"roles":[]}
>=20
> My main concern in coding this was validating that malformed data
> can't be stored. I've fixed this to throw errors in the case of
> malformed data (instead of ignoring it).
>=20
>>=20
>> (8) Point (7) is arguably a simple bug which can be fixed, but I'd =
prefer
>> for couchdb to be fail-safe; that is, an empty ACL means nobody has =
access.
>=20
> The goal with this patch was to add some easy ACL functionality
> without changing the default behavior. People who are happy to use
> Couch in admin-party mode shouldn't see any changes.
>=20
>>=20
>> One way to achieve this would be for two new roles, "_anon" and =
"_user",
>> granted to all unauthenticated and authenticated users respectively. =
Then a
>> fully public database would have roles:["_anon","_user"], and this =
would
>> be added to a new database unless you ask otherwise (see point (6)).
>=20
> This ["_anon","_user"] business seems fine to me. It's a lot more
> complexity for basically the same capabilities (although I do like the
> ability to lock out anon users on a per-db basis, that's pretty cool.)
>=20
> If anyone writes this, I'd be happy to see it. This won't be a small
> patch because it's got to introduce those roles as well as use them in
> the reader checks.
>=20
>>=20
>> (9) The _users db itself is world-readable (showing not only who your =
users
>> are, but their password hashes). Highly undesirable.
>=20
> I actually consider this a feature. We'd like to get some stronger
> password hashing (see the bcrypt threads) which should help with the
> password parts.
>=20
>>=20
>> You can set a _readers ACL on it, but it has consequences:
>> * users can't sign up for new accounts
>> * users can't change their own passwords
>> This forces such things through a privileged external interface. =
Actually
>> I'm fine with that, because I want to validate signups anyway, but =
others
>> might not be.
>>=20
>> (10) I don't think you can replicate _readers, _admins and _security, =
unless
>> (a) you are doing rsync filesystem-level replication, or (b) you =
explicitly
>> GET and PUT these resources from one DB to another. This is arguably =
a
>> feature not a bug.
>>=20
>> (11) Trivial problem of the day: _security resources which are not =
objects
>> give an erlang-flavoured error.
>>=20
>> $ curl -X PUT -d '["foo","bar"]' =
http://brianadmin:brianadmin@127.0.0.1:5984/briantest/_security
>> {"error":"unknown_error","reason":"function_clause"}
>>=20
>> Sorry for the long post, and if I really am barking up the wrong tree =
here,
>> please tell me so.
>=20
> Thanks for pointing out a lot of small bugs. The ones I think have =
merit are:
>=20
> 1, 4, 6, 7, 8, 11
>=20
> Of these I've fixed 4, 7 and 11 this morning.
>=20
> If you can create Jira tickets for 1, 6, and 8 that'd be nice.
>=20
> Chris
>=20
>>=20
>> Regards,
>>=20
>> Brian.
>>=20
>=20
>=20
>=20
> --=20
> Chris Anderson
> http://jchrisa.net
> http://couch.io