[Freeipa-devel] [PATCH] 0506 Default read ACIs for hosts

Mon Apr 14 08:58:29 UTC 2014

On 04/14/2014 10:54 AM, Martin Kosek wrote:
> On 04/11/2014 02:53 PM, Petr Viktorin wrote:
>> On 04/11/2014 02:36 PM, Simo Sorce wrote:
>>> On Fri, 2014-04-11 at 09:48 +0200, Martin Kosek wrote:
>>>> On 04/10/2014 05:29 PM, Petr Viktorin wrote:
>>>>> On 04/10/2014 03:04 PM, Martin Kosek wrote:
>>>>>> On 04/10/2014 02:52 PM, Simo Sorce wrote:
>>>>>>> On Thu, 2014-04-10 at 13:56 +0200, Petr Viktorin wrote:
>>>>>>>> On 04/09/2014 12:25 PM, Martin Kosek wrote:
>>>>>>>>> On 04/03/2014 12:09 PM, Petr Viktorin wrote:
>>>>>>>>>> Hello,
>>>>>>>>>> This adds read permissions to read hosts.
>>>>>>>>>>
>>>>>>>>>> Read access is given to all authenticated users.
>>>>>>>>>> For reading host membership info, there is a separate permission that
>>>>>>>>>> also
>>>>>>>>>> defaults to all authenticated users.
>>>>>>>>>>
>>>>>>>>>> The userPassword attribute is not included for obvious reasons.
>>>>>>>>>
>>>>>>>>> 1) We decided to show hosts only to authenticated users by default. I
>>>>>>>>> am just
>>>>>>>>> thinking - should some portion of hosts be readable just like groups and
>>>>>>>>> users
>>>>>>>>> are? For example at least the host core defined by nsHost objectlass?
>>>>>>>>>
>>>>>>>>> objectClasses: ( nsHost-oid NAME 'nsHost' DESC 'Netscape defined
>>>>>>>>> objectclass'
>>>>>>>>>      SUP top STRUCTURAL MUST cn MAY ( serverHostName $ description $ l $
>>>>>>>>> nsHostLoc
>>>>>>>>>      ation $ nsHardwarePlatform $ nsOsVersion ) X-ORIGIN 'Netscape' )
>>>>>>>>>
>>>>>>>>> Are application supposed to be able to anonymously read that information?
>>>>>>>>
>>>>>>>> I'm not sure. Simo?
>>>>>>>
>>>>>>> Good question, probably not by default, we offer DNS and do not
>>>>>>> recommend people to rely on exposed host maps.
>>>>>>
>>>>>> Question is if should have a separate permission like "System: Read Host Core
>>>>>> Attributes", "System: Read Host", "System: Read Host Membership" or we are
>>>>>> fine
>>>>>> with just "System: Read Host", "System: Read Host Membership".
>>>>>>
>>>>>> If we do not expect people and programs to often read the list of host or the
>>>>>> basic host information anonymously, I would stick with the latter.
>>>>>
>>>>> Let's do that then. It's easy enough to add a custom permission.
>>>>>
>>>>>>>>> 2) Do we want to count enrolledBy and managedBy attribute to "System: Read
>>>>>>>>> Host
>>>>>>>>> Membership" permission or should it be included in the "Read Hosts"
>>>>>>>>> permission?
>>>>>>>>>
>>>>>>>>> If we want to stick with previous behavior, we would want to have only
>>>>>>>>> "memberOf" listed as this is how our original member handling ACI looks
>>>>>>>>> like:
>>>>>>>>>
>>>>>>>>> install/share/default-aci.ldif:aci: (targetattr = "memberOf ||
>>>>>>>>> memberHost ||
>>>>>>>>> memberUser")(version 3.0; acl "No anonymous access to member information";
>>>>>>>>> deny
>>>>>>>>> (read,search,compare) userdn != "ldap:///all";
>>>>>>>>
>>>>>>>> What was the reasoning behind enrolledBy and managedBy? I got it from
>>>>>>>> the notes from devconf; I don't think there was much discussion.
>>>>>>>
>>>>>>> I have no recollection :(
>>>>>>
>>>>>> There was no discussion about these particular attributes. I added then to
>>>>>> Membership permission because they were DN-ish, but when I think more
>>>>>> about it,
>>>>>> it does not seem as a membership in the sense of the ACI above. I would
>>>>>> personally only keep member/memberOf in the Membership attributes.
>>>>>
>>>>> Moved to Read Host.
>>>>>
>>>>>>>>> 3) I could not functionally test if e.g. clients and replicas still
>>>>>>>>> enroll as
>>>>>>>>> we do not have an ACI for krbtpolicy/krbRealmContainer yet and
>>>>>>>>> ipa-client-install searches for it.
>>>>>>>>>
>>>>>>>>> Speaking of which, we will need to have an ACI for reading a portion of
>>>>>>>>> krbRealmContainer anonymously to enable IPA client autodiscovery
>>>>>>>>> (cn+objectclass should be sufficient).
>>>>>>>
>>>>>>> Sad we ended up depending on this, I would have preferred to not depend
>>>>>>> on keeping this around if we ever want to change something.
>>>>>>
>>>>>> Yeah... But we should be OK with exposing just the CN which is not really a
>>>>>> secret as we know what is the realm name...
>>>>>>
>>>>>> Martin
>>>>>>
>>>>
>>>> The basis looks good now. However, when I was checking host's objectclass and
>>>> attributes we allow, I saw many attributes not listed in our DevConf meeting
>>>> minutes, but which are now missing in the entry when I read it as authenticated
>>>> used.
>>>
>>> As an admin, or as a regular user ?
>>>
>>>> krbPrincipalAux attributes like krbCanonicalName, krbExtraData,
>>>> krbLastAdminUnlock, krbLastFailedAuth...
>>>> We may want to list them all, except the ones we chose to now show, like
>>>> krbPrincipalKey :) I would welcome Simo's recommendation in this place.
>>>
>>> I do not think we necessarily need to expose most of them to regular
>>> users (which include keytab bearing services) by default.
>>>
>>>> krbTicketPolicyAux attributes like krbMaxRenewableAge
>>>>
>>>> Objectclasses for reference:
>>>>
>>>> objectClasses: ( 2.16.840.1.113719.1.301.6.8.1 NAME 'krbPrincipalAux' AUXILIAR
>>>>    Y MAY ( krbPrincipalName $ krbCanonicalName $ krbUPEnabled $ krbPrincipalKey
>>>>    $ krbTicketPolicyReference $ krbPrincipalExpiration $ krbPasswordExpiration $
>>>>     krbPwdPolicyReference $ krbPrincipalType $ krbPwdHistory $ krbLastPwdChange
>>>>    $ krbPrincipalAliases $ krbLastSuccessfulAuth $ krbLastFailedAuth $ krbLoginF
>>>>    ailedCount $ krbExtraData $ krbLastAdminUnlock ) )
>>>
>>> Of these we may want to show krbPrincipalName, krbCanonicalName,
>>> krbPrincipalAliases by default.
>>
>> Added.
>>
>>> I am uncertain about krbPrincipalExpiration, krbPasswordExpiration,
>>> krbLastPwdChange, but they may be needed for clients like SSSD to do
>>> proper checking and for power users to figure out themselves when they
>>> need to change passwords or ask for an account validity extension etc..
>>
>> Let's show them then, to make their lives easier. Paranoid admins can always
>> hide them.
>>
>>> We certainly should not expose:
>>> krbPrincipalKey, krbPwdHistory (though we do not use this IIRC, yet),
>>> krbExtraData
>>>
>>> The rest I lean more on the not showing by default, although they could
>>> be safely shown to authenticated users if necessary.
>>
>> Similarly, generous admins can always give more access. I've not added these.
>>
>>>> objectClasses: ( 2.16.840.1.113719.1.301.6.16.1 NAME 'krbTicketPolicyAux' AUXI
>>>>    LIARY MAY ( krbTicketFlags $ krbMaxTicketLife $ krbMaxRenewableAge ) )
>>>
>>> I do not think we need to expose these, although exposing them is not
>>> problematic, you can infer what they are by simply asking for tickets
>>> anyway.
>>
>> Not added.
>>
>>
>> I assume the same will also apply to  users and services?
>>
>
> IMO yes. We will see if any issue caused by unreadable attribute pops up. For
> example, user-status command won't work properly without additional permissions
> as it reads krbLastSuccessfulAuth and krbLastFailedAuth. But we will get to
> that part when we get to users.
>
> As for this patch, it works OK so it is an ACK from me.

Thanks, pushed to master: c08f8d2f1e61e607a493980880a81120ad01a045


-- 
Petr³