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.

Martin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to