[Freeipa-devel] Design Review Keytab Retrieval

[Martin Kosek]

On 06/27/2014 10:00 AM, Alexander Bokovoy wrote:
> On Thu, 26 Jun 2014, Simo Sorce wrote:
>> On Thu, 2014-06-26 at 10:20 -0400, Simo Sorce wrote:
>>> On Thu, 2014-06-26 at 15:33 +0300, Alexander Bokovoy wrote:
>>> > On Thu, 26 Jun 2014, Martin Kosek wrote:
>>> > >On 06/26/2014 04:29 AM, Nathaniel McCallum wrote:
>>> > >> On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote:
>>> > >>> On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote:
>>> > >>>> ----- Original Message -----
>>> > >>>>> ----- Original Message -----
>>> > >>>>>>> Can you check if ipaProtectedOperation is in the aci attribute in the
>>> > >>>>>>> base tree object ?
>>> > >>>>>>> It should be there as excluded, and that should cause admin to not be
>>> > >>>>>>> able to retrieve keytabs.
>>> > >>>>>>
>>> > >>>>>> It was not. While running ipa-ldap-updater I got the following:
>>> > >>>>>> InvalidSyntax: ACL Syntax Error(-5):(targetattr=
>>> > >>>>>> \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are
>>> > >>>>>> allowed to rekey any entity\22; allow(write) groupdn =
>>> > >>>>>> \22ldap:///cn=admins: Invalid syntax.
>>> > >>>>>
>>> > >>>>> Uhmm I do not see anything obviously wrong with ACI instruction, it
>>> looks
>>> > >>>>> just like the one I replace, Ideas ?
>>> > >>>>> Do you have ipaProtectedOperation in the schema ?
>>> > >>>>>
>>> > >>>>> (I rebased patch 3 but will wait to send a patchset until we
>>> understand (and
>>> > >>>>> fix) why this is failing to update.
>>> > >>>>
>>> > >>>> Ok, apparently it was a quoting issue in the .update files, hopefully
>>> that's
>>> > >>>> the only issue (I am at a conference today and do not have my test
>>> env. handy).
>>> > >>>>
>>> > >>>> The attached patches are rebased on the latest master.
>>> > >>>
>>> > >>> 0001: Line 555 has very wrong indentation.
>>> > >>>
>>> > >>> I don't see anything else wrong in the other patches. I've tested
>>> > >>> everything and it works as designed.
>>> > >>>
>>> > >>> I have CC'd everyone who was involved with review at any point on these
>>> > >>> patches. This serves as my public notice that I'd like to ACK the next
>>> > >>> round of patches. If anyone has anything else to add, please do it
>>> > >>> before tomorrow evening. Thanks!
>>> > >>>
>>> > >>> Nathaniel
>>> > >>
>>> > >> ACK
>>> > >>
>>> > >> Nathaniel
>>> > >
>>> > >Pushed all 6 patches to master. Thanks for careful review!
>>> >
>>> > Unfortunately, at least enctype marshalling is wrong with these patches.
>>> > Samba does not work anymore with the keytab fetched in new version.
>>> >
>>> > We see following in the keytab:
>>> > Keytab name: FILE:/etc/samba/samba.keytab
>>> > KVNO Timestamp           Principal
>>> > ----
>>> -------------------------------------------------------------------------
>>> >  1 06/26/2014 13:03:01
>>> cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat.com at DOM136.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
>>> (etype 274)
>>> >  1 06/26/2014 13:03:01
>>> cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat.com at DOM136.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
>>> (etype 273)
>>> >  1 06/26/2014 13:03:01
>>> cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat.com at DOM136.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
>>> (etype 272)
>>> >  1 06/26/2014 13:03:01
>>> cifs/vm-136.dom136.tbad.idm.lab.eng.brq.redhat.com at DOM136.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
>>> (etype 279)
>>> >
>>> > Note that etype is unresolvable. In the build without these patches we
>>> > get something like
>>> >    1 06/23/2014 16:28:59
>>> cifs/vm-139.dom139.tbad.idm.lab.eng.brq.redhat.com at DOM139.TBAD.IDM.LAB.ENG.BRQ.REDHAT.COM
>>> (aes256-cts-hmac-sha1-96)
>>> >
>>> > So this patchset needs an improvement before release.
>>>
>>> Working on this.
>>> I know, roughly what's going on, but still trying to pinpoint exactly
>>> the offender. (It is the ber marshalling/unmarshalling indeed).
>>>
>>> Simo.
>>>
>>
>> The attached patch fixes it for me.
> ACK, works for me too.
> 
> Martin: it makes sense to merge both this and the indentation fix
> together prior to commit.

+1. (The bad indentation fix is my fault as I wanted to fix that before
pushing, based on Nathaniel's point, but did not notice the source use tabs).

Merged both patches, pushed to master.

Martin