[Freeipa-devel] [PATCH] 761 keytab manipulation permission management

[Petr Vobornik]

On 6.10.2014 16:17, Martin Kosek wrote:
> On 10/06/2014 04:15 PM, Petr Vobornik wrote:
>> On 6.10.2014 15:49, Martin Kosek wrote:
>>> On 10/06/2014 03:01 PM, Simo Sorce wrote:
>>>> On Mon, 06 Oct 2014 12:53:57 +0200
>>>> Martin Kosek <mkosek at redhat.com> wrote:
>>>>
>>>>> On 10/06/2014 10:33 AM, Jan Cholasta wrote:
>>>>>> Dne 3.10.2014 v 17:02 Martin Kosek napsal(a):
>>>>>>> On 10/03/2014 04:59 PM, Jan Cholasta wrote:
>>>>>>>> Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a):
>>>>>>>>> On 3.10.2014 16:24, Martin Kosek wrote:
>>>>>>>>>> NACK. I will not comment on mechanics, if you get an ACK from
>>>>>>>>>> Honza, it is good enough. I just do not like the API. It is
>>>>>>>>>> hard to guess what "host-add-retrieve-keytab" means. That word
>>>>>>>>>> does not even make much sense.
>>>>>>>>>>
>>>>>>>>>> Can we use something more readable? For example:
>>>>>>>>>>
>>>>>>>>>> ipa host-add-allowed-operation HOSTNAME --operation read_keys
>>>>>>>>>> --users=STR --groups STR
>>>>>>>>>> ipa host-add-allowed-operation HOSTNAME --operation write_keys
>>>>>>>>>> --users=STR --groups STR
>>>>>>>>>>
>>>>>>>>>> and
>>>>>>>>>>
>>>>>>>>>> ipa host-remove-allowed-operation HOSTNAME --operation read_keys
>>>>>>>>>> --users=STR --groups STR
>>>>>>>>>> ipa host-remove-allowed-operation HOSTNAME --operation
>>>>>>>>>> write_keys --users=STR --groups STR
>>>>>>>>>>
>>>>>>>>>> Same with services. At least to me, it looks more readable.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Martin
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Seems to me as adding of allowed operation. Not allowing an
>>>>>>>>> operation.
>>>>>>>>
>>>>>>>> +1
>>>>>>>>
>>>>>>>>>
>>>>>>>>> What about:
>>>>>>>>>
>>>>>>>>> ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR
>>>>>>>>> ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups
>>>>>>>>> STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups
>>>>>>>>> STR ipa host-disallow-create-keytab HOSTNAME --users=STR
>>>>>>>>> --groups STR
>>>>>>>>
>>>>>>>> I like these the best. Maybe with a -to or -by suffix.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> or if we expect more operations in a future:
>>>>>>>>>
>>>>>>>>> ipa host-allow-operation HOSTNAME --operation read-keys
>>>>>>>>> --users=STR --groups STR
>>>>>>>>> ipa host-disallow-operation HOSTNAME --operation read-keys
>>>>>>>>> --users=STR --groups STR
>>>>>>>>> ipa host-allow-operation HOSTNAME --operation write-keys
>>>>>>>>> --users=STR --groups STR
>>>>>>>>> ipa host-disallow-operation HOSTNAME --operation write-keys
>>>>>>>>> --users=STR --groups STR
>>>>>>>>>
>>>>>>>>> or if we want to keep 'add' and 'remove' in command names:
>>>>>>>>>
>>>>>>>>> ipa host-add-retrieve-keytab-right HOSTNAME --users=STR
>>>>>>>>> --groups=STR ipa host-add-create-keytab-right HOSTNAME
>>>>>>>>> --users=STR --groups=STR ipa host-remove-retrieve-keytab-right
>>>>>>>>> HOSTNAME --users=STR --groups=STR ipa
>>>>>>>>> host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> personally I'm not a fan o the --operation switch, but could be
>>>>>>>>> persuaded by a 'future' usage.
>>>>>>>>
>>>>>>>> Not a fan either, because it is not consistent with the rest of
>>>>>>>> the framework.
>>>>>>>> Also, non-optional options are not really options.
>>>>>>>
>>>>>>> Right. Though mandatory options is a concept already existing in
>>>>>>> FreeIPA framework in many places.
>>>>>>
>>>>>> That does not make it right.
>>>>>
>>>>> Right :-)
>>>>>
>>>>>>> What I see as a deal breaker is that with
>>>>>>> --operation switch, we are ready for dozens of potential future
>>>>>>> operations. With operation hardcoded in command name, we are not.
>>>>>>
>>>>>> I don't see dozens of operations coming in the near future, there's
>>>>>> no need for a premature optimization like this.
>>>>>
>>>>> My point was that it will be difficult to switch from having
>>>>> per-operation commands to one general command for all operations
>>>>> later, however far the future is.
>>>>>
>>>>> Given there is no clear agreement on the API (ipa
>>>>> host-allow-operation vs.
>>>>> host-allow-read-keytab+host-allow-write-keytab) yet, I would like to
>>>>> ask Rob or Simo for their opinion/vote here too so that we can select
>>>>> an approach and go with it.
>>>>
>>>> I am not even sure why we are tying this to hosts to be honest.
>>>
>>> Isn't the use case to for example allow several load balancing nodes
>>> host/serverX.example.com get a keytab for host/server.example.com?
>>>
>>>> The allow-operation plugin is generic, and we should probably have a
>>>> command that reflect that like:
>>>> ipa operations-add/mod/del and options to say what the
>>>> operation does and what it applies to.
>>>>
>>>> Of course the naming needs more thought, but I do not think having a
>>>> command for this specific narrowed down operations is wise.
>>>>
>>>> Actually it may even fit right into the permissions commands (Add a
>>>> --operation switch ?), as these operations are just a particular type
>>>> of ACIs/Permissions that apply to abstract operations rather than
>>>> LDAP operations, so it is a natural extension of our permissions.
>>>>
>>>> Simo.
>>>
>>> Ok, now that's a 180° turn from what we were discussing until now... I really
>>> do not think we can go the permission route ATM, the permission plugin is now
>>> very bound to how ACIs work and how they are used in FreeIPA tree. Changing the
>>> backend from ACI to ipaAllowedOperation would be something completely different.
>>>
>>> The super-general operation-* command is interesting idea, though it decouples
>>> these operations from their targets. I think there is a benefit in seeing an
>>> command allowing writing/reading a keytab right in the list of host commands or
>>> in host page Web UI. Given the short time before 4.1 release, I think it will
>>> be more straightforward to go with
>>>
>>> ipa host-allow-operation
>>> or
>>> ipa host-allow-read-keytab/host-allow-write-keytab
>>>
>>> route.
>>>
>>> Martin
>>>
>>
>> It also works a bit differently. Permissions defined by admin, using a
>> permission plugin, are part of RBAC. Allowed operations aren't. Binding it with
>> permission plugin might cause confusion.
>>
>> On the other hand, I'm afraid that current implementation clutters up both CLI
>> and Web UI. We don't distinguish common commands and less used ones. Therefore
>> they all are equally visible - not good UX. We will have to focus on this issue
>> sooner or later.
>>
>> The general operaion-* commands might be a subject of discussion. I don't like
>> it because the operations are always bound with particular object.
>>
>> I agree with Martin and would go with 'ipa
>> host-allow-read-keytab/host-allow-write-keytab' commands.
>
> Note that Martin still prefers host-allow-operation with operation being it's
> argument, as Petr3 suggested.

I'm not against it even though I prefer the former one.

>
>> Also terminology is not united: write vs create + read vs retrieve. Since
>> ipa-get-keytab uses --retrieve, I would go with 'create' and 'retrieve'.
>
> The operation itself in LDAP is named read_key and write_key - so we need to
> chose what it the canonical naming - ipa-getkeytab CLI or LDAP.

User should not need to know how it's implemented. We should use the 
same terminology in all our CLIs.

>
> Martin
>
-- 
Petr Vobornik