[Freeipa-devel] [RANT] --setattr validation is a minefield.

[Petr Viktorin]

On 04/10/2012 07:53 PM, Martin Kosek wrote:
> On Tue, 2012-04-10 at 19:25 +0200, Petr Viktorin wrote:
>> On 04/10/2012 07:07 PM, Martin Kosek wrote:
>>> On Tue, 2012-04-10 at 17:03 +0200, Jan Cholasta wrote:
>>>> On 10.4.2012 16:00, Petr Viktorin wrote:
> [snip]
>>>> Like you said above, we should either not validate --{set,add,del}attr
>>>> or don't allow them on known attributes.
>>>
>>> IMHO, validating attributes we manage in the same way for both --setattr
>>> and standard attrs is not that wrong. It is a good precaution, because
>>> if we let an unvalidated value in, it can make even a bigger mess later
>>> in our pre_callbacks or post_callbacks where we assume that at this
>>> point everything is valid.
>>
>> Then we should validate *exactly* the same way, including not allowing
>> no_update attributes to be updated.
>
> That makes some sense, I could agree with that.
>

Now that I have a ticket on this 
(https://fedorahosted.org/freeipa/ticket/2580), I would like to get some 
wider agreement here.

The no_update/no_create attributes are mainly "enabled" flags 
(ipaenabledflag, nsaccountlock, idnszoneactive), administrative 
(krbprincipalname, ipauniqueid, ipacertificatesubjectbase), DNS record 
type and data, and various virtual attributes.

If setattr etc. is disabled for all of these, it will mainly matter for 
the "enabled" flags. To be honest I don't know why we only allow 
modifying those through special commands.
If there's some security reason for that, then setattr etc. should be 
disabled for them; otherwise I think they should be changeable through 
xyz-mod.

Either way, setattr etc. should honor the no_update flags. Any objections?

-- 
Petr³



PS. For reference, our no_create/no_update params are:

automember
     automemberdefaultgroup: no_create no_search no_update
     key: no_create no_search no_update
automountkey
     description: no_create no_output no_search no_update
config
     ipacertificatesubjectbase: no_update
dnsrecord
     dnsrecords: no_create no_search no_update
     dnstype: no_create no_search no_update
     dnsdata: no_create no_search no_update
     a_extra_create_reverse: dnsrecord_extra no_update virtual_attribute
     aaaa_extra_create_reverse: dnsrecord_extra no_update virtual_attribute
dnszone
     idnszoneactive: no_create no_update
entitle
     uuid: no_create no_update
     ipaentitlementid: no_create no_update
hbacrule
     ipaenabledflag: no_create no_search no_update
     memberuser_user: no_create no_search no_update
     memberuser_group: no_create no_search no_update
     memberhost_host: no_create no_search no_update
     memberhost_hostgroup: no_create no_search no_update
     sourcehost_host: no_create no_search no_update
     sourcehost_hostgroup: no_create no_search no_update
     memberservice_hbacsvc: no_create no_search no_update
     memberservice_hbacsvcgroup: no_create no_search no_update
host
     randompassword: no_create no_search no_update virtual_attribute
     krbprincipalname: no_create no_search no_update
     sshpubkeyfp: no_create no_search no_update virtual_attribute
netgroup
     ipauniqueid: no_create no_update
selinuxusermap
     ipaenabledflag: no_create no_search no_update
     memberuser_user: no_create no_search no_update
     memberuser_group: no_create no_search no_update
     memberhost_host: no_create no_search no_update
     memberhost_hostgroup: no_create no_search no_update
sudocmdgroup
     membercmd_sudocmd: no_create no_search no_update
     membercmd_sudocmdgroup: no_create no_search no_update
sudorule
     ipaenabledflag: no_create no_search no_update
     memberuser_user: no_create no_search no_update
     memberuser_group: no_create no_search no_update
     memberhost_host: no_create no_search no_update
     memberhost_hostgroup: no_create no_search no_update
     memberallowcmd_sudocmd: no_create no_search no_update
     memberdenycmd_sudocmd: no_create no_search no_update
     memberallowcmd_sudocmdgroup: no_create no_search no_update
     memberdenycmd_sudocmdgroup: no_create no_search no_update
     ipasudorunas_user: no_create no_search no_update
     ipasudorunas_group: no_create no_search no_update
     ipasudoopt: no_create no_search no_update
     ipasudorunasgroup_group: no_create no_search no_update
user
     krbprincipalname: no_update
     randompassword: no_create no_search no_update virtual_attribute
     nsaccountlock: no_create no_search no_update
     sshpubkeyfp: no_create no_search no_update virtual_attribute