On 09/07/2013 04:45 PM, Simo Sorce wrote:
Sorry to come late to this thread.
I think I like some of Petr plan, but not all of it.
On Fri, 2013-09-06 at 08:46 -0400, Rob Crittenden wrote:
[...]
I'm not sure I follow, what are you trying to achieve here? The more ACIs the
slower the processing.
One of the main reason for this feature is to get rid of the global allowing
ACI:
And this is good, one-size-fit-all, doesn't work anymore, however we
need to try to keep the number of ACIs low anyway, this can be done my
smartly and programmatically generate ACIs, however we need a very
complete test suite and in order to have a complete one, we need enough
information to know what is the intended behavior. Plus I think for
robustness the test suite should be generated from different code, so
that bugs in the ACI generation code does not also cause blind spots in
the ACI testing code.
I don't want to build a "smart" system; it should rather be simple and
straightforward. Other than that I agree, good tests are always necessary.
[...]
Right, I just want to know why it was proposed to add 2 ACIs for every
container.
I am puzzled as well.
Consider that point dropped; I trusted the bug description without checking.
How to handle passwords and other non-public attributes? I'm thinking
about keeping a global list [...]
No I will veto anything that overhauls ACIs and keeps negative filtering
lists.
Any big change to the ACIs system *MUST* get rid of ( targetattr !=
list ).
We already risked many times of exposing data and a few years ago had to
issue a CVE because of this. We do not want to keep doing it. Especially
if you are building this system to allow plugins to change permissions
you do not want to rely on the authors to remember to add negative
entries.
+1, a negative list doesn't make sense any more.
It could get ugly real fast, and potentially cause a lot of extra processing. I
think the object(s) for each attribute should be considered so these wouldn't
have to be applied to every ACI but only those that are affected. We don't need
to worry about userPassword in groups, for example.
I think that a decision that a param should not be included in default read
rule should be included in the param object itself, see below.
I really do not want to see the ACI templates/definition/call it how you
want int the framework, because the framework will need to change with
versions of IPA but the data may remain in LDAP for long. therefore any
ACI template should be stored in LDAP, probably under cn=etc,$suffix.
It may make sense to have cn=aci-templates,cn=etc,$suffix, and then
within that container (accessible only by security admins) we have one
template per object/container, that is used to generate ACIs.
Something like:
dn: cn=users,cn=aci-templates,cn=etc,$suffix
objectclass: ipaAciTemplate
anonReadAttributes: <multivalue list>
authReadAttributes: <multivalue list>
selfReadAttributes: <multivalue list>
selfWriteAttributes: <multivalue list>
and so on.
These are the defaults, only attributes that must be disclosed are
listed, *no* negative lists, the default is changed globaly to deny all,
and if there is no allow ACI an attribute is inaccessible by default.
This allows easy customization, if someone decides 'description' should
be kept confidential, all he needs to do is to remove it from the right
list (and perhaps add it to a privileged list), and just run a tool that
regenerates and changes the default ACIs for the container.
* No need to modify framework code anywhere. *
+1 to storing the attribute lists in LDAP. I don't like the Template
idea though.
First, on IPA upgrades, we need to be able to add attributes to the
ACIs. Automatically.
We cannot make the IPA-generated lists user-modifiable. If some past
upgrade added a readable 'description' attribute, but the admin decides
that it should not be readable, the next upgrade should not re-add it.
So we need to store the IPA defaults and user changes separately.
Second, we already have a place where users can customize ACIs: the
permissions. I don't see the point of another layer.
The only thing we're adding here over regular permissions is the
aforementioned ability to auto-add new attributes on upgrades.
We just need a special "managed" permission for each object's "anon
read", "auth read", "self read", "self write", etc.
I propose a new Permission type with three lists: attributes added by
IPA, attributes added by the admin, and attributes removed by the admin.
On IPA updates, new items can be added to the first list, and the ACI
gets regenerated from all three. This ensures that an admin's changes
get preserved across updates.
If we mark a Permission as SYSTEM, old IPA versions won't try to locate
or touch its ACI, but they'll still be able to add it to privileges and
roles using the existing API/CLI/UI.
So we'd have something like:
cn=Read Users,cn=permissions,cn=pbac,$SUFFIX
objectclass: ipaPermission
objectclass: ipaManagedPermission
ipaDefaultAttributes: <multivalue list>
ipaAdminAddedAttributes: <multivalue list>
ipaAdminExcludedAttributes: <multivalue list>
ipaPermissionType: SYSTEM
Admins that don't want IPA updates messing with their ACIs can just
remove the permission from privileges and add custom plain permissions
instead.
(Or possibly even convert the default permission to non-managed, or
delete it -- if we write UI to do+undo these actions.)
# Combining read rights
I think (read, search, compare) should be exposed in permission objects
as a single right. Or is there a reason to keep it split?
Yes, they are separate for a reason. Using only search and compare isn't
common, but it isn't unheard of either. For example, to be able to detect the
presence of an attribute you can provide just the search permission.
Wouldn't most users use the (read, search, compare) triple? It would lower our
number of ACIs significantly if we do not have 3 permissions per each object.
There is nothing to prevent an ACI from containing multiple permissions.
I wasn't proposing that. But rolling these three logically into the same
thing in IPA I think is short-sighted.
I think it would make sense for an optimizer ACI generator to come later
at a second stage, where we go through the generated list and combine
ACIs together.
Ie the first version could generate three distinct ACIs for read, search
and compare, and later on we add an optimizer that consolidates them
into a single ACI.
For the default permissions I'd rather generate ACIs with all three.
Admins (or IPA if needed) can add an additional permission if some
attribute only needs search.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
