[Freeipa-devel] [PATCH 556-557] Add option to disable setkeytab extended operations

[Martin Kosek]

On 12/02/2015 07:16 PM, Simo Sorce wrote:
> On Tue, 2015-12-01 at 16:44 +0100, Petr Vobornik wrote:
>> On 12/01/2015 04:20 PM, Alexander Bokovoy wrote:
>>> On Tue, 01 Dec 2015, Martin Kosek wrote:
>>>> On 12/01/2015 02:59 PM, Simo Sorce wrote:
>>>>> On Tue, 2015-12-01 at 14:42 +0100, Martin Kosek wrote:
>>>>>> On 12/01/2015 02:38 PM, Simo Sorce wrote:
>>>>>>> On Tue, 2015-12-01 at 10:11 +0200, Alexander Bokovoy wrote:
>>>>>>>> On Mon, 30 Nov 2015, Simo Sorce wrote:
>>>>>>>>> On Wed, 2015-11-25 at 09:47 -0500, Simo Sorce wrote:
>>>>>>>>>> On Wed, 2015-11-25 at 09:02 -0500, Rob Crittenden wrote:
>>>>>>>>>>> Jan Cholasta wrote:
>>>>>>>>>>>> On 24.11.2015 22:17, Simo Sorce wrote:
>>>>>>>>>>>>> On Tue, 2015-11-24 at 14:57 -0500, Simo Sorce wrote:
>>>>>>>>>>>>>> On Tue, 2015-11-24 at 14:42 -0500, Simo Sorce wrote:
>>>>>>>>>>>>>>> Since some time we use the getkeytab operation to fetch
>>>>>>>>>>>>>>> keytabs on
>>>>>>>>>>>>>>> newer
>>>>>>>>>>>>>>> clients. According to bug #232 setkeytab can be used to
>>>>>>>>>>>>>>> circumvent
>>>>>>>>>>>>>>> password quality controls so it needs to be slowly retired.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The attached patches implement #5485 in 2 parts.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The first introduces the option DisableSetKeytab which
>>>>>>>>>>>>>>> globally
>>>>>>>>>>>>>>> disables
>>>>>>>>>>>>>>> the setkeytab extended operation. This is set to false by
>>>>>>>>>>>>>>> default for
>>>>>>>>>>>>>>> backwards compatibility.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The second introduces an option called
>>>>>>>>>>>>>>> DisableUserSetKeytab, which is
>>>>>>>>>>>>>>> active by default in new installs (but not in upgraded
>>>>>>>>>>>>>>> ones), and only
>>>>>>>>>>>>>>> disables the use of setkeytab for ipa suers, but not for
>>>>>>>>>>>>>>> hosts/services.
>>>>>>>>>>>>>>> This is because user's are the ones that may abuse the
>>>>>>>>>>>>>>> interface to
>>>>>>>>>>>>>>> escape password policies and users also normally do not
>>>>>>>>>>>>>>> acquire
>>>>>>>>>>>>>>> keytabs,
>>>>>>>>>>>>>>> so it is a safe bet to disable just them by default in new
>>>>>>>>>>>>>>> installs.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> (Testing in progress)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Tested and working as expected.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I realized that adding options to ipaConfig require to add
>>>>>>>>>>>>> them in the
>>>>>>>>>>>>> UI as well, attached patches add options in API.txt and
>>>>>>>>>>>>> config.py
>>>>>>>>>>>>> Make now complain I should change API Major or Minor, but it
>>>>>>>>>>>>> is not
>>>>>>>>>>>>> clear to me why given this are additional values and no real
>>>>>>>>>>>>> change or
>>>>>>>>>>>>> new function is introduced. What's the recommendation ?
>>>>>>>>>>>>
>>>>>>>>>>>> When does make complain? It is supposed to complain only when
>>>>>>>>>>>> API.txt
>>>>>>>>>>>> does not match code.
>>>>>>>>>>>>
>>>>>>>>>>>> Anyway, we usually bump minor version even for backward
>>>>>>>>>>>> compatible
>>>>>>>>>>>> changes, see e.g. commit 9549a59.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The point of API.txt (and the heavy client) was to save a
>>>>>>>>>>> round-trip.
>>>>>>>>>>> Being able to pass in an invalid option would void that rule hence
>>>>>>>>>>> having to update the API when new values are added.
>>>>>>>>>>>
>>>>>>>>>>> rob
>>>>>>>>>>
>>>>>>>>>> Ok added version change to the second patch (so we bump it only
>>>>>>>>>> once
>>>>>>>>>> given these are basically related changes.
>>>>>>>>>
>>>>>>>>> Bump, is this ok ?
>>>>>>>> This patch is fine but please fix setkeytab use in ipa-sam before
>>>>>>>> committing this patch.
>>>>>>>
>>>>>>> This patch does not disable setkeytab yet, so it can go in right away
>>>>>>> (it blocks other patches already). We have a bug to change ipa-sam,
>>>>>>> should we mark it blocker for 4.4 ?
>>>>>>
>>>>>> We also have to fix the permission to change keytab, so that it uses
>>>>>> the new
>>>>>> style (https://fedorahosted.org/freeipa/ticket/5487). I would
>>>>>> actually make
>>>>>> this ticket and the ipa-sam ticket a blocker for this patch set.
>>>>>>
>>>>>> Otherwise you are actually introducing a switch that breaks FreeIPA
>>>>>> as some of
>>>>>> it's core server functions still use the old method.
>>>>>
>>>>> The point of this patchset is to introduce the switch early so that
>>>>> current server will support the off switch when newer servers down the
>>>>> road are ready to flip it. The defaults are still to allow these
>>>>> operations for services/hosts.
>>>>
>>>> I still do not get the logic about an early switch. Now, if switch is
>>>> turned
>>>> on, FreeIPA server breaks on several places. I would really rather
>>>> expect the
>>>> switch to be introduced when the server itself supports it. Then,
>>>> admin would
>>>> enable it when the clients are sufficiently updated and can use the
>>>> new method.
>>>>
>>>> Why would admin want to enable the switch early if it breaks FreeIPA
>>>> some of
>>>> the FreeIPA servers? Permission can be upgraded in newer version and get
>>>> replicated, that's fine. But ipa-sam would be still broken on this old
>>>> FreeIPA
>>>> server.
>>> Old server is not a problem here: ipa-sam only talks to the
>>> localhost-based server as we always use ldapi protocol. So if server is
>>> running old-behavior FreeIPA, ipa-sam on the same server will work
>>> against it.
>>>
>>> What I don't want to have is a situation where setkeytab is disabled and
>>> a new server obeys it but ipa-sam on this new server is not updated and
>>> will expectedly fail. We are not that forced to do the change right now
>>> in 4.3, given that the default will still be to keep setkeytab, thus we
>>> can wait with this patch until ipa-sam is fixed and then push both
>>> patches into the closest release, be it 4.3.x (x>=0) or whatever is the
>>> next one.
>>>
>>
>> +1, fixing ipa-sam would be then a release blocker for 4.3 which is not 
>> desired.
>>
>> Also what about adding support for "ipaProtectedoperation check" for 
>> user principals?
>>
>> I'm afraid that forbidding getting user principal might be regarded as a 
>> regression which might cause that admins won't set DisableSetKeytab.
> 
> One thing at a time.
> We have bugs open for all these things, but I see no reason not to add
> an *optional* setting just because some things break when it is turned
> on.

One thing at a time, yes - but at the right order. I would only agree with you
if you add a proper label for this optional option or rename it to

DisableUserSetKeytabAndBreakIPASamAndBreakKeytabRenewalToo :-)

Martin