[Freeipa-devel] [PATCH 0002] Port from python-krbV to python-gssapi

[David Kupka]

On 26/08/15 09:42, Jan Cholasta wrote:
> On 25.8.2015 21:00, Simo Sorce wrote:
>> On Tue, 2015-08-25 at 20:45 +0200, Michael Šimáček wrote:
>>>
>>> On 2015-08-25 18:43, Robbie Harwood wrote:
>>>> Jan Cholasta <jcholast at redhat.com> writes:
>>>>
>>>>> On 25.8.2015 12:46, Michael Šimáček wrote:
>>>>>> On 2015-08-25 12:38, Alexander Bokovoy wrote:
>>>>>>> On Tue, 25 Aug 2015, Michael Šimáček wrote:
>>>>>>>> On 2015-08-24 20:29, Robbie Harwood wrote:
>>>>>>>>> Michael Šimáček <msimacek at redhat.com> writes:
>>>>>>>>>> On 2015-08-24 17:49, Simo Sorce wrote:
>>>>>>>>>>> On Mon, 2015-08-24 at 17:18 +0200, Michael Šimáček wrote:
>>>>>>>>>>>> On 2015-08-24 14:50, Jan Cholasta wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Fixed. python-gssapi has a display_as method that could pull
>>>>>>>>>>>> the
>>>>>>>>>>>> name
>>>>>>>>>>>> from it, but it doesn't work in current version, therefore
>>>>>>>>>>>> using
>>>>>>>>>>>> partition to split on '@'
>>>>>>>>>
>>>>>>>>> It's actually a bug in MIT Krb5, as we noted in your bug[0].
>>>>>>>>> So this:
>>>>>>>>>
>>>>>>>>>> -        user =
>>>>>>>>>> api.Command.user_show(unicode(principal[0]))['result']
>>>>>>>>>> +        user =
>>>>>>>>>> api.Command.user_show(principal.partition('@')[0])['result']
>>>>>>>>>
>>>>>>>>> is working around a bug in specific Kerberos versions.  If
>>>>>>>>> people are
>>>>>>>>> okay with merging such code, then I guess this is fine; I would
>>>>>>>>> personally not do so because there is not a clear point at
>>>>>>>>> which it can
>>>>>>>>> be removed.  At the very least, we should wait until we see what
>>>>>>>>> versions of krb5 MIT is going to fix.
>>>>>>>>>
>>>>>>>>> Otherwise, looks good.
>>>>>>>>>
>>>>>>>>> [0]: https://github.com/pythongssapi/python-gssapi/issues/79
>>>>>>>>>
>>>>>>>>
>>>>>>>> python-krbV migration is blocking support for Python 3. The bug
>>>>>>>> doesn't have any fix upstream yet and there are two bugs
>>>>>>>> actually, the
>>>>>>>> second one is in python-gssapi, which I've just reported [1].
>>>>>>>> Waiting
>>>>>>>> for two bugs to be fixed could be detrimental to py3 migration
>>>>>>>> as we
>>>>>>>> don't have much time left. And I'm no longer sure that display_as
>>>>>>>
>>>>>>> I don't buy this.
>>>>>>>
>>>>>>> We have plenty of time for solving these bugs. Remember, that Samba
>>>>>>> DCE RPC bindings aren't migrated to Python 3 either and will not be
>>>>>>> before release of Samba 4.4. For Samba 4.3 it is simply too late.
>>>>>>>
>>>>>>> So we are still far away from full Python3 migration for FreeIPA and
>>>>>>> waiting for solving these two bugs is OK.
>>>>>>
>>>>>> If fixing them solves anything at all. I planned to use
>>>>>> display_as(NameType.user), but when trying it on Name object with
>>>>>> name_type set (which doesn't trigger the segfault), it doesn't
>>>>>> seem to
>>>>>> work either. I get:
>>>>>> gssapi.raw.exceptions.OperationUnavailableError: Major (1048576): The
>>>>>> operation or option is not available or unsupported, Minor (0):
>>>>>> Unknown
>>>>>> error
>>>>>>
>>>>>> Robbie, can you clarify whether display_as could be actually used
>>>>>> to get
>>>>>> the first component of the principal reliably?
>>>>
>>>> display_as should behave in accordance with its docs; anything else
>>>> is a
>>>> bug report, which you filed.  I don't know what you're asking me for
>>>> beyond that.
>>>>
>>>
>>> Why I mentioned display_as at all is that I initially assumed it could
>>> be used for this, but it was only an assumption because I couldn't get
>>> around the segfault. Later on, the cause of the segfault was found and I
>>> was able to try the method and I found out that it probably cannot be
>>> used for this purpose (i. e. extracting the first component of the
>>> principal) regardless of the two bugs. How I thought it would be used:
>>> import gssapi
>>> cred = gssapi.Credentials()
>>> user = cred.name.display_as(gssapi.NameType.user)
>>>
>>> What I got:
>>> gssapi.raw.exceptions.OperationUnavailableError: Major (1048576): The
>>> operation or option is not available or unsupported, Minor (0): Unknown
>>> error
>>>
>>> This seems more like the method is not intended to be used this way. So
>>> I'm asking you whether it is a bug or whether there is another way to do
>>> it. Otherwise display_as cannot be used here.
>>>
>>>>> As I have written in the other thread, we use
>>>>> "principal.split('@')" in
>>>>> other parts of IPA, so "principal.partition('@')" should be OK as
>>>>> well.
>>>>>
>>>>> This patch works for me, so ACK.
>>>>>
>>>>> Unless there are any further objections, I would like to push it.
>>>>
>>>> I think the newest iteration of this
>>>>
>>>>> user =
>>>>> api.Command.user_show(principal.partition('@')[0].partition('/')[0])['result']
>>>>>
>>>>
>>>> is even worse, but if it is decided to merge, then hopefully we can be
>>>> rid of it quickly.
>>>
>>> It is splitting a string of known format in a way that is used in other
>>> places of freeipa. What is specifically so bad about it? What do you
>>> suggest as an alternative?
>>
>> Given display_as() currently does not work for you go ahead with this
>> code. We'll revisit display_as later once we figure out more about the
>> bug that makes it fail.
>
> OK.
>
> Pushed to master: aad73fad601f576dd83b758f4448839b4e8e87df
>

I think this patch is causing tracebacks when expired or missing 
kerberos ticket (https://fedorahosted.org/freeipa/ticket/5272).


-- 
David Kupka