Simo Sorce wrote:
Hello,
this is a patchset I have been brewing for quite some time.
It addresses primarily ticket #3859 however the implementation
implicitly also addresses tickets #232 (effective only if we change
permissions and break the old interface so only potentially but not
immediately) and #233.
The patchset is marked [RFC] because it involves the clever use of ACIs
to introduce a new ipaPermittedOperations attribute that is used to
allow to define a 'virtual' operation as a subtype. This clever use of
ACIs is also what stalled this patchset because of 389DS bugs #47569 and
#47571 which have since been fixed and I was finally able to verify.
Also another blocker for this patchset is that we need to wait for 4.0
when we change the Permission model and stop allowing anyone to read all
attributes.
Another reason this is still RFC is that the admin user apparently is
allowed to retrieve any keytab with the current code and default ACIs as
augmented by the 3rd patch. It is not entirely clear to me why that
happens, I think it maybe due to the broad permissions granted to the
admins group. This is *not* something we want to allow in the default
case so help to figure out how to avoid it will go a great way into
allowing this patchset to be acceptable.
However due to the various changes I want to post it to the list for
feedback, to see if someone can poke holes in the general architecture
of the patches.
Thanks for reading this far :-)
patch 1 looks ok, though I didn't test it. It seems like fairly
straightforward refactoring.
In patch 2 shouldn't pwd be initialized with:
krb5_data pwd = { 0 , 0, NULL};
In patch 3 I'd prefer that each ber failure have a distinct error
message so we can more clearly see where the request failed. There is
also some inconsistency when LOG_FATAL is called and when it isn't.
Is this the sort of thing where seeing the value from the request would
be helpful in error messages? For example, where the principalname is
not found, should that be included in the error?
For the ACIs it would be helpful to have owner and group in the ACI
comment. Right now they are all the same for each type.
Patch 4, do we want a knob to tune the timeout?
Similar duplicated error messages.
Typo in "Incopatible options ...."
rob
Simo.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
